golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
121.57k stars 17.41k forks source link

x/tools/gopls: spurious "would shadow" error when renaming parameter #57479

Open adonovan opened 1 year ago

adonovan commented 1 year ago

When renaming a parameter using gopls's rename command, and the new name is identical to the name of an imported package referenced by one of the function parameters, gopls reports an error, even though the renaming might be sound. For example:

import "fmt"

func _(x fmt.Stringer) {} // rename x to "fmt" gives renaming this var "x" to "fmt" would shadow this reference to the imported package name declared here

(Unfortunately the pronouns in the error message don't actually link to source locations, at least not when I use Emacs+eglot.)

The same thing happens when the package is referenced by a later parameter (e.g. func(x int, y fmt.Stringer)) or by a named constant in an array type (e.g. func(x [math.MaxInt]bool)).

IIRC the gorename tool got this right.

gopherbot commented 1 month ago

Change https://go.dev/cl/590976 mentions this issue: gopls/internal/test: add test case for parameter rename match import

suzmue commented 1 month ago

Using a go version >= go1.22, fixes this issue.

I was able to investigate how this upgrade was able to fix this, and it appears the scopePos for the result object in the block.LookupParent call is NoPos in go < 1.22, and has a valid pos starting in go1.22.

Once new versions of gopls require >= go1.22 (see https://go.dev/issue/65917), this issue will be resolved.

adonovan commented 1 month ago

Ah, that suggests this was a consequence of the scope bug #64292, fixed in go1.22 by https://go.dev/cl/544035. Thanks for investigating!