From b3b5c13b291f9653da6f31b95db100a2e26bd186 Mon Sep 17 00:00:00 2001 From: Robert Findley Date: Tue, 26 Jul 2022 17:01:06 -0400 Subject: [PATCH] internal/lsp/cache: invalidate packages with missing deps when files are 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 Reviewed-by: Suzy Mueller gopls-CI: kokoro TryBot-Result: Gopher Robot --- gopls/internal/regtest/watch/watch_test.go | 4 +--- internal/lsp/cache/snapshot.go | 22 +++++++++++++++++++--- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/gopls/internal/regtest/watch/watch_test.go b/gopls/internal/regtest/watch/watch_test.go index 04414f6b744..2890f401a90 100644 --- 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 index 64e7f17c994..b183bc5f88f 100644 --- 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.