Skip to content

Commit 4ee50fe

Browse files
xieyuschengopherbot
authored andcommitted
gopls/internal/analysis/modernize: rangeint: avoid offering wrong fix
This change adds additional checking to ensure that rangeint won't offer a fix in cases where RHS of 'i < limit' depends on loop var. Given the code snippet below, this change will no longer offer a wrong fix as it did before. var n, kd int for j := 0; j < min(n-j, kd+1); j++ { } - offered fix before(build error 'undefined: j') var n, kd int for j := range min(n-j, kd+1){ } Fixes golang/go#72726 Change-Id: I78c5457406258c44dd2fa861aa43d9ddb9c707fc Reviewed-on: https://go-review.googlesource.com/c/tools/+/656975 Reviewed-by: Alan Donovan <[email protected]> Auto-Submit: Alan Donovan <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]> Reviewed-by: Robert Findley <[email protected]>
1 parent bf70295 commit 4ee50fe

File tree

3 files changed

+50
-0
lines changed

3 files changed

+50
-0
lines changed

gopls/internal/analysis/modernize/rangeint.go

+8
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,15 @@ func rangeint(pass *analysis.Pass) {
6262
compare.Op == token.LSS &&
6363
equalSyntax(compare.X, init.Lhs[0]) {
6464
// Have: for i = 0; i < limit; ... {}
65+
6566
limit := compare.Y
67+
curLimit, _ := curLoop.FindNode(limit)
68+
// Don't offer a fix if the limit expression depends on the loop index.
69+
for cur := range curLimit.Preorder((*ast.Ident)(nil)) {
70+
if cur.Node().(*ast.Ident).Name == index.Name {
71+
continue nextLoop
72+
}
73+
}
6674

6775
// Skip loops up to b.N in benchmarks; see [bloop].
6876
if sel, ok := limit.(*ast.SelectorExpr); ok &&

gopls/internal/analysis/modernize/testdata/src/rangeint/rangeint.go

+21
Original file line numberDiff line numberDiff line change
@@ -77,3 +77,24 @@ func issue71847d() {
7777
for i := 0; i < limit2; i++ { // want "for loop can be modernized using range over int"
7878
}
7979
}
80+
81+
func issue72726() {
82+
var n, kd int
83+
for i := 0; i < n; i++ { // want "for loop can be modernized using range over int"
84+
// nope: j will be invisible once it's refactored to 'for j := range min(n-j, kd+1)'
85+
for j := 0; j < min(n-j, kd+1); j++ { // nope
86+
_, _ = i, j
87+
}
88+
}
89+
90+
for i := 0; i < i; i++ { // nope
91+
}
92+
93+
var i int
94+
for i = 0; i < i/2; i++ { // nope
95+
}
96+
97+
var arr []int
98+
for i = 0; i < arr[i]; i++ { // nope
99+
}
100+
}

gopls/internal/analysis/modernize/testdata/src/rangeint/rangeint.go.golden

+21
Original file line numberDiff line numberDiff line change
@@ -77,3 +77,24 @@ func issue71847d() {
7777
for range int(limit2) { // want "for loop can be modernized using range over int"
7878
}
7979
}
80+
81+
func issue72726() {
82+
var n, kd int
83+
for i := range n { // want "for loop can be modernized using range over int"
84+
// nope: j will be invisible once it's refactored to 'for j := range min(n-j, kd+1)'
85+
for j := 0; j < min(n-j, kd+1); j++ { // nope
86+
_, _ = i, j
87+
}
88+
}
89+
90+
for i := 0; i < i; i++ { // nope
91+
}
92+
93+
var i int
94+
for i = 0; i < i/2; i++ { // nope
95+
}
96+
97+
var arr []int
98+
for i = 0; i < arr[i]; i++ { // nope
99+
}
100+
}

0 commit comments

Comments
 (0)