From fe58b07b84cecebfbeee9434860b09cb353b03c5 Mon Sep 17 00:00:00 2001 From: Rob Findley Date: Fri, 28 Jul 2023 10:19:12 -0400 Subject: [PATCH] gopls/internal/lsp/source: fix renaming of type parameters Type parameters were incorrectly being treated as exported for the purpose of renaming checks, and imported type parameters do not have the correct scope information for rename checks to work. Fixes golang/go#61635 Change-Id: I4261c5e7b3622affbeb81a96ea4f510a5b9e4fe3 Reviewed-on: https://go-review.googlesource.com/c/tools/+/513916 gopls-CI: kokoro Reviewed-by: Hyang-Ah Hana Kim Run-TryBot: Robert Findley TryBot-Result: Gopher Robot --- gopls/internal/lsp/source/rename.go | 9 +- .../marker/testdata/rename/generics.txt | 117 ++++++++++++++++++ .../marker/testdata/rename/issue61614.txt | 35 ------ 3 files changed, 124 insertions(+), 37 deletions(-) create mode 100644 gopls/internal/regtest/marker/testdata/rename/generics.txt delete mode 100644 gopls/internal/regtest/marker/testdata/rename/issue61614.txt diff --git a/gopls/internal/lsp/source/rename.go b/gopls/internal/lsp/source/rename.go index 02bdaededaf..0f49cdcd56b 100644 --- a/gopls/internal/lsp/source/rename.go +++ b/gopls/internal/lsp/source/rename.go @@ -350,8 +350,13 @@ func renameOrdinary(ctx context.Context, snapshot Snapshot, f FileHandle, pp pro // So we take a scenic route. switch obj.(type) { // avoid "obj :=" since cases reassign the var case *types.TypeName: - if named, ok := obj.Type().(*types.Named); ok { - obj = named.Obj() + switch t := obj.Type().(type) { + case *types.Named: + obj = t.Obj() + case *typeparams.TypeParam: + // As with capitalized function parameters below, type parameters are + // local. + goto skipObjectPath } case *types.Func: obj = funcOrigin(obj.(*types.Func)) diff --git a/gopls/internal/regtest/marker/testdata/rename/generics.txt b/gopls/internal/regtest/marker/testdata/rename/generics.txt new file mode 100644 index 00000000000..6f308b126d4 --- /dev/null +++ b/gopls/internal/regtest/marker/testdata/rename/generics.txt @@ -0,0 +1,117 @@ +This test exercises various renaming features on generic code. + +Fixed bugs: + +- golang/go#61614: renaming a method of a type in a package that uses type + parameter composite lits used to panic, because previous iterations of the + satisfy analysis did not account for this language feature. + +- golang/go#61635: renaming type parameters did not work when they were + capitalized and the package was imported by another package. + +-- flags -- +-min_go=go1.18 + +-- go.mod -- +module example.com +go 1.20 + +-- a.go -- +package a + +type I int + +func (I) m() {} //@rename("m", M, mToM) + +func _[P ~[]int]() { + _ = P{} +} + +-- @mToM/a.go -- +package a + +type I int + +func (I) M() {} //@rename("m", M, mToM) + +func _[P ~[]int]() { + _ = P{} +} + +-- g.go -- +package a + +type S[P any] struct { //@rename("P", Q, PToQ) + P P + F func(P) P +} + +func F[R any](r R) { + var _ R //@rename("R", S, RToS) +} + +-- @PToQ/g.go -- +package a + +type S[Q any] struct { //@rename("P", Q, PToQ) + P Q + F func(Q) Q +} + +func F[R any](r R) { + var _ R //@rename("R", S, RToS) +} + +-- @RToS/g.go -- +package a + +type S[P any] struct { //@rename("P", Q, PToQ) + P P + F func(P) P +} + +func F[S any](r S) { + var _ S //@rename("R", S, RToS) +} + +-- issue61635/p.go -- +package issue61635 + +type builder[S ~[]F, F ~string] struct { //@rename("S", T, SToT) + name string + elements S + elemData map[F][]ElemData[F] + // other fields... +} + +type ElemData[F ~string] struct { + Name F + // other fields... +} + +type BuilderImpl[S ~[]F, F ~string] struct{ builder[S, F] } + +-- importer/i.go -- +package importer + +import "example.com/issue61635" // importing is necessary to repro golang/go#61635 + +var _ issue61635.ElemData[string] + +-- @SToT/issue61635/p.go -- +package issue61635 + +type builder[T ~[]F, F ~string] struct { //@rename("S", T, SToT) + name string + elements T + elemData map[F][]ElemData[F] + // other fields... +} + +type ElemData[F ~string] struct { + Name F + // other fields... +} + +type BuilderImpl[S ~[]F, F ~string] struct{ builder[S, F] } + diff --git a/gopls/internal/regtest/marker/testdata/rename/issue61614.txt b/gopls/internal/regtest/marker/testdata/rename/issue61614.txt deleted file mode 100644 index cfb37b19ac5..00000000000 --- a/gopls/internal/regtest/marker/testdata/rename/issue61614.txt +++ /dev/null @@ -1,35 +0,0 @@ -This test renames a method of a type in a package that uses type parameter -composite lits. Previous iterations of the satisfy analysis did not account for -this language feature. - -See issue #60789. - --- flags -- --min_go=go1.18 - --- go.mod -- -module example.com -go 1.20 - --- a.go -- -package a - -type I int - -func (I) m() {} //@rename("m", M, mToM) - -func _[P ~[]int]() { - _ = P{} -} - --- @mToM/a.go -- -package a - -type I int - -func (I) M() {} //@rename("m", M, mToM) - -func _[P ~[]int]() { - _ = P{} -} -