go/analysis/passes/modernize: fix rangeint rewriting
The rangeint modernizer looks for references to "i"
within the loop body and does not suggest a fix if any
of them is an l-value. However, it does not count the
key of a RangeStmt as an l-value when it should,
resulting in invalid suggested fixes.
This CL modifes the l-value logic and adds a test.
Fixes golang/go#77034
Change-Id: Ic0cdbaf5ba64f82c5dba41773ad29f06953aab5c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/734480
Reviewed-by: Alan Donovan <adonovan@google.com>
Auto-Submit: Madeline Kalil <mkalil@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
diff --git a/go/analysis/passes/modernize/rangeint.go b/go/analysis/passes/modernize/rangeint.go
index fccd6e5..600c561 100644
--- a/go/analysis/passes/modernize/rangeint.go
+++ b/go/analysis/passes/modernize/rangeint.go
@@ -332,6 +332,11 @@
if v, ok := info.Defs[id]; ok && v.Pos() != id.Pos() {
return true // reassignment of i (i, j := 1, 2)
}
+ case edge.RangeStmt_Key:
+ rng := cur.Parent().Node().(*ast.RangeStmt)
+ if rng.Tok == token.ASSIGN {
+ return true // "for k, v = range x" is like an AssignStmt to k, v
+ }
case edge.IncDecStmt_X:
return true // i++, i--
case edge.UnaryExpr_X:
diff --git a/go/analysis/passes/modernize/testdata/src/rangeint/rangeint.go b/go/analysis/passes/modernize/testdata/src/rangeint/rangeint.go
index a14dc38..0ec4374 100644
--- a/go/analysis/passes/modernize/testdata/src/rangeint/rangeint.go
+++ b/go/analysis/passes/modernize/testdata/src/rangeint/rangeint.go
@@ -307,3 +307,20 @@
break NotUsedAfter
}
}
+
+// Don't allow rewriting of an outer loop when its inner loop modifies the outer
+// loop variable.
+func issue77034() {
+ for i := 0; i < 5; i++ {
+ for i = range 10 {
+ }
+ }
+}
+
+func issue77034_define_inner() {
+ for i := 0; i < 5; i++ { // want "for loop can be modernized using range over int"
+ for i := range 10 { // inner "i" doesn't modify outer "i"
+ println(i)
+ }
+ }
+}
diff --git a/go/analysis/passes/modernize/testdata/src/rangeint/rangeint.go.golden b/go/analysis/passes/modernize/testdata/src/rangeint/rangeint.go.golden
index 0e92147..ad98069 100644
--- a/go/analysis/passes/modernize/testdata/src/rangeint/rangeint.go.golden
+++ b/go/analysis/passes/modernize/testdata/src/rangeint/rangeint.go.golden
@@ -306,3 +306,20 @@
break NotUsedAfter
}
}
+
+// Don't allow rewriting of an outer loop when its inner loop modifies the outer
+// loop variable.
+func issue77034() {
+ for i := 0; i < 5; i++ {
+ for i = range 10 {
+ }
+ }
+}
+
+func issue77034_define_inner() {
+ for range 5 { // want "for loop can be modernized using range over int"
+ for i := range 10 { // inner "i" doesn't modify outer "i"
+ println(i)
+ }
+ }
+}
