x/tools/gopls/internal/analysis/modernize: forvar do not fixes redeclaration of second var

[skewb1k]

Steps to reproduce

go.mod

module main

go 1.24.5 //1.22+

main.go

package main

func main() {
	for i, v := range []string{"a", "b", "c"} {
		i := i
		v := v
		println(i, v)
	}
}

Run

modernize -fix ./...

Only the first redeclaration is removed. The second remains:

for i, v := range []string{"a", "b", "c"} {
	v := v
	println(i, v)
}

And whats even worse, if you run the tool again, it removes the remaining redeclaration, leading to a different result on the second run. This is unexpected and unacceptable for a static analysis tool - it should be idempotent and consistent across runs.

This issue also affected the gopls source itself, which is how I discovered it. See this change:
https://go-review.googlesource.com/c/tools/+/665215/1/gopls/internal/golang/references.go

There is a test case that expects this behavior, but I believe it is flawed.
The correct behavior should be to remove both redundant redeclarations in a single pass.

[gopherbot]

Change https://go.dev/cl/692615 mentions this issue: gopls/internal/analysis/modernize/forvar: provide fix for second loop var

[skewb1k]

Opened CL with a working but maybe not the best solution for this.
I'm open to iterating on it if the problem and the proposed behavior make sense.

[gabyhelp]

Related Issues

• x/tools/gopls/internal/analysis/modernize: replacing non-equivalent code #73073 (closed)

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}