internal/lsp: remove PackageHandle
Just like ParseGoHandle, PackageHandle isn't very useful as part of the
public API. Remove it.
Having PackagesForFile take a URI rather than a FileHandle seems
reasonable, and made me wonder if that logic applies to other calls like
ParseGo. For now I'm going to stop here. I could also revert that part
of the change.
Change-Id: Idba8e9fdba0b0c48e841a698eb97e47fd5f23cf5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244637
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
diff --git a/internal/lsp/cache/analysis.go b/internal/lsp/cache/analysis.go
index d2ebdb9..ed74ee1 100644
--- a/internal/lsp/cache/analysis.go
+++ b/internal/lsp/cache/analysis.go
@@ -84,14 +84,14 @@
func (s *snapshot) actionHandle(ctx context.Context, id packageID, a *analysis.Analyzer) (*actionHandle, error) {
ph := s.getPackage(id, source.ParseFull)
if ph == nil {
- return nil, errors.Errorf("no PackageHandle for %s", id)
+ return nil, errors.Errorf("no package for %s", id)
}
act := s.getActionHandle(id, ph.mode, a)
if act != nil {
return act, nil
}
if len(ph.key) == 0 {
- return nil, errors.Errorf("no key for PackageHandle %s", id)
+ return nil, errors.Errorf("no key for package %s", id)
}
pkg, err := ph.check(ctx, s)
if err != nil {
diff --git a/internal/lsp/cache/check.go b/internal/lsp/cache/check.go
index 8ac2fb7..6a96b98 100644
--- a/internal/lsp/cache/check.go
+++ b/internal/lsp/cache/check.go
@@ -27,7 +27,6 @@
type packageHandleKey string
-// packageHandle implements source.PackageHandle.
type packageHandle struct {
handle *memoize.Handle
@@ -58,13 +57,13 @@
err error
}
-// buildPackageHandle returns a source.PackageHandle for a given package and config.
+// buildPackageHandle returns a packageHandle for a given package and mode.
func (s *snapshot) buildPackageHandle(ctx context.Context, id packageID, mode source.ParseMode) (*packageHandle, error) {
if ph := s.getPackage(id, mode); ph != nil {
return ph, nil
}
- // Build the PackageHandle for this ID and its dependencies.
+ // Build the packageHandle for this ID and its dependencies.
ph, deps, err := s.buildKey(ctx, id, mode)
if err != nil {
return nil, err
@@ -98,7 +97,7 @@
})
ph.handle = h
- // Cache the PackageHandle in the snapshot. If a package handle has already
+ // Cache the handle in the snapshot. If a package handle has already
// been cached, addPackage will return the cached value. This is fine,
// since the original package handle above will have no references and be
// garbage collected.
diff --git a/internal/lsp/cache/mod_tidy.go b/internal/lsp/cache/mod_tidy.go
index 8e7305c..4998ace 100644
--- a/internal/lsp/cache/mod_tidy.go
+++ b/internal/lsp/cache/mod_tidy.go
@@ -88,21 +88,10 @@
if err != nil {
return nil, err
}
- wsPhs, err := s.WorkspacePackages(ctx)
- if ctx.Err() != nil {
- return nil, ctx.Err()
- }
+ workspacePkgs, err := s.WorkspacePackages(ctx)
if err != nil {
return nil, err
}
- var workspacePkgs []source.Package
- for _, ph := range wsPhs {
- pkg, err := ph.Check(ctx, s)
- if err != nil {
- return nil, err
- }
- workspacePkgs = append(workspacePkgs, pkg)
- }
importHash, err := hashImports(ctx, workspacePkgs)
if err != nil {
return nil, err
diff --git a/internal/lsp/cache/pkg.go b/internal/lsp/cache/pkg.go
index 1ea98d7..ebe440e 100644
--- a/internal/lsp/cache/pkg.go
+++ b/internal/lsp/cache/pkg.go
@@ -71,7 +71,7 @@
return gf, nil
}
}
- return nil, errors.Errorf("no ParseGoHandle for %s", uri)
+ return nil, errors.Errorf("no parsed file for %s in %v", uri, p.m.id)
}
func (p *pkg) GetSyntax() []*ast.File {
diff --git a/internal/lsp/cache/snapshot.go b/internal/lsp/cache/snapshot.go
index 5beb5de..d9fb962 100644
--- a/internal/lsp/cache/snapshot.go
+++ b/internal/lsp/cache/snapshot.go
@@ -28,7 +28,6 @@
"golang.org/x/tools/internal/packagesinternal"
"golang.org/x/tools/internal/span"
"golang.org/x/tools/internal/typesinternal"
- errors "golang.org/x/xerrors"
)
type snapshot struct {
@@ -258,17 +257,13 @@
return hashContents([]byte(strings.Join(unsaved, "")))
}
-func (s *snapshot) PackageHandles(ctx context.Context, fh source.FileHandle) ([]source.PackageHandle, error) {
- if fh.Kind() != source.Go {
- panic("called PackageHandles on a non-Go FileHandle")
- }
-
- ctx = event.Label(ctx, tag.URI.Of(fh.URI()))
+func (s *snapshot) PackagesForFile(ctx context.Context, uri span.URI) ([]source.Package, error) {
+ ctx = event.Label(ctx, tag.URI.Of(uri))
// Check if we should reload metadata for the file. We don't invalidate IDs
// (though we should), so the IDs will be a better source of truth than the
// metadata. If there are no IDs for the file, then we should also reload.
- ids := s.getIDsForURI(fh.URI())
+ ids := s.getIDsForURI(uri)
reload := len(ids) == 0
for _, id := range ids {
// Reload package metadata if any of the metadata has missing
@@ -283,40 +278,31 @@
// calls to packages.Load. Determine what we should do instead.
}
if reload {
- if err := s.load(ctx, fileURI(fh.URI())); err != nil {
+ if err := s.load(ctx, fileURI(uri)); err != nil {
return nil, err
}
}
// Get the list of IDs from the snapshot again, in case it has changed.
- var phs []source.PackageHandle
- for _, id := range s.getIDsForURI(fh.URI()) {
- ph, err := s.packageHandle(ctx, id, source.ParseFull)
+ var pkgs []source.Package
+ for _, id := range s.getIDsForURI(uri) {
+ pkg, err := s.checkedPackage(ctx, id, source.ParseFull)
if err != nil {
return nil, err
}
- phs = append(phs, ph)
+ pkgs = append(pkgs, pkg)
}
- return phs, nil
+ return pkgs, nil
}
-// packageHandle returns a PackageHandle for the given ID. It assumes that
-// the metadata for the given ID has already been loaded, but if the
-// PackageHandle has not been constructed, it will rebuild it.
-func (s *snapshot) packageHandle(ctx context.Context, id packageID, mode source.ParseMode) (*packageHandle, error) {
- ph := s.getPackage(id, mode)
- if ph != nil {
- return ph, nil
+func (s *snapshot) checkedPackage(ctx context.Context, id packageID, mode source.ParseMode) (*pkg, error) {
+ ph, err := s.buildPackageHandle(ctx, id, mode)
+ if err != nil {
+ return nil, err
}
- // Don't reload metadata in this function.
- // Callers of this function must reload metadata themselves.
- m := s.getMetadata(id)
- if m == nil {
- return nil, errors.Errorf("%s has no metadata", id)
- }
- return s.buildPackageHandle(ctx, m.id, mode)
+ return ph.check(ctx, s)
}
-func (s *snapshot) GetReverseDependencies(ctx context.Context, id string) ([]source.PackageHandle, error) {
+func (s *snapshot) GetReverseDependencies(ctx context.Context, id string) ([]source.Package, error) {
if err := s.awaitLoaded(ctx); err != nil {
return nil, err
}
@@ -326,15 +312,15 @@
// Make sure to delete the original package ID from the map.
delete(ids, packageID(id))
- var results []source.PackageHandle
+ var pkgs []source.Package
for id := range ids {
- ph, err := s.packageHandle(ctx, id, source.ParseFull)
+ pkg, err := s.checkedPackage(ctx, id, source.ParseFull)
if err != nil {
return nil, err
}
- results = append(results, ph)
+ pkgs = append(pkgs, pkg)
}
- return results, nil
+ return pkgs, nil
}
// transitiveReverseDependencies populates the uris map with file URIs
@@ -431,64 +417,55 @@
return ids
}
-func (s *snapshot) WorkspacePackages(ctx context.Context) ([]source.PackageHandle, error) {
+func (s *snapshot) WorkspacePackages(ctx context.Context) ([]source.Package, error) {
if err := s.awaitLoaded(ctx); err != nil {
return nil, err
}
- var results []source.PackageHandle
+ var pkgs []source.Package
for _, pkgID := range s.workspacePackageIDs() {
- ph, err := s.packageHandle(ctx, pkgID, source.ParseFull)
+ pkg, err := s.checkedPackage(ctx, pkgID, source.ParseFull)
if err != nil {
return nil, err
}
- results = append(results, ph)
+ pkgs = append(pkgs, pkg)
}
- return results, nil
+ return pkgs, nil
}
-func (s *snapshot) KnownPackages(ctx context.Context) ([]source.PackageHandle, error) {
+func (s *snapshot) KnownPackages(ctx context.Context) ([]source.Package, error) {
if err := s.awaitLoaded(ctx); err != nil {
return nil, err
}
- // Collect PackageHandles for all of the workspace packages first.
- // They may need to be reloaded if their metadata has been invalidated.
- wsPackages := make(map[packageID]bool)
- s.mu.Lock()
- for id := range s.workspacePackages {
- wsPackages[id] = true
- }
- s.mu.Unlock()
- var results []source.PackageHandle
- for pkgID := range wsPackages {
- ph, err := s.packageHandle(ctx, pkgID, source.ParseFull)
- if err != nil {
- return nil, err
- }
- results = append(results, ph)
- }
-
- // Once all workspace packages have been checked, the metadata will be up-to-date.
- // Add all packages known in the workspace (that haven't already been added).
- pkgIDs := make(map[packageID]bool)
+ // The WorkspaceSymbols implementation relies on this function returning
+ // workspace packages first.
+ wsPackages := s.workspacePackageIDs()
+ var otherPackages []packageID
s.mu.Lock()
for id := range s.metadata {
- if !wsPackages[id] {
- pkgIDs[id] = true
+ if _, ok := s.workspacePackages[id]; ok {
+ continue
}
+ otherPackages = append(otherPackages, id)
}
s.mu.Unlock()
- for pkgID := range pkgIDs {
- // Metadata for these packages should already be up-to-date,
- // so just build the package handle directly (without a reload).
- ph, err := s.buildPackageHandle(ctx, pkgID, source.ParseExported)
+ var pkgs []source.Package
+ for _, id := range wsPackages {
+ pkg, err := s.checkedPackage(ctx, id, source.ParseFull)
if err != nil {
return nil, err
}
- results = append(results, ph)
+ pkgs = append(pkgs, pkg)
}
- return results, nil
+ for _, id := range otherPackages {
+ pkg, err := s.checkedPackage(ctx, id, source.ParseExported)
+ if err != nil {
+ return nil, err
+ }
+ pkgs = append(pkgs, pkg)
+ }
+ return pkgs, nil
}
func (s *snapshot) CachedImportPaths(ctx context.Context) (map[string]source.Package, error) {
@@ -507,7 +484,7 @@
}
for importPath, newPkg := range cachedPkg.imports {
if oldPkg, ok := results[string(importPath)]; ok {
- // Using the same trick as NarrowestPackageHandle, prefer non-variants.
+ // Using the same trick as NarrowestPackage, prefer non-variants.
if len(newPkg.compiledGoFiles) < len(oldPkg.(*pkg).compiledGoFiles) {
results[string(importPath)] = newPkg
}
diff --git a/internal/lsp/cache/view.go b/internal/lsp/cache/view.go
index e2bb085..cbe70ec 100644
--- a/internal/lsp/cache/view.go
+++ b/internal/lsp/cache/view.go
@@ -235,10 +235,10 @@
if sumFH != nil {
sumContents, err := sumFH.Read()
if err != nil {
- return "", nil, err
+ return "", cleanup, err
}
if err := ioutil.WriteFile(tmpSumName, sumContents, 0655); err != nil {
- return "", nil, err
+ return "", cleanup, err
}
}
diff --git a/internal/lsp/code_action.go b/internal/lsp/code_action.go
index 29c99ed..bd19f55 100644
--- a/internal/lsp/code_action.go
+++ b/internal/lsp/code_action.go
@@ -118,16 +118,16 @@
if ctx.Err() != nil {
return nil, ctx.Err()
}
- phs, err := snapshot.PackageHandles(ctx, fh)
+ pkgs, err := snapshot.PackagesForFile(ctx, fh.URI())
if err != nil {
return nil, err
}
- ph, err := source.WidestPackageHandle(phs)
+ pkg, err := source.WidestPackage(pkgs)
if err != nil {
return nil, err
}
if (wanted[protocol.QuickFix] || wanted[protocol.SourceFixAll]) && len(diagnostics) > 0 {
- analysisQuickFixes, highConfidenceEdits, err := analysisFixes(ctx, snapshot, ph, diagnostics)
+ analysisQuickFixes, highConfidenceEdits, err := analysisFixes(ctx, snapshot, pkg, diagnostics)
if err != nil {
return nil, err
}
@@ -159,14 +159,14 @@
}
// Add any suggestions that do not necessarily fix any diagnostics.
if wanted[protocol.RefactorRewrite] {
- fixes, err := convenienceFixes(ctx, snapshot, ph, uri, params.Range)
+ fixes, err := convenienceFixes(ctx, snapshot, pkg, uri, params.Range)
if err != nil {
return nil, err
}
codeActions = append(codeActions, fixes...)
}
if wanted[protocol.RefactorExtract] {
- fixes, err := extractionFixes(ctx, snapshot, ph, uri, params.Range)
+ fixes, err := extractionFixes(ctx, snapshot, pkg, uri, params.Range)
if err != nil {
return nil, err
}
@@ -237,7 +237,7 @@
return results
}
-func analysisFixes(ctx context.Context, snapshot source.Snapshot, ph source.PackageHandle, diagnostics []protocol.Diagnostic) ([]protocol.CodeAction, []protocol.TextDocumentEdit, error) {
+func analysisFixes(ctx context.Context, snapshot source.Snapshot, pkg source.Package, diagnostics []protocol.Diagnostic) ([]protocol.CodeAction, []protocol.TextDocumentEdit, error) {
if len(diagnostics) == 0 {
return nil, nil, nil
}
@@ -246,7 +246,7 @@
sourceFixAllEdits []protocol.TextDocumentEdit
)
for _, diag := range diagnostics {
- srcErr, analyzer, ok := findSourceError(ctx, snapshot, ph.ID(), diag)
+ srcErr, analyzer, ok := findSourceError(ctx, snapshot, pkg.ID(), diag)
if !ok {
continue
}
@@ -341,7 +341,7 @@
return nil
}
-func convenienceFixes(ctx context.Context, snapshot source.Snapshot, ph source.PackageHandle, uri span.URI, rng protocol.Range) ([]protocol.CodeAction, error) {
+func convenienceFixes(ctx context.Context, snapshot source.Snapshot, pkg source.Package, uri span.URI, rng protocol.Range) ([]protocol.CodeAction, error) {
var analyzers []*analysis.Analyzer
for _, a := range snapshot.View().Options().ConvenienceAnalyzers {
if !a.Enabled(snapshot) {
@@ -353,7 +353,7 @@
}
analyzers = append(analyzers, a.Analyzer)
}
- diagnostics, err := snapshot.Analyze(ctx, ph.ID(), analyzers...)
+ diagnostics, err := snapshot.Analyze(ctx, pkg.ID(), analyzers...)
if err != nil {
return nil, err
}
@@ -407,7 +407,7 @@
}, nil
}
-func extractionFixes(ctx context.Context, snapshot source.Snapshot, ph source.PackageHandle, uri span.URI, rng protocol.Range) ([]protocol.CodeAction, error) {
+func extractionFixes(ctx context.Context, snapshot source.Snapshot, pkg source.Package, uri span.URI, rng protocol.Range) ([]protocol.CodeAction, error) {
if rng.Start == rng.End {
return nil, nil
}
diff --git a/internal/lsp/diagnostics.go b/internal/lsp/diagnostics.go
index 93f66e4..5491178 100644
--- a/internal/lsp/diagnostics.go
+++ b/internal/lsp/diagnostics.go
@@ -89,7 +89,7 @@
}
// Diagnose all of the packages in the workspace.
- wsHandles, err := snapshot.WorkspacePackages(ctx)
+ wsPkgs, err := snapshot.WorkspacePackages(ctx)
if err != nil {
// Try constructing a more helpful error message out of this error.
if s.handleFatalErrors(ctx, snapshot, modErr, err) {
@@ -110,16 +110,11 @@
showMsg *protocol.ShowMessageParams
wg sync.WaitGroup
)
- for _, ph := range wsHandles {
+ for _, pkg := range wsPkgs {
wg.Add(1)
- go func(ph source.PackageHandle) {
+ go func(pkg source.Package) {
defer wg.Done()
- pkg, err := ph.Check(ctx, snapshot)
- if err != nil {
- event.Error(ctx, "warning: diagnose package", err, tag.Snapshot.Of(snapshot.ID()), tag.Package.Of(ph.ID()))
- return
- }
// Only run analyses for packages with open files.
withAnalysis := alwaysAnalyze
for _, pgf := range pkg.CompiledGoFiles() {
@@ -140,7 +135,7 @@
}
}
if err != nil {
- event.Error(ctx, "warning: diagnose package", err, tag.Snapshot.Of(snapshot.ID()), tag.Package.Of(ph.ID()))
+ event.Error(ctx, "warning: diagnose package", err, tag.Snapshot.Of(snapshot.ID()), tag.Package.Of(pkg.ID()))
return
}
@@ -159,7 +154,7 @@
}
}
reportsMu.Unlock()
- }(ph)
+ }(pkg)
}
wg.Wait()
return reports, showMsg
diff --git a/internal/lsp/link.go b/internal/lsp/link.go
index 24a91a7..73c21d8 100644
--- a/internal/lsp/link.go
+++ b/internal/lsp/link.go
@@ -101,11 +101,11 @@
func goLinks(ctx context.Context, snapshot source.Snapshot, fh source.FileHandle) ([]protocol.DocumentLink, error) {
view := snapshot.View()
- phs, err := snapshot.PackageHandles(ctx, fh)
+ pkgs, err := snapshot.PackagesForFile(ctx, fh.URI())
if err != nil {
return nil, err
}
- ph, err := source.WidestPackageHandle(phs)
+ pkg, err := source.WidestPackage(pkgs)
if err != nil {
return nil, err
}
@@ -142,7 +142,7 @@
if view.IsGoPrivatePath(target) {
continue
}
- if mod, version, ok := moduleAtVersion(ctx, snapshot, target, ph); ok && strings.ToLower(view.Options().LinkTarget) == "pkg.go.dev" {
+ if mod, version, ok := moduleAtVersion(ctx, snapshot, target, pkg); ok && strings.ToLower(view.Options().LinkTarget) == "pkg.go.dev" {
target = strings.Replace(target, mod, mod+"@"+version, 1)
}
// Account for the quotation marks in the positions.
@@ -175,11 +175,7 @@
return links, nil
}
-func moduleAtVersion(ctx context.Context, snapshot source.Snapshot, target string, ph source.PackageHandle) (string, string, bool) {
- pkg, err := ph.Check(ctx, snapshot)
- if err != nil {
- return "", "", false
- }
+func moduleAtVersion(ctx context.Context, snapshot source.Snapshot, target string, pkg source.Package) (string, string, bool) {
impPkg, err := pkg.GetImport(target)
if err != nil {
return "", "", false
diff --git a/internal/lsp/lsp_test.go b/internal/lsp/lsp_test.go
index e5943aa..bca0740 100644
--- a/internal/lsp/lsp_test.go
+++ b/internal/lsp/lsp_test.go
@@ -933,10 +933,6 @@
t.Fatal(err)
}
got = tests.FilterWorkspaceSymbols(got, dirs)
- if len(got) != len(expectedSymbols) {
- t.Errorf("want %d symbols, got %d", len(expectedSymbols), len(got))
- return
- }
if diff := tests.DiffWorkspaceSymbols(expectedSymbols, got); diff != "" {
t.Error(diff)
}
diff --git a/internal/lsp/source/code_lens.go b/internal/lsp/source/code_lens.go
index 90c5b97..d3cfb4b 100644
--- a/internal/lsp/source/code_lens.go
+++ b/internal/lsp/source/code_lens.go
@@ -52,7 +52,7 @@
if !strings.HasSuffix(fh.URI().Filename(), "_test.go") {
return nil, nil
}
- pkg, pgf, err := getParsedFile(ctx, snapshot, fh, WidestPackageHandle)
+ pkg, pgf, err := getParsedFile(ctx, snapshot, fh, WidestPackage)
if err != nil {
return nil, err
}
diff --git a/internal/lsp/source/command.go b/internal/lsp/source/command.go
index 375bd54..0103ab4 100644
--- a/internal/lsp/source/command.go
+++ b/internal/lsp/source/command.go
@@ -184,7 +184,7 @@
// getAllSuggestedFixInputs is a helper function to collect all possible needed
// inputs for an AppliesFunc or SuggestedFixFunc.
func getAllSuggestedFixInputs(ctx context.Context, snapshot Snapshot, fh FileHandle, pRng protocol.Range) (*token.FileSet, span.Range, []byte, *ast.File, *protocol.ColumnMapper, *types.Package, *types.Info, error) {
- pkg, pgf, err := getParsedFile(ctx, snapshot, fh, NarrowestPackageHandle)
+ pkg, pgf, err := getParsedFile(ctx, snapshot, fh, NarrowestPackage)
if err != nil {
return nil, span.Range{}, nil, nil, nil, nil, nil, fmt.Errorf("getting file for Identifier: %w", err)
}
diff --git a/internal/lsp/source/completion.go b/internal/lsp/source/completion.go
index d00eddd..5485088 100644
--- a/internal/lsp/source/completion.go
+++ b/internal/lsp/source/completion.go
@@ -434,7 +434,7 @@
startTime := time.Now()
- pkg, pgf, err := getParsedFile(ctx, snapshot, fh, NarrowestPackageHandle)
+ pkg, pgf, err := getParsedFile(ctx, snapshot, fh, NarrowestPackage)
if err != nil {
return nil, nil, fmt.Errorf("getting file for Completion: %w", err)
}
diff --git a/internal/lsp/source/diagnostics.go b/internal/lsp/source/diagnostics.go
index 542c1be..5fa1275 100644
--- a/internal/lsp/source/diagnostics.go
+++ b/internal/lsp/source/diagnostics.go
@@ -129,15 +129,7 @@
if err != nil {
return FileIdentity{}, nil, err
}
- phs, err := snapshot.PackageHandles(ctx, fh)
- if err != nil {
- return FileIdentity{}, nil, err
- }
- ph, err := NarrowestPackageHandle(phs)
- if err != nil {
- return FileIdentity{}, nil, err
- }
- pkg, err := ph.Check(ctx, snapshot)
+ pkg, _, err := getParsedFile(ctx, snapshot, fh, NarrowestPackage)
if err != nil {
return FileIdentity{}, nil, err
}
diff --git a/internal/lsp/source/highlight.go b/internal/lsp/source/highlight.go
index 4d4e79f..9d20b6c 100644
--- a/internal/lsp/source/highlight.go
+++ b/internal/lsp/source/highlight.go
@@ -22,7 +22,7 @@
ctx, done := event.Start(ctx, "source.Highlight")
defer done()
- pkg, pgf, err := getParsedFile(ctx, snapshot, fh, WidestPackageHandle)
+ pkg, pgf, err := getParsedFile(ctx, snapshot, fh, WidestPackage)
if err != nil {
return nil, fmt.Errorf("getting file for Highlight: %w", err)
}
diff --git a/internal/lsp/source/identifier.go b/internal/lsp/source/identifier.go
index 8520d8a..acb3bfd 100644
--- a/internal/lsp/source/identifier.go
+++ b/internal/lsp/source/identifier.go
@@ -57,7 +57,7 @@
ctx, done := event.Start(ctx, "source.Identifier")
defer done()
- pkg, pgf, err := getParsedFile(ctx, snapshot, fh, NarrowestPackageHandle)
+ pkg, pgf, err := getParsedFile(ctx, snapshot, fh, NarrowestPackage)
if err != nil {
return nil, fmt.Errorf("getting file for Identifier: %w", err)
}
diff --git a/internal/lsp/source/implementation.go b/internal/lsp/source/implementation.go
index 11de86c..e508622 100644
--- a/internal/lsp/source/implementation.go
+++ b/internal/lsp/source/implementation.go
@@ -94,11 +94,7 @@
if err != nil {
return nil, err
}
- for _, ph := range knownPkgs {
- pkg, err := ph.Check(ctx, s)
- if err != nil {
- return nil, err
- }
+ for _, pkg := range knownPkgs {
pkgs[pkg.GetTypes()] = pkg
info := pkg.GetTypesInfo()
for _, obj := range info.Defs {
@@ -205,17 +201,13 @@
// referenced at the given position. An object will be returned for
// every package that the file belongs to.
func qualifiedObjsAtProtocolPos(ctx context.Context, s Snapshot, fh FileHandle, pp protocol.Position) ([]qualifiedObject, error) {
- phs, err := s.PackageHandles(ctx, fh)
+ pkgs, err := s.PackagesForFile(ctx, fh.URI())
if err != nil {
return nil, err
}
// Check all the packages that the file belongs to.
var qualifiedObjs []qualifiedObject
- for _, ph := range phs {
- searchpkg, err := ph.Check(ctx, s)
- if err != nil {
- return nil, err
- }
+ for _, searchpkg := range pkgs {
astFile, pos, err := getASTFile(searchpkg, fh, pp)
if err != nil {
return nil, err
diff --git a/internal/lsp/source/references.go b/internal/lsp/source/references.go
index 3ba71dd..54d8d09 100644
--- a/internal/lsp/source/references.go
+++ b/internal/lsp/source/references.go
@@ -76,13 +76,7 @@
if err != nil {
return nil, err
}
- for _, ph := range reverseDeps {
- pkg, err := ph.Check(ctx, s)
- if err != nil {
- return nil, err
- }
- searchPkgs = append(searchPkgs, pkg)
- }
+ searchPkgs = append(searchPkgs, reverseDeps...)
}
// Add the package in which the identifier is declared.
searchPkgs = append(searchPkgs, qo.pkg)
diff --git a/internal/lsp/source/signature_help.go b/internal/lsp/source/signature_help.go
index 1b4d630..c471d1e 100644
--- a/internal/lsp/source/signature_help.go
+++ b/internal/lsp/source/signature_help.go
@@ -22,7 +22,7 @@
ctx, done := event.Start(ctx, "source.SignatureHelp")
defer done()
- pkg, pgf, err := getParsedFile(ctx, snapshot, fh, NarrowestPackageHandle)
+ pkg, pgf, err := getParsedFile(ctx, snapshot, fh, NarrowestPackage)
if err != nil {
return nil, 0, fmt.Errorf("getting file for SignatureHelp: %w", err)
}
diff --git a/internal/lsp/source/source_test.go b/internal/lsp/source/source_test.go
index 417e1e6..047adfd 100644
--- a/internal/lsp/source/source_test.go
+++ b/internal/lsp/source/source_test.go
@@ -820,10 +820,6 @@
t.Fatal(err)
}
got = tests.FilterWorkspaceSymbols(got, dirs)
- if len(got) != len(expectedSymbols) {
- t.Errorf("want %d symbols, got %d", len(expectedSymbols), len(got))
- return
- }
if diff := tests.DiffWorkspaceSymbols(expectedSymbols, got); diff != "" {
t.Error(diff)
}
diff --git a/internal/lsp/source/symbols.go b/internal/lsp/source/symbols.go
index dddaa26..efb6155 100644
--- a/internal/lsp/source/symbols.go
+++ b/internal/lsp/source/symbols.go
@@ -18,7 +18,7 @@
ctx, done := event.Start(ctx, "source.DocumentSymbols")
defer done()
- pkg, pgf, err := getParsedFile(ctx, snapshot, fh, NarrowestPackageHandle)
+ pkg, pgf, err := getParsedFile(ctx, snapshot, fh, NarrowestPackage)
if err != nil {
return nil, fmt.Errorf("getting file for DocumentSymbols: %w", err)
}
diff --git a/internal/lsp/source/util.go b/internal/lsp/source/util.go
index 6227fb9..da9d2f1 100644
--- a/internal/lsp/source/util.go
+++ b/internal/lsp/source/util.go
@@ -66,18 +66,14 @@
return s.m.URI
}
-// getParsedFile is a convenience function that extracts the Package and ParseGoHandle for a File in a Snapshot.
+// getParsedFile is a convenience function that extracts the Package and ParsedGoFile for a File in a Snapshot.
// selectPackage is typically Narrowest/WidestPackageHandle below.
func getParsedFile(ctx context.Context, snapshot Snapshot, fh FileHandle, selectPackage PackagePolicy) (Package, *ParsedGoFile, error) {
- phs, err := snapshot.PackageHandles(ctx, fh)
+ phs, err := snapshot.PackagesForFile(ctx, fh.URI())
if err != nil {
return nil, nil, err
}
- ph, err := selectPackage(phs)
- if err != nil {
- return nil, nil, err
- }
- pkg, err := ph.Check(ctx, snapshot)
+ pkg, err := selectPackage(phs)
if err != nil {
return nil, nil, err
}
@@ -85,63 +81,49 @@
return pkg, pgh, err
}
-type PackagePolicy func([]PackageHandle) (PackageHandle, error)
+type PackagePolicy func([]Package) (Package, error)
-// NarrowestPackageHandle picks the "narrowest" package for a given file.
+// NarrowestPackage picks the "narrowest" package for a given file.
//
// By "narrowest" package, we mean the package with the fewest number of files
// that includes the given file. This solves the problem of test variants,
// as the test will have more files than the non-test package.
-func NarrowestPackageHandle(handles []PackageHandle) (PackageHandle, error) {
- if len(handles) < 1 {
- return nil, errors.Errorf("no PackageHandles")
+func NarrowestPackage(pkgs []Package) (Package, error) {
+ if len(pkgs) < 1 {
+ return nil, errors.Errorf("no packages")
}
- result := handles[0]
- for _, handle := range handles[1:] {
+ result := pkgs[0]
+ for _, handle := range pkgs[1:] {
if result == nil || len(handle.CompiledGoFiles()) < len(result.CompiledGoFiles()) {
result = handle
}
}
if result == nil {
- return nil, errors.Errorf("nil PackageHandles have been returned")
+ return nil, errors.Errorf("no packages in input")
}
return result, nil
}
-// WidestPackageHandle returns the PackageHandle containing the most files.
+// WidestPackage returns the Package containing the most files.
//
// This is useful for something like diagnostics, where we'd prefer to offer diagnostics
// for as many files as possible.
-func WidestPackageHandle(handles []PackageHandle) (PackageHandle, error) {
- if len(handles) < 1 {
- return nil, errors.Errorf("no PackageHandles")
+func WidestPackage(pkgs []Package) (Package, error) {
+ if len(pkgs) < 1 {
+ return nil, errors.Errorf("no packages")
}
- result := handles[0]
- for _, handle := range handles[1:] {
+ result := pkgs[0]
+ for _, handle := range pkgs[1:] {
if result == nil || len(handle.CompiledGoFiles()) > len(result.CompiledGoFiles()) {
result = handle
}
}
if result == nil {
- return nil, errors.Errorf("nil PackageHandles have been returned")
+ return nil, errors.Errorf("no packages in input")
}
return result, nil
}
-// SpecificPackageHandle creates a PackagePolicy to select a
-// particular PackageHandle when you alread know the one you want.
-func SpecificPackageHandle(desiredID string) PackagePolicy {
- return func(handles []PackageHandle) (PackageHandle, error) {
- for _, h := range handles {
- if h.ID() == desiredID {
- return h, nil
- }
- }
-
- return nil, fmt.Errorf("no package handle with expected id %q", desiredID)
- }
-}
-
func IsGenerated(ctx context.Context, snapshot Snapshot, uri span.URI) bool {
fh, err := snapshot.GetFile(ctx, uri)
if err != nil {
diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go
index f3d099a..db76226 100644
--- a/internal/lsp/source/view.go
+++ b/internal/lsp/source/view.go
@@ -92,13 +92,12 @@
// takes a ParseModHandle.
ModTidyHandle(ctx context.Context) (ModTidyHandle, error)
- // PackageHandles returns the PackageHandles for the packages that this file
- // belongs to.
- PackageHandles(ctx context.Context, fh FileHandle) ([]PackageHandle, error)
+ // PackagesForFile returns the packages that this file belongs to.
+ PackagesForFile(ctx context.Context, uri span.URI) ([]Package, error)
// GetActiveReverseDeps returns the active files belonging to the reverse
// dependencies of this file's package.
- GetReverseDependencies(ctx context.Context, id string) ([]PackageHandle, error)
+ GetReverseDependencies(ctx context.Context, id string) ([]Package, error)
// CachedImportPaths returns all the imported packages loaded in this snapshot,
// indexed by their import path.
@@ -107,27 +106,10 @@
// KnownPackages returns all the packages loaded in this snapshot.
// Workspace packages may be parsed in ParseFull mode, whereas transitive
// dependencies will be in ParseExported mode.
- KnownPackages(ctx context.Context) ([]PackageHandle, error)
+ KnownPackages(ctx context.Context) ([]Package, error)
- // WorkspacePackages returns the PackageHandles for the snapshot's
- // top-level packages.
- WorkspacePackages(ctx context.Context) ([]PackageHandle, error)
-}
-
-// PackageHandle represents a handle to a specific version of a package.
-// It is uniquely defined by the file handles that make up the package.
-type PackageHandle interface {
- // ID returns the ID of the package associated with the PackageHandle.
- ID() string
-
- // CompiledGoFiles returns the URIs of the files composing the package.
- CompiledGoFiles() []span.URI
-
- // Check returns the type-checked Package for the PackageHandle.
- Check(ctx context.Context, snapshot Snapshot) (Package, error)
-
- // Cached returns the Package for the PackageHandle if it has already been stored.
- Cached() (Package, error)
+ // WorkspacePackages returns the snapshot's top-level packages.
+ WorkspacePackages(ctx context.Context) ([]Package, error)
}
// View represents a single workspace.
diff --git a/internal/lsp/source/workspace_symbol.go b/internal/lsp/source/workspace_symbol.go
index 30f9b2c..e6ed5b2 100644
--- a/internal/lsp/source/workspace_symbol.go
+++ b/internal/lsp/source/workspace_symbol.go
@@ -51,8 +51,8 @@
if err != nil {
return nil, err
}
- for _, ph := range knownPkgs {
- pkg, err := ph.Check(ctx, view.Snapshot())
+ // TODO: apply some kind of ordering to the search, and sort the results.
+ for _, pkg := range knownPkgs {
symbolMatcher := makePackageSymbolMatcher(style, pkg, queryMatcher)
if err != nil {
return nil, err
