Skip to content

Commit dfebd28

Browse files
muirdmstamblerre
authored andcommitted
internal/lsp: fix find-references to search from all package variants
We previously searched the reverse dependencies of the "widest" package that contained out starting identifier, but if our package has tests then the widest package is the ".test" variant, and it has no reverse dependencies. Fix by searching through all of the packages that contain our starting identifier. For example: -- foo/foo.go -- package foo func Foo() {} -- foo/foo_test.go -- package foo func TestFoo(t *testing.T) {} -- bar/bar.go -- import "foo" func _() { foo.Foo() } We would start searching from the foo.test variant, but we wouldn't search package bar at all because bar does not import foo.test, it imports plain foo. Now we search from both foo and foo.test (you still need search foo.test to find references within foo_test.go). Fixes golang/go#35936. Change-Id: I5fd2f7bb130a421ed6fad92da11179995c99a2cf Reviewed-on: https://go-review.googlesource.com/c/tools/+/210537 Run-TryBot: Rebecca Stambler <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Rebecca Stambler <[email protected]>
1 parent ad473c0 commit dfebd28

File tree

3 files changed

+45
-24
lines changed

3 files changed

+45
-24
lines changed

internal/lsp/references.go

+43-23
Original file line numberDiff line numberDiff line change
@@ -29,48 +29,68 @@ func (s *Server) references(ctx context.Context, params *protocol.ReferenceParam
2929
if f.Kind() != source.Go {
3030
return nil, nil
3131
}
32-
ident, err := source.Identifier(ctx, snapshot, f, params.Position, source.WidestCheckPackageHandle)
32+
phs, err := snapshot.PackageHandles(ctx, snapshot.Handle(ctx, f))
3333
if err != nil {
3434
return nil, nil
3535
}
36-
references, err := ident.References(ctx)
37-
if err != nil {
38-
log.Error(ctx, "no references", err, tag.Of("Identifier", ident.Name))
39-
}
4036

4137
// Get the location of each reference to return as the result.
42-
locations := make([]protocol.Location, 0, len(references))
43-
seen := make(map[span.Span]bool)
44-
for _, ref := range references {
45-
refSpan, err := ref.Span()
38+
var (
39+
locations []protocol.Location
40+
seen = make(map[span.Span]bool)
41+
lastIdent *source.IdentifierInfo
42+
)
43+
for _, ph := range phs {
44+
ident, err := source.Identifier(ctx, snapshot, f, params.Position, source.SpecificPackageHandle(ph.ID()))
4645
if err != nil {
47-
return nil, err
46+
if err == source.ErrNoIdentFound {
47+
return nil, err
48+
}
49+
log.Error(ctx, "no identifier", err, tag.Of("Identifier", ident.Name))
50+
continue
4851
}
49-
if seen[refSpan] {
50-
continue // already added this location
51-
}
52-
seen[refSpan] = true
53-
refRange, err := ref.Range()
52+
53+
lastIdent = ident
54+
55+
references, err := ident.References(ctx)
5456
if err != nil {
55-
return nil, err
57+
log.Error(ctx, "no references", err, tag.Of("Identifier", ident.Name))
58+
continue
59+
}
60+
61+
for _, ref := range references {
62+
refSpan, err := ref.Span()
63+
if err != nil {
64+
return nil, err
65+
}
66+
if seen[refSpan] {
67+
continue // already added this location
68+
}
69+
seen[refSpan] = true
70+
refRange, err := ref.Range()
71+
if err != nil {
72+
return nil, err
73+
}
74+
locations = append(locations, protocol.Location{
75+
URI: protocol.NewURI(ref.URI()),
76+
Range: refRange,
77+
})
5678
}
57-
locations = append(locations, protocol.Location{
58-
URI: protocol.NewURI(ref.URI()),
59-
Range: refRange,
60-
})
6179
}
80+
6281
// Only add the identifier's declaration if the client requests it.
63-
if params.Context.IncludeDeclaration {
64-
rng, err := ident.Declaration.Range()
82+
if params.Context.IncludeDeclaration && lastIdent != nil {
83+
rng, err := lastIdent.Declaration.Range()
6584
if err != nil {
6685
return nil, err
6786
}
6887
locations = append([]protocol.Location{
6988
{
70-
URI: protocol.NewURI(ident.Declaration.URI()),
89+
URI: protocol.NewURI(lastIdent.Declaration.URI()),
7190
Range: rng,
7291
},
7392
}, locations...)
7493
}
94+
7595
return locations, nil
7696
}

internal/lsp/source/util.go

+1
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ package source
66

77
import (
88
"context"
9+
"fmt"
910
"go/ast"
1011
"go/printer"
1112
"go/token"

internal/lsp/testdata/summary.txt.golden

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
-- summary --
2-
CompletionsCount = 220
2+
CompletionsCount = 221
33
CompletionSnippetCount = 51
44
UnimportedCompletionsCount = 4
55
DeepCompletionsCount = 5

0 commit comments

Comments
 (0)