Skip to content

Commit

Permalink
quickfix: delete QF1000
Browse files Browse the repository at this point in the history
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
  • Loading branch information
dominikh committed May 24, 2021
1 parent c68974c commit 440ad19
Show file tree
Hide file tree
Showing 5 changed files with 0 additions and 161 deletions.
4 changes: 0 additions & 4 deletions quickfix/analysis.go
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down
6 changes: 0 additions & 6 deletions quickfix/doc.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
107 changes: 0 additions & 107 deletions quickfix/lint.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down

This file was deleted.

This file was deleted.

0 comments on commit 440ad19

Please sign in to comment.