From 3e6f5d44f491a5fd95c40f388549a999ce45d6e0 Mon Sep 17 00:00:00 2001 From: Rebecca Stambler Date: Thu, 9 Jan 2020 19:22:08 -0500 Subject: [PATCH] internal/lsp: don't invalidate workspace when a mod file is opened Opening a mod file is not sufficient cause to invalidate in the workspace, so don't. Change-Id: I2b7703008528e4469be312165deb17fe6856fd74 Reviewed-on: https://go-review.googlesource.com/c/tools/+/214259 Run-TryBot: Rebecca Stambler TryBot-Result: Gobot Gobot Reviewed-by: Heschi Kreinick --- internal/lsp/cache/load.go | 9 ++++++--- internal/lsp/cache/parse.go | 3 +++ internal/lsp/cache/snapshot.go | 22 ++++++++++++---------- 3 files changed, 21 insertions(+), 13 deletions(-) diff --git a/internal/lsp/cache/load.go b/internal/lsp/cache/load.go index 568427455c..1993954515 100644 --- a/internal/lsp/cache/load.go +++ b/internal/lsp/cache/load.go @@ -83,11 +83,14 @@ func (c *cache) shouldLoad(ctx context.Context, s *snapshot, originalFH, current if originalFH == nil { return true } - // If the file is a mod file, we should always load. - if originalFH.Identity().Kind == currentFH.Identity().Kind && currentFH.Identity().Kind == source.Mod { + // If the file hasn't changed, there's no need to reload. + if originalFH.Identity().String() == currentFH.Identity().String() { + return false + } + // If a go.mod file's contents have changed, always invalidate metadata. + if kind := originalFH.Identity().Kind; kind == source.Mod { return true } - // Get the original and current parsed files in order to check package name and imports. original, _, _, originalErr := c.ParseGoHandle(originalFH, source.ParseHeader).Parse(ctx) current, _, _, currentErr := c.ParseGoHandle(currentFH, source.ParseHeader).Parse(ctx) diff --git a/internal/lsp/cache/parse.go b/internal/lsp/cache/parse.go index a556124a99..9031fc8b6f 100644 --- a/internal/lsp/cache/parse.go +++ b/internal/lsp/cache/parse.go @@ -114,6 +114,9 @@ func parseGo(ctx context.Context, fset *token.FileSet, fh source.FileHandle, mod ctx, done := trace.StartSpan(ctx, "cache.parseGo", telemetry.File.Of(fh.Identity().URI.Filename())) defer done() + if fh.Identity().Kind != source.Go { + return nil, nil, nil, errors.Errorf("cannot parse non-Go file %s", fh.Identity().URI) + } buf, _, err := fh.Read(ctx) if err != nil { return nil, nil, nil, err diff --git a/internal/lsp/cache/snapshot.go b/internal/lsp/cache/snapshot.go index ea4bef8f40..c1f13d9c13 100644 --- a/internal/lsp/cache/snapshot.go +++ b/internal/lsp/cache/snapshot.go @@ -562,16 +562,22 @@ func (s *snapshot) clone(ctx context.Context, withoutURI span.URI, withoutFileKi directIDs[id] = struct{}{} } - // If we are invalidating a go.mod file then we should invalidate all of the packages in the module - if withoutFileKind == source.Mod { + // Get the current and original FileHandles for this URI. + currentFH := s.view.session.GetFile(withoutURI) + originalFH := s.files[withoutURI] + + // Check if the file's package name or imports have changed, + // and if so, invalidate this file's packages' metadata. + invalidateMetadata := s.view.session.cache.shouldLoad(ctx, s, originalFH, currentFH) + + // If a go.mod file's contents have changed, invalidate the metadata + // for all of the packages in the workspace. + if invalidateMetadata && currentFH.Identity().Kind == source.Mod { for id := range s.workspacePackages { directIDs[id] = struct{}{} } } - // Get the original FileHandle for the URI, if it exists. - originalFH := s.files[withoutURI] - // If this is a file we don't yet know about, // then we do not yet know what packages it should belong to. // Make a rough estimate of what metadata to invalidate by finding the package IDs @@ -631,8 +637,8 @@ func (s *snapshot) clone(ctx context.Context, withoutURI span.URI, withoutFileKi for k, v := range s.files { result.files[k] = v } + // Handle the invalidated file; it may have new contents or not exist. - currentFH := s.view.session.GetFile(withoutURI) if _, _, err := currentFH.Read(ctx); os.IsNotExist(err) { delete(result.files, withoutURI) } else { @@ -662,10 +668,6 @@ func (s *snapshot) clone(ctx context.Context, withoutURI span.URI, withoutFileKi result.workspacePackages[k] = v } - // Check if the file's package name or imports have changed, - // and if so, invalidate this file's packages' metadata. - invalidateMetadata := s.view.session.cache.shouldLoad(ctx, s, originalFH, currentFH) - // Copy the package metadata. We only need to invalidate packages directly // containing the affected file, and only if it changed in a relevant way. for k, v := range s.metadata {