internal/lsp: invalidate metadata and type info more selectively

Say you have foo.go and foo_test.go yielding packages "foo" and
"foo.test". Previously when you changed foo_test.go we would
invalidate the foo.test and foo packages. Invalidating foo is not
necessary since it does not depend on any test files. Furthermore, it
caused problems because nothing would refetch foo's metadata until
foo.go changed, so various things (such as finding implementations in
packages that depend on "foo") would be broken.

Now we only invalidate metadata from packages that contain the
modified file. We only invalidate type info from packages that contain
the modified file, or from such packages' transitive reverse
dependencies.

Change-Id: I23d1af91bcdf22fad4faa1b048afe17ef4e403a1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/210460
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
diff --git a/internal/lsp/cache/snapshot.go b/internal/lsp/cache/snapshot.go
index 7280658..d18ae9d 100644
--- a/internal/lsp/cache/snapshot.go
+++ b/internal/lsp/cache/snapshot.go
@@ -452,7 +452,7 @@
 	return s.files[f.URI()]
 }
 
-func (s *snapshot) clone(ctx context.Context, withoutURI span.URI, withoutTypes, withoutMetadata map[span.URI]struct{}) *snapshot {
+func (s *snapshot) clone(ctx context.Context, withoutURI span.URI, withoutTypes, withoutMetadata map[packageID]struct{}) *snapshot {
 	s.mu.Lock()
 	defer s.mu.Unlock()
 
@@ -475,46 +475,32 @@
 		result.files[k] = v
 	}
 	// Collect the IDs for the packages associated with the excluded URIs.
-	withoutMetadataIDs := make(map[packageID]struct{})
-	withoutTypesIDs := make(map[packageID]struct{})
 	for k, ids := range s.ids {
-		// Map URIs to IDs for exclusion.
-		if _, ok := withoutTypes[k]; ok {
-			for _, id := range ids {
-				withoutTypesIDs[id] = struct{}{}
-			}
-		}
-		if _, ok := withoutMetadata[k]; ok {
-			for _, id := range ids {
-				withoutMetadataIDs[id] = struct{}{}
-			}
-			continue
-		}
 		result.ids[k] = ids
 	}
 	// Copy the package type information.
 	for k, v := range s.packages {
-		if _, ok := withoutTypesIDs[k.id]; ok {
+		if _, ok := withoutTypes[k.id]; ok {
 			continue
 		}
-		if _, ok := withoutMetadataIDs[k.id]; ok {
+		if _, ok := withoutMetadata[k.id]; ok {
 			continue
 		}
 		result.packages[k] = v
 	}
 	// Copy the package analysis information.
 	for k, v := range s.actions {
-		if _, ok := withoutTypesIDs[k.pkg.id]; ok {
+		if _, ok := withoutTypes[k.pkg.id]; ok {
 			continue
 		}
-		if _, ok := withoutMetadataIDs[k.pkg.id]; ok {
+		if _, ok := withoutMetadata[k.pkg.id]; ok {
 			continue
 		}
 		result.actions[k] = v
 	}
 	// Copy the package metadata.
 	for k, v := range s.metadata {
-		if _, ok := withoutMetadataIDs[k]; ok {
+		if _, ok := withoutMetadata[k]; ok {
 			continue
 		}
 		result.metadata[k] = v
@@ -539,8 +525,8 @@
 // Note: The logic in this function is convoluted. Do not change without significant thought.
 func (v *view) invalidateContent(ctx context.Context, f source.File, action source.FileAction) bool {
 	var (
-		withoutTypes    = make(map[span.URI]struct{})
-		withoutMetadata = make(map[span.URI]struct{})
+		withoutTypes    = make(map[packageID]struct{})
+		withoutMetadata = make(map[packageID]struct{})
 		ids             = make(map[packageID]struct{})
 	)
 
@@ -591,15 +577,8 @@
 	}
 
 	// Remove the package and all of its reverse dependencies from the cache.
-	reverseDependencies := make(map[packageID]struct{})
 	for id := range ids {
-		v.snapshot.transitiveReverseDependencies(id, reverseDependencies)
-	}
-	for id := range reverseDependencies {
-		m := v.snapshot.getMetadata(id)
-		for _, uri := range m.compiledGoFiles {
-			withoutTypes[uri] = struct{}{}
-		}
+		v.snapshot.transitiveReverseDependencies(id, withoutTypes)
 	}
 
 	// If we are deleting a file, make sure to clear out the overlay.
@@ -614,9 +593,7 @@
 	// and if so, invalidate this file's packages' metadata.
 	if v.session.cache.shouldLoad(ctx, v.snapshot, originalFH, currentFH) {
 		for id := range ids {
-			for _, uri := range v.snapshot.getMetadata(id).compiledGoFiles {
-				withoutMetadata[uri] = struct{}{}
-			}
+			withoutMetadata[id] = struct{}{}
 
 			// TODO: If a package's name has changed,
 			// we should invalidate the metadata for the new package name (if it exists).