From 440ad19b0e9cf67333d7ba95f934a4e7f8a899cc Mon Sep 17 00:00:00 2001 From: Dominik Honnef Date: Mon, 24 May 2021 23:17:59 +0200 Subject: [PATCH] quickfix: delete QF1000 The upstream position is that one should always use strings.Index when dealing with literals, and we don't want to offer a suggestion that goes against that. IndexByte has a small performance improvement over Index, but this should be addressed by improvements to strings.Index and the compiler. We could still recommend to use IndexByte when not using literals, but that would be entirely new code, not part of the existing check. It is not clear what to do for the bytes package, but it is probably best to err on the side of not making any suggestions. Closes gh-1007 --- quickfix/analysis.go | 4 - quickfix/doc.go | 6 - quickfix/lint.go | 107 ------------------ .../CheckStringsIndexByte.go | 22 ---- .../CheckStringsIndexByte.go.golden | 22 ---- 5 files changed, 161 deletions(-) delete mode 100644 quickfix/testdata/src/CheckStringsIndexByte/CheckStringsIndexByte.go delete mode 100644 quickfix/testdata/src/CheckStringsIndexByte/CheckStringsIndexByte.go.golden diff --git a/quickfix/analysis.go b/quickfix/analysis.go index 88dd5f5ae..de24d2c71 100644 --- a/quickfix/analysis.go +++ b/quickfix/analysis.go @@ -10,10 +10,6 @@ import ( ) var Analyzers = lint.InitializeAnalyzers(Docs, map[string]*analysis.Analyzer{ - "QF1000": { - Run: CheckStringsIndexByte, - Requires: []*analysis.Analyzer{inspect.Analyzer}, - }, "QF1001": { Run: CheckDeMorgan, Requires: []*analysis.Analyzer{inspect.Analyzer}, diff --git a/quickfix/doc.go b/quickfix/doc.go index e34f47013..9cc88b215 100644 --- a/quickfix/doc.go +++ b/quickfix/doc.go @@ -3,12 +3,6 @@ package quickfix import "honnef.co/go/tools/analysis/lint" var Docs = lint.Markdownify(map[string]*lint.Documentation{ - "QF1000": { - Title: "Use byte-specific indexing function", - Since: "Unreleased", - Severity: lint.SeverityHint, - }, - "QF1001": { Title: "Apply De Morgan's law", Since: "Unreleased", diff --git a/quickfix/lint.go b/quickfix/lint.go index fc396f292..3a7a937d2 100644 --- a/quickfix/lint.go +++ b/quickfix/lint.go @@ -18,113 +18,6 @@ import ( "golang.org/x/tools/go/analysis" ) -var ( - stringsIndexQ = pattern.MustParse(` - (CallExpr - fn@(Or - (Function "strings.Index") - (Function "strings.LastIndex") - (Function "strings.IndexByte") - (Function "strings.LastIndexByte")) - [arg1 lit@(BasicLit (Or "STRING" "CHAR") _)])`) - bytesIndexQ = pattern.MustParse(` - (CallExpr - fn@(Or - (Function "bytes.Index") - (Function "bytes.LastIndex")) - [ - arg1 - (Or - (CompositeLit _ [b]) - (CallExpr (ArrayType nil _) lit@(BasicLit "STRING" _)))])`) - - stringsIndexR = pattern.MustParse(`(CallExpr (SelectorExpr (Ident "strings") (Ident replacement)) [arg1 (BasicLit "CHAR" lit)])`) - stringsIndexByteR = pattern.MustParse(`(CallExpr (SelectorExpr (Ident "strings") (Ident replacement)) [arg1 (BasicLit "STRING" lit)])`) - bytesIndexR = pattern.MustParse(`(CallExpr (SelectorExpr (Ident "bytes") (Ident replacement)) [arg1 b])`) -) - -func CheckStringsIndexByte(pass *analysis.Pass) (interface{}, error) { - // FIXME(dh): create proper suggested fix for renamed import - - fn := func(node ast.Node) { - if matcher, ok := code.Match(pass, stringsIndexQ, node); ok { - var replacement string - isString := false - fn := typeutil.FuncName(matcher.State["fn"].(*types.Func)) - switch fn { - case "strings.Index": - replacement = "IndexByte" - isString = true - case "strings.LastIndex": - replacement = "LastIndexByte" - isString = true - case "strings.IndexByte": - replacement = "Index" - case "strings.LastIndexByte": - replacement = "LastIndex" - } - - var rPattern pattern.Pattern - lit := matcher.State["lit"].(*ast.BasicLit).Value - var newLit string - if isString { - if q, err := strconv.Unquote(lit); err != nil || len(q) != 1 { - return - } - newLit = "'" + lit[1:len(lit)-1] + "'" - rPattern = stringsIndexR - } else { - newLit = `"` + lit[1:len(lit)-1] + `"` - rPattern = stringsIndexByteR - } - - state := pattern.State{ - "arg1": matcher.State["arg1"], - "lit": newLit, - "replacement": replacement, - } - report.Report(pass, node, fmt.Sprintf("could use strings.%s instead of %s", replacement, fn), - report.Fixes(edit.Fix(fmt.Sprintf("Use strings.%s instead of %s", replacement, fn), edit.ReplaceWithPattern(pass, rPattern, state, node)))) - } else if matcher, ok := code.Match(pass, bytesIndexQ, node); ok { - var replacement string - fn := typeutil.FuncName(matcher.State["fn"].(*types.Func)) - switch fn { - case "bytes.Index": - replacement = "IndexByte" - case "bytes.LastIndex": - replacement = "LastIndexByte" - } - - if _, ok := matcher.State["b"]; ok { - state := pattern.State{ - "arg1": matcher.State["arg1"], - "b": matcher.State["b"], - "replacement": replacement, - } - report.Report(pass, node, fmt.Sprintf("could use bytes.%s instead of %s", replacement, fn), - report.Fixes(edit.Fix(fmt.Sprintf("Use bytes.%s instead of %s", replacement, fn), edit.ReplaceWithPattern(pass, bytesIndexR, state, node)))) - } else { - lit := matcher.State["lit"].(*ast.BasicLit).Value - if q, err := strconv.Unquote(lit); err != nil || len(q) != 1 { - return - } - state := pattern.State{ - "arg1": matcher.State["arg1"], - "b": &ast.BasicLit{ - Kind: token.CHAR, - Value: "'" + lit[1:len(lit)-1] + "'", - }, - "replacement": replacement, - } - report.Report(pass, node, fmt.Sprintf("could use bytes.%s instead of %s", replacement, fn), - report.Fixes(edit.Fix(fmt.Sprintf("Use bytes.%s instead of %s", replacement, fn), edit.ReplaceWithPattern(pass, bytesIndexR, state, node)))) - } - } - } - code.Preorder(pass, fn, (*ast.CallExpr)(nil)) - return nil, nil -} - func negateDeMorgan(expr ast.Expr, recursive bool) ast.Expr { switch expr := expr.(type) { case *ast.BinaryExpr: diff --git a/quickfix/testdata/src/CheckStringsIndexByte/CheckStringsIndexByte.go b/quickfix/testdata/src/CheckStringsIndexByte/CheckStringsIndexByte.go deleted file mode 100644 index 2b9e1119e..000000000 --- a/quickfix/testdata/src/CheckStringsIndexByte/CheckStringsIndexByte.go +++ /dev/null @@ -1,22 +0,0 @@ -package pkg - -import ( - "bytes" - "strings" -) - -func fn() { - strings.Index("", "0") // want `could use strings.IndexByte instead of strings.Index` - strings.LastIndex("", "0") // want `could use strings.LastIndexByte instead of strings.LastIndex` - strings.IndexByte("", '0') // want `could use strings.Index instead of strings.IndexByte` - strings.LastIndexByte("", '0') // want `could use strings.LastIndex instead of strings.LastIndexByte` - bytes.Index(nil, []byte{'0'}) // want `could use bytes.IndexByte instead of bytes.Index` - bytes.LastIndex(nil, []byte{'0'}) // want `could use bytes.LastIndexByte instead of bytes.LastIndex` - bytes.Index(nil, []byte("0")) // want `could use bytes.IndexByte instead of bytes.Index` - - strings.Index("", "µ") - strings.Index("", "00") - strings.LastIndex("", "00") - bytes.LastIndex(nil, []byte{'0', '0'}) - bytes.Index(nil, []byte("µ")) -} diff --git a/quickfix/testdata/src/CheckStringsIndexByte/CheckStringsIndexByte.go.golden b/quickfix/testdata/src/CheckStringsIndexByte/CheckStringsIndexByte.go.golden deleted file mode 100644 index 7a8a5ec18..000000000 --- a/quickfix/testdata/src/CheckStringsIndexByte/CheckStringsIndexByte.go.golden +++ /dev/null @@ -1,22 +0,0 @@ -package pkg - -import ( - "bytes" - "strings" -) - -func fn() { - strings.IndexByte("", '0') // want `could use strings.IndexByte instead of strings.Index` - strings.LastIndexByte("", '0') // want `could use strings.LastIndexByte instead of strings.LastIndex` - strings.Index("", "0") // want `could use strings.Index instead of strings.IndexByte` - strings.LastIndex("", "0") // want `could use strings.LastIndex instead of strings.LastIndexByte` - bytes.IndexByte(nil, '0') // want `could use bytes.IndexByte instead of bytes.Index` - bytes.LastIndexByte(nil, '0') // want `could use bytes.LastIndexByte instead of bytes.LastIndex` - bytes.IndexByte(nil, '0') // want `could use bytes.IndexByte instead of bytes.Index` - - strings.Index("", "µ") - strings.Index("", "00") - strings.LastIndex("", "00") - bytes.LastIndex(nil, []byte{'0', '0'}) - bytes.Index(nil, []byte("µ")) -}