From e13f15d1b9df00fb1b01890cd1382a7bf41b8ab7 Mon Sep 17 00:00:00 2001 From: Rohan Challa Date: Tue, 26 Nov 2019 15:03:20 -0500 Subject: [PATCH] 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 TryBot-Result: Gobot Gobot Reviewed-by: Rebecca Stambler --- internal/lsp/source/implementation.go | 36 +++++++++---------- .../testdata/implementation/implementation.go | 10 ++++++ .../testdata/implementation/other/other.go | 6 ++++ 3 files changed, 34 insertions(+), 18 deletions(-) diff --git a/internal/lsp/source/implementation.go b/internal/lsp/source/implementation.go index 61e80cfb9c..9853490f0f 100644 --- a/internal/lsp/source/implementation.go +++ b/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{} - // 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 @@ 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] - 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 @@ func (i *IdentifierInfo) implementations(ctx context.Context) (implementsResult, } } } - 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 1dba704fc5..2b1070123a 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 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), +} diff --git a/internal/lsp/testdata/implementation/other/other.go b/internal/lsp/testdata/implementation/other/other.go index ae7adf10bf..e8410aff87 100644 --- a/internal/lsp/testdata/implementation/other/other.go +++ b/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") +}