Skip to content

Commit

Permalink
internal/lsp/cache: invalidate packages with missing deps when files are
Browse files Browse the repository at this point in the history
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 <[email protected]>
Reviewed-by: Suzy Mueller <[email protected]>
gopls-CI: kokoro <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
  • Loading branch information
findleyr committed Jul 27, 2022
1 parent 39a4e36 commit b3b5c13
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 6 deletions.
4 changes: 1 addition & 3 deletions gopls/internal/regtest/watch/watch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
)
})
}
Expand Down
22 changes: 19 additions & 3 deletions internal/lsp/cache/snapshot.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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,
Expand Down Expand Up @@ -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.
Expand Down

0 comments on commit b3b5c13

Please sign in to comment.