From 1561060c98f9376d002b792330e31239df635502 Mon Sep 17 00:00:00 2001 From: Rob Findley Date: Fri, 28 Jul 2023 10:34:23 -0400 Subject: [PATCH] gopls/internal/lsp/source: fix incorrect 'origin' logic for named types Type names of named types are always canonical. The existing logic to canonicalize them was ineffective, and had the side effect of breaking aliases. Also add more tests of generic name renaming. Fixes golang/go#61625 Change-Id: I318a758176aea6712da16dde29394ffb08bdf715 Reviewed-on: https://go-review.googlesource.com/c/tools/+/513917 Reviewed-by: Hyang-Ah Hana Kim Run-TryBot: Robert Findley TryBot-Result: Gopher Robot gopls-CI: kokoro --- gopls/internal/lsp/source/rename.go | 16 +-- .../regtest/marker/testdata/rename/basic.txt | 22 +++- .../marker/testdata/rename/generics.txt | 123 ++++++++++++++++++ 3 files changed, 149 insertions(+), 12 deletions(-) diff --git a/gopls/internal/lsp/source/rename.go b/gopls/internal/lsp/source/rename.go index 0f49cdcd56b..c1db0e5fd5d 100644 --- a/gopls/internal/lsp/source/rename.go +++ b/gopls/internal/lsp/source/rename.go @@ -343,17 +343,15 @@ func renameOrdinary(ctx context.Context, snapshot Snapshot, f FileHandle, pp pro // Find objectpath, if object is exported ("" otherwise). var declObjPath objectpath.Path if obj.Exported() { - // objectpath.For requires the origin of a generic - // function or type, not an instantiation (a bug?). - // Unfortunately we can't call {Func,TypeName}.Origin - // as these are not available in go/types@go1.18. - // So we take a scenic route. + // objectpath.For requires the origin of a generic function or type, not an + // instantiation (a bug?). Unfortunately we can't call Func.Origin as this + // is not available in go/types@go1.18. So we take a scenic route. + // + // Note that unlike Funcs, TypeNames are always canonical (they are "left" + // of the type parameters, unlike methods). switch obj.(type) { // avoid "obj :=" since cases reassign the var case *types.TypeName: - switch t := obj.Type().(type) { - case *types.Named: - obj = t.Obj() - case *typeparams.TypeParam: + if _, ok := obj.Type().(*typeparams.TypeParam); ok { // As with capitalized function parameters below, type parameters are // local. goto skipObjectPath diff --git a/gopls/internal/regtest/marker/testdata/rename/basic.txt b/gopls/internal/regtest/marker/testdata/rename/basic.txt index fe723cf9f0f..28de07c3482 100644 --- a/gopls/internal/regtest/marker/testdata/rename/basic.txt +++ b/gopls/internal/regtest/marker/testdata/rename/basic.txt @@ -3,12 +3,28 @@ This test performs basic coverage of 'rename' within a single package. -- basic.go -- package p -func f(x int) { println(x) } //@rename("x", y, param_x) +func f(x int) { println(x) } //@rename("x", y, xToy) --- @param_x/basic.go -- +-- @xToy/basic.go -- package p -func f(y int) { println(y) } //@rename("x", y, param_x) +func f(y int) { println(y) } //@rename("x", y, xToy) + +-- alias.go -- +package p + +// from golang/go#61625 +type LongNameHere struct{} +type A = LongNameHere //@rename("A", B, AToB) +func Foo() A + +-- @AToB/alias.go -- +package p + +// from golang/go#61625 +type LongNameHere struct{} +type B = LongNameHere //@rename("A", B, AToB) +func Foo() B -- errors.go -- package p diff --git a/gopls/internal/regtest/marker/testdata/rename/generics.txt b/gopls/internal/regtest/marker/testdata/rename/generics.txt index 6f308b126d4..9f015ee2d08 100644 --- a/gopls/internal/regtest/marker/testdata/rename/generics.txt +++ b/gopls/internal/regtest/marker/testdata/rename/generics.txt @@ -115,3 +115,126 @@ type ElemData[F ~string] struct { type BuilderImpl[S ~[]F, F ~string] struct{ builder[S, F] } +-- instances/type.go -- +package instances + +type R[P any] struct { //@rename("R", u, Rtou) + Next *R[P] //@rename("R", s, RTos) +} + +func (rv R[P]) Do(R[P]) R[P] { //@rename("Do", Do1, DoToDo1) + var x R[P] + return rv.Do(x) //@rename("Do", Do2, DoToDo2) +} + +func _() { + var x R[int] //@rename("R", r, RTor) + x = x.Do(x) +} + +-- @RTos/instances/type.go -- +package instances + +type s[P any] struct { //@rename("R", u, Rtou) + Next *s[P] //@rename("R", s, RTos) +} + +func (rv s[P]) Do(s[P]) s[P] { //@rename("Do", Do1, DoToDo1) + var x s[P] + return rv.Do(x) //@rename("Do", Do2, DoToDo2) +} + +func _() { + var x s[int] //@rename("R", r, RTor) + x = x.Do(x) +} + +-- @Rtou/instances/type.go -- +package instances + +type u[P any] struct { //@rename("R", u, Rtou) + Next *u[P] //@rename("R", s, RTos) +} + +func (rv u[P]) Do(u[P]) u[P] { //@rename("Do", Do1, DoToDo1) + var x u[P] + return rv.Do(x) //@rename("Do", Do2, DoToDo2) +} + +func _() { + var x u[int] //@rename("R", r, RTor) + x = x.Do(x) +} + +-- @DoToDo1/instances/type.go -- +package instances + +type R[P any] struct { //@rename("R", u, Rtou) + Next *R[P] //@rename("R", s, RTos) +} + +func (rv R[P]) Do1(R[P]) R[P] { //@rename("Do", Do1, DoToDo1) + var x R[P] + return rv.Do1(x) //@rename("Do", Do2, DoToDo2) +} + +func _() { + var x R[int] //@rename("R", r, RTor) + x = x.Do1(x) +} + +-- @DoToDo2/instances/type.go -- +package instances + +type R[P any] struct { //@rename("R", u, Rtou) + Next *R[P] //@rename("R", s, RTos) +} + +func (rv R[P]) Do2(R[P]) R[P] { //@rename("Do", Do1, DoToDo1) + var x R[P] + return rv.Do2(x) //@rename("Do", Do2, DoToDo2) +} + +func _() { + var x R[int] //@rename("R", r, RTor) + x = x.Do2(x) +} + +-- instances/func.go -- +package instances + +func Foo[P any](p P) { //@rename("Foo", Bar, FooToBar) + Foo(p) //@rename("Foo", Baz, FooToBaz) +} + +-- @FooToBar/instances/func.go -- +package instances + +func Bar[P any](p P) { //@rename("Foo", Bar, FooToBar) + Bar(p) //@rename("Foo", Baz, FooToBaz) +} + +-- @FooToBaz/instances/func.go -- +package instances + +func Baz[P any](p P) { //@rename("Foo", Bar, FooToBar) + Baz(p) //@rename("Foo", Baz, FooToBaz) +} + +-- @RTor/instances/type.go -- +package instances + +type r[P any] struct { //@rename("R", u, Rtou) + Next *r[P] //@rename("R", s, RTos) +} + +func (rv r[P]) Do(r[P]) r[P] { //@rename("Do", Do1, DoToDo1) + var x r[P] + return rv.Do(x) //@rename("Do", Do2, DoToDo2) +} + +func _() { + var x r[int] //@rename("R", r, RTor) + x = x.Do(x) +} +