golang / golang/go

x/tools/go/analysis/passes/modernize: slicesbackward: invalid variable shadowing in loop

Open
#80,960 3 comments 0 reactions 0 assignees View on GitHub
NeedsInvestigation Tools
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.