internal/lsp: fix find-implementation for promoted methods
We weren't returning promoted methods as implementations when the
promoted method was defined in a different package than the type
implementing the interface.
Fix by properly mapping the implementer types.Object to its containing
source.Package.
I generalized the implementations() result to just contain the
implementer objects and their containing package. This allowed me to
get rid of some result prep code in Implementation().
Fixes golang/go#35972.
Change-Id: I867f2114c34e2ad39515ee3c8b6354c1cd35f7af
Reviewed-on: https://go-review.googlesource.com/c/tools/+/210280
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
diff --git a/internal/lsp/implementation.go b/internal/lsp/implementation.go
index 37faeaf..344deab 100644
--- a/internal/lsp/implementation.go
+++ b/internal/lsp/implementation.go
@@ -50,7 +50,7 @@
locs, err := ident.Implementation(ctx)
if err != nil {
- if err == source.ErrNotAMethod {
+ if err == source.ErrNotAnInterface {
return nil, err
}
log.Error(ctx, "failed to find Implemenation", err)
diff --git a/internal/lsp/source/implementation.go b/internal/lsp/source/implementation.go
index e1578cd..9fe59e4 100644
--- a/internal/lsp/source/implementation.go
+++ b/internal/lsp/source/implementation.go
@@ -2,20 +2,12 @@
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.
-// The code in this file is based largely on the code in
-// cmd/guru/implements.go. The guru implementation supports
-// looking up "implementers" of methods also, but that
-// code has been cut out here for now for simplicity.
-
package source
import (
"context"
- "fmt"
- "go/token"
"go/types"
- "golang.org/x/tools/go/types/typeutil"
"golang.org/x/tools/internal/lsp/protocol"
"golang.org/x/tools/internal/lsp/telemetry"
"golang.org/x/tools/internal/telemetry/log"
@@ -25,75 +17,35 @@
func (i *IdentifierInfo) Implementation(ctx context.Context) ([]protocol.Location, error) {
ctx = telemetry.Package.With(ctx, i.pkg.ID())
- res, err := i.implementations(ctx)
+ impls, err := i.implementations(ctx)
if err != nil {
return nil, err
}
- var objs []types.Object
- pkgs := map[token.Pos]Package{}
-
- if res.toMethod != nil {
- // If we looked up a method, results are in toMethod.
- for _, s := range res.toMethod {
- if pkgs[s.Obj().Pos()] != nil {
- continue
- }
- // Determine package of receiver.
- recv := s.Recv()
- if p, ok := recv.(*types.Pointer); ok {
- recv = p.Elem()
- }
- if n, ok := recv.(*types.Named); ok {
- pkg := res.pkgs[n]
- pkgs[s.Obj().Pos()] = pkg
- }
- // Add object to objs.
- objs = append(objs, s.Obj())
- }
- } else {
- // Otherwise, the results are in to.
- for _, t := range res.to {
- // We'll provide implementations that are named types and pointers to named types.
- if p, ok := t.(*types.Pointer); ok {
- t = p.Elem()
- }
- if n, ok := t.(*types.Named); ok {
- if pkgs[n.Obj().Pos()] != nil {
- continue
- }
- pkg := res.pkgs[n]
- pkgs[n.Obj().Pos()] = pkg
- objs = append(objs, n.Obj())
- }
- }
- }
-
var locations []protocol.Location
- for _, obj := range objs {
- pkg := pkgs[obj.Pos()]
- if pkgs[obj.Pos()] == nil || len(pkg.CompiledGoFiles()) == 0 {
+ for _, impl := range impls {
+ if impl.pkg == nil || len(impl.pkg.CompiledGoFiles()) == 0 {
continue
}
- file, _, err := i.Snapshot.View().FindPosInPackage(pkgs[obj.Pos()], obj.Pos())
+
+ file, _, err := i.Snapshot.View().FindPosInPackage(impl.pkg, impl.obj.Pos())
if err != nil {
log.Error(ctx, "Error getting file for object", err)
continue
}
- ident, err := findIdentifier(i.Snapshot, pkg, file, obj.Pos())
+
+ ident, err := findIdentifier(i.Snapshot, impl.pkg, file, impl.obj.Pos())
if err != nil {
log.Error(ctx, "Error getting ident for object", err)
continue
}
+
decRange, err := ident.Declaration.Range()
if err != nil {
log.Error(ctx, "Error getting range for object", err)
continue
}
- // Do not add interface itself to the list.
- if ident.Declaration.spanRange == i.Declaration.spanRange {
- continue
- }
+
locations = append(locations, protocol.Location{
URI: protocol.NewURI(ident.Declaration.URI()),
Range: decRange,
@@ -102,75 +54,95 @@
return locations, nil
}
-var ErrNotAMethod = errors.New("this function is not a method")
+var ErrNotAnInterface = errors.New("not an interface or interface method")
-func (i *IdentifierInfo) implementations(ctx context.Context) (implementsResult, error) {
- var T types.Type
- var method *types.Func
- if i.Type.Object == nil {
- // This isn't a type. Is it a method?
- obj, ok := i.Declaration.obj.(*types.Func)
- if !ok {
- return implementsResult{}, fmt.Errorf("no type info object for identifier %q", i.Name)
- }
- recv := obj.Type().(*types.Signature).Recv()
- if recv == nil {
- return implementsResult{}, ErrNotAMethod
- }
+func (i *IdentifierInfo) implementations(ctx context.Context) ([]implementation, error) {
+ var (
+ T *types.Interface
+ method *types.Func
+ )
+
+ switch obj := i.Declaration.obj.(type) {
+ case *types.Func:
method = obj
- T = recv.Type()
- } else {
- T = i.Type.Object.Type()
+ if recv := obj.Type().(*types.Signature).Recv(); recv != nil {
+ T, _ = recv.Type().Underlying().(*types.Interface)
+ }
+ case *types.TypeName:
+ T, _ = obj.Type().Underlying().(*types.Interface)
+ }
+
+ if T == nil {
+ return nil, ErrNotAnInterface
+ }
+
+ if T.NumMethods() == 0 {
+ return nil, nil
}
// Find all named types, even local types (which can have methods
- // due to promotion). We ignore aliases 'type M = N' to avoid
- // duplicate reporting of the Named type N.
- var allNamed []*types.Named
- pkgs := map[*types.Named]Package{}
+ // due to promotion).
+ var (
+ allNamed []*types.Named
+ pkgs = make(map[*types.Package]Package)
+ )
for _, pkg := range i.Snapshot.KnownPackages(ctx) {
+ pkgs[pkg.GetTypes()] = pkg
+
info := pkg.GetTypesInfo()
for _, obj := range info.Defs {
+ // We ignore aliases 'type M = N' to avoid duplicate reporting
+ // of the Named type N.
if obj, ok := obj.(*types.TypeName); ok && !obj.IsAlias() {
+ // We skip interface types since we only want concrete
+ // implementations.
if named, ok := obj.Type().(*types.Named); ok && !isInterface(named) {
allNamed = append(allNamed, named)
- pkgs[named] = pkg
}
}
}
}
- var msets typeutil.MethodSetCache
+ var (
+ impls []implementation
+ seen = make(map[types.Object]bool)
+ )
- // Test each named type.
- var to []types.Type
+ // Find all the named types that implement our interface.
for _, U := range allNamed {
- if isInterface(T) {
- if msets.MethodSet(T).Len() == 0 {
- continue // empty interface
+ var concrete types.Type = U
+ if !types.AssignableTo(concrete, T) {
+ // We also accept T if *T implements our interface.
+ concrete = types.NewPointer(concrete)
+ if !types.AssignableTo(concrete, T) {
+ continue
}
+ }
- // T interface, U concrete
- if types.AssignableTo(U, T) {
- to = append(to, U)
- } else if pU := types.NewPointer(U); types.AssignableTo(pU, T) {
- to = append(to, pU)
- }
+ var obj types.Object = U.Obj()
+ if method != nil {
+ obj = types.NewMethodSet(concrete).Lookup(method.Pkg(), method.Name()).Obj()
}
- }
- var toMethod []*types.Selection // contain nils
- if method != nil {
- for _, t := range to {
- toMethod = append(toMethod,
- types.NewMethodSet(t).Lookup(method.Pkg(), method.Name()))
+
+ if obj == method || seen[obj] {
+ continue
}
+
+ seen[obj] = true
+
+ impls = append(impls, implementation{
+ obj: obj,
+ pkg: pkgs[obj.Pkg()],
+ })
}
- return implementsResult{pkgs, to, toMethod}, nil
+
+ return impls, nil
}
-// implementsResult contains the results of an implements query.
-type implementsResult struct {
- pkgs map[*types.Named]Package
- to []types.Type // named or ptr-to-named types assignable to interface T
- toMethod []*types.Selection
+type implementation struct {
+ // obj is the implementation, either a *types.TypeName or *types.Func.
+ obj types.Object
+
+ // pkg is the Package that contains obj's definition.
+ pkg Package
}
diff --git a/internal/lsp/testdata/implementation/implementation.go b/internal/lsp/testdata/implementation/implementation.go
index 40da1da..5c18004 100644
--- a/internal/lsp/testdata/implementation/implementation.go
+++ b/internal/lsp/testdata/implementation/implementation.go
@@ -25,7 +25,7 @@
}
type U interface {
- U() //TODO: fix flaky @implementations("U", ImpU)
+ U() //@implementations("U", ImpU)
}
type cryer int
diff --git a/internal/lsp/testdata/summary.txt.golden b/internal/lsp/testdata/summary.txt.golden
index 4695f20..1067353 100644
--- a/internal/lsp/testdata/summary.txt.golden
+++ b/internal/lsp/testdata/summary.txt.golden
@@ -20,5 +20,5 @@
SymbolsCount = 1
SignaturesCount = 22
LinksCount = 6
-ImplementationsCount = 4
+ImplementationsCount = 5
