From 6799a7ae926804d94311fff9145861c22f77ba3a Mon Sep 17 00:00:00 2001 From: Robert Findley Date: Mon, 14 Mar 2022 23:59:05 -0400 Subject: [PATCH] internal/lsp/source: canonicalize objects in reference/rename requests With generics, instantiated object may have differing pointer identities. Fix references/rename requests for instantiated methods/fields by using a canonical object identity of (pos, pkg, name). Fixes golang/go#51672 Change-Id: I0021ca562b8a74dadb616cf6864cb0bdd0165cc3 Reviewed-on: https://go-review.googlesource.com/c/tools/+/392480 Trust: Robert Findley Run-TryBot: Robert Findley Reviewed-by: Hyang-Ah Hana Kim gopls-CI: kokoro TryBot-Result: Gopher Robot --- internal/lsp/source/references.go | 18 ++- .../lsp/testdata/rename/generics/embedded.go | 10 ++ .../rename/generics/embedded.go.golden | 12 ++ .../lsp/testdata/rename/generics/generics.go | 25 ++++ .../rename/generics/generics.go.golden | 108 ++++++++++++++++++ .../lsp/testdata/rename/generics/unions.go | 10 ++ .../testdata/rename/generics/unions.go.golden | 24 ++++ .../lsp/testdata/summary_go1.18.txt.golden | 2 +- 8 files changed, 204 insertions(+), 5 deletions(-) create mode 100644 internal/lsp/testdata/rename/generics/embedded.go create mode 100644 internal/lsp/testdata/rename/generics/embedded.go.golden create mode 100644 internal/lsp/testdata/rename/generics/generics.go create mode 100644 internal/lsp/testdata/rename/generics/generics.go.golden create mode 100644 internal/lsp/testdata/rename/generics/unions.go create mode 100644 internal/lsp/testdata/rename/generics/unions.go.golden diff --git a/internal/lsp/source/references.go b/internal/lsp/source/references.go index 993b9f8a14f..5d3eac33781 100644 --- a/internal/lsp/source/references.go +++ b/internal/lsp/source/references.go @@ -109,10 +109,13 @@ func references(ctx context.Context, snapshot Snapshot, qos []qualifiedObject, i searchPkgs = append(searchPkgs, qo.pkg) for _, pkg := range searchPkgs { for ident, obj := range pkg.GetTypesInfo().Uses { - if obj != qo.obj { - // If ident is not a use of qo.obj, skip it, with one exception: uses - // of an embedded field can be considered references of the embedded - // type name. + // For instantiated objects (as in methods or fields on instantiated + // types), we may not have pointer-identical objects but still want to + // consider them references. + if !equalOrigin(obj, qo.obj) { + // If ident is not a use of qo.obj, skip it, with one exception: + // uses of an embedded field can be considered references of the + // embedded type name if !includeEmbeddedRefs { continue } @@ -167,6 +170,13 @@ func references(ctx context.Context, snapshot Snapshot, qos []qualifiedObject, i return references, nil } +// equalOrigin reports whether obj1 and obj2 have equivalent origin object. +// This may be the case even if obj1 != obj2, if one or both of them is +// instantiated. +func equalOrigin(obj1, obj2 types.Object) bool { + return obj1.Pkg() == obj2.Pkg() && obj1.Pos() == obj2.Pos() && obj1.Name() == obj2.Name() +} + // interfaceReferences returns the references to the interfaces implemented by // the type or method at the given position. func interfaceReferences(ctx context.Context, s Snapshot, f FileHandle, pp protocol.Position) ([]*ReferenceInfo, error) { diff --git a/internal/lsp/testdata/rename/generics/embedded.go b/internal/lsp/testdata/rename/generics/embedded.go new file mode 100644 index 00000000000..b44bab8809b --- /dev/null +++ b/internal/lsp/testdata/rename/generics/embedded.go @@ -0,0 +1,10 @@ +//go:build go1.18 +// +build go1.18 + +package generics + +type foo[P any] int //@rename("foo","bar") + +var x struct{ foo[int] } + +var _ = x.foo diff --git a/internal/lsp/testdata/rename/generics/embedded.go.golden b/internal/lsp/testdata/rename/generics/embedded.go.golden new file mode 100644 index 00000000000..faa9afb69f6 --- /dev/null +++ b/internal/lsp/testdata/rename/generics/embedded.go.golden @@ -0,0 +1,12 @@ +-- bar-rename -- +//go:build go1.18 +// +build go1.18 + +package generics + +type bar[P any] int //@rename("foo","bar") + +var x struct{ bar[int] } + +var _ = x.bar + diff --git a/internal/lsp/testdata/rename/generics/generics.go b/internal/lsp/testdata/rename/generics/generics.go new file mode 100644 index 00000000000..977589c0c59 --- /dev/null +++ b/internal/lsp/testdata/rename/generics/generics.go @@ -0,0 +1,25 @@ +//go:build go1.18 +// +build go1.18 + +package generics + +type G[P any] struct { + F int +} + +func (G[_]) M() {} + +func F[P any](P) { + var p P //@rename("P", "Q") + _ = p +} + +func _() { + var x G[int] //@rename("G", "H") + _ = x.F //@rename("F", "K") + x.M() //@rename("M", "N") + + var y G[string] + _ = y.F + y.M() +} diff --git a/internal/lsp/testdata/rename/generics/generics.go.golden b/internal/lsp/testdata/rename/generics/generics.go.golden new file mode 100644 index 00000000000..b46a0837d13 --- /dev/null +++ b/internal/lsp/testdata/rename/generics/generics.go.golden @@ -0,0 +1,108 @@ +-- Q-rename -- +//go:build go1.18 +// +build go1.18 + +package generics + +type G[P any] struct { + F int +} + +func (G[_]) M() {} + +func F[Q any](Q) { + var p Q //@rename("P", "Q") + _ = p +} + +func _() { + var x G[int] //@rename("G", "H") + _ = x.F //@rename("F", "K") + x.M() //@rename("M", "N") + + var y G[string] + _ = y.F + y.M() +} + +-- H-rename -- +//go:build go1.18 +// +build go1.18 + +package generics + +type H[P any] struct { + F int +} + +func (H[_]) M() {} + +func F[P any](P) { + var p P //@rename("P", "Q") + _ = p +} + +func _() { + var x H[int] //@rename("G", "H") + _ = x.F //@rename("F", "K") + x.M() //@rename("M", "N") + + var y H[string] + _ = y.F + y.M() +} + +-- K-rename -- +//go:build go1.18 +// +build go1.18 + +package generics + +type G[P any] struct { + K int +} + +func (G[_]) M() {} + +func F[P any](P) { + var p P //@rename("P", "Q") + _ = p +} + +func _() { + var x G[int] //@rename("G", "H") + _ = x.K //@rename("F", "K") + x.M() //@rename("M", "N") + + var y G[string] + _ = y.K + y.M() +} + +-- N-rename -- +//go:build go1.18 +// +build go1.18 + +package generics + +type G[P any] struct { + F int +} + +func (G[_]) N() {} + +func F[P any](P) { + var p P //@rename("P", "Q") + _ = p +} + +func _() { + var x G[int] //@rename("G", "H") + _ = x.F //@rename("F", "K") + x.N() //@rename("M", "N") + + var y G[string] + _ = y.F + y.N() +} + diff --git a/internal/lsp/testdata/rename/generics/unions.go b/internal/lsp/testdata/rename/generics/unions.go new file mode 100644 index 00000000000..c737b5c27e2 --- /dev/null +++ b/internal/lsp/testdata/rename/generics/unions.go @@ -0,0 +1,10 @@ +//go:build go1.18 +// +build go1.18 + +package generics + +type T string //@rename("T", "R") + +type C interface { + T | ~int //@rename("T", "S") +} diff --git a/internal/lsp/testdata/rename/generics/unions.go.golden b/internal/lsp/testdata/rename/generics/unions.go.golden new file mode 100644 index 00000000000..463289629c5 --- /dev/null +++ b/internal/lsp/testdata/rename/generics/unions.go.golden @@ -0,0 +1,24 @@ +-- R-rename -- +//go:build go1.18 +// +build go1.18 + +package generics + +type R string //@rename("T", "R") + +type C interface { + R | ~int //@rename("T", "S") +} + +-- S-rename -- +//go:build go1.18 +// +build go1.18 + +package generics + +type S string //@rename("T", "R") + +type C interface { + S | ~int //@rename("T", "S") +} + diff --git a/internal/lsp/testdata/summary_go1.18.txt.golden b/internal/lsp/testdata/summary_go1.18.txt.golden index 6bb06713965..284ef645e18 100644 --- a/internal/lsp/testdata/summary_go1.18.txt.golden +++ b/internal/lsp/testdata/summary_go1.18.txt.golden @@ -20,7 +20,7 @@ DefinitionsCount = 108 TypeDefinitionsCount = 18 HighlightsCount = 69 ReferencesCount = 27 -RenamesCount = 41 +RenamesCount = 48 PrepareRenamesCount = 7 SymbolsCount = 5 WorkspaceSymbolsCount = 20