internal/lsp/cache: invalidate metadata when parsing issues resolve

Package loading (at least via go list) fails when the import header
doesn't parse, so we need to invalidate metadata upon a state change
from non parsing->parsing. Refactor metadata invalidation to implement
this feature, add additional documentation, and avoid unnecessary work.

This change revealed a latent bug (via TestDeleteDirectory): when
statting a deleted directory failed, we could fail to invalidate any
package IDs, even those we already knew about. This bug was masked by
the somewhat complicated semantics of pkgNameChanged. The semantics of
pkgNameChanged are simplified to encapsulate any change to a
package->file association.

Also refactor the parsing API somewhat, and add a bug report if we don't
get a ParseGoFile while inspecting for metadata changes.

Update most regtests to panic upon receiving bug reports.

Fixes golang/go#52981

Change-Id: I1838963ecc9c01e316f887aa9d8f1260662281ab
Reviewed-on: https://go-review.googlesource.com/c/tools/+/407501
Reviewed-by: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>

1 files changed, 21 insertions, 13 deletions

diff --git a/internal/lsp/cache/parse.go b/internal/lsp/cache/parse.go
index 5d751ebe5..1ab1fa9e5 100644
--- a/internal/lsp/cache/parse.go
+++ b/internal/lsp/cache/parse.go
@@ -72,27 +72,19 @@ func (s *snapshot) parseGoHandle(ctx context.Context, fh source.FileHandle, mode
 }
 
 func (pgh *parseGoHandle) String() string {
-	return pgh.File().URI().Filename()
-}
-
-func (pgh *parseGoHandle) File() source.FileHandle {
-	return pgh.file
-}
-
-func (pgh *parseGoHandle) Mode() source.ParseMode {
-	return pgh.mode
+	return pgh.file.URI().Filename()
 }
 
 func (s *snapshot) ParseGo(ctx context.Context, fh source.FileHandle, mode source.ParseMode) (*source.ParsedGoFile, error) {
-	pgh := s.parseGoHandle(ctx, fh, mode)
-	pgf, _, err := s.parseGo(ctx, pgh)
+	pgf, _, err := s.parseGo(ctx, fh, mode)
 	return pgf, err
 }
 
-func (s *snapshot) parseGo(ctx context.Context, pgh *parseGoHandle) (*source.ParsedGoFile, bool, error) {
-	if pgh.mode == source.ParseExported {
+func (s *snapshot) parseGo(ctx context.Context, fh source.FileHandle, mode source.ParseMode) (*source.ParsedGoFile, bool, error) {
+	if mode == source.ParseExported {
 		panic("only type checking should use Exported")
 	}
+	pgh := s.parseGoHandle(ctx, fh, mode)
 	d, err := pgh.handle.Get(ctx, s.generation, s)
 	if err != nil {
 		return nil, false, err
@@ -101,6 +93,22 @@ func (s *snapshot) parseGo(ctx context.Context, pgh *parseGoHandle) (*source.Par
 	return data.parsed, data.fixed, data.err
 }
 
+// cachedPGF returns the cached ParsedGoFile for the given ParseMode, if it
+// has already been computed. Otherwise, it returns nil.
+func (s *snapshot) cachedPGF(fh source.FileHandle, mode source.ParseMode) *source.ParsedGoFile {
+	key := parseKey{file: fh.FileIdentity(), mode: mode}
+	if pgh := s.getGoFile(key); pgh != nil {
+		cached := pgh.handle.Cached(s.generation)
+		if cached != nil {
+			cached := cached.(*parseGoData)
+			if cached.parsed != nil {
+				return cached.parsed
+			}
+		}
+	}
+	return nil
+}
+
 type astCacheKey struct {
 	pkg packageHandleKey
 	uri span.URI