internal/lsp/cache: don't forget files just because they change

The situation in golang/go#35638 was as follows:

didOpen main.go creates a snapshot that knows main.go is in package
"mod.com".
didChange main.go creates a snapshot. When a file changes, we discard
its contents by leaving the file handle out of the "files" map.
didOpen const.go creates a snapshot, and attempts to invalidate the
metadata for packages in the same directory.

The way we detect packages in the same directory is by iterating through
the files in the snapshot. But we threw away the only file in "mod.com"
in step 2 when its contents changed. If a diagnostics run happened to
get in between the two steps, it would re-load main.go and the bug would
go away. If not, step 3 would find no files and fail to invalidate
"mod.com".

The best way to fix this is to insert the new file handle eagerly during
cloning. That way there's no confusion.

Fixes golang/go#35638.

Change-Id: I340bd28a96ad7b4cc912032065f3c2732c380bb2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/211578
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This commit is contained in:
Heschi Kreinick 2019-12-16 16:34:44 -05:00
parent ac28154776
commit 84f0c7cf60
1 changed files with 10 additions and 7 deletions

View File

@ -536,13 +536,19 @@ func (s *snapshot) clone(ctx context.Context, withoutFile source.File) *snapshot
files: make(map[span.URI]source.FileHandle), files: make(map[span.URI]source.FileHandle),
workspacePackages: make(map[packageID]bool), workspacePackages: make(map[packageID]bool),
} }
// Copy all of the FileHandles except for the one that was invalidated.
// Copy all of the FileHandles.
for k, v := range s.files { for k, v := range s.files {
if k == withoutFile.URI() {
continue
}
result.files[k] = v result.files[k] = v
} }
// Handle the invalidated file; it may have new contents or not exist.
currentFH := s.view.session.GetFile(withoutFile.URI(), withoutFile.Kind())
if _, _, err := currentFH.Read(ctx); os.IsNotExist(err) {
delete(result.files, withoutFile.URI())
} else {
result.files[withoutFile.URI()] = currentFH
}
// Collect the IDs for the packages associated with the excluded URIs. // Collect the IDs for the packages associated with the excluded URIs.
for k, ids := range s.ids { for k, ids := range s.ids {
result.ids[k] = ids result.ids[k] = ids
@ -566,9 +572,6 @@ func (s *snapshot) clone(ctx context.Context, withoutFile source.File) *snapshot
result.workspacePackages[k] = v result.workspacePackages[k] = v
} }
// Get the current FileHandle for the URI.
currentFH := s.view.session.GetFile(withoutFile.URI(), withoutFile.Kind())
// Check if the file's package name or imports have changed, // Check if the file's package name or imports have changed,
// and if so, invalidate this file's packages' metadata. // and if so, invalidate this file's packages' metadata.
invalidateMetadata := s.view.session.cache.shouldLoad(ctx, s, originalFH, currentFH) invalidateMetadata := s.view.session.cache.shouldLoad(ctx, s, originalFH, currentFH)