internal/lsp: fixes premature return in find implementations
Find implementations sometimes returns no results, as it prematurely returns when it
finds an invalid object. Instead the behavior should be to check all the objects in case
a later object is a valid interface.
Fixes #35602
Change-Id: I0e3e2aa8d3afeaa34e392c2fe3ef8cdcd13b3d1e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/208959
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
diff --git a/internal/lsp/source/implementation.go b/internal/lsp/source/implementation.go
index 61e80cf..9853490 100644
--- a/internal/lsp/source/implementation.go
+++ b/internal/lsp/source/implementation.go
@@ -17,24 +17,25 @@
"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"
)
func (i *IdentifierInfo) Implementation(ctx context.Context) ([]protocol.Location, error) {
+ ctx = telemetry.Package.With(ctx, i.pkg.ID())
+
res, err := i.implementations(ctx)
if err != nil {
return nil, err
}
var objs []types.Object
- pkgs := map[types.Object]Package{}
- // To ensure that we get one object per position, the seen map tracks object positions.
- var seen map[token.Pos]bool
+ pkgs := map[token.Pos]Package{}
if res.toMethod != nil {
- seen = make(map[token.Pos]bool, len(res.toMethod))
// If we looked up a method, results are in toMethod.
for _, s := range res.toMethod {
- if seen[s.Obj().Pos()] {
+ if pkgs[s.Obj().Pos()] != nil {
continue
}
// Determine package of receiver.
@@ -44,14 +45,12 @@
}
if n, ok := recv.(*types.Named); ok {
pkg := res.pkgs[n]
- pkgs[s.Obj()] = pkg
+ pkgs[s.Obj().Pos()] = pkg
}
// Add object to objs.
objs = append(objs, s.Obj())
- seen[s.Obj().Pos()] = true
}
} else {
- seen = make(map[token.Pos]bool, len(res.to))
// Otherwise, the results are in to.
for _, t := range res.to {
// We'll provide implementations that are named types and pointers to named types.
@@ -59,34 +58,36 @@
t = p.Elem()
}
if n, ok := t.(*types.Named); ok {
- if seen[n.Obj().Pos()] {
+ if pkgs[n.Obj().Pos()] != nil {
continue
}
pkg := res.pkgs[n]
- pkgs[n.Obj()] = pkg
+ pkgs[n.Obj().Pos()] = pkg
objs = append(objs, n.Obj())
- seen[n.Obj().Pos()] = true
}
}
}
var locations []protocol.Location
for _, obj := range objs {
- pkg := pkgs[obj]
- if pkgs[obj] == nil || len(pkg.CompiledGoFiles()) == 0 {
+ pkg := pkgs[obj.Pos()]
+ if pkgs[obj.Pos()] == nil || len(pkg.CompiledGoFiles()) == 0 {
continue
}
- file, _, err := i.Snapshot.View().FindPosInPackage(pkgs[obj], obj.Pos())
+ file, _, err := i.Snapshot.View().FindPosInPackage(pkgs[obj.Pos()], obj.Pos())
if err != nil {
- return nil, err
+ log.Error(ctx, "Error getting file for object", err)
+ continue
}
ident, err := findIdentifier(i.Snapshot, pkg, file, obj.Pos())
if err != nil {
- return nil, err
+ log.Error(ctx, "Error getting ident for object", err)
+ continue
}
decRange, err := ident.Declaration.Range()
if err != nil {
- return nil, err
+ 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 {
@@ -136,7 +137,6 @@
}
}
}
-
allNamed = append(allNamed, types.Universe.Lookup("error").Type().(*types.Named))
var msets typeutil.MethodSetCache
diff --git a/internal/lsp/testdata/implementation/implementation.go b/internal/lsp/testdata/implementation/implementation.go
index 1dba704..2b10701 100644
--- a/internal/lsp/testdata/implementation/implementation.go
+++ b/internal/lsp/testdata/implementation/implementation.go
@@ -1,5 +1,7 @@
package implementation
+import "implementation/other"
+
type ImpP struct{} //@ImpP
func (*ImpP) Laugh() { //@mark(LaughP, "Laugh")
@@ -17,3 +19,11 @@
type Laugher interface { //@Laugher,implementations("augher", ImpP),implementations("augher", OtherImpP),implementations("augher", ImpI),implementations("augher", ImpS),implementations("augher", OtherImpI),implementations("augher", OtherImpS),
Laugh() //@mark(LaughL, "Laugh"),implementations("augh", LaughP),implementations("augh", OtherLaughP),implementations("augh", LaughI),implementations("augh", LaughS),implementations("augh", OtherLaughI),implementations("augh", OtherLaughS)
}
+
+type Foo struct {
+ other.Foo
+}
+
+type U interface {
+ U() //@mark(IntU, "U"),implementations("U", ImpU),
+}
diff --git a/internal/lsp/testdata/implementation/other/other.go b/internal/lsp/testdata/implementation/other/other.go
index ae7adf1..e8410af 100644
--- a/internal/lsp/testdata/implementation/other/other.go
+++ b/internal/lsp/testdata/implementation/other/other.go
@@ -13,3 +13,9 @@
type ImpI interface { //@mark(OtherImpI, "ImpI")
Laugh() //@mark(OtherLaughI, "Laugh")
}
+
+type Foo struct {
+}
+
+func (Foo) U() { //@mark(ImpU, "U")
+}
