internal/cache: invalidate broken imports when package files change

When a file->package association changes, it may fix broken imports.
Fix this invalidation in Snapshot.clone.

Fixes golang/go#66384

Change-Id: If0f491548043a30bb6302bf207733f6f458f2574
Reviewed-on: https://go-review.googlesource.com/c/tools/+/588764
Reviewed-by: Alan Donovan <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>

Author: findleyr

gopls/internal/cache/snapshot.go

@@ -1775,16 +1775,18 @@ func (s *Snapshot) clone(ctx, bgCtx context.Context, changed StateChange, done f
 	// Compute invalidations based on file changes.
 	anyImportDeleted := false      // import deletions can resolve cycles
 	anyFileOpenedOrClosed := false // opened files affect workspace packages
-	anyFileAdded := false          // adding a file can resolve missing dependencies
+	anyPkgFileChanged := false     // adding a file to a package can resolve missing dependencies
 
 	for uri, newFH := range changedFiles {
 		// The original FileHandle for this URI is cached on the snapshot.
 		oldFH := oldFiles[uri] // may be nil
 		_, oldOpen := oldFH.(*overlay)
 		_, newOpen := newFH.(*overlay)
 
+		// TODO(rfindley): consolidate with 'metadataChanges' logic below, which
+		// also considers existential changes.
 		anyFileOpenedOrClosed = anyFileOpenedOrClosed || (oldOpen != newOpen)
-		anyFileAdded = anyFileAdded || (oldFH == nil || !fileExists(oldFH)) && fileExists(newFH)
+		anyPkgFileChanged = anyPkgFileChanged || (oldFH == nil || !fileExists(oldFH)) && fileExists(newFH)
 
 		// If uri is a Go file, check if it has changed in a way that would
 		// invalidate metadata. Note that we can't use s.view.FileKind here,
@@ -1802,6 +1804,7 @@ func (s *Snapshot) clone(ctx, bgCtx context.Context, changed StateChange, done f
 
 		invalidateMetadata = invalidateMetadata || reinit
 		anyImportDeleted = anyImportDeleted || importDeleted
+		anyPkgFileChanged = anyPkgFileChanged || pkgFileChanged
 
 		// Mark all of the package IDs containing the given file.
 		filePackageIDs := invalidatedPackageIDs(uri, s.meta.IDs, pkgFileChanged)
@@ -1878,7 +1881,7 @@ func (s *Snapshot) clone(ctx, bgCtx context.Context, changed StateChange, done f
 	// We could be smart here and try to guess which packages may have been
 	// fixed, but until that proves necessary, just invalidate metadata for any
 	// package with missing dependencies.
-	if anyFileAdded {
+	if anyPkgFileChanged {
 		for id, mp := range s.meta.Packages {
 			for _, impID := range mp.DepsByImpPath {
 				if impID == "" { // missing import

gopls/internal/test/integration/diagnostics/invalidation_test.go

@@ -27,7 +27,7 @@ func _() {
 	x := 2
 }
 `
-	Run(t, files, func(_ *testing.T, env *Env) { // Create a new workspace-level directory and empty file.
+	Run(t, files, func(t *testing.T, env *Env) { // Create a new workspace-level directory and empty file.
 		env.OpenFile("main.go")
 		var afterOpen protocol.PublishDiagnosticsParams
 		env.AfterChange(
@@ -70,7 +70,7 @@ func _() {
 // Irrelevant comment #0
 `
 
-	Run(t, files, func(_ *testing.T, env *Env) { // Create a new workspace-level directory and empty file.
+	Run(t, files, func(t *testing.T, env *Env) { // Create a new workspace-level directory and empty file.
 		env.OpenFile("main.go")
 		var d protocol.PublishDiagnosticsParams
 		env.AfterChange(
@@ -104,3 +104,38 @@ func _() {
 		}
 	})
 }
+
+func TestCreatingPackageInvalidatesDiagnostics_Issue66384(t *testing.T) {
+	const files = `
+-- go.mod --
+module example.com
+
+go 1.15
+-- main.go --
+package main
+
+import "example.com/pkg"
+
+func main() {
+	var _ pkg.Thing
+}
+`
+	Run(t, files, func(t *testing.T, env *Env) {
+		env.OnceMet(
+			InitialWorkspaceLoad,
+			Diagnostics(env.AtRegexp("main.go", `"example.com/pkg"`)),
+		)
+		// In order for this test to reproduce golang/go#66384, we have to create
+		// the buffer, wait for loads, and *then* "type out" the contents. Doing so
+		// reproduces the conditions of the bug report, that typing the package
+		// name itself doesn't invalidate the broken import.
+		env.CreateBuffer("pkg/pkg.go", "")
+		env.AfterChange()
+		env.EditBuffer("pkg/pkg.go", protocol.TextEdit{NewText: "package pkg\ntype Thing struct{}\n"})
+		env.AfterChange()
+		env.SaveBuffer("pkg/pkg.go")
+		env.AfterChange(NoDiagnostics())
+		env.SetBufferContent("pkg/pkg.go", "package pkg")
+		env.AfterChange(Diagnostics(env.AtRegexp("main.go", "Thing")))
+	})
+}