From d36a54b56b46cb3de852f65cec98b6a563ff8011 Mon Sep 17 00:00:00 2001 From: Rob Findley Date: Fri, 9 Jul 2021 19:30:35 -0400 Subject: [PATCH] internal/lsp: improve package search in a couple places When we open a file in a package, independent of whether it is in the workspace, we type check in ParseFull mode. However, several other code paths don't find this better parse mode. We need a better abstraction, but for now improve a couple code paths specifically for the purpose of fixing Hover content. Updates golang/go#46158 Updates golang/go#46902 Change-Id: I34c0432fdba406d569ea963ab4366336068767a2 Reviewed-on: https://go-review.googlesource.com/c/tools/+/333689 Trust: Robert Findley Run-TryBot: Robert Findley gopls-CI: kokoro TryBot-Result: Go Bot Reviewed-by: Rebecca Stambler --- internal/lsp/cache/check.go | 2 ++ internal/lsp/cache/pkg.go | 4 ++++ internal/lsp/source/identifier.go | 4 ++++ internal/lsp/source/util.go | 24 ++++++++++++++++++++---- internal/lsp/source/view.go | 1 + 5 files changed, 31 insertions(+), 4 deletions(-) diff --git a/internal/lsp/cache/check.go b/internal/lsp/cache/check.go index 277ad8b91d2..89094b0e3e9 100644 --- a/internal/lsp/cache/check.go +++ b/internal/lsp/cache/check.go @@ -539,8 +539,10 @@ func doTypeCheck(ctx context.Context, snapshot *snapshot, m *metadata, mode sour for _, cgf := range pkg.compiledGoFiles { files = append(files, cgf.File) } + // Type checking errors are handled via the config, so ignore them here. _ = check.Files(files) + // If the context was cancelled, we may have returned a ton of transient // errors to the type checker. Swallow them. if ctx.Err() != nil { diff --git a/internal/lsp/cache/pkg.go b/internal/lsp/cache/pkg.go index aa075641f03..5a87a149bee 100644 --- a/internal/lsp/cache/pkg.go +++ b/internal/lsp/cache/pkg.go @@ -61,6 +61,10 @@ func (p *pkg) PkgPath() string { return string(p.m.pkgPath) } +func (p *pkg) ParseMode() source.ParseMode { + return p.mode +} + func (p *pkg) CompiledGoFiles() []*source.ParsedGoFile { return p.compiledGoFiles } diff --git a/internal/lsp/source/identifier.go b/internal/lsp/source/identifier.go index ff86eaa1a3e..ee8684bdee1 100644 --- a/internal/lsp/source/identifier.go +++ b/internal/lsp/source/identifier.go @@ -85,6 +85,10 @@ func Identifier(ctx context.Context, snapshot Snapshot, fh FileHandle, pos proto return nil, fmt.Errorf("no packages for file %v", fh.URI()) } 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 diff --git a/internal/lsp/source/util.go b/internal/lsp/source/util.go index a917a54067a..a30cc75e8e3 100644 --- a/internal/lsp/source/util.go +++ b/internal/lsp/source/util.go @@ -274,19 +274,35 @@ func CompareDiagnostic(a, b *Diagnostic) int { return 1 } -// FindPackageFromPos finds the parsed file for a position in a given search -// package. +// FindPackageFromPos finds the first package containing pos in its +// type-checked AST. func FindPackageFromPos(ctx context.Context, snapshot Snapshot, pos token.Pos) (Package, error) { tok := snapshot.FileSet().File(pos) if tok == nil { return nil, errors.Errorf("no file for pos %v", pos) } uri := span.URIFromPath(tok.Name()) - pkgs, err := snapshot.PackagesForFile(ctx, uri, TypecheckWorkspace) + // Search all packages: some callers may be working with packages not + // type-checked in workspace mode. + pkgs, err := snapshot.PackagesForFile(ctx, uri, TypecheckAll) if err != nil { return nil, err } - return pkgs[0], nil + // Only return the package if it actually type-checked the given position. + for _, pkg := range pkgs { + parsed, err := pkg.File(uri) + if err != nil { + return nil, err + } + if parsed == nil { + continue + } + if parsed.Tok.Base() != tok.Base() { + continue + } + return pkg, nil + } + return nil, errors.Errorf("no package for given file position") } // findFileInDeps finds uri in pkg or its dependencies. diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go index 748bdb1a6a8..74b77ca325c 100644 --- a/internal/lsp/source/view.go +++ b/internal/lsp/source/view.go @@ -581,6 +581,7 @@ type Package interface { Version() *module.Version HasListOrParseErrors() bool HasTypeErrors() bool + ParseMode() ParseMode } type CriticalError struct {