Skip to content

Commit bf4db91

Browse files
adonovangopherbot
authored andcommitted
gopls/internal/analysis/modernize: for i := 0; i < n; i++ -> range n
This CL adds a modernizer for 3-clause for loops that offers a fix to replace them with go1.22 "range n" loops. Updates golang/go#70815 Change-Id: I347179b8a308f380a9a894aa811ced66f7605df1 Reviewed-on: https://go-review.googlesource.com/c/tools/+/646916 Auto-Submit: Alan Donovan <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]> Reviewed-by: Robert Findley <[email protected]>
1 parent 73edff8 commit bf4db91

File tree

10 files changed

+253
-5
lines changed

10 files changed

+253
-5
lines changed

go/ast/inspector/inspector.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -279,6 +279,6 @@ func (v *visitor) pop(node ast.Node) {
279279
node: node,
280280
typ: current.typAccum,
281281
index: current.index,
282-
parent: int32(current.edgeKindAndIndex), // see [unpackEdgeKindAndIndex]
282+
parent: current.edgeKindAndIndex, // see [unpackEdgeKindAndIndex]
283283
})
284284
}

gopls/doc/analyzers.md

+3-1
Original file line numberDiff line numberDiff line change
@@ -493,9 +493,11 @@ existing code by using more modern features of Go, such as:
493493
added in go1.19;
494494
- replacing uses of context.WithCancel in tests with t.Context, added in
495495
go1.24;
496-
- replacing omitempty by omitzero on structs, added in go 1.24;
496+
- replacing omitempty by omitzero on structs, added in go1.24;
497497
- replacing append(s[:i], s[i+1]...) by slices.Delete(s, i, i+1),
498498
added in go1.21
499+
- replacing a 3-clause for i := 0; i < n; i++ {} loop by
500+
for i := range n {}, added in go1.22;
499501

500502
Default: on.
501503

gopls/internal/analysis/modernize/doc.go

+3-1
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,9 @@
2525
// added in go1.19;
2626
// - replacing uses of context.WithCancel in tests with t.Context, added in
2727
// go1.24;
28-
// - replacing omitempty by omitzero on structs, added in go 1.24;
28+
// - replacing omitempty by omitzero on structs, added in go1.24;
2929
// - replacing append(s[:i], s[i+1]...) by slices.Delete(s, i, i+1),
3030
// added in go1.21
31+
// - replacing a 3-clause for i := 0; i < n; i++ {} loop by
32+
// for i := range n {}, added in go1.22;
3133
package modernize

gopls/internal/analysis/modernize/modernize.go

+1
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ func run(pass *analysis.Pass) (any, error) {
6767
mapsloop(pass)
6868
minmax(pass)
6969
omitzero(pass)
70+
rangeint(pass)
7071
slicescontains(pass)
7172
slicesdelete(pass)
7273
sortslice(pass)

gopls/internal/analysis/modernize/modernize_test.go

+1
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ func Test(t *testing.T) {
2020
"mapsloop",
2121
"minmax",
2222
"omitzero",
23+
"rangeint",
2324
"slicescontains",
2425
"slicesdelete",
2526
"sortslice",
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,163 @@
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+
"fmt"
9+
"go/ast"
10+
"go/token"
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/internal/analysisinternal"
16+
"golang.org/x/tools/internal/astutil/cursor"
17+
"golang.org/x/tools/internal/astutil/edge"
18+
)
19+
20+
// rangeint offers a fix to replace a 3-clause 'for' loop:
21+
//
22+
// for i := 0; i < limit; i++ {}
23+
//
24+
// by a range loop with an integer operand:
25+
//
26+
// for i := range limit {}
27+
//
28+
// Variants:
29+
// - The ':=' may be replaced by '='.
30+
// - The fix may remove "i :=" if it would become unused.
31+
//
32+
// Restrictions:
33+
// - The variable i must not be assigned or address-taken within the
34+
// loop, because a "for range int" loop does not respect assignments
35+
// to the loop index.
36+
// - The limit must not be b.N, to avoid redundancy with bloop's fixes.
37+
//
38+
// Caveats:
39+
// - The fix will cause the limit expression to be evaluated exactly
40+
// once, instead of once per iteration. The limit may be a function call
41+
// (e.g. seq.Len()). The fix may change the cardinality of side effects.
42+
func rangeint(pass *analysis.Pass) {
43+
info := pass.TypesInfo
44+
45+
inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
46+
for curFile := range filesUsing(inspect, info, "go1.22") {
47+
nextLoop:
48+
for curLoop := range curFile.Preorder((*ast.ForStmt)(nil)) {
49+
loop := curLoop.Node().(*ast.ForStmt)
50+
if init, ok := loop.Init.(*ast.AssignStmt); ok &&
51+
isSimpleAssign(init) &&
52+
is[*ast.Ident](init.Lhs[0]) &&
53+
isZeroLiteral(init.Rhs[0]) {
54+
// Have: for i = 0; ... (or i := 0)
55+
index := init.Lhs[0].(*ast.Ident)
56+
57+
if compare, ok := loop.Cond.(*ast.BinaryExpr); ok &&
58+
compare.Op == token.LSS &&
59+
equalSyntax(compare.X, init.Lhs[0]) {
60+
// Have: for i = 0; i < limit; ... {}
61+
limit := compare.Y
62+
63+
// Skip loops up to b.N in benchmarks; see [bloop].
64+
if sel, ok := limit.(*ast.SelectorExpr); ok &&
65+
sel.Sel.Name == "N" &&
66+
analysisinternal.IsPointerToNamed(info.TypeOf(sel.X), "testing", "B") {
67+
continue // skip b.N
68+
}
69+
70+
if inc, ok := loop.Post.(*ast.IncDecStmt); ok &&
71+
inc.Tok == token.INC &&
72+
equalSyntax(compare.X, inc.X) {
73+
// Have: for i = 0; i < limit; i++ {}
74+
75+
// Find references to i within the loop body.
76+
v := info.Defs[index]
77+
used := false
78+
for curId := range curLoop.Child(loop.Body).Preorder((*ast.Ident)(nil)) {
79+
id := curId.Node().(*ast.Ident)
80+
if info.Uses[id] == v {
81+
used = true
82+
83+
// Reject if any is an l-value (assigned or address-taken):
84+
// a "for range int" loop does not respect assignments to
85+
// the loop variable.
86+
if isScalarLvalue(curId) {
87+
continue nextLoop
88+
}
89+
}
90+
}
91+
92+
// If i is no longer used, delete "i := ".
93+
var edits []analysis.TextEdit
94+
if !used && init.Tok == token.DEFINE {
95+
edits = append(edits, analysis.TextEdit{
96+
Pos: index.Pos(),
97+
End: init.Rhs[0].Pos(),
98+
})
99+
}
100+
101+
pass.Report(analysis.Diagnostic{
102+
Pos: init.Pos(),
103+
End: inc.End(),
104+
Category: "rangeint",
105+
Message: "for loop can be modernized using range over int",
106+
SuggestedFixes: []analysis.SuggestedFix{{
107+
Message: fmt.Sprintf("Replace for loop with range %s",
108+
analysisinternal.Format(pass.Fset, limit)),
109+
TextEdits: append(edits, []analysis.TextEdit{
110+
// for i := 0; i < limit; i++ {}
111+
// ----- ---
112+
// -------
113+
// for i := range limit {}
114+
{
115+
Pos: init.Rhs[0].Pos(),
116+
End: limit.Pos(),
117+
NewText: []byte("range "),
118+
},
119+
{
120+
Pos: limit.End(),
121+
End: inc.End(),
122+
},
123+
}...),
124+
}},
125+
})
126+
}
127+
}
128+
}
129+
}
130+
}
131+
}
132+
133+
// isScalarLvalue reports whether the specified identifier is
134+
// address-taken or appears on the left side of an assignment.
135+
//
136+
// This function is valid only for scalars (x = ...),
137+
// not for aggregates (x.a[i] = ...)
138+
func isScalarLvalue(curId cursor.Cursor) bool {
139+
// Unfortunately we can't simply use info.Types[e].Assignable()
140+
// as it is always true for a variable even when that variable is
141+
// used only as an r-value. So we must inspect enclosing syntax.
142+
143+
cur := curId
144+
145+
// Strip enclosing parens.
146+
ek, _ := cur.Edge()
147+
for ek == edge.ParenExpr_X {
148+
cur = cur.Parent()
149+
ek, _ = cur.Edge()
150+
}
151+
152+
switch ek {
153+
case edge.AssignStmt_Lhs:
154+
return true // i = j
155+
case edge.IncDecStmt_X:
156+
return true // i++, i--
157+
case edge.UnaryExpr_X:
158+
if cur.Parent().Node().(*ast.UnaryExpr).Op == token.AND {
159+
return true // &i
160+
}
161+
}
162+
return false
163+
}

gopls/internal/analysis/modernize/slicescontains.go

+5
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,11 @@ func slicescontains(pass *analysis.Pass) {
179179
}
180180

181181
// Last statement of body must return/break out of the loop.
182+
//
183+
// TODO(adonovan): opt:consider avoiding FindNode with new API of form:
184+
// curRange.Get(edge.RangeStmt_Body, -1).
185+
// Get(edge.BodyStmt_List, 0).
186+
// Get(edge.IfStmt_Body)
182187
curBody, _ := curRange.FindNode(body)
183188
curLastStmt, _ := curBody.LastChild()
184189

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
package rangeint
2+
3+
func _(i int, s struct{ i int }) {
4+
for i := 0; i < 10; i++ { // want "for loop can be modernized using range over int"
5+
println(i)
6+
}
7+
for i = 0; i < f(); i++ { // want "for loop can be modernized using range over int"
8+
}
9+
for i := 0; i < 10; i++ { // want "for loop can be modernized using range over int"
10+
// i unused within loop
11+
}
12+
13+
// nope
14+
for i := 0; i < 10; { // nope: missing increment
15+
}
16+
for i := 0; i < 10; i-- { // nope: negative increment
17+
}
18+
for i := 0; ; i++ { // nope: missing comparison
19+
}
20+
for i := 0; i <= 10; i++ { // nope: wrong comparison
21+
}
22+
for ; i < 10; i++ { // nope: missing init
23+
}
24+
for s.i = 0; s.i < 10; s.i++ { // nope: not an ident
25+
}
26+
for i := 0; i < 10; i++ { // nope: takes address of i
27+
println(&i)
28+
}
29+
for i := 0; i < 10; i++ { // nope: increments i
30+
i++
31+
}
32+
for i := 0; i < 10; i++ { // nope: assigns i
33+
i = 8
34+
}
35+
}
36+
37+
func f() int { return 0 }
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
package rangeint
2+
3+
func _(i int, s struct{ i int }) {
4+
for i := range 10 { // want "for loop can be modernized using range over int"
5+
println(i)
6+
}
7+
for i = range f() { // want "for loop can be modernized using range over int"
8+
}
9+
for range 10 { // want "for loop can be modernized using range over int"
10+
// i unused within loop
11+
}
12+
13+
// nope
14+
for i := 0; i < 10; { // nope: missing increment
15+
}
16+
for i := 0; i < 10; i-- { // nope: negative increment
17+
}
18+
for i := 0; ; i++ { // nope: missing comparison
19+
}
20+
for i := 0; i <= 10; i++ { // nope: wrong comparison
21+
}
22+
for ; i < 10; i++ { // nope: missing init
23+
}
24+
for s.i = 0; s.i < 10; s.i++ { // nope: not an ident
25+
}
26+
for i := 0; i < 10; i++ { // nope: takes address of i
27+
println(&i)
28+
}
29+
for i := 0; i < 10; i++ { // nope: increments i
30+
i++
31+
}
32+
for i := 0; i < 10; i++ { // nope: assigns i
33+
i = 8
34+
}
35+
}
36+
37+
func f() int { return 0 }

gopls/internal/doc/api.json

+2-2
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 go 1.24;\n - replacing append(s[:i], s[i+1]...) by slices.Delete(s, i, i+1),\n added in go1.21",
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;",
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 go 1.24;\n - replacing append(s[:i], s[i+1]...) by slices.Delete(s, i, i+1),\n added in go1.21",
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;",
11931193
"URL": "https://pkg.go.dev/golang.org/x/tools/gopls/internal/analysis/modernize",
11941194
"Default": true
11951195
},

0 commit comments

Comments
 (0)