internal/lsp/cache: clean up getOrLoadIDsForURI

Address some TODOs in getOrLoadIDsForURI. In particular, after this
change files will be marked as unloadable if getOrLoadIDsForURI returns
fails to load valid IDs.

Change-Id: I1f09d1c7edd776e46bf8178157bcf9e439b9f293
Reviewed-on: https://go-review.googlesource.com/c/tools/+/447742
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
This commit is contained in:
Robert Findley 2022-11-03 19:06:31 -04:00
parent 5a4eba5af0
commit 23056f613e
1 changed files with 7 additions and 17 deletions

View File

@ -734,23 +734,14 @@ func (s *snapshot) getOrLoadIDsForURI(ctx context.Context, uri span.URI) ([]Pack
// Start with the set of package associations derived from the last load.
ids := s.meta.ids[uri]
hasValidID := false // whether we have any valid package metadata containing uri
shouldLoad := false // whether any packages containing uri are marked 'shouldLoad'
for _, id := range ids {
// TODO(rfindley): remove the defensiveness here. s.meta.metadata[id] must
// exist.
if _, ok := s.meta.metadata[id]; ok {
hasValidID = true
}
if len(s.shouldLoad[id]) > 0 {
shouldLoad = true
}
}
// Check if uri is known to be unloadable.
//
// TODO(rfindley): shouldn't we also mark uri as unloadable if the load below
// fails? Otherwise we endlessly load files with no packages.
_, unloadable := s.unloadableFiles[uri]
s.mu.Unlock()
@ -759,7 +750,7 @@ func (s *snapshot) getOrLoadIDsForURI(ctx context.Context, uri span.URI) ([]Pack
// - uri is not contained in any valid packages
// - ...or one of the packages containing uri is marked 'shouldLoad'
// - ...but uri is not unloadable
if (shouldLoad || !hasValidID) && !unloadable {
if (shouldLoad || len(ids) == 0) && !unloadable {
scope := fileLoadScope(uri)
err := s.load(ctx, false, scope)
@ -788,14 +779,11 @@ func (s *snapshot) getOrLoadIDsForURI(ctx context.Context, uri span.URI) ([]Pack
s.mu.Lock()
ids = s.meta.ids[uri]
var validIDs []PackageID
for _, id := range ids {
// TODO(rfindley): remove the defensiveness here as well.
if _, ok := s.meta.metadata[id]; ok {
validIDs = append(validIDs, id)
}
// metadata is only ever added by loading, so if we get here and still have
// no ids, uri is unloadable.
if !unloadable && len(ids) == 0 {
s.unloadableFiles[uri] = struct{}{}
}
ids = validIDs
s.mu.Unlock()
return ids, nil
@ -1745,6 +1733,8 @@ func (s *snapshot) clone(ctx, bgCtx context.Context, changes map[span.URI]*fileC
// TODO(rfindley): this looks wrong. Shouldn't we clear unloadableFiles on
// changes to environment or workspace layout, or more generally on any
// metadata change?
//
// Maybe not, as major configuration changes cause a new view.
for k, v := range s.unloadableFiles {
result.unloadableFiles[k] = v
}