Skip to content

Commit

Permalink
gopls/internal/lsp/source: fix incorrect 'origin' logic for named types
Browse files Browse the repository at this point in the history
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 <hyangah@gmail.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
  • Loading branch information
findleyr committed Jul 28, 2023
1 parent fe58b07 commit 1561060
Show file tree
Hide file tree
Showing 3 changed files with 149 additions and 12 deletions.
16 changes: 7 additions & 9 deletions gopls/internal/lsp/source/rename.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
22 changes: 19 additions & 3 deletions gopls/internal/regtest/marker/testdata/rename/basic.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
123 changes: 123 additions & 0 deletions gopls/internal/regtest/marker/testdata/rename/generics.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

0 comments on commit 1561060

Please sign in to comment.