internal/lsp: refactor (*snapshot).clone to handle multiple URIs
This is the first in a series of changes to batch file changes. First,
we have to support invalidating a snaphot with multiple files.
Updates golang/go#31553
Change-Id: I7cd83d9280e3274549a72393bb9010c47eb5dd1e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215902
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
diff --git a/internal/lsp/cache/snapshot.go b/internal/lsp/cache/snapshot.go
index f2d6602..bd31dff 100644
--- a/internal/lsp/cache/snapshot.go
+++ b/internal/lsp/cache/snapshot.go
@@ -596,85 +596,10 @@
}
}
-func (s *snapshot) clone(ctx context.Context, withoutURI span.URI) *snapshot {
+func (s *snapshot) clone(ctx context.Context, withoutURIs []span.URI) *snapshot {
s.mu.Lock()
defer s.mu.Unlock()
- directIDs := map[packageID]struct{}{}
- // Collect all of the package IDs that correspond to the given file.
- // TODO: if the file has moved into a new package, we should invalidate that too.
- for _, id := range s.ids[withoutURI] {
- directIDs[id] = struct{}{}
- }
-
- // Get the current and original FileHandles for this URI.
- currentFH := s.view.session.GetFile(withoutURI)
- originalFH := s.files[withoutURI]
-
- // Check if the file's package name or imports have changed,
- // and if so, invalidate this file's packages' metadata.
- invalidateMetadata := s.view.session.cache.shouldLoad(ctx, s, originalFH, currentFH)
-
- // If a go.mod file's contents have changed, invalidate the metadata
- // for all of the packages in the workspace.
- if invalidateMetadata && currentFH.Identity().Kind == source.Mod {
- for id := range s.workspacePackages {
- directIDs[id] = struct{}{}
- }
- }
-
- // If this is a file we don't yet know about,
- // then we do not yet know what packages it should belong to.
- // Make a rough estimate of what metadata to invalidate by finding the package IDs
- // of all of the files in the same directory as this one.
- // TODO(rstambler): Speed this up by mapping directories to filenames.
- if len(directIDs) == 0 {
- if dirStat, err := os.Stat(filepath.Dir(withoutURI.Filename())); err == nil {
- for uri := range s.files {
- if fdirStat, err := os.Stat(filepath.Dir(uri.Filename())); err == nil {
- if os.SameFile(dirStat, fdirStat) {
- for _, id := range s.ids[uri] {
- directIDs[id] = struct{}{}
- }
- }
- }
- }
- }
- }
-
- // Invalidate reverse dependencies too.
- // TODO(heschi): figure out the locking model and use transitiveReverseDeps?
- transitiveIDs := make(map[packageID]struct{})
- var addRevDeps func(packageID)
- addRevDeps = func(id packageID) {
- if _, seen := transitiveIDs[id]; seen {
- return
- }
- transitiveIDs[id] = struct{}{}
- for _, rid := range s.getImportedByLocked(id) {
- addRevDeps(rid)
- }
- }
- for id := range directIDs {
- addRevDeps(id)
- }
- // Invalidate metadata for the transitive dependencies,
- // if they are x_tests and test variants.
- //
- // An example:
- //
- // The only way to reload the metadata for
- // golang.org/x/tools/internal/lsp/cache [golang.org/x/tools/internal/lsp/source.test]
- // is by reloading golang.org/x/tools/internal/lsp/source.
- // That means we have to invalidate the metadata for
- // golang.org/x/tools/internal/lsp/source_test when invalidating metadata for
- // golang.org/x/tools/internal/lsp/cache.
- for id := range transitiveIDs {
- if m := s.metadata[id]; m != nil && m.forTest != "" {
- directIDs[id] = struct{}{}
- }
- }
-
result := &snapshot{
id: s.id + 1,
view: s.view,
@@ -692,17 +617,86 @@
result.files[k] = v
}
- // Handle the invalidated file; it may have new contents or not exist.
- if _, _, err := currentFH.Read(ctx); os.IsNotExist(err) {
- delete(result.files, withoutURI)
- } else {
- result.files[withoutURI] = currentFH
+ // transitiveIDs keeps track of transitive reverse dependencies.
+ // If an ID is present in the map, invalidate its types.
+ // If an ID's value is true, invalidate its metadata too.
+ transitiveIDs := make(map[packageID]bool)
+
+ for _, withoutURI := range withoutURIs {
+ directIDs := map[packageID]struct{}{}
+
+ // Collect all of the package IDs that correspond to the given file.
+ // TODO: if the file has moved into a new package, we should invalidate that too.
+ for _, id := range s.ids[withoutURI] {
+ directIDs[id] = struct{}{}
+ }
+ // Get the current and original FileHandles for this URI.
+ currentFH := s.view.session.GetFile(withoutURI)
+ originalFH := s.files[withoutURI]
+
+ // Check if the file's package name or imports have changed,
+ // and if so, invalidate this file's packages' metadata.
+ invalidateMetadata := s.view.session.cache.shouldLoad(ctx, s, originalFH, currentFH)
+
+ // If a go.mod file's contents have changed, invalidate the metadata
+ // for all of the packages in the workspace.
+ if invalidateMetadata && currentFH.Identity().Kind == source.Mod {
+ for id := range s.workspacePackages {
+ directIDs[id] = struct{}{}
+ }
+ }
+
+ // If this is a file we don't yet know about,
+ // then we do not yet know what packages it should belong to.
+ // Make a rough estimate of what metadata to invalidate by finding the package IDs
+ // of all of the files in the same directory as this one.
+ // TODO(rstambler): Speed this up by mapping directories to filenames.
+ if len(directIDs) == 0 {
+ if dirStat, err := os.Stat(filepath.Dir(withoutURI.Filename())); err == nil {
+ for uri := range s.files {
+ if fdirStat, err := os.Stat(filepath.Dir(uri.Filename())); err == nil {
+ if os.SameFile(dirStat, fdirStat) {
+ for _, id := range s.ids[uri] {
+ directIDs[id] = struct{}{}
+ }
+ }
+ }
+ }
+ }
+ }
+
+ // Invalidate reverse dependencies too.
+ // TODO(heschi): figure out the locking model and use transitiveReverseDeps?
+ var addRevDeps func(packageID)
+ addRevDeps = func(id packageID) {
+ if _, seen := transitiveIDs[id]; seen {
+ return
+ }
+ transitiveIDs[id] = invalidateMetadata
+ for _, rid := range s.getImportedByLocked(id) {
+ addRevDeps(rid)
+ }
+ }
+ for id := range directIDs {
+ addRevDeps(id)
+ }
+
+ // Handle the invalidated file; it may have new contents or not exist.
+ if _, _, err := currentFH.Read(ctx); os.IsNotExist(err) {
+ delete(result.files, withoutURI)
+ } else {
+ result.files[withoutURI] = currentFH
+ }
}
// Collect the IDs for the packages associated with the excluded URIs.
for k, ids := range s.ids {
result.ids[k] = ids
}
+ // Copy the set of initally loaded packages.
+ for k, v := range s.workspacePackages {
+ result.workspacePackages[k] = v
+ }
// Copy the package type information.
for k, v := range s.packages {
if _, ok := transitiveIDs[k.id]; ok {
@@ -717,21 +711,17 @@
}
result.actions[k] = v
}
- // Copy the set of initally loaded packages.
- for k, v := range s.workspacePackages {
- result.workspacePackages[k] = v
- }
-
// Copy the package metadata. We only need to invalidate packages directly
// containing the affected file, and only if it changed in a relevant way.
for k, v := range s.metadata {
- if _, ok := directIDs[k]; invalidateMetadata && ok {
+ if invalidateMetadata, ok := transitiveIDs[k]; invalidateMetadata && ok {
continue
}
result.metadata[k] = v
}
// Don't bother copying the importedBy graph,
// as it changes each time we update metadata.
+
return result
}
diff --git a/internal/lsp/cache/view.go b/internal/lsp/cache/view.go
index 1bb84c8..d480f22 100644
--- a/internal/lsp/cache/view.go
+++ b/internal/lsp/cache/view.go
@@ -594,7 +594,7 @@
v.snapshotMu.Lock()
defer v.snapshotMu.Unlock()
- v.snapshot = v.snapshot.clone(ctx, uri)
+ v.snapshot = v.snapshot.clone(ctx, []span.URI{uri})
return v.snapshot
}
