Skip to content

Commit

Permalink
gopls/internal/lsp/source: use PackageFromFile in Identifier
Browse files Browse the repository at this point in the history
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 <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Run-TryBot: Robert Findley <[email protected]>
gopls-CI: kokoro <[email protected]>
  • Loading branch information
findleyr committed Oct 5, 2022
1 parent ff4ff8b commit c5514b7
Showing 1 changed file with 11 additions and 32 deletions.
43 changes: 11 additions & 32 deletions gopls/internal/lsp/source/identifier.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import (
"go/parser"
"go/token"
"go/types"
"sort"
"strconv"

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

// TODO(rfindley): Why isn't this PackageForFile? A single package should
// suffice to find identifier info.
pkgs, err := snapshot.PackagesForFile(ctx, fh.URI(), TypecheckAll, false)
pkg, err := snapshot.PackageForFile(ctx, fh.URI(), TypecheckFull, NarrowestPackage)
if err != nil {
return nil, err
}
if len(pkgs) == 0 {
return nil, fmt.Errorf("no packages for file %v", fh.URI())
pgf, err := pkg.File(fh.URI())
if err != nil {
// We shouldn't get a package from PackagesForFile that doesn't actually
// contain the file.
bug.Report("missing package file", bug.Data{"pkg": pkg.ID(), "file": fh.URI()})
return nil, err
}
sort.Slice(pkgs, func(i, j int) bool {
// Prefer packages with a more complete parse mode.
if pkgs[i].ParseMode() != pkgs[j].ParseMode() {
return pkgs[i].ParseMode() > pkgs[j].ParseMode()
}
return len(pkgs[i].CompiledGoFiles()) < len(pkgs[j].CompiledGoFiles())
})
var findErr error
for _, pkg := range pkgs {
pgf, err := pkg.File(fh.URI())
if err != nil {
// We shouldn't get a package from PackagesForFile that doesn't actually
// contain the file.
bug.Report("missing package file", bug.Data{"pkg": pkg.ID(), "file": fh.URI()})
return nil, err
}
pos, err := pgf.Mapper.Pos(position)
if err != nil {
return nil, err
}
var ident *IdentifierInfo
ident, findErr = findIdentifier(ctx, snapshot, pkg, pgf, pos)
if findErr == nil {
return ident, nil
}
pos, err := pgf.Mapper.Pos(position)
if err != nil {
return nil, err
}
return nil, findErr
return findIdentifier(ctx, snapshot, pkg, pgf, pos)
}

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

0 comments on commit c5514b7

Please sign in to comment.