gopls/internal/analysis/modernize: fix bug in rangeint

for and range loops leave their index variables with a different
final value: limit, and limit-1, respectively. Thus we must
not offer a fix if the loop variable is used after the loop.

+ test

Fixes golang/go#71952

Change-Id: Iaabd20792724166ace0ed5fd9dd997edaa96a435
Reviewed-on: https://go-review.googlesource.com/c/tools/+/652496
Auto-Submit: Alan Donovan <[email protected]>
Reviewed-by: Jonathan Amsterdam <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>

Author: adonovan

gopls/internal/analysis/modernize/rangeint.go

@@ -100,6 +100,18 @@ func rangeint(pass *analysis.Pass) {
 							})
 						}
 
+						// If i is used after the loop,
+						// don't offer a fix, as a range loop
+						// leaves i with a different final value (limit-1).
+						if init.Tok == token.ASSIGN {
+							for curId := range curLoop.Parent().Preorder((*ast.Ident)(nil)) {
+								id := curId.Node().(*ast.Ident)
+								if id.Pos() > loop.End() && info.Uses[id] == v {
+									continue nextLoop
+								}
+							}
+						}
+
 						// If limit is len(slice),
 						// simplify "range len(slice)" to "range slice".
 						if call, ok := limit.(*ast.CallExpr); ok &&

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

@@ -4,7 +4,11 @@ func _(i int, s struct{ i int }, slice []int) {
 	for i := 0; i < 10; i++ { // want "for loop can be modernized using range over int"
 		println(i)
 	}
-	for i = 0; i < f(); i++ { // want "for loop can be modernized using range over int"
+	{
+		var i int
+		for i = 0; i < f(); i++ { // want "for loop can be modernized using range over int"
+		}
+		// NB: no uses of i after loop.
 	}
 	for i := 0; i < 10; i++ { // want "for loop can be modernized using range over int"
 		// i unused within loop
@@ -51,3 +55,14 @@ func _(s string) {
 		}
 	}
 }
+
+// Repro for #71952: for and range loops have different final values
+// on i (n and n-1, respectively) so we can't offer the fix if i is
+// used after the loop.
+func nopePostconditionDiffers() {
+	i := 0
+	for i = 0; i < 5; i++ {
+		println(i)
+	}
+	println(i) // must print 5, not 4
+}

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

@@ -4,7 +4,11 @@ func _(i int, s struct{ i int }, slice []int) {
 	for i := range 10 { // want "for loop can be modernized using range over int"
 		println(i)
 	}
-	for i = range f() { // want "for loop can be modernized using range over int"
+	{
+		var i int
+		for i = range f() { // want "for loop can be modernized using range over int"
+		}
+		// NB: no uses of i after loop.
 	}
 	for range 10 { // want "for loop can be modernized using range over int"
 		// i unused within loop
@@ -51,3 +55,14 @@ func _(s string) {
 		}
 	}
 }
+
+// Repro for #71952: for and range loops have different final values
+// on i (n and n-1, respectively) so we can't offer the fix if i is
+// used after the loop.
+func nopePostconditionDiffers() {
+	i := 0
+	for i = 0; i < 5; i++ {
+		println(i)
+	}
+	println(i) // must print 5, not 4
+}