Skip to content

Commit c5514b7

Browse files
committed
gopls/internal/lsp/source: use PackageFromFile in Identifier
When searching for declaration information about a position, it should suffice to search the narrowest fully type-checked package containing the file. This should significantly improve performance when there are many test variants of the current package, that have not yet been type-checked in the ParseFull mode (as reported in golang/go#55293). For golang/go#55293 Change-Id: I89a1999f9fe82dea51dd47db769c90b69be5e454 Reviewed-on: https://go-review.googlesource.com/c/tools/+/438496 Reviewed-by: Alan Donovan <adonovan@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Robert Findley <rfindley@google.com> gopls-CI: kokoro <noreply+kokoro@google.com>
1 parent ff4ff8b commit c5514b7

File tree

1 file changed

+11
-32
lines changed

1 file changed

+11
-32
lines changed

gopls/internal/lsp/source/identifier.go

+11-32
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import (
1212
"go/parser"
1313
"go/token"
1414
"go/types"
15-
"sort"
1615
"strconv"
1716

1817
"golang.org/x/tools/go/ast/astutil"
@@ -81,42 +80,22 @@ func Identifier(ctx context.Context, snapshot Snapshot, fh FileHandle, position
8180
ctx, done := event.Start(ctx, "source.Identifier")
8281
defer done()
8382

84-
// TODO(rfindley): Why isn't this PackageForFile? A single package should
85-
// suffice to find identifier info.
86-
pkgs, err := snapshot.PackagesForFile(ctx, fh.URI(), TypecheckAll, false)
83+
pkg, err := snapshot.PackageForFile(ctx, fh.URI(), TypecheckFull, NarrowestPackage)
8784
if err != nil {
8885
return nil, err
8986
}
90-
if len(pkgs) == 0 {
91-
return nil, fmt.Errorf("no packages for file %v", fh.URI())
87+
pgf, err := pkg.File(fh.URI())
88+
if err != nil {
89+
// We shouldn't get a package from PackagesForFile that doesn't actually
90+
// contain the file.
91+
bug.Report("missing package file", bug.Data{"pkg": pkg.ID(), "file": fh.URI()})
92+
return nil, err
9293
}
93-
sort.Slice(pkgs, func(i, j int) bool {
94-
// Prefer packages with a more complete parse mode.
95-
if pkgs[i].ParseMode() != pkgs[j].ParseMode() {
96-
return pkgs[i].ParseMode() > pkgs[j].ParseMode()
97-
}
98-
return len(pkgs[i].CompiledGoFiles()) < len(pkgs[j].CompiledGoFiles())
99-
})
100-
var findErr error
101-
for _, pkg := range pkgs {
102-
pgf, err := pkg.File(fh.URI())
103-
if err != nil {
104-
// We shouldn't get a package from PackagesForFile that doesn't actually
105-
// contain the file.
106-
bug.Report("missing package file", bug.Data{"pkg": pkg.ID(), "file": fh.URI()})
107-
return nil, err
108-
}
109-
pos, err := pgf.Mapper.Pos(position)
110-
if err != nil {
111-
return nil, err
112-
}
113-
var ident *IdentifierInfo
114-
ident, findErr = findIdentifier(ctx, snapshot, pkg, pgf, pos)
115-
if findErr == nil {
116-
return ident, nil
117-
}
94+
pos, err := pgf.Mapper.Pos(position)
95+
if err != nil {
96+
return nil, err
11897
}
119-
return nil, findErr
98+
return findIdentifier(ctx, snapshot, pkg, pgf, pos)
12099
}
121100

122101
// ErrNoIdentFound is error returned when no identifier is found at a particular position

0 commit comments

Comments
 (0)