gopls/internal/lsp/source: document {All,Workspace}Metadata
This change is the residue of a failed attempt to reduce the scope
of the Implementations query (and others) based on a misunderstanding.
It merely improves the documentation around these two concepts.
What is the German word for a lengthy effort that ends in
nothing more than a small amount of wistful documentation?
Also, return errors correctly in InlayHint, which I touched along
the way.
Change-Id: I5023d0abffed8d0d9cfd8a18668ba859e8fabbc2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/493625
Run-TryBot: Alan Donovan <adonovan@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
diff --git a/gopls/internal/lsp/mod/inlayhint.go b/gopls/internal/lsp/mod/inlayhint.go
index e494cc7..4f08dd2 100644
--- a/gopls/internal/lsp/mod/inlayhint.go
+++ b/gopls/internal/lsp/mod/inlayhint.go
@@ -18,46 +18,46 @@
if err != nil {
return nil, err
}
- return unexpectedVersion(ctx, snapshot, pm), nil
-}
-// Compare the version of the module used in the snapshot's metadata with the
-// version requested by the module, in both cases, taking replaces into account.
-// Produce an InlayHint when the version is the module is not the one usedd.
-func unexpectedVersion(ctx context.Context, snapshot source.Snapshot, pm *source.ParsedModule) []protocol.InlayHint {
- var ans []protocol.InlayHint
- if pm.File == nil {
- return nil
- }
+ // Compare the version of the module used in the snapshot's metadata with the
+ // version requested by the module, in both cases, taking replaces into account.
+ // Produce an InlayHint when the version is the module is not the one used.
+
replaces := make(map[string]*modfile.Replace)
- requires := make(map[string]*modfile.Require)
for _, x := range pm.File.Replace {
replaces[x.Old.Path] = x
}
+
+ requires := make(map[string]*modfile.Require)
for _, x := range pm.File.Require {
requires[x.Mod.Path] = x
}
- am, _ := snapshot.AllMetadata(ctx)
+
+ am, err := snapshot.AllMetadata(ctx)
+ if err != nil {
+ return nil, err
+ }
+
+ var ans []protocol.InlayHint
seen := make(map[string]bool)
for _, meta := range am {
- if meta == nil || meta.Module == nil || seen[meta.Module.Path] {
+ if meta.Module == nil || seen[meta.Module.Path] {
continue
}
seen[meta.Module.Path] = true
- metaMod := meta.Module
- metaVersion := metaMod.Version
- if metaMod.Replace != nil {
- metaVersion = metaMod.Replace.Version
+ metaVersion := meta.Module.Version
+ if meta.Module.Replace != nil {
+ metaVersion = meta.Module.Replace.Version
}
// These versions can be blank, as in gopls/go.mod's local replace
- if oldrepl, ok := replaces[metaMod.Path]; ok && oldrepl.New.Version != metaVersion {
+ if oldrepl, ok := replaces[meta.Module.Path]; ok && oldrepl.New.Version != metaVersion {
ih := genHint(oldrepl.Syntax, oldrepl.New.Version, metaVersion, pm.Mapper)
if ih != nil {
ans = append(ans, *ih)
}
- } else if oldreq, ok := requires[metaMod.Path]; ok && oldreq.Mod.Version != metaVersion {
+ } else if oldreq, ok := requires[meta.Module.Path]; ok && oldreq.Mod.Version != metaVersion {
// maybe it was replaced:
- if _, ok := replaces[metaMod.Path]; ok {
+ if _, ok := replaces[meta.Module.Path]; ok {
continue
}
ih := genHint(oldreq.Syntax, oldreq.Mod.Version, metaVersion, pm.Mapper)
@@ -66,7 +66,7 @@
}
}
}
- return ans
+ return ans, nil
}
func genHint(mline *modfile.Line, oldVersion, newVersion string, m *protocol.Mapper) *protocol.InlayHint {
diff --git a/gopls/internal/lsp/source/completion/completion.go b/gopls/internal/lsp/source/completion/completion.go
index e6e53e0..f92790f 100644
--- a/gopls/internal/lsp/source/completion/completion.go
+++ b/gopls/internal/lsp/source/completion/completion.go
@@ -1174,7 +1174,8 @@
// not assume global Pos/Object realms and then use export
// data instead of the quick parse approach taken here.
- // First, we search among packages in the workspace.
+ // First, we search among packages in the forward transitive
+ // closure of the workspace.
// We'll use a fast parse to extract package members
// from those that match the name/path criterion.
all, err := c.snapshot.AllMetadata(ctx)
@@ -1610,7 +1611,7 @@
count := 0
- // Search packages across the entire workspace.
+ // Search the forward transitive closure of the workspace.
all, err := c.snapshot.AllMetadata(ctx)
if err != nil {
return err
diff --git a/gopls/internal/lsp/source/implementation.go b/gopls/internal/lsp/source/implementation.go
index bcb8b94..00559cf 100644
--- a/gopls/internal/lsp/source/implementation.go
+++ b/gopls/internal/lsp/source/implementation.go
@@ -152,8 +152,9 @@
return nil, nil
}
- // The global search needs to look at every package in the workspace;
- // see package ./methodsets.
+ // The global search needs to look at every package in the
+ // forward transitive closure of the workspace; see package
+ // ./methodsets.
//
// For now we do all the type checking before beginning the search.
// TODO(adonovan): opt: search in parallel topological order
diff --git a/gopls/internal/lsp/source/known_packages.go b/gopls/internal/lsp/source/known_packages.go
index 4414852..b1c90a9 100644
--- a/gopls/internal/lsp/source/known_packages.go
+++ b/gopls/internal/lsp/source/known_packages.go
@@ -51,7 +51,7 @@
}
}
- // Now find candidates among known packages.
+ // Now find candidates among all known packages.
knownPkgs, err := snapshot.AllMetadata(ctx)
if err != nil {
return nil, err
diff --git a/gopls/internal/lsp/source/linkname.go b/gopls/internal/lsp/source/linkname.go
index 961e8f2..84890a6 100644
--- a/gopls/internal/lsp/source/linkname.go
+++ b/gopls/internal/lsp/source/linkname.go
@@ -117,7 +117,7 @@
func findLinkname(ctx context.Context, snapshot Snapshot, pkgPath PackagePath, name string) (Package, *ParsedGoFile, token.Pos, error) {
// Typically the linkname refers to a forward dependency
// or a reverse dependency, but in general it may refer
- // to any package in the workspace.
+ // to any package that is linked with this one.
var pkgMeta *Metadata
metas, err := snapshot.AllMetadata(ctx)
if err != nil {
diff --git a/gopls/internal/lsp/source/rename.go b/gopls/internal/lsp/source/rename.go
index 55bc95b..60fb48d 100644
--- a/gopls/internal/lsp/source/rename.go
+++ b/gopls/internal/lsp/source/rename.go
@@ -718,8 +718,8 @@
// directory.
//
// It updates package clauses and import paths for the renamed package as well
-// as any other packages affected by the directory renaming among packages
-// described by allMetadata.
+// as any other packages affected by the directory renaming among all packages
+// known to the snapshot.
func renamePackage(ctx context.Context, s Snapshot, f FileHandle, newName PackageName) (map[span.URI][]diff.Edit, error) {
if strings.HasSuffix(string(newName), "_test") {
return nil, fmt.Errorf("cannot rename to _test package")
diff --git a/gopls/internal/lsp/source/view.go b/gopls/internal/lsp/source/view.go
index a86874a..d736f3fe 100644
--- a/gopls/internal/lsp/source/view.go
+++ b/gopls/internal/lsp/source/view.go
@@ -175,9 +175,23 @@
// WorkspaceMetadata returns a new, unordered slice containing
// metadata for all ordinary and test packages (but not
// intermediate test variants) in the workspace.
+ //
+ // The workspace is the set of modules typically defined by a
+ // go.work file. It is not transitively closed: for example,
+ // the standard library is not usually part of the workspace
+ // even though every module in the workspace depends on it.
+ //
+ // Operations that must inspect all the dependencies of the
+ // workspace packages should instead use AllMetadata.
WorkspaceMetadata(ctx context.Context) ([]*Metadata, error)
- // AllMetadata returns a new unordered array of metadata for all packages in the workspace.
+ // AllMetadata returns a new unordered array of metadata for
+ // all packages known to this snapshot, which includes the
+ // packages of all workspace modules plus their transitive
+ // import dependencies.
+ //
+ // It may also contain ad-hoc packages for standalone files.
+ // It includes all test variants.
AllMetadata(ctx context.Context) ([]*Metadata, error)
// Metadata returns the metadata for the specified package,
