-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
gopls/internal/analysis/modernize: strings.Split -> SplitSeq
This CL defines a modernizer for calls to strings.Split and bytes.Split, that offers a fix to instead use go1.24's SplitSeq, which avoids allocating an array. The fix is offered only if Split is used as the operand of a range statement, either directly, or indirectly via a variable whose sole use is the range statement. + tests Change-Id: I7c6c128d21ccf7f8b3c7745538177d2d162f62de Reviewed-on: https://go-review.googlesource.com/c/tools/+/647438 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Jonathan Amsterdam <jba@google.com> Auto-Submit: Alan Donovan <adonovan@google.com>
- Loading branch information
Showing
9 changed files
with
205 additions
and
2 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,112 @@ | ||
| // Copyright 2025 The Go Authors. All rights reserved. | ||
| // Use of this source code is governed by a BSD-style | ||
| // license that can be found in the LICENSE file. | ||
|
|
||
| package modernize | ||
|
|
||
| import ( | ||
| "go/ast" | ||
| "go/token" | ||
| "go/types" | ||
|
|
||
| "golang.org/x/tools/go/analysis" | ||
| "golang.org/x/tools/go/analysis/passes/inspect" | ||
| "golang.org/x/tools/go/ast/inspector" | ||
| "golang.org/x/tools/go/types/typeutil" | ||
| "golang.org/x/tools/internal/analysisinternal" | ||
| "golang.org/x/tools/internal/astutil/edge" | ||
| ) | ||
|
|
||
| // splitseq offers a fix to replace a call to strings.Split with | ||
| // SplitSeq when it is the operand of a range loop, either directly: | ||
| // | ||
| // for _, line := range strings.Split() {...} | ||
| // | ||
| // or indirectly, if the variable's sole use is the range statement: | ||
| // | ||
| // lines := strings.Split() | ||
| // for _, line := range lines {...} | ||
| // | ||
| // Variants: | ||
| // - bytes.SplitSeq | ||
| func splitseq(pass *analysis.Pass) { | ||
| if !analysisinternal.Imports(pass.Pkg, "strings") && | ||
| !analysisinternal.Imports(pass.Pkg, "bytes") { | ||
| return | ||
| } | ||
| info := pass.TypesInfo | ||
| inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector) | ||
| for curFile := range filesUsing(inspect, info, "go1.24") { | ||
| for curRange := range curFile.Preorder((*ast.RangeStmt)(nil)) { | ||
| rng := curRange.Node().(*ast.RangeStmt) | ||
|
|
||
| // Reject "for i, line := ..." since SplitSeq is not an iter.Seq2. | ||
| // (We require that i is blank.) | ||
| if id, ok := rng.Key.(*ast.Ident); ok && id.Name != "_" { | ||
| continue | ||
| } | ||
|
|
||
| // Find the call operand of the range statement, | ||
| // whether direct or indirect. | ||
| call, ok := rng.X.(*ast.CallExpr) | ||
| if !ok { | ||
| if id, ok := rng.X.(*ast.Ident); ok { | ||
| if v, ok := info.Uses[id].(*types.Var); ok { | ||
| if ek, idx := curRange.Edge(); ek == edge.BlockStmt_List && idx > 0 { | ||
| curPrev, _ := curRange.PrevSibling() | ||
| if assign, ok := curPrev.Node().(*ast.AssignStmt); ok && | ||
| assign.Tok == token.DEFINE && | ||
| len(assign.Lhs) == 1 && | ||
| len(assign.Rhs) == 1 && | ||
| info.Defs[assign.Lhs[0].(*ast.Ident)] == v && | ||
| soleUse(info, v) == id { | ||
| // Have: | ||
| // lines := ... | ||
| // for _, line := range lines {...} | ||
| // and no other uses of lines. | ||
| call, _ = assign.Rhs[0].(*ast.CallExpr) | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if call != nil { | ||
| var edits []analysis.TextEdit | ||
| if rng.Key != nil { | ||
| // Delete (blank) RangeStmt.Key: | ||
| // for _, line := -> for line := | ||
| // for _, _ := -> for | ||
| // for _ := -> for | ||
| end := rng.Range | ||
| if rng.Value != nil { | ||
| end = rng.Value.Pos() | ||
| } | ||
| edits = append(edits, analysis.TextEdit{ | ||
| Pos: rng.Key.Pos(), | ||
| End: end, | ||
| }) | ||
| } | ||
|
|
||
| if sel, ok := call.Fun.(*ast.SelectorExpr); ok && | ||
| (analysisinternal.IsFunctionNamed(typeutil.Callee(info, call), "strings", "Split") || | ||
| analysisinternal.IsFunctionNamed(typeutil.Callee(info, call), "bytes", "Split")) { | ||
| pass.Report(analysis.Diagnostic{ | ||
| Pos: sel.Pos(), | ||
| End: sel.End(), | ||
| Category: "splitseq", | ||
| Message: "Ranging over SplitSeq is more efficient", | ||
| SuggestedFixes: []analysis.SuggestedFix{{ | ||
| Message: "Replace Split with SplitSeq", | ||
| TextEdits: append(edits, analysis.TextEdit{ | ||
| // Split -> SplitSeq | ||
| Pos: sel.Sel.Pos(), | ||
| End: sel.Sel.End(), | ||
| NewText: []byte("SplitSeq")}), | ||
| }}, | ||
| }) | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } |
42 changes: 42 additions & 0 deletions
42
gopls/internal/analysis/modernize/testdata/src/splitseq/splitseq.go
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| //go:build go1.24 | ||
|
|
||
| package splitseq | ||
|
|
||
| import ( | ||
| "bytes" | ||
| "strings" | ||
| ) | ||
|
|
||
| func _() { | ||
| for _, line := range strings.Split("", "") { // want "Ranging over SplitSeq is more efficient" | ||
| println(line) | ||
| } | ||
| for i, line := range strings.Split("", "") { // nope: uses index var | ||
| println(i, line) | ||
| } | ||
| for i, _ := range strings.Split("", "") { // nope: uses index var | ||
| println(i) | ||
| } | ||
| for i := range strings.Split("", "") { // nope: uses index var | ||
| println(i) | ||
| } | ||
| for _ = range strings.Split("", "") { // want "Ranging over SplitSeq is more efficient" | ||
| } | ||
| for range strings.Split("", "") { // want "Ranging over SplitSeq is more efficient" | ||
| } | ||
| for range bytes.Split(nil, nil) { // want "Ranging over SplitSeq is more efficient" | ||
| } | ||
| { | ||
| lines := strings.Split("", "") // want "Ranging over SplitSeq is more efficient" | ||
| for _, line := range lines { | ||
| println(line) | ||
| } | ||
| } | ||
| { | ||
| lines := strings.Split("", "") // nope: lines is used not just by range | ||
| for _, line := range lines { | ||
| println(line) | ||
| } | ||
| println(lines) | ||
| } | ||
| } |
42 changes: 42 additions & 0 deletions
42
gopls/internal/analysis/modernize/testdata/src/splitseq/splitseq.go.golden
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| //go:build go1.24 | ||
|
|
||
| package splitseq | ||
|
|
||
| import ( | ||
| "bytes" | ||
| "strings" | ||
| ) | ||
|
|
||
| func _() { | ||
| for line := range strings.SplitSeq("", "") { // want "Ranging over SplitSeq is more efficient" | ||
| println(line) | ||
| } | ||
| for i, line := range strings.Split("", "") { // nope: uses index var | ||
| println(i, line) | ||
| } | ||
| for i, _ := range strings.Split("", "") { // nope: uses index var | ||
| println(i) | ||
| } | ||
| for i := range strings.Split("", "") { // nope: uses index var | ||
| println(i) | ||
| } | ||
| for range strings.SplitSeq("", "") { // want "Ranging over SplitSeq is more efficient" | ||
| } | ||
| for range strings.SplitSeq("", "") { // want "Ranging over SplitSeq is more efficient" | ||
| } | ||
| for range bytes.SplitSeq(nil, nil) { // want "Ranging over SplitSeq is more efficient" | ||
| } | ||
| { | ||
| lines := strings.SplitSeq("", "") // want "Ranging over SplitSeq is more efficient" | ||
| for line := range lines { | ||
| println(line) | ||
| } | ||
| } | ||
| { | ||
| lines := strings.Split("", "") // nope: lines is used not just by range | ||
| for _, line := range lines { | ||
| println(line) | ||
| } | ||
| println(lines) | ||
| } | ||
| } |
1 change: 1 addition & 0 deletions
1
gopls/internal/analysis/modernize/testdata/src/splitseq/splitseq_go123.go
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| package splitseq |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters