x/tools/go/analysis/passes/modernize: slicesbackward: invalid variable shadowing in loop
- Dominant language
- Go
- Stars
- 139k
- Forks
- 19.4k
- PR merge metrics
- PR metrics pending
Description
### Go version
go version go1.27.0 darwin/arm64
### Output of `go env` in your module/workspace:
```shell
AR='ar'
CC='clang'
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_ENABLED='1'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
CXX='clang++'
GCCGO='gccgo'
GO111MODULE='on'
GOARCH='arm64'
GOARM64='v8.0'
GOAUTH='netrc'
GOBIN='/go/bin'
GOCACHE='/go-build'
GOCACHEPROG=''
GODEBUG=''
GOENV='/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFIPS140='off'
GOFLAGS=''
GOGCCFLAGS='-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/ht/pthg90p54xx6vf8ktlv9nqbc0000gn/T/go-build432207319=/tmp/go-build -gno-record-gcc-switches -fno-common'
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMOD='/go.mod'
GOMODCACHE='/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='darwin'
GOPACKAGESDRIVER=''
GOPATH='/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org'
GOROOT='/usr/local/go/current'
GOSUMDB='sum.golang.org'
GOTELEMETRY='local'
GOTELEMETRYDIR='/go/telemetry'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/usr/local/go/current/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='go1.27.0'
GOWORK=''
PKG_CONFIG='pkg-config'
```
### What did you do?
- Updated to Go 1.27
- Run `go fix ./...`
- Run `go test ./...`
### What did you see happen?
The fix applied the following diff, which no longer compiles:
```go
- for i := len(s.entries) - 1; i >= 0; i-- {
+ for i, v := range slices.Backward(s.entries) {
col := s.table[i] // this entry's column in the solution table
for j := range col {
// Value and gain of j shares of this entry.
- v := s.entries[i].Value * currency.Value(j)
- g := s.entries[i].Gain * currency.Value(j)
+ v := v.Value * currency.Value(j)
+ g := v.Gain * currency.Value(j)
```
Citing:
```
solver/solver.go:86:11: v.Gain undefined (type currency.Value has no field or method Gain)
```
The reuse of the name `v` would ordinarily be fine, since the inner scope has its own binding, but in this case the (existing) inner binding wound up shadowing the variable chosen by the fix, and the types no longer match.
### What did you expect to see?
I would have been content with a diagnostic saying "I couldn't apply this fix". Ideally, the fix would have noticed the inner binding has a different type, and chosen a fresh name for the loop variable that would not conflict with that.
Such a collision won't always cause a problem, and could be avoided if the types match, but that may be too tedious for a fix to chase down.
Contributor guide
Research direction
Start by reproducing the issue with `go fix ./...` and `go test ./...`, then inspect the `slicesbackward` pass named in the issue and the failing use at `solver/solver.go:86`. Trace how the fix chooses its loop variable name and add coverage for the reported shadowing case; done means the transformed code compiles or the fix emits a diagnostic instead.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- tooling
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 52/100