From 351fb220e1b4c8af14d9e2125e0577b82dbe1e2a Mon Sep 17 00:00:00 2001 From: Rebecca Stambler Date: Fri, 17 Jan 2020 15:20:54 -0500 Subject: [PATCH] internal/lsp: invalidate directories if we have no direct IDs Previously, we were only invalidating packages in directories if we didn't have a file handle. We should also invalidate if we have an unparseable file handle - that is, a file with no content or no package name. Fixes golang/go#36608. Change-Id: Ia12fad962c06ddeeac382185d3220961f5c584e0 Reviewed-on: https://go-review.googlesource.com/c/tools/+/215318 Run-TryBot: Rebecca Stambler Reviewed-by: Heschi Kreinick TryBot-Result: Gobot Gobot --- internal/lsp/cache/load.go | 2 +- internal/lsp/cache/snapshot.go | 18 +++--------------- 2 files changed, 4 insertions(+), 16 deletions(-) diff --git a/internal/lsp/cache/load.go b/internal/lsp/cache/load.go index 8f8df23100..98b083a9fe 100644 --- a/internal/lsp/cache/load.go +++ b/internal/lsp/cache/load.go @@ -74,7 +74,7 @@ func (s *snapshot) load(ctx context.Context, scopes ...interface{}) ([]*metadata return nil, ctx.Err() } - log.Print(ctx, "go/packages.Load", tag.Of("query", query), tag.Of("packages", len(pkgs))) + log.Print(ctx, "go/packages.Load", tag.Of("snapshot", s.ID()), tag.Of("query", query), tag.Of("packages", len(pkgs))) if len(pkgs) == 0 { if err == nil { err = errors.Errorf("no packages found for query %s", query) diff --git a/internal/lsp/cache/snapshot.go b/internal/lsp/cache/snapshot.go index d06253dd61..d24c9bc720 100644 --- a/internal/lsp/cache/snapshot.go +++ b/internal/lsp/cache/snapshot.go @@ -8,7 +8,6 @@ import ( "context" "os" "path/filepath" - "strings" "sync" "golang.org/x/tools/go/analysis" @@ -629,10 +628,10 @@ func (s *snapshot) clone(ctx context.Context, withoutURI span.URI) *snapshot { // Make a rough estimate of what metadata to invalidate by finding the package IDs // of all of the files in the same directory as this one. // TODO(rstambler): Speed this up by mapping directories to filenames. - if originalFH == nil { - if dirStat, err := os.Stat(dir(withoutURI.Filename())); err == nil { + if len(directIDs) == 0 { + if dirStat, err := os.Stat(filepath.Dir(withoutURI.Filename())); err == nil { for uri := range s.files { - if fdirStat, err := os.Stat(dir(uri.Filename())); err == nil { + if fdirStat, err := os.Stat(filepath.Dir(uri.Filename())); err == nil { if os.SameFile(dirStat, fdirStat) { for _, id := range s.ids[uri] { directIDs[id] = struct{}{} @@ -643,13 +642,6 @@ func (s *snapshot) clone(ctx context.Context, withoutURI span.URI) *snapshot { } } - // If there is no known FileHandle and no known IDs for the given file, - // there is nothing to invalidate. - if len(directIDs) == 0 && originalFH == nil { - // TODO(heschi): clone anyway? Seems like this is just setting us up for trouble. - return s - } - // Invalidate reverse dependencies too. // TODO(heschi): figure out the locking model and use transitiveReverseDeps? transitiveIDs := make(map[packageID]struct{}) @@ -743,10 +735,6 @@ func (s *snapshot) clone(ctx context.Context, withoutURI span.URI) *snapshot { return result } -func dir(filename string) string { - return strings.ToLower(filepath.Dir(filename)) -} - func (s *snapshot) ID() uint64 { return s.id }