Skip to content

Commit 5f9967d

Browse files
adonovangopherbot
adonovan
authored and
gopherbot
committed
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>
1 parent a1eb5fd commit 5f9967d

File tree

9 files changed

+205
-2
lines changed

9 files changed

+205
-2
lines changed

‎gopls/doc/analyzers.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -497,6 +497,8 @@ existing code by using more modern features of Go, such as:
497497
added in go1.21
498498
- replacing a 3-clause for i := 0; i < n; i++ {} loop by
499499
for i := range n {}, added in go1.22;
500+
- replacing Split in "for range strings.Split(...)" by go1.24's
501+
more efficient SplitSeq;
500502

501503
Default: on.
502504

‎gopls/internal/analysis/modernize/doc.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,4 +30,6 @@
3030
// added in go1.21
3131
// - replacing a 3-clause for i := 0; i < n; i++ {} loop by
3232
// for i := range n {}, added in go1.22;
33+
// - replacing Split in "for range strings.Split(...)" by go1.24's
34+
// more efficient SplitSeq;
3335
package modernize

‎gopls/internal/analysis/modernize/modernize.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ func run(pass *analysis.Pass) (any, error) {
7070
rangeint(pass)
7171
slicescontains(pass)
7272
slicesdelete(pass)
73+
splitseq(pass)
7374
sortslice(pass)
7475
testingContext(pass)
7576

‎gopls/internal/analysis/modernize/modernize_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ func Test(t *testing.T) {
2323
"rangeint",
2424
"slicescontains",
2525
"slicesdelete",
26+
"splitseq",
2627
"sortslice",
2728
"testingcontext",
2829
)

‎gopls/internal/analysis/modernize/splitseq.go

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
// Copyright 2025 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
package modernize
6+
7+
import (
8+
"go/ast"
9+
"go/token"
10+
"go/types"
11+
12+
"golang.org/x/tools/go/analysis"
13+
"golang.org/x/tools/go/analysis/passes/inspect"
14+
"golang.org/x/tools/go/ast/inspector"
15+
"golang.org/x/tools/go/types/typeutil"
16+
"golang.org/x/tools/internal/analysisinternal"
17+
"golang.org/x/tools/internal/astutil/edge"
18+
)
19+
20+
// splitseq offers a fix to replace a call to strings.Split with
21+
// SplitSeq when it is the operand of a range loop, either directly:
22+
//
23+
// for _, line := range strings.Split() {...}
24+
//
25+
// or indirectly, if the variable's sole use is the range statement:
26+
//
27+
// lines := strings.Split()
28+
// for _, line := range lines {...}
29+
//
30+
// Variants:
31+
// - bytes.SplitSeq
32+
func splitseq(pass *analysis.Pass) {
33+
if !analysisinternal.Imports(pass.Pkg, "strings") &&
34+
!analysisinternal.Imports(pass.Pkg, "bytes") {
35+
return
36+
}
37+
info := pass.TypesInfo
38+
inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
39+
for curFile := range filesUsing(inspect, info, "go1.24") {
40+
for curRange := range curFile.Preorder((*ast.RangeStmt)(nil)) {
41+
rng := curRange.Node().(*ast.RangeStmt)
42+
43+
// Reject "for i, line := ..." since SplitSeq is not an iter.Seq2.
44+
// (We require that i is blank.)
45+
if id, ok := rng.Key.(*ast.Ident); ok && id.Name != "_" {
46+
continue
47+
}
48+
49+
// Find the call operand of the range statement,
50+
// whether direct or indirect.
51+
call, ok := rng.X.(*ast.CallExpr)
52+
if !ok {
53+
if id, ok := rng.X.(*ast.Ident); ok {
54+
if v, ok := info.Uses[id].(*types.Var); ok {
55+
if ek, idx := curRange.Edge(); ek == edge.BlockStmt_List && idx > 0 {
56+
curPrev, _ := curRange.PrevSibling()
57+
if assign, ok := curPrev.Node().(*ast.AssignStmt); ok &&
58+
assign.Tok == token.DEFINE &&
59+
len(assign.Lhs) == 1 &&
60+
len(assign.Rhs) == 1 &&
61+
info.Defs[assign.Lhs[0].(*ast.Ident)] == v &&
62+
soleUse(info, v) == id {
63+
// Have:
64+
// lines := ...
65+
// for _, line := range lines {...}
66+
// and no other uses of lines.
67+
call, _ = assign.Rhs[0].(*ast.CallExpr)
68+
}
69+
}
70+
}
71+
}
72+
}
73+
74+
if call != nil {
75+
var edits []analysis.TextEdit
76+
if rng.Key != nil {
77+
// Delete (blank) RangeStmt.Key:
78+
// for _, line := -> for line :=
79+
// for _, _ := -> for
80+
// for _ := -> for
81+
end := rng.Range
82+
if rng.Value != nil {
83+
end = rng.Value.Pos()
84+
}
85+
edits = append(edits, analysis.TextEdit{
86+
Pos: rng.Key.Pos(),
87+
End: end,
88+
})
89+
}
90+
91+
if sel, ok := call.Fun.(*ast.SelectorExpr); ok &&
92+
(analysisinternal.IsFunctionNamed(typeutil.Callee(info, call), "strings", "Split") ||
93+
analysisinternal.IsFunctionNamed(typeutil.Callee(info, call), "bytes", "Split")) {
94+
pass.Report(analysis.Diagnostic{
95+
Pos: sel.Pos(),
96+
End: sel.End(),
97+
Category: "splitseq",
98+
Message: "Ranging over SplitSeq is more efficient",
99+
SuggestedFixes: []analysis.SuggestedFix{{
100+
Message: "Replace Split with SplitSeq",
101+
TextEdits: append(edits, analysis.TextEdit{
102+
// Split -> SplitSeq
103+
Pos: sel.Sel.Pos(),
104+
End: sel.Sel.End(),
105+
NewText: []byte("SplitSeq")}),
106+
}},
107+
})
108+
}
109+
}
110+
}
111+
}
112+
}

‎gopls/internal/analysis/modernize/testdata/src/splitseq/splitseq.go

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
//go:build go1.24
2+
3+
package splitseq
4+
5+
import (
6+
"bytes"
7+
"strings"
8+
)
9+
10+
func _() {
11+
for _, line := range strings.Split("", "") { // want "Ranging over SplitSeq is more efficient"
12+
println(line)
13+
}
14+
for i, line := range strings.Split("", "") { // nope: uses index var
15+
println(i, line)
16+
}
17+
for i, _ := range strings.Split("", "") { // nope: uses index var
18+
println(i)
19+
}
20+
for i := range strings.Split("", "") { // nope: uses index var
21+
println(i)
22+
}
23+
for _ = range strings.Split("", "") { // want "Ranging over SplitSeq is more efficient"
24+
}
25+
for range strings.Split("", "") { // want "Ranging over SplitSeq is more efficient"
26+
}
27+
for range bytes.Split(nil, nil) { // want "Ranging over SplitSeq is more efficient"
28+
}
29+
{
30+
lines := strings.Split("", "") // want "Ranging over SplitSeq is more efficient"
31+
for _, line := range lines {
32+
println(line)
33+
}
34+
}
35+
{
36+
lines := strings.Split("", "") // nope: lines is used not just by range
37+
for _, line := range lines {
38+
println(line)
39+
}
40+
println(lines)
41+
}
42+
}

‎gopls/internal/analysis/modernize/testdata/src/splitseq/splitseq.go.golden

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
//go:build go1.24
2+
3+
package splitseq
4+
5+
import (
6+
"bytes"
7+
"strings"
8+
)
9+
10+
func _() {
11+
for line := range strings.SplitSeq("", "") { // want "Ranging over SplitSeq is more efficient"
12+
println(line)
13+
}
14+
for i, line := range strings.Split("", "") { // nope: uses index var
15+
println(i, line)
16+
}
17+
for i, _ := range strings.Split("", "") { // nope: uses index var
18+
println(i)
19+
}
20+
for i := range strings.Split("", "") { // nope: uses index var
21+
println(i)
22+
}
23+
for range strings.SplitSeq("", "") { // want "Ranging over SplitSeq is more efficient"
24+
}
25+
for range strings.SplitSeq("", "") { // want "Ranging over SplitSeq is more efficient"
26+
}
27+
for range bytes.SplitSeq(nil, nil) { // want "Ranging over SplitSeq is more efficient"
28+
}
29+
{
30+
lines := strings.SplitSeq("", "") // want "Ranging over SplitSeq is more efficient"
31+
for line := range lines {
32+
println(line)
33+
}
34+
}
35+
{
36+
lines := strings.Split("", "") // nope: lines is used not just by range
37+
for _, line := range lines {
38+
println(line)
39+
}
40+
println(lines)
41+
}
42+
}

‎gopls/internal/analysis/modernize/testdata/src/splitseq/splitseq_go123.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
package splitseq

‎gopls/internal/doc/api.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -510,7 +510,7 @@
510510
},
511511
{
512512
"Name": "\"modernize\"",
513-
"Doc": "simplify code by using modern constructs\n\nThis analyzer reports opportunities for simplifying and clarifying\nexisting code by using more modern features of Go, such as:\n\n - replacing an if/else conditional assignment by a call to the\n built-in min or max functions added in go1.21;\n - replacing sort.Slice(x, func(i, j int) bool) { return s[i] \u003c s[j] }\n by a call to slices.Sort(s), added in go1.21;\n - replacing interface{} by the 'any' type added in go1.18;\n - replacing append([]T(nil), s...) by slices.Clone(s) or\n slices.Concat(s), added in go1.21;\n - replacing a loop around an m[k]=v map update by a call\n to one of the Collect, Copy, Clone, or Insert functions\n from the maps package, added in go1.21;\n - replacing []byte(fmt.Sprintf...) by fmt.Appendf(nil, ...),\n added in go1.19;\n - replacing uses of context.WithCancel in tests with t.Context, added in\n go1.24;\n - replacing omitempty by omitzero on structs, added in go1.24;\n - replacing append(s[:i], s[i+1]...) by slices.Delete(s, i, i+1),\n added in go1.21\n - replacing a 3-clause for i := 0; i \u003c n; i++ {} loop by\n for i := range n {}, added in go1.22;",
513+
"Doc": "simplify code by using modern constructs\n\nThis analyzer reports opportunities for simplifying and clarifying\nexisting code by using more modern features of Go, such as:\n\n - replacing an if/else conditional assignment by a call to the\n built-in min or max functions added in go1.21;\n - replacing sort.Slice(x, func(i, j int) bool) { return s[i] \u003c s[j] }\n by a call to slices.Sort(s), added in go1.21;\n - replacing interface{} by the 'any' type added in go1.18;\n - replacing append([]T(nil), s...) by slices.Clone(s) or\n slices.Concat(s), added in go1.21;\n - replacing a loop around an m[k]=v map update by a call\n to one of the Collect, Copy, Clone, or Insert functions\n from the maps package, added in go1.21;\n - replacing []byte(fmt.Sprintf...) by fmt.Appendf(nil, ...),\n added in go1.19;\n - replacing uses of context.WithCancel in tests with t.Context, added in\n go1.24;\n - replacing omitempty by omitzero on structs, added in go1.24;\n - replacing append(s[:i], s[i+1]...) by slices.Delete(s, i, i+1),\n added in go1.21\n - replacing a 3-clause for i := 0; i \u003c n; i++ {} loop by\n for i := range n {}, added in go1.22;\n - replacing Split in \"for range strings.Split(...)\" by go1.24's\n more efficient SplitSeq;",
514514
"Default": "true"
515515
},
516516
{
@@ -1189,7 +1189,7 @@
11891189
},
11901190
{
11911191
"Name": "modernize",
1192-
"Doc": "simplify code by using modern constructs\n\nThis analyzer reports opportunities for simplifying and clarifying\nexisting code by using more modern features of Go, such as:\n\n - replacing an if/else conditional assignment by a call to the\n built-in min or max functions added in go1.21;\n - replacing sort.Slice(x, func(i, j int) bool) { return s[i] \u003c s[j] }\n by a call to slices.Sort(s), added in go1.21;\n - replacing interface{} by the 'any' type added in go1.18;\n - replacing append([]T(nil), s...) by slices.Clone(s) or\n slices.Concat(s), added in go1.21;\n - replacing a loop around an m[k]=v map update by a call\n to one of the Collect, Copy, Clone, or Insert functions\n from the maps package, added in go1.21;\n - replacing []byte(fmt.Sprintf...) by fmt.Appendf(nil, ...),\n added in go1.19;\n - replacing uses of context.WithCancel in tests with t.Context, added in\n go1.24;\n - replacing omitempty by omitzero on structs, added in go1.24;\n - replacing append(s[:i], s[i+1]...) by slices.Delete(s, i, i+1),\n added in go1.21\n - replacing a 3-clause for i := 0; i \u003c n; i++ {} loop by\n for i := range n {}, added in go1.22;",
1192+
"Doc": "simplify code by using modern constructs\n\nThis analyzer reports opportunities for simplifying and clarifying\nexisting code by using more modern features of Go, such as:\n\n - replacing an if/else conditional assignment by a call to the\n built-in min or max functions added in go1.21;\n - replacing sort.Slice(x, func(i, j int) bool) { return s[i] \u003c s[j] }\n by a call to slices.Sort(s), added in go1.21;\n - replacing interface{} by the 'any' type added in go1.18;\n - replacing append([]T(nil), s...) by slices.Clone(s) or\n slices.Concat(s), added in go1.21;\n - replacing a loop around an m[k]=v map update by a call\n to one of the Collect, Copy, Clone, or Insert functions\n from the maps package, added in go1.21;\n - replacing []byte(fmt.Sprintf...) by fmt.Appendf(nil, ...),\n added in go1.19;\n - replacing uses of context.WithCancel in tests with t.Context, added in\n go1.24;\n - replacing omitempty by omitzero on structs, added in go1.24;\n - replacing append(s[:i], s[i+1]...) by slices.Delete(s, i, i+1),\n added in go1.21\n - replacing a 3-clause for i := 0; i \u003c n; i++ {} loop by\n for i := range n {}, added in go1.22;\n - replacing Split in \"for range strings.Split(...)\" by go1.24's\n more efficient SplitSeq;",
11931193
"URL": "https://pkg.go.dev/golang.org/x/tools/gopls/internal/analysis/modernize",
11941194
"Default": true
11951195
},

0 commit comments

Comments
 (0)