internal/lsp/cache: sort Metadata.Deps, for determinism
...so that each client doesn't have to.
Change-Id: I039c493031c5c90c4479741cf6f7572dad480808
Reviewed-on: https://go-review.googlesource.com/c/tools/+/415502
Run-TryBot: Alan Donovan <adonovan@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Auto-Submit: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
diff --git a/internal/lsp/cache/analysis.go b/internal/lsp/cache/analysis.go
index 847ac2d..db2ca2a 100644
--- a/internal/lsp/cache/analysis.go
+++ b/internal/lsp/cache/analysis.go
@@ -10,7 +10,6 @@
"go/ast"
"go/types"
"reflect"
- "sort"
"sync"
"golang.org/x/sync/errgroup"
@@ -122,13 +121,8 @@
// An analysis that consumes/produces facts
// must run on the package's dependencies too.
if len(a.FactTypes) > 0 {
- importIDs := make([]string, 0, len(ph.m.Deps))
for _, importID := range ph.m.Deps {
- importIDs = append(importIDs, string(importID))
- }
- sort.Strings(importIDs) // for determinism
- for _, importID := range importIDs {
- depActionHandle, err := s.actionHandle(ctx, PackageID(importID), a)
+ depActionHandle, err := s.actionHandle(ctx, importID, a)
if err != nil {
return nil, err
}
diff --git a/internal/lsp/cache/check.go b/internal/lsp/cache/check.go
index aeb4563..5ded278 100644
--- a/internal/lsp/cache/check.go
+++ b/internal/lsp/cache/check.go
@@ -102,13 +102,6 @@
return nil, fmt.Errorf("no metadata for %s", id)
}
- // For key stability, sort depList.
- // TODO(adonovan): make m.Deps have a well defined order.
- depList := append([]PackageID{}, m.Deps...)
- sort.Slice(depList, func(i, j int) bool {
- return depList[i] < depList[j]
- })
-
// Begin computing the key by getting the depKeys for all dependencies.
// This requires reading the transitive closure of dependencies' source files.
//
@@ -122,8 +115,8 @@
// for each package is computed by at most one thread, then do
// the recursive key building of dependencies in parallel.
deps := make(map[PackagePath]*packageHandle)
- depKeys := make([]packageHandleKey, len(depList))
- for i, depID := range depList {
+ depKeys := make([]packageHandleKey, len(m.Deps))
+ for i, depID := range m.Deps {
depHandle, err := s.buildPackageHandle(ctx, depID, s.workspaceParseMode(depID))
// Don't use invalid metadata for dependencies if the top-level
// metadata is valid. We only load top-level packages, so if the
diff --git a/internal/lsp/cache/load.go b/internal/lsp/cache/load.go
index d613dc3..00d4ab2 100644
--- a/internal/lsp/cache/load.go
+++ b/internal/lsp/cache/load.go
@@ -539,6 +539,7 @@
event.Error(ctx, "error in dependency", err)
}
}
+ sort.Slice(m.Deps, func(i, j int) bool { return m.Deps[i] < m.Deps[j] }) // for determinism
return nil
}
diff --git a/internal/lsp/cache/metadata.go b/internal/lsp/cache/metadata.go
index b4da713..486035f 100644
--- a/internal/lsp/cache/metadata.go
+++ b/internal/lsp/cache/metadata.go
@@ -32,7 +32,7 @@
ForTest PackagePath
TypesSizes types.Sizes
Errors []packages.Error
- Deps []PackageID
+ Deps []PackageID // direct dependencies, in string order
MissingDeps map[PackagePath]struct{}
Module *packages.Module
depsErrors []*packagesinternal.PackageError
