x/tools/gopls/internal/analysis/modernize: range over int rule doesn't consider potential slice appends

[jakebailey]

Go version

go version go1.24.1 linux/amd64

Output of go env in your module/workspace:

AR='ar'
CC='gcc'
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_ENABLED='1'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
CXX='g++'
GCCGO='gccgo'
GO111MODULE=''
GOAMD64='v1'
GOARCH='amd64'
GOAUTH='netrc'
GOBIN=''
GOCACHE='/home/jabaile/.cache/go-build'
GOCACHEPROG=''
GODEBUG=''
GOENV='/home/jabaile/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFIPS140='off'
GOFLAGS=''
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build4045231923=/tmp/go-build -gno-record-gcc-switches'
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMOD='/home/jabaile/work/TypeScript-go/go.mod'
GOMODCACHE='/home/jabaile/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/home/jabaile/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/usr/local/go'
GOSUMDB='sum.golang.org'
GOTELEMETRY='local'
GOTELEMETRYDIR='/home/jabaile/.config/go/telemetry'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/usr/local/go/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.24.1'
GOWORK=''
PKG_CONFIG='pkg-config'

What did you do?

I have an insertion ordered map implementation; a subset of it is:

// OrderedMap is an insertion ordered map.
type OrderedMap[K comparable, V any] struct {
	keys []K
	mp   map[K]V
}

// ...

// Keys returns an iterator over the keys in the map.
// A slice of the keys can be obtained by calling `slices.Collect`.
func (m *OrderedMap[K, V]) Keys() iter.Seq[K] {
	return func(yield func(K) bool) {
		if m == nil {
			return
		}

		// We use a for loop here to ensure we enumerate new items added during iteration.
		for i := 0; i < len(m.keys); i++ {
			if !yield(m.keys[i]) {
				break
			}
		}
	}
}

The loop here is explicitly a plain for loop such that the len is reevaluated each iteration, so that adding new entries to the map still yield them.

What did you see happen?

Modernizer reports that the plain for loop could be turned into an int range loop:

.../internal/collections/ordered_map.go:116:7: for loop can be modernized using range over int
.../internal/collections/ordered_map.go:134:7: for loop can be modernized using range over int
.../internal/collections/ordered_map.go:151:7: for loop can be modernized using range over int

What did you expect to see?

Potentially no suggestion of a range over int, since iterating over a mutable slice has differing behavior. However, this case is certainly rare, most people don't do this.

(Mainly I'm worried for the potential future where modernizers are run by go vet or similar and thus cannot be ignored.)

[gabyhelp]

Related Issues

• x/tools/gopls/internal/analysis/modernize: rangeint: transformation unsound when loop variable is defined outside loop #73009 (closed)
• x/tools/gopls: yet more modernizer bugs #71847 (closed)
• x/tools/gopls/internal/analysis/modernize: rangeint generates correct but not idiomatic transformation #71725 (closed)
• x/tools/gopls/internal/analysis: add "modernizer" analyzers #70815 (closed)
• x/tools/gopls/internal/analysis/modernize: rangeint should be silent when RHS of 'i < limit' depends on loop var #72726 (closed)
• x/tools/gopls/internal/analysis/modernize: rangeint: transformation unsound when loop body has effects on iterated array #72917

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

[jakebailey]

Dang, sorry, I tried to find the dupe beforehand!

[adonovan]

No worries, thanks for reporting it--I'd rather have two reports than none.