[tools] go/analysis/passes/modernize: avoid changing assignment-form slicesbackward loops

0 views

Skip to first unread message

shuang cui (Gerrit)

unread,

Aug 11, 2026, 9:55:47 PM (6 hours ago) Aug 11

to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com

shuang cui has uploaded the change for review

Commit message

go/analysis/passes/modernize: avoid changing assignment-form slicesbackward loops

The slicesbackward analyzer must not rewrite loops whose init statement
assigns to a pre-existing variable. Replacing

    for i = len(s)-1; i >= 0; i-- {}

with a range loop declares a new iteration variable and changes the value
observed after the loop.

Only offer the suggested fix for := initialization, and add a regression test.

Fixes golang/go#80838

Change-Id: I381a9f373ede43b097dac177cd01b86915d1282f

Change diff

diff --git a/go/analysis/passes/modernize/slicesbackward.go b/go/analysis/passes/modernize/slicesbackward.go
index c65a6c7..f9b363a 100644
--- a/go/analysis/passes/modernize/slicesbackward.go
+++ b/go/analysis/passes/modernize/slicesbackward.go
@@ -71,11 +71,17 @@
 		for curLoop := range curFile.Preorder((*ast.ForStmt)(nil)) {
 			loop := curLoop.Node().(*ast.ForStmt)
 
-			// Match init:  i := len(s) - 1   or   i = len(s) - 1
+			// Match init:  i := len(s) - 1.
 			init, ok := loop.Init.(*ast.AssignStmt)
 			if !ok || !isSimpleAssign(init) {
 				continue
 			}
+			// A range loop always declares a new iteration variable with :=.
+			// Rewriting an assignment to a pre-existing variable would change
+			// its value after the loop (the original loop leaves it at -1).
+			if init.Tok != token.DEFINE {
+				continue
+			}
 			indexIdent, ok := init.Lhs[0].(*ast.Ident)
 			if !ok {
 				continue
diff --git a/go/analysis/passes/modernize/testdata/src/slicesbackward/slicesbackward.go b/go/analysis/passes/modernize/testdata/src/slicesbackward/slicesbackward.go
index 9e026fe..94157a9 100644
--- a/go/analysis/passes/modernize/testdata/src/slicesbackward/slicesbackward.go
+++ b/go/analysis/passes/modernize/testdata/src/slicesbackward/slicesbackward.go
@@ -107,15 +107,25 @@
 	}
 }
 
-// Should fire: i is declared before the loop but not address-taken.
+// Should NOT fire: init assigns to a pre-existing variable.
 func iDeclaredBeforeLoop(s []int) {
 	var i int
-	for i = len(s) - 1; i >= 0; i-- { // want "backward loop over slice can be modernized using slices.Backward"
+	for i = len(s) - 1; i >= 0; i-- {
 		println(s[i])
 	}
 	_ = i
 }
 
+// Should NOT fire: the value of a pre-existing index variable is observable
+// after the loop and must remain -1 after the original loop.
+func preexistingIndexValue(s []int) int {
+	i := 123
+	for i = len(s) - 1; i >= 0; i-- {
+		println(s[i])
+	}
+	return i
+}
+
 // Should NOT fire: i is address-taken before the loop (init uses =, not :=).
 func iAddressTakenBeforeLoop(s []int) {
 	var i int
diff --git a/go/analysis/passes/modernize/testdata/src/slicesbackward/slicesbackward.go.golden b/go/analysis/passes/modernize/testdata/src/slicesbackward/slicesbackward.go.golden
index 1db10b1..2aeb3e9 100644
--- a/go/analysis/passes/modernize/testdata/src/slicesbackward/slicesbackward.go.golden
+++ b/go/analysis/passes/modernize/testdata/src/slicesbackward/slicesbackward.go.golden
@@ -107,15 +107,25 @@
 	}
 }
 
-// Should fire: i is declared before the loop but not address-taken.
+// Should NOT fire: init assigns to a pre-existing variable.
 func iDeclaredBeforeLoop(s []int) {
 	var i int
-	for _, v := range slices.Backward(s) { // want "backward loop over slice can be modernized using slices.Backward"
-		println(v)
+	for i = len(s) - 1; i >= 0; i-- {
+		println(s[i])
 	}
 	_ = i
 }
 
+// Should NOT fire: the value of a pre-existing index variable is observable
+// after the loop and must remain -1 after the original loop.
+func preexistingIndexValue(s []int) int {
+	i := 123
+	for i = len(s) - 1; i >= 0; i-- {
+		println(s[i])
+	}
+	return i
+}
+
 // Should NOT fire: i is address-taken before the loop (init uses =, not :=).
 func iAddressTakenBeforeLoop(s []int) {
 	var i int
@@ -274,4 +284,3 @@
 		s[v] = 5
 	}
 }
-

Change information

Files:

M go/analysis/passes/modernize/slicesbackward.go
M go/analysis/passes/modernize/testdata/src/slicesbackward/slicesbackward.go
M go/analysis/passes/modernize/testdata/src/slicesbackward/slicesbackward.go.golden

Change size: S

Delta: 3 files changed, 32 insertions(+), 7 deletions(-)

Open in Gerrit

Related details

Attention set is empty

Submit Requirements:

Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

unsatisfied_requirement

satisfied_requirement

open

diffy

shuang cui (Gerrit)

unread,

Aug 11, 2026, 9:59:50 PM (5 hours ago) Aug 11

to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com

shuang cui voted Commit-Queue+1

Commit-Queue