internal/lsp/cache: invalidate packages with missing deps when files are

findleyr · findleyr · commit b3b5c13b291f · 2022-07-27T16:29:04.000Z
added Add logic to invalidate any packages with missing dependencies when a file is added. Also fix a latent bug overwriting 'anyFileOpenenedOrClosed' for each loop iteration. Fixes golang/go#54073 Change-Id: I000ceb354885bd4863a1dfdda09e4d5f0e5481f3 Reviewed-on: https://go-review.googlesource.com/c/tools/+/419501 Run-TryBot: Robert Findley <rfindley@google.com> Reviewed-by: Suzy Mueller <suzmue@golang.org> gopls-CI: kokoro <noreply+kokoro@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
diff --git a/gopls/internal/regtest/watch/watch_test.go b/gopls/internal/regtest/watch/watch_test.go
@@ -199,14 +199,12 @@ func _() {
 }
 `
 	Run(t, missing, func(t *testing.T, env *Env) {
-		t.Skip("the initial workspace load fails and never retries")
-
 		env.Await(
 			env.DiagnosticAtRegexp("a/a.go", "\"mod.com/c\""),
 		)
 		env.WriteWorkspaceFile("c/c.go", `package c; func C() {};`)
 		env.Await(
-			EmptyDiagnostics("c/c.go"),
+			EmptyDiagnostics("a/a.go"),
 		)
 	})
 }
diff --git a/internal/lsp/cache/snapshot.go b/internal/lsp/cache/snapshot.go
@@ -1695,8 +1695,10 @@ func (s *snapshot) clone(ctx, bgCtx context.Context, changes map[span.URI]*fileC
 
 	// Compute invalidations based on file changes.
 	changedPkgFiles := map[PackageID]bool{} // packages whose file set may have changed
-	anyImportDeleted := false
-	anyFileOpenedOrClosed := false
+	anyImportDeleted := false               // import deletions can resolve cycles
+	anyFileOpenedOrClosed := false          // opened files affect workspace packages
+	anyFileAdded := false                   // adding a file can resolve missing dependencies
+
 	for uri, change := range changes {
 		// Maybe reinitialize the view if we see a change in the vendor
 		// directory.
@@ -1709,7 +1711,8 @@ func (s *snapshot) clone(ctx, bgCtx context.Context, changes map[span.URI]*fileC
 		var originalOpen, newOpen bool
 		_, originalOpen = originalFH.(*overlay)
 		_, newOpen = change.fileHandle.(*overlay)
-		anyFileOpenedOrClosed = originalOpen != newOpen
+		anyFileOpenedOrClosed = anyFileOpenedOrClosed || (originalOpen != newOpen)
+		anyFileAdded = anyFileAdded || (originalFH == nil && change.fileHandle != nil)
 
 		// 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,
@@ -1779,6 +1782,19 @@ func (s *snapshot) clone(ctx, bgCtx context.Context, changes map[span.URI]*fileC
 		}
 	}
 
+	// Adding a file can resolve missing dependencies from existing packages.
+	//
+	// 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 {
+		for id, metadata := range s.meta.metadata {
+			if len(metadata.MissingDeps) > 0 {
+				directIDs[id] = true
+			}
+		}
+	}
+
 	// Invalidate reverse dependencies too.
 	// idsToInvalidate keeps track of transitive reverse dependencies.
 	// If an ID is present in the map, invalidate its types.

Original file line number	Diff line number	Diff line change
`@@ -199,14 +199,12 @@ func _() {`
`199`	`199`	`}`
`200`	`200`	`
`201`	`201`	`Run(t, missing, func(t testing.T, env Env) {`
`202`		`- t.Skip("the initial workspace load fails and never retries")`
`203`		`-`
`204`	`202`	`env.Await(`
`205`	`203`	`env.DiagnosticAtRegexp("a/a.go", "\"mod.com/c\""),`
`206`	`204`	`)`
`207`	`205`	env.WriteWorkspaceFile("c/c.go", `package c; func C() {};`)
`208`	`206`	`env.Await(`
`209`		`- EmptyDiagnostics("c/c.go"),`
	`207`	`+ EmptyDiagnostics("a/a.go"),`
`210`	`208`	`)`
`211`	`209`	`})`
`212`	`210`	`}`