gopls/modernize: "for i = range n {}" fix is wrong if there are later uses of i

[adonovan]

@dethi reports:

I found one additional bug in the transformation of i := 0; for i = 0; i < n; i++ -> for i = range n if i is used outside of the for loop body. See the minimal reproduction: https://go.dev/play/p/9tlB2oilyJn

In particular, the postcondition of the original loop is that i has the value n, whereas for the range loop it has the value n-1.

[gopherbot]

Change https://go.dev/cl/652496 mentions this issue: gopls/internal/analysis/modernize: fix bug in rangeint

[08d2]

Forgive me if this has already been addressed, but I think there may still be an issue here.

package main

func main() {
	x := &X{x: []int{1, 2, 3, 4, 5, 6}}
	for i := 0; i < x.Len(); i++ {
		println(i, x.Len())
		x.Drop()
	}
}

type X struct{ x []int }

func (x *X) Len() int { return len(x.x) }
func (x *X) Drop()    { x.x = x.x[1:] }

$ go run main.go
0 6
1 5
2 4

$ go run golang.org/x/tools/gopls/internal/analysis/modernize/cmd/modernize@latest -fix .

--- main.go (old)
+++ main.go (new)
@@ -2,7 +2,7 @@
 

 func main() {
        x := &X{x: []int{1, 2, 3, 4, 5, 6}}
-       for i := 0; i < x.Len(); i++ {
+       for i := range x.Len() {
                println(i, x.Len())
                x.Drop()
        }

$ go run main.go
0 6
1 5
2 4
3 3
4 2
5 1

[adonovan]

Forgive me if this has already been addressed, but I think there may still be an issue here.

You're right that this is a bug, but it's a different one: #72917. It was fixed at master but has not yet been released.