gopls/internal/lsp/source: KnownPackagePaths: avoid loading
This operation is logically just about metadata.
This change makes it avoid package loading.
(This is a warm-up exercise for more invasive variants
in many other places.)
Change-Id: I408c754f10665d3959ee0f4ff6cf95c54ec94f3e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/452056
Reviewed-by: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
diff --git a/gopls/internal/lsp/cache/check.go b/gopls/internal/lsp/cache/check.go
index 663cf25..8adec22 100644
--- a/gopls/internal/lsp/cache/check.go
+++ b/gopls/internal/lsp/cache/check.go
@@ -721,7 +721,7 @@
for _, depErr := range relevantErrors {
for i := len(depErr.ImportStack) - 1; i >= 0; i-- {
item := depErr.ImportStack[i]
- m := s.getMetadata(PackageID(item))
+ m := s.Metadata(PackageID(item))
if m == nil || m.Module == nil {
continue
}
diff --git a/gopls/internal/lsp/cache/mod_tidy.go b/gopls/internal/lsp/cache/mod_tidy.go
index 0a4b88e..9495f3f 100644
--- a/gopls/internal/lsp/cache/mod_tidy.go
+++ b/gopls/internal/lsp/cache/mod_tidy.go
@@ -193,7 +193,7 @@
// workspace.
// TODO(adonovan): opt: opportunities for parallelism abound.
for _, id := range snapshot.workspacePackageIDs() {
- m := snapshot.getMetadata(id)
+ m := snapshot.Metadata(id)
if m == nil {
return nil, fmt.Errorf("no metadata for %s", id)
}
diff --git a/gopls/internal/lsp/cache/snapshot.go b/gopls/internal/lsp/cache/snapshot.go
index 28d6fff..8b02dea 100644
--- a/gopls/internal/lsp/cache/snapshot.go
+++ b/gopls/internal/lsp/cache/snapshot.go
@@ -1249,10 +1249,9 @@
return match
}
-func (s *snapshot) getMetadata(id PackageID) *source.Metadata {
+func (s *snapshot) Metadata(id PackageID) *source.Metadata {
s.mu.Lock()
defer s.mu.Unlock()
-
return s.meta.metadata[id]
}
diff --git a/gopls/internal/lsp/source/known_packages.go b/gopls/internal/lsp/source/known_packages.go
index 6f3bf8c..19abf8f 100644
--- a/gopls/internal/lsp/source/known_packages.go
+++ b/gopls/internal/lsp/source/known_packages.go
@@ -7,6 +7,8 @@
import (
"context"
"fmt"
+ "go/parser"
+ "go/token"
"sort"
"strings"
"sync"
@@ -20,78 +22,96 @@
// packages in the package graph that could potentially be imported by
// the given file. The list is ordered lexicographically, except that
// all dot-free paths (standard packages) appear before dotful ones.
+//
+// It is part of the gopls.list_known_packages command.
func KnownPackagePaths(ctx context.Context, snapshot Snapshot, fh VersionedFileHandle) ([]PackagePath, error) {
- // TODO(adonovan): this whole algorithm could be more
- // simply expressed in terms of Metadata, not Packages.
- // All we need below is:
- // - for fh: Metadata.{DepsByPkgPath,Path,Name}
- // - for all cached packages: Metadata.{Path,Name,ForTest,DepsByPkgPath}.
- pkg, pgf, err := GetParsedFile(ctx, snapshot, fh, NarrowestPackage)
+ // This algorithm is expressed in terms of Metadata, not Packages,
+ // so it doesn't cause or wait for type checking.
+
+ // Find a Metadata containing the file.
+ metas, err := snapshot.MetadataForFile(ctx, fh.URI())
if err != nil {
- return nil, fmt.Errorf("GetParsedFile: %w", err)
+ return nil, err // e.g. context cancelled
}
- alreadyImported := map[PackagePath]struct{}{}
- for _, imp := range pkg.Imports() {
- alreadyImported[imp.PkgPath()] = struct{}{}
+ if len(metas) == 0 {
+ return nil, fmt.Errorf("no loaded package contain file %s", fh.URI())
}
- pkgs, err := snapshot.CachedImportPaths(ctx)
+ current := metas[0] // pick one arbitrarily (they should all have the same package path)
+
+ // Parse the file's imports so we can compute which
+ // PackagePaths are imported by this specific file.
+ src, err := fh.Read()
if err != nil {
return nil, err
}
- var (
- seen = make(map[PackagePath]struct{})
- paths []PackagePath
- )
- for path, knownPkg := range pkgs {
+ file, err := parser.ParseFile(token.NewFileSet(), fh.URI().Filename(), src, parser.ImportsOnly)
+ if err != nil {
+ return nil, err
+ }
+ imported := make(map[PackagePath]bool)
+ for _, imp := range file.Imports {
+ if id := current.DepsByImpPath[UnquoteImportPath(imp)]; id != "" {
+ if m := snapshot.Metadata(id); m != nil {
+ imported[m.PkgPath] = true
+ }
+ }
+ }
+
+ // Now find candidates among known packages.
+ knownPkgs, err := snapshot.AllValidMetadata(ctx)
+ if err != nil {
+ return nil, err
+ }
+ seen := make(map[PackagePath]bool)
+ for _, knownPkg := range knownPkgs {
// package main cannot be imported
- if knownPkg.Name() == "main" {
+ if knownPkg.Name == "main" {
continue
}
// test packages cannot be imported
- if knownPkg.ForTest() != "" {
+ if knownPkg.ForTest != "" {
continue
}
- // no need to import what the file already imports
- if _, ok := alreadyImported[path]; ok {
+ // No need to import what the file already imports.
+ // This check is based on PackagePath, not PackageID,
+ // so that all test variants are filtered out too.
+ if imported[knownPkg.PkgPath] {
continue
}
- // snapshot.CachedImportPaths could have multiple versions of a pkg
- if _, ok := seen[path]; ok {
- continue
- }
- seen[path] = struct{}{}
// make sure internal packages are importable by the file
- if !IsValidImport(pkg.PkgPath(), path) {
+ if !IsValidImport(current.PkgPath, knownPkg.PkgPath) {
continue
}
// naive check on cyclical imports
- if isDirectlyCyclical(pkg, knownPkg) {
+ if isDirectlyCyclical(current, knownPkg) {
continue
}
- paths = append(paths, path)
- seen[path] = struct{}{}
+ // AllValidMetadata may have multiple variants of a pkg.
+ seen[knownPkg.PkgPath] = true
}
- err = snapshot.RunProcessEnvFunc(ctx, func(o *imports.Options) error {
- var mu sync.Mutex
+
+ // Augment the set by invoking the goimports algorithm.
+ if err := snapshot.RunProcessEnvFunc(ctx, func(o *imports.Options) error {
ctx, cancel := context.WithTimeout(ctx, time.Millisecond*80)
defer cancel()
- return imports.GetAllCandidates(ctx, func(ifix imports.ImportFix) {
- mu.Lock()
- defer mu.Unlock()
+ var seenMu sync.Mutex
+ wrapped := func(ifix imports.ImportFix) {
+ seenMu.Lock()
+ defer seenMu.Unlock()
// TODO(adonovan): what if the actual package path has a vendor/ prefix?
- path := PackagePath(ifix.StmtInfo.ImportPath)
- if _, ok := seen[path]; ok {
- return
- }
- paths = append(paths, path)
- seen[path] = struct{}{}
- }, "", pgf.URI.Filename(), string(pkg.Name()), o.Env)
- })
- if err != nil {
- // if an error occurred, we still have a decent list we can
- // show to the user through snapshot.CachedImportPaths
+ seen[PackagePath(ifix.StmtInfo.ImportPath)] = true
+ }
+ return imports.GetAllCandidates(ctx, wrapped, "", fh.URI().Filename(), string(current.Name), o.Env)
+ }); err != nil {
+ // If goimports failed, proceed with just the candidates from the metadata.
event.Error(ctx, "imports.GetAllCandidates", err)
}
+
+ // Sort lexicographically, but with std before non-std packages.
+ paths := make([]PackagePath, 0, len(seen))
+ for path := range seen {
+ paths = append(paths, path)
+ }
sort.Slice(paths, func(i, j int) bool {
importI, importJ := paths[i], paths[j]
iHasDot := strings.Contains(string(importI), ".")
@@ -101,6 +121,7 @@
}
return importI < importJ
})
+
return paths, nil
}
@@ -109,11 +130,11 @@
// a list of importable packages already generates a very large list
// and having a few false positives in there could be worth the
// performance snappiness.
-func isDirectlyCyclical(pkg, imported Package) bool {
- for _, imp := range imported.Imports() {
- if imp.PkgPath() == pkg.PkgPath() {
- return true
- }
- }
- return false
+//
+// TODO(adonovan): ensure that metadata graph is always cyclic!
+// Many algorithms will get confused or even stuck in the
+// presence of cycles. Then replace this function by 'false'.
+func isDirectlyCyclical(pkg, imported *Metadata) bool {
+ _, ok := imported.DepsByPkgPath[pkg.PkgPath]
+ return ok
}
diff --git a/gopls/internal/lsp/source/view.go b/gopls/internal/lsp/source/view.go
index 52d9978..1084c74 100644
--- a/gopls/internal/lsp/source/view.go
+++ b/gopls/internal/lsp/source/view.go
@@ -185,6 +185,9 @@
// CachedImportPaths returns all the imported packages loaded in this
// snapshot, indexed by their package path (not import path, despite the name)
// and checked in TypecheckWorkspace mode.
+ //
+ // To reduce latency, it does not wait for type-checking to complete.
+ // It is intended for use only in completions.
CachedImportPaths(ctx context.Context) (map[PackagePath]Package, error)
// KnownPackages returns a new unordered list of all packages
@@ -210,7 +213,11 @@
// Symbols returns all symbols in the snapshot.
Symbols(ctx context.Context) map[span.URI][]Symbol
- // Metadata returns a new slice containing metadata for each
+ // Metadata returns the metadata for the specified package,
+ // or nil if it was not found.
+ Metadata(id PackageID) *Metadata
+
+ // MetadataForFile returns a new slice containing metadata for each
// package containing the Go file identified by uri, ordered by the
// number of CompiledGoFiles (i.e. "narrowest" to "widest" package).
// It returns an error if the context was cancelled.
