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>

internal/lsp/source/implementation.go

@@ -17,24 +17,25 @@ import (
 
 	"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{}
+	pkgs := map[token.Pos]Package{}
-	// To ensure that we get one object per position, the seen map tracks object positions.
-	var seen map[token.Pos]bool
 
 	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 @@ func (i *IdentifierInfo) Implementation(ctx context.Context) ([]protocol.Locatio
 			}
 			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 @@ func (i *IdentifierInfo) Implementation(ctx context.Context) ([]protocol.Locatio
 				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]
+		pkg := pkgs[obj.Pos()]
-		if pkgs[obj] == nil || len(pkg.CompiledGoFiles()) == 0 {
+		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 @@ func (i *IdentifierInfo) implementations(ctx context.Context) (implementsResult,
 			}
 		}
 	}
-
 	allNamed = append(allNamed, types.Universe.Lookup("error").Type().(*types.Named))
 
 	var msets typeutil.MethodSetCache

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 ImpI interface { //@ImpI
 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),
+}

internal/lsp/testdata/implementation/other/other.go

@@ -13,3 +13,9 @@ func (ImpS) Laugh() { //@mark(OtherLaughS, "Laugh")
 type ImpI interface { //@mark(OtherImpI, "ImpI")
 	Laugh() //@mark(OtherLaughI, "Laugh")
 }
+
+type Foo struct {
+}
+
+func (Foo) U() { //@mark(ImpU, "U")
+}