gopls/internal/lsp/source: referencesV2: support unexported methods
This change adds support for unexported methods to the incremental
implementation of the 'references' operation, which previously
had to fall back to a type-check-the-world strategy for methods.
(Exported methods are still to do.)
The implementation strategy is to record the receiver type R of
the target method M, and search types.Info.Uses for all objects
that are methods of the same name as M, whose receiver is mutually
assignable with R.
Change-Id: I4121f34df822662ccbaac5d53f3e2a5578be5431
Reviewed-on: https://go-review.googlesource.com/c/tools/+/462915
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
diff --git a/gopls/internal/lsp/lsp_test.go b/gopls/internal/lsp/lsp_test.go
index 6a6a8ca..eb5baf9 100644
--- a/gopls/internal/lsp/lsp_test.go
+++ b/gopls/internal/lsp/lsp_test.go
@@ -867,6 +867,8 @@
want := make(map[protocol.Location]bool)
for i, pos := range itemList {
// We don't want the first result if we aren't including the declaration.
+ // TODO(adonovan): don't assume a single declaration:
+ // there may be >1 if corresponding methods are considered.
if i == 0 && !includeDeclaration {
continue
}
diff --git a/gopls/internal/lsp/source/references2.go b/gopls/internal/lsp/source/references2.go
index 9ebeace..1fc01dc 100644
--- a/gopls/internal/lsp/source/references2.go
+++ b/gopls/internal/lsp/source/references2.go
@@ -30,6 +30,7 @@
"golang.org/x/tools/go/types/objectpath"
"golang.org/x/tools/gopls/internal/lsp/protocol"
"golang.org/x/tools/gopls/internal/lsp/safetoken"
+ "golang.org/x/tools/gopls/internal/lsp/source/methodsets"
"golang.org/x/tools/gopls/internal/span"
"golang.org/x/tools/internal/event"
)
@@ -248,18 +249,7 @@
break
}
if obj == nil {
- return nil, ErrNoIdentFound
- }
-
- // If the object is a method, we need to search for all
- // matching implementations and/or interfaces, without
- // type-checking everything.
- //
- // That will require an approach such as the one sketched in
- // go.dev/cl/452060. Until then, we simply fall back to the
- // old implementation for now. TODO(adonovan): fix.
- if fn, ok := obj.(*types.Func); ok && fn.Type().(*types.Signature).Recv() != nil {
- return nil, ErrFallback
+ return nil, ErrNoIdentFound // can't happen
}
// nil, error, iota, or other built-in?
@@ -303,6 +293,18 @@
var exportedObjectPath objectpath.Path
if path, err := objectpath.For(obj); err == nil && obj.Exported() {
exportedObjectPath = path
+
+ // If the object is an exported method, we need to search for
+ // all matching implementations (using the incremental
+ // implementation of 'implementations') and then search for
+ // the set of corresponding methods (requiring the incremental
+ // implementation of 'references' to be generalized to a set
+ // of search objects).
+ // Until then, we simply fall back to the old implementation for now.
+ // TODO(adonovan): fix.
+ if fn, ok := obj.(*types.Func); ok && fn.Type().(*types.Signature).Recv() != nil {
+ return nil, ErrFallback
+ }
}
// If it is exported, how far need we search?
@@ -368,7 +370,7 @@
return refs, nil
}
-// localReferences reports (concurrently) each reference to the object
+// localReferences reports each reference to the object
// declared at the specified URI/offset within its enclosing package m.
func localReferences(ctx context.Context, snapshot Snapshot, declURI span.URI, declOffset int, m *Metadata, report func(loc protocol.Location, isDecl bool)) error {
pkgs, err := snapshot.TypeCheck(ctx, TypecheckFull, m.ID)
@@ -393,15 +395,57 @@
}
// Report the locations of the declaration(s).
+ // TODO(adonovan): what about for corresponding methods? Add tests.
for _, node := range targets {
report(mustLocation(pgf, node), true)
}
+ // receiver returns the effective receiver type for method-set
+ // comparisons for obj, if it is a method, or nil otherwise.
+ receiver := func(obj types.Object) types.Type {
+ if fn, ok := obj.(*types.Func); ok {
+ if recv := fn.Type().(*types.Signature).Recv(); recv != nil {
+ return methodsets.EnsurePointer(recv.Type())
+ }
+ }
+ return nil
+ }
+
+ // If we're searching for references to a method, broaden the
+ // search to include references to corresponding methods of
+ // mutually assignable receiver types.
+ // (We use a slice, but objectsAt never returns >1 methods.)
+ var methodRecvs []types.Type
+ var methodName string // name of an arbitrary target, iff a method
+ for obj := range targets {
+ if t := receiver(obj); t != nil {
+ methodRecvs = append(methodRecvs, t)
+ methodName = obj.Name()
+ }
+ }
+
+ // matches reports whether obj either is or corresponds to a target.
+ // (Correspondence is defined as usual for interface methods.)
+ matches := func(obj types.Object) bool {
+ if targets[obj] != nil {
+ return true
+ } else if methodRecvs != nil && obj.Name() == methodName {
+ if orecv := receiver(obj); orecv != nil {
+ for _, mrecv := range methodRecvs {
+ if concreteImplementsIntf(orecv, mrecv) {
+ return true
+ }
+ }
+ }
+ }
+ return false
+ }
+
// Scan through syntax looking for uses of one of the target objects.
for _, pgf := range pkg.CompiledGoFiles() {
ast.Inspect(pgf.File, func(n ast.Node) bool {
if id, ok := n.(*ast.Ident); ok {
- if used, ok := pkg.GetTypesInfo().Uses[id]; ok && targets[used] != nil {
+ if obj, ok := pkg.GetTypesInfo().Uses[id]; ok && matches(obj) {
report(mustLocation(pgf, id), false)
}
}
@@ -453,10 +497,14 @@
}
targets[obj] = leaf
}
+
+ if len(targets) == 0 {
+ return nil, fmt.Errorf("objectAt: internal error: no targets") // can't happen
+ }
return targets, nil
}
-// globalReferences reports (concurrently) each cross-package
+// globalReferences reports each cross-package
// reference to the object identified by (pkgPath, objectPath).
func globalReferences(ctx context.Context, snapshot Snapshot, m *Metadata, pkgPath PackagePath, objectPath objectpath.Path, report func(loc protocol.Location, isDecl bool)) error {
// TODO(adonovan): opt: don't actually type-check here,
diff --git a/gopls/internal/regtest/misc/references_test.go b/gopls/internal/regtest/misc/references_test.go
index 19704ff..f105f4c 100644
--- a/gopls/internal/regtest/misc/references_test.go
+++ b/gopls/internal/regtest/misc/references_test.go
@@ -52,6 +52,8 @@
// This reproduces and tests golang/go#48400.
func TestReferencesPanicOnError(t *testing.T) {
+ // Ideally this would actually return the correct answer,
+ // instead of merely failing gracefully.
const files = `
-- go.mod --
module mod.com
