gopls/internal/lsp/cache: invalidate metadata after vendor change
This change causes the snapshot to watch the vendor/modules.txt
file and to invalidate the metadata if it changes, since it
could cause modules to move.
Also, add a regression test for the bug in which running
go mod vendor causes a module to move, but the Definitions
operation reports the old location. (And the reverse case
of deleting the vendor tree.)
Also, add persistent.Map.String method, for debugging.
Fixes golang/go#55995
Change-Id: I48302416586c763b4a4de7d67aaa88fde52ea400
Reviewed-on: https://go-review.googlesource.com/c/tools/+/454315
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Alan Donovan <adonovan@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
diff --git a/gopls/internal/lsp/cache/snapshot.go b/gopls/internal/lsp/cache/snapshot.go
index 5a15641..80de11a 100644
--- a/gopls/internal/lsp/cache/snapshot.go
+++ b/gopls/internal/lsp/cache/snapshot.go
@@ -917,7 +917,7 @@
dirName := dir.Filename()
// If the directory is within the view's folder, we're already watching
- // it with the pattern above.
+ // it with the first pattern above.
if source.InDir(s.view.folder.Filename(), dirName) {
continue
}
@@ -1654,16 +1654,18 @@
// Most likely, each call site of inVendor needs to be reconsidered to
// understand and correctly implement the desired behavior.
func inVendor(uri span.URI) bool {
- if !strings.Contains(string(uri), "/vendor/") {
- return false
- }
- // Only packages in _subdirectories_ of /vendor/ are considered vendored
+ _, after, found := cut(string(uri), "/vendor/")
+ // Only subdirectories of /vendor/ are considered vendored
// (/vendor/a/foo.go is vendored, /vendor/foo.go is not).
- split := strings.Split(string(uri), "/vendor/")
- if len(split) < 2 {
- return false
+ return found && strings.Contains(after, "/")
+}
+
+// TODO(adonovan): replace with strings.Cut when we can assume go1.18.
+func cut(s, sep string) (before, after string, found bool) {
+ if i := strings.Index(s, sep); i >= 0 {
+ return s[:i], s[i+len(sep):], true
}
- return strings.Contains(split[1], "/")
+ return s, "", false
}
// unappliedChanges is a file source that handles an uncloned snapshot.
@@ -1691,14 +1693,16 @@
s.mu.Lock()
defer s.mu.Unlock()
- // If there is an initialization error and a vendor directory changed, try to
- // reinit.
- if s.initializedErr != nil {
- for uri := range changes {
- if inVendor(uri) {
- reinit = true
- break
- }
+ // Changes to vendor tree may require reinitialization,
+ // either because of an initialization error
+ // (e.g. "inconsistent vendoring detected"), or because
+ // one or more modules may have moved into or out of the
+ // vendor tree after 'go mod vendor' or 'rm -fr vendor/'.
+ for uri := range changes {
+ if inVendor(uri) && s.initializedErr != nil ||
+ strings.HasSuffix(string(uri), "/vendor/modules.txt") {
+ reinit = true
+ break
}
}
@@ -1782,7 +1786,7 @@
}
// directIDs keeps track of package IDs that have directly changed.
- // It maps id->invalidateMetadata.
+ // Note: this is not a set, it's a map from id to invalidateMetadata.
directIDs := map[PackageID]bool{}
// Invalidate all package metadata if the workspace module has changed.
@@ -1821,7 +1825,7 @@
// Mark all of the package IDs containing the given file.
filePackageIDs := invalidatedPackageIDs(uri, s.meta.ids, pkgFileChanged)
for id := range filePackageIDs {
- directIDs[id] = directIDs[id] || invalidateMetadata
+ directIDs[id] = directIDs[id] || invalidateMetadata // may insert 'false'
}
// Invalidate the previous modTidyHandle if any of the files have been
