go/analysis/passes/modernize: use strings.Contains in strings.Cut modernizer
This CL is a follow-up to the strings.Cut modernizer, which offers to replace instances of strings.Index, strings.IndexByte, bytes.Index, and bytes.IndexByte, with either strings.Cut or bytes.Cut.
When the index is used only to check for the presence of the substring or byte slice, a suggested fix would look like:
_, _, ok := strings.Cut(s, substr)
Instead, the analyzer suggests the use of strings.Contains which avoids the blank assignments for unused variables:
found := strings.Contains(s, substr)
Change-Id: Idf50116812e2056f2fa973ef08c87672a2e86a47
Reviewed-on: https://go-review.googlesource.com/c/tools/+/718760
Reviewed-by: Alan Donovan <adonovan@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
diff --git a/go/analysis/passes/modernize/doc.go b/go/analysis/passes/modernize/doc.go
index 04440e7..7469002 100644
--- a/go/analysis/passes/modernize/doc.go
+++ b/go/analysis/passes/modernize/doc.go
@@ -351,6 +351,20 @@
return before
}
+And:
+
+ idx := strings.Index(s, substr)
+ if idx >= 0 {
+ return
+ }
+
+is replaced by:
+
+ found := strings.Contains(s, substr)
+ if found {
+ return
+ }
+
It also handles variants using [strings.IndexByte] instead of Index, or the bytes package instead of strings.
Fixes are offered only in cases in which there are no potential modifications of the idx, s, or substr expressions between their definition and use.
diff --git a/go/analysis/passes/modernize/stringscut.go b/go/analysis/passes/modernize/stringscut.go
index 1342965..598920d 100644
--- a/go/analysis/passes/modernize/stringscut.go
+++ b/go/analysis/passes/modernize/stringscut.go
@@ -45,9 +45,10 @@
goplsexport.StringsCutModernizer = stringscutAnalyzer
}
-// stringscut offers a fix to replace an occurrence of strings.Index, strings.IndexByte, bytes.Index,
-// or bytes.IndexByte with strings.Cut or bytes.Cut.
-// Consider some candidate for replacement i := strings.Index(s, substr). The following must hold for a replacement to occur:
+// stringscut offers a fix to replace an occurrence of strings.Index{,Byte} with
+// strings.{Cut,Contains}, and similar fixes for functions in the bytes package.
+// Consider some candidate for replacement i := strings.Index(s, substr).
+// The following must hold for a replacement to occur:
//
// 1. All instances of i and s must be in one of these forms.
// Binary expressions:
@@ -91,6 +92,23 @@
// If the condition is negated (e.g. establishes `i < 0`), we use `if !ok` instead.
// If the slices of `s` match `s[:i]` or `s[i+len(substr):]` or their variants listed above,
// then we replace them with before and after.
+//
+// When the index `i` is used only to check for the presence of the substring or byte slice,
+// the suggested fix uses Contains() instead of Cut.
+//
+// For example:
+//
+// i := strings.Index(s, substr)
+// if i >= 0 {
+// return
+// }
+//
+// Would become:
+//
+// found := strings.Contains(s, substr)
+// if found {
+// return
+// }
func stringscut(pass *analysis.Pass) (any, error) {
skipGenerated(pass)
var (
@@ -172,16 +190,14 @@
}
// If the only uses are ok and !ok, don't suggest a Cut() fix - these should be using Contains()
- // TODO(mkalil): implement text edits for strings.Contains
- if (len(lessZero) > 0 || len(greaterNegOne) > 0) && len(beforeSlice) == 0 && len(afterSlice) == 0 {
- continue
- }
+ isContains := (len(lessZero) > 0 || len(greaterNegOne) > 0) && len(beforeSlice) == 0 && len(afterSlice) == 0
scope := iObj.Parent()
var (
okVarName = refactor.FreshName(scope, iIdent.Pos(), "ok")
beforeVarName = refactor.FreshName(scope, iIdent.Pos(), "before")
afterVarName = refactor.FreshName(scope, iIdent.Pos(), "after")
+ foundVarName = refactor.FreshName(scope, iIdent.Pos(), "found") // for Contains()
)
// If there will be no uses of ok, before, or after, use the
@@ -206,30 +222,53 @@
})
}
}
-
- replace(lessZero, "!"+okVarName) // idx < 0 -> !ok
- replace(greaterNegOne, okVarName) // idx > -1 -> ok
- replace(beforeSlice, beforeVarName) // s[:idx] -> before
- replace(afterSlice, afterVarName) // s[idx+k:] -> after
-
// Get the ident for the call to strings.Index, which could just be
// "Index" if the strings package is dot imported.
indexCallId := typesinternal.UsedIdent(info, indexCall.Fun)
- // Replace the call to Index or IndexByte with a call to Cut.
- edits = append(edits, analysis.TextEdit{
- Pos: indexCallId.Pos(),
- End: indexCallId.End(),
- NewText: []byte("Cut"),
- })
+ replacedFunc := "Cut"
+ if isContains {
+ replacedFunc = "Contains"
+ replace(lessZero, "!"+foundVarName) // idx < 0 -> !found
+ replace(greaterNegOne, foundVarName) // idx > -1 -> found
- // Replace the assignment with before, after, ok.
- edits = append(edits, analysis.TextEdit{
- Pos: iIdent.Pos(),
- End: iIdent.End(),
- NewText: fmt.Appendf(nil, "%s, %s, %s", beforeVarName, afterVarName, okVarName),
- })
+ // Replace the assignment with found, and replace the call to
+ // Index or IndexByte with a call to Contains.
+ // i := strings.Index (...)
+ // ----- --------
+ // found := strings.Contains(...)
+ edits = append(edits, analysis.TextEdit{
+ Pos: iIdent.Pos(),
+ End: iIdent.End(),
+ NewText: []byte(foundVarName),
+ }, analysis.TextEdit{
+ Pos: indexCallId.Pos(),
+ End: indexCallId.End(),
+ NewText: []byte("Contains"),
+ })
+ } else {
+ replace(lessZero, "!"+okVarName) // idx < 0 -> !ok
+ replace(greaterNegOne, okVarName) // idx > -1 -> ok
+ replace(beforeSlice, beforeVarName) // s[:idx] -> before
+ replace(afterSlice, afterVarName) // s[idx+k:] -> after
+
+ // Replace the assignment with before, after, ok, and replace
+ // the call to Index or IndexByte with a call to Cut.
+ // i := strings.Index(...)
+ // ----------------- -----
+ // before, after, ok := strings.Cut (...)
+ edits = append(edits, analysis.TextEdit{
+ Pos: iIdent.Pos(),
+ End: iIdent.End(),
+ NewText: fmt.Appendf(nil, "%s, %s, %s", beforeVarName, afterVarName, okVarName),
+ }, analysis.TextEdit{
+ Pos: indexCallId.Pos(),
+ End: indexCallId.End(),
+ NewText: []byte("Cut"),
+ })
+ }
+
// Calls to IndexByte have a byte as their second arg, which
- // must be converted to a string or []byte to be a valid arg for Cut.
+ // must be converted to a string or []byte to be a valid arg for Cut/Contains.
if obj.Name() == "IndexByte" {
switch obj.Pkg().Name() {
case "strings":
@@ -250,7 +289,7 @@
} else {
// substr is a byte constant
val, _ := constant.Int64Val(searchByteVal) // inv: must be a valid byte
- // strings.Cut requires a string, so convert byte literal to string literal; e.g. 'a' -> "a", 55 -> "7"
+ // strings.Cut/Contains requires a string, so convert byte literal to string literal; e.g. 'a' -> "a", 55 -> "7"
edits = append(edits, analysis.TextEdit{
Pos: substr.Pos(),
End: substr.End(),
@@ -258,7 +297,7 @@
})
}
case "bytes":
- // bytes.Cut requires a []byte, so wrap substr in a []byte{}
+ // bytes.Cut/Contains requires a []byte, so wrap substr in a []byte{}
edits = append(edits, []analysis.TextEdit{
{
Pos: substr.Pos(),
@@ -274,11 +313,11 @@
pass.Report(analysis.Diagnostic{
Pos: indexCall.Fun.Pos(),
End: indexCall.Fun.End(),
- Message: fmt.Sprintf("%s.%s can be simplified using %s.Cut",
- obj.Pkg().Name(), obj.Name(), obj.Pkg().Name()),
+ Message: fmt.Sprintf("%s.%s can be simplified using %s.%s",
+ obj.Pkg().Name(), obj.Name(), obj.Pkg().Name(), replacedFunc),
Category: "stringscut",
SuggestedFixes: []analysis.SuggestedFix{{
- Message: fmt.Sprintf("Simplify %s.%s call using %s.Cut", obj.Pkg().Name(), obj.Name(), obj.Pkg().Name()),
+ Message: fmt.Sprintf("Simplify %s.%s call using %s.%s", obj.Pkg().Name(), obj.Name(), obj.Pkg().Name(), replacedFunc),
TextEdits: edits,
}},
})
diff --git a/go/analysis/passes/modernize/testdata/src/stringscut/stringscut.go b/go/analysis/passes/modernize/testdata/src/stringscut/stringscut.go
index 6b9aef1..85fb82a 100644
--- a/go/analysis/passes/modernize/testdata/src/stringscut/stringscut.go
+++ b/go/analysis/passes/modernize/testdata/src/stringscut/stringscut.go
@@ -14,10 +14,33 @@
func basic_contains(s string) bool {
s = "reassigned"
- i := strings.Index(s, "=") // don't modernize for now - since only use is "ok", this should be strings.Contains
+ i := strings.Index(s, "=") // want "strings.Index can be simplified using strings.Contains"
return i >= 0
}
+func contains_variety(s, sub string) {
+ i := strings.Index(s, sub) // want "strings.Index can be simplified using strings.Contains"
+ if i >= 0 {
+ print("found")
+ }
+ if i < 0 {
+ print("not found")
+ }
+ if i <= -1 {
+ print("not found")
+ }
+}
+
+func basic_contains_bytes(s string) bool {
+ i := strings.IndexByte(s, '=') // want "strings.IndexByte can be simplified using strings.Contains"
+ return i < 0
+}
+
+func basic_contains_bytes_byte(s []byte) bool {
+ i := bytes.IndexByte(s, 22) // want "bytes.IndexByte can be simplified using bytes.Contains"
+ return i < 0
+}
+
func skip_var_decl(s string) bool {
var i int
i = strings.Index(s, "=") // don't modernize - i might be reassigned
diff --git a/go/analysis/passes/modernize/testdata/src/stringscut/stringscut.go.golden b/go/analysis/passes/modernize/testdata/src/stringscut/stringscut.go.golden
index 75b778e..c3ddd69 100644
--- a/go/analysis/passes/modernize/testdata/src/stringscut/stringscut.go.golden
+++ b/go/analysis/passes/modernize/testdata/src/stringscut/stringscut.go.golden
@@ -14,8 +14,31 @@
func basic_contains(s string) bool {
s = "reassigned"
- i := strings.Index(s, "=") // don't modernize for now - since only use is "ok", this should be strings.Contains
- return i >= 0
+ found := strings.Contains(s, "=") // want "strings.Index can be simplified using strings.Contains"
+ return found
+}
+
+func contains_variety(s, sub string) {
+ found := strings.Contains(s, sub) // want "strings.Index can be simplified using strings.Contains"
+ if found {
+ print("found")
+ }
+ if !found {
+ print("not found")
+ }
+ if !found {
+ print("not found")
+ }
+}
+
+func basic_contains_bytes(s string) bool {
+ found := strings.Contains(s, "=") // want "strings.IndexByte can be simplified using strings.Contains"
+ return !found
+}
+
+func basic_contains_bytes_byte(s []byte) bool {
+ found := bytes.Contains(s, []byte{22}) // want "bytes.IndexByte can be simplified using bytes.Contains"
+ return !found
}
func skip_var_decl(s string) bool {
diff --git a/gopls/doc/analyzers.md b/gopls/doc/analyzers.md
index 16151a5..dfd5f00 100644
--- a/gopls/doc/analyzers.md
+++ b/gopls/doc/analyzers.md
@@ -4063,6 +4063,20 @@
return before
}
+And:
+
+ idx := strings.Index(s, substr)
+ if idx >= 0 {
+ return
+ }
+
+is replaced by:
+
+ found := strings.Contains(s, substr)
+ if found {
+ return
+ }
+
It also handles variants using [strings.IndexByte](/strings#IndexByte) instead of Index, or the bytes package instead of strings.
Fixes are offered only in cases in which there are no potential modifications of the idx, s, or substr expressions between their definition and use.
diff --git a/gopls/internal/doc/api.json b/gopls/internal/doc/api.json
index 5fb02a1..66ab0ce 100644
--- a/gopls/internal/doc/api.json
+++ b/gopls/internal/doc/api.json
@@ -1708,7 +1708,7 @@
},
{
"Name": "\"stringscut\"",
- "Doc": "replace strings.Index etc. with strings.Cut\n\nThis analyzer replaces certain patterns of use of [strings.Index] and string slicing by [strings.Cut], added in go1.18.\n\nFor example:\n\n\tidx := strings.Index(s, substr)\n\tif idx \u003e= 0 {\n\t return s[:idx]\n\t}\n\nis replaced by:\n\n\tbefore, _, ok := strings.Cut(s, substr)\n\tif ok {\n\t return before\n\t}\n\nIt also handles variants using [strings.IndexByte] instead of Index, or the bytes package instead of strings.\n\nFixes are offered only in cases in which there are no potential modifications of the idx, s, or substr expressions between their definition and use.",
+ "Doc": "replace strings.Index etc. with strings.Cut\n\nThis analyzer replaces certain patterns of use of [strings.Index] and string slicing by [strings.Cut], added in go1.18.\n\nFor example:\n\n\tidx := strings.Index(s, substr)\n\tif idx \u003e= 0 {\n\t return s[:idx]\n\t}\n\nis replaced by:\n\n\tbefore, _, ok := strings.Cut(s, substr)\n\tif ok {\n\t return before\n\t}\n\nAnd:\n\n\tidx := strings.Index(s, substr)\n\tif idx \u003e= 0 {\n\t return\n\t}\n\nis replaced by:\n\n\tfound := strings.Contains(s, substr)\n\tif found {\n\t return\n\t}\n\nIt also handles variants using [strings.IndexByte] instead of Index, or the bytes package instead of strings.\n\nFixes are offered only in cases in which there are no potential modifications of the idx, s, or substr expressions between their definition and use.",
"Default": "true",
"Status": ""
},
@@ -3611,7 +3611,7 @@
},
{
"Name": "stringscut",
- "Doc": "replace strings.Index etc. with strings.Cut\n\nThis analyzer replaces certain patterns of use of [strings.Index] and string slicing by [strings.Cut], added in go1.18.\n\nFor example:\n\n\tidx := strings.Index(s, substr)\n\tif idx \u003e= 0 {\n\t return s[:idx]\n\t}\n\nis replaced by:\n\n\tbefore, _, ok := strings.Cut(s, substr)\n\tif ok {\n\t return before\n\t}\n\nIt also handles variants using [strings.IndexByte] instead of Index, or the bytes package instead of strings.\n\nFixes are offered only in cases in which there are no potential modifications of the idx, s, or substr expressions between their definition and use.",
+ "Doc": "replace strings.Index etc. with strings.Cut\n\nThis analyzer replaces certain patterns of use of [strings.Index] and string slicing by [strings.Cut], added in go1.18.\n\nFor example:\n\n\tidx := strings.Index(s, substr)\n\tif idx \u003e= 0 {\n\t return s[:idx]\n\t}\n\nis replaced by:\n\n\tbefore, _, ok := strings.Cut(s, substr)\n\tif ok {\n\t return before\n\t}\n\nAnd:\n\n\tidx := strings.Index(s, substr)\n\tif idx \u003e= 0 {\n\t return\n\t}\n\nis replaced by:\n\n\tfound := strings.Contains(s, substr)\n\tif found {\n\t return\n\t}\n\nIt also handles variants using [strings.IndexByte] instead of Index, or the bytes package instead of strings.\n\nFixes are offered only in cases in which there are no potential modifications of the idx, s, or substr expressions between their definition and use.",
"URL": "https://pkg.go.dev/golang.org/x/tools/gopls/internal/analysis/modernize#stringscut",
"Default": true
},
