Skip to content

Commit

Permalink
gopls/internal/lsp/cache: narrow reloadOrphanedFiles to open files
Browse files Browse the repository at this point in the history
This CL implements a narrower fix for #57053 than completely removing
orphaned file reloading, as done in CL 454116.

One side-effect of orphaned file reloading that we still depend on is to
ensure we at least try to load each open file. Elsewhere in the code
(for example, in GetCriticalError) we assume that we've tried loading
all open files.

For golang/go#57053

Change-Id: I33b77859b3c2bf1437558b64b8262985730a7215
Reviewed-on: https://go-review.googlesource.com/c/tools/+/454559
Reviewed-by: Alan Donovan <[email protected]>
Run-TryBot: Robert Findley <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Auto-Submit: Robert Findley <[email protected]>
gopls-CI: kokoro <[email protected]>
  • Loading branch information
findleyr authored and gopherbot committed Dec 2, 2022
1 parent 6002d6e commit 4f69bf3
Showing 1 changed file with 12 additions and 9 deletions.
21 changes: 12 additions & 9 deletions gopls/internal/lsp/cache/snapshot.go
Original file line number Diff line number Diff line change
Expand Up @@ -1390,6 +1390,10 @@ func (s *snapshot) GetCriticalError(ctx context.Context) *source.CriticalError {
// ID "command-line-arguments" are usually a symptom of a bad workspace
// configuration.
//
// This heuristic is path-dependent: we only get command-line-arguments
// packages when we've loaded using file scopes, which only occurs
// on-demand or via orphaned file reloading.
//
// TODO(rfindley): re-evaluate this heuristic.
if containsCommandLineArguments(wsPkgs) {
return s.workspaceLayoutError(ctx)
Expand Down Expand Up @@ -1492,7 +1496,7 @@ func (s *snapshot) awaitLoadedAllErrors(ctx context.Context) *source.CriticalErr
}
}

if err := s.reloadOrphanedFiles(ctx); err != nil {
if err := s.reloadOrphanedOpenFiles(ctx); err != nil {
diags := s.extractGoCommandErrors(ctx, err)
return &source.CriticalError{
MainError: err,
Expand Down Expand Up @@ -1560,12 +1564,12 @@ func (s *snapshot) reloadWorkspace(ctx context.Context) error {
return err
}

func (s *snapshot) reloadOrphanedFiles(ctx context.Context) error {
func (s *snapshot) reloadOrphanedOpenFiles(ctx context.Context) error {
// When we load ./... or a package path directly, we may not get packages
// that exist only in overlays. As a workaround, we search all of the files
// available in the snapshot and reload their metadata individually using a
// file= query if the metadata is unavailable.
files := s.orphanedFiles()
files := s.orphanedOpenFiles()

// Files without a valid package declaration can't be loaded. Don't try.
var scopes []loadScope
Expand Down Expand Up @@ -1612,12 +1616,16 @@ func (s *snapshot) reloadOrphanedFiles(ctx context.Context) error {
return nil
}

func (s *snapshot) orphanedFiles() []source.VersionedFileHandle {
func (s *snapshot) orphanedOpenFiles() []source.VersionedFileHandle {
s.mu.Lock()
defer s.mu.Unlock()

var files []source.VersionedFileHandle
s.files.Range(func(uri span.URI, fh source.VersionedFileHandle) {
// Only consider open files, which will be represented as overlays.
if _, isOverlay := fh.(*overlay); !isOverlay {
return
}
// Don't try to reload metadata for go.mod files.
if s.view.FileKind(fh) != source.Go {
return
Expand All @@ -1627,11 +1635,6 @@ func (s *snapshot) orphanedFiles() []source.VersionedFileHandle {
if !source.InDir(s.view.folder.Filename(), uri.Filename()) {
return
}
// If the file is not open and is in a vendor directory, don't treat it
// like a workspace package.
if _, ok := fh.(*overlay); !ok && inVendor(uri) {
return
}
// Don't reload metadata for files we've already deemed unloadable.
if _, ok := s.unloadableFiles[uri]; ok {
return
Expand Down

0 comments on commit 4f69bf3

Please sign in to comment.