gopls/internal/lsp: fix a latent bug in orphaned file reloading
When determining if a file is orphaned, we were checking the length of
ids, not ids[uri]. Fortunately, this bug was latent because we *also*
mark files as unloadable in MetadataForFile.
It seems like orphaned file reloading is probably unnecessary at this
point, or at least needs to be rethought. It has been a source of
confusion and bugs in the past. However, that is left for a later
change.
On that note, fix an inconsistency in orphaned file reloading that led
to a test failure after the fix above: orphaned file reloading was
skipping files with no package declaration, because (per the commit
adding that logic), the Go command didn't recognize them. That appears
to no longer be the case, even in the oldest supported Go versions: they
get synthesized into command-line-arguments packages just like other
orphaned files.
Change-Id: Ib542aa22a0cd24f99bcb57361d3ae94d066d51e3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/510675
Run-TryBot: Robert Findley <rfindley@google.com>
Auto-Submit: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
diff --git a/gopls/internal/lsp/cache/snapshot.go b/gopls/internal/lsp/cache/snapshot.go
index e6f76d4f..dbb3a41 100644
--- a/gopls/internal/lsp/cache/snapshot.go
+++ b/gopls/internal/lsp/cache/snapshot.go
@@ -1616,10 +1616,6 @@
for _, file := range files {
file := file
g.Go(func() error {
- pgf, err := s.ParseGo(ctx, file, source.ParseHeader)
- if err != nil || !pgf.File.Package.IsValid() {
- return nil // need a valid header
- }
return s.load(ctx, false, fileLoadScope(file.URI()))
})
}
@@ -1654,7 +1650,7 @@
// TODO(rfindley): instead of locking here, we should have load return the
// metadata graph that resulted from loading.
uri := file.URI()
- if len(s.meta.ids) == 0 {
+ if len(s.meta.ids[uri]) == 0 {
s.unloadableFiles[uri] = struct{}{}
}
}
@@ -2142,6 +2138,10 @@
}
// Make sure to remove the changed file from the unloadable set.
+ //
+ // TODO(rfindley): this also looks wrong, as typing in an unloadable file
+ // will result in repeated reloads. We should only delete if metadata
+ // changed.
delete(result.unloadableFiles, uri)
}
