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 <rfindley@google.com> Run-TryBot: Robert Findley <rfindley@google.com> gopls-CI: kokoro <noreply+kokoro@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Rebecca Stambler <rstambler@golang.org>
golang · Jul 13, 2021 · d36a54b · d36a54b
1 parent 3844600
commit d36a54b
Show file tree

Hide file tree

Showing 5 changed files with 31 additions and 4 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -581,6 +581,7 @@ type Package interface {
 	Version() *module.Version
 	HasListOrParseErrors() bool
 	HasTypeErrors() bool
+	ParseMode() ParseMode
 }
 
 type CriticalError struct {