internal/lsp/cache: store parseGoHandles

parseGoHandles have lifetimes separate from the packages they belong to.
For example, a package may be invalidated by a change to one of its
files, but we still want to retain the parse results for all the rest.
Track them explicitly.

Change-Id: I03a4ffe283bf2b252d2d838bdb2cf332cd981075
Reviewed-on: https://go-review.googlesource.com/c/tools/+/245059
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>

internal/lsp/cache/check.go

@@ -238,7 +238,7 @@ func (s *snapshot) parseGoHandles(ctx context.Context, files []span.URI, mode so
 		if err != nil {
 			return nil, err
 		}
-		pghs = append(pghs, s.view.session.cache.parseGoHandle(ctx, fh, mode))
+		pghs = append(pghs, s.parseGoHandle(ctx, fh, mode))
 	}
 	return pghs, nil
 }
@@ -288,7 +288,7 @@ func typeCheck(ctx context.Context, snapshot *snapshot, m *metadata, mode source
 				actualErrors[i] = err
 				return
 			}
-			pgh := snapshot.view.session.cache.parseGoHandle(ctx, fh, mode)
+			pgh := snapshot.parseGoHandle(ctx, fh, mode)
 			pgf, fixed, err := snapshot.parseGo(ctx, pgh)
 			if err != nil {
 				actualErrors[i] = err

internal/lsp/cache/parse.go

@@ -51,27 +51,31 @@ type parseGoData struct {
 	err   error // any other errors
 }
 
-func (c *Cache) parseGoHandle(ctx context.Context, fh source.FileHandle, mode source.ParseMode) *parseGoHandle {
+func (s *snapshot) parseGoHandle(ctx context.Context, fh source.FileHandle, mode source.ParseMode) *parseGoHandle {
 	key := parseKey{
 		file: fh.FileIdentity(),
 		mode: mode,
 	}
-	parseHandle := c.store.Bind(key, func(ctx context.Context, arg memoize.Arg) interface{} {
+	if pgh := s.getGoFile(key); pgh != nil {
+		return pgh
+	}
+	parseHandle := s.view.session.cache.store.Bind(key, func(ctx context.Context, arg memoize.Arg) interface{} {
 		snapshot := arg.(*snapshot)
 		return parseGo(ctx, snapshot.view.session.cache.fset, fh, mode)
 	})
 
-	astHandle := c.store.Bind(astCacheKey(key), func(ctx context.Context, arg memoize.Arg) interface{} {
+	astHandle := s.view.session.cache.store.Bind(astCacheKey(key), func(ctx context.Context, arg memoize.Arg) interface{} {
 		snapshot := arg.(*snapshot)
 		return buildASTCache(ctx, snapshot, parseHandle)
 	})
 
-	return &parseGoHandle{
+	pgh := &parseGoHandle{
 		handle:         parseHandle,
 		file:           fh,
 		mode:           mode,
 		astCacheHandle: astHandle,
 	}
+	return s.addGoFile(key, pgh)
 }
 
 func (pgh *parseGoHandle) String() string {
@@ -87,7 +91,7 @@ func (pgh *parseGoHandle) Mode() source.ParseMode {
 }
 
 func (s *snapshot) ParseGo(ctx context.Context, fh source.FileHandle, mode source.ParseMode) (*source.ParsedGoFile, error) {
-	pgh := s.view.session.cache.parseGoHandle(ctx, fh, mode)
+	pgh := s.parseGoHandle(ctx, fh, mode)
 	pgf, _, err := s.parseGo(ctx, pgh)
 	return pgf, err
 }
@@ -107,18 +111,14 @@ func (s *snapshot) PosToDecl(ctx context.Context, pgf *source.ParsedGoFile) (map
 		return nil, err
 	}
 
-	pgh := s.view.session.cache.parseGoHandle(ctx, fh, pgf.Mode)
+	pgh := s.parseGoHandle(ctx, fh, pgf.Mode)
 	d, err := pgh.astCacheHandle.Get(ctx, s)
 	if err != nil {
 		return nil, err
 	}
 
 	data := d.(*astCacheData)
-	if data.err != nil {
-		return nil, data.err
-	}
-
-	return data.posToDecl, nil
+	return data.posToDecl, data.err
 }
 
 func (s *snapshot) PosToField(ctx context.Context, pgf *source.ParsedGoFile) (map[token.Pos]*ast.Field, error) {
@@ -127,17 +127,14 @@ func (s *snapshot) PosToField(ctx context.Context, pgf *source.ParsedGoFile) (ma
 		return nil, err
 	}
 
-	pgh := s.view.session.cache.parseGoHandle(ctx, fh, pgf.Mode)
+	pgh := s.parseGoHandle(ctx, fh, pgf.Mode)
 	d, err := pgh.astCacheHandle.Get(ctx, s)
 	if err != nil || d == nil {
 		return nil, err
 	}
 
 	data := d.(*astCacheData)
-	if data.err != nil {
-		return nil, data.err
-	}
-	return data.posToField, nil
+	return data.posToField, data.err
 }
 
 type astCacheData struct {
@@ -317,6 +314,7 @@ func parseGo(ctx context.Context, fset *token.FileSet, fh source.FileHandle, mod
 		Converter: span.NewTokenConverter(fset, tok),
 		Content:   buf,
 	}
+
 	return &parseGoData{
 		parsed: &source.ParsedGoFile{
 			URI:      fh.URI(),

internal/lsp/cache/session.go

@@ -178,6 +178,7 @@ func (s *Session) createView(ctx context.Context, name string, folder span.URI,
 			ids:               make(map[span.URI][]packageID),
 			metadata:          make(map[packageID]*metadata),
 			files:             make(map[span.URI]source.VersionedFileHandle),
+			goFiles:           make(map[parseKey]*parseGoHandle),
 			importedBy:        make(map[packageID][]packageID),
 			actions:           make(map[actionKey]*actionHandle),
 			workspacePackages: make(map[packageID]packagePath),

internal/lsp/cache/snapshot.go

@@ -9,6 +9,7 @@ import (
 	"context"
 	"fmt"
 	"go/ast"
+	"go/parser"
 	"go/token"
 	"go/types"
 	"io"
@@ -60,6 +61,9 @@ type snapshot struct {
 	// It may invalidated when a file's content changes.
 	files map[span.URI]source.VersionedFileHandle
 
+	// goFiles maps a parseKey to its parseGoHandle.
+	goFiles map[parseKey]*parseGoHandle
+
 	// packages maps a packageKey to a set of packageHandles to which that file belongs.
 	// It may be invalidated when a file's content changes.
 	packages map[packageKey]*packageHandle
@@ -317,6 +321,22 @@ func (s *snapshot) transitiveReverseDependencies(id packageID, ids map[packageID
 	}
 }
 
+func (s *snapshot) getGoFile(key parseKey) *parseGoHandle {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+	return s.goFiles[key]
+}
+
+func (s *snapshot) addGoFile(key parseKey, pgh *parseGoHandle) *parseGoHandle {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+	if existing, ok := s.goFiles[key]; ok {
+		return existing
+	}
+	s.goFiles[key] = pgh
+	return pgh
+}
+
 func (s *snapshot) getModHandle(uri span.URI) *parseModHandle {
 	s.mu.Lock()
 	defer s.mu.Unlock()
@@ -763,6 +783,7 @@ func (s *snapshot) clone(ctx context.Context, withoutURIs map[span.URI]source.Ve
 		packages:          make(map[packageKey]*packageHandle),
 		actions:           make(map[actionKey]*actionHandle),
 		files:             make(map[span.URI]source.VersionedFileHandle),
+		goFiles:           make(map[parseKey]*parseGoHandle),
 		workspacePackages: make(map[packageID]packagePath),
 		unloadableFiles:   make(map[span.URI]struct{}),
 		parseModHandles:   make(map[span.URI]*parseModHandle),
@@ -784,6 +805,13 @@ func (s *snapshot) clone(ctx context.Context, withoutURIs map[span.URI]source.Ve
 		result.parseModHandles[k] = v
 	}
 
+	for k, v := range s.goFiles {
+		if _, ok := withoutURIs[k.file.URI]; ok {
+			continue
+		}
+		result.goFiles[k] = v
+	}
+
 	// transitiveIDs keeps track of transitive reverse dependencies.
 	// If an ID is present in the map, invalidate its types.
 	// If an ID's value is true, invalidate its metadata too.
@@ -964,27 +992,36 @@ func (s *snapshot) shouldInvalidateMetadata(ctx context.Context, originalFH, cur
 		return originalFH.URI() == s.view.modURI
 	}
 	// Get the original and current parsed files in order to check package name and imports.
-	original, originalErr := s.ParseGo(ctx, originalFH, source.ParseHeader)
-	current, currentErr := s.ParseGo(ctx, currentFH, source.ParseHeader)
+	// Use the direct parsing API to avoid modifying the snapshot we're cloning.
+	parse := func(fh source.FileHandle) (*ast.File, error) {
+		data, err := fh.Read()
+		if err != nil {
+			return nil, err
+		}
+		fset := token.NewFileSet()
+		return parser.ParseFile(fset, fh.URI().Filename(), data, parser.ImportsOnly)
+	}
+	original, originalErr := parse(originalFH)
+	current, currentErr := parse(currentFH)
 	if originalErr != nil || currentErr != nil {
 		return (originalErr == nil) != (currentErr == nil)
 	}
 	// Check if the package's metadata has changed. The cases handled are:
 	//    1. A package's name has changed
 	//    2. A file's imports have changed
-	if original.File.Name.Name != current.File.Name.Name {
+	if original.Name.Name != current.Name.Name {
 		return true
 	}
 	// If the package's imports have increased, definitely re-run `go list`.
-	if len(original.File.Imports) < len(current.File.Imports) {
+	if len(original.Imports) < len(current.Imports) {
 		return true
 	}
 	importSet := make(map[string]struct{})
-	for _, importSpec := range original.File.Imports {
+	for _, importSpec := range original.Imports {
 		importSet[importSpec.Path.Value] = struct{}{}
 	}
 	// If any of the current imports were not in the original imports.
-	for _, importSpec := range current.File.Imports {
+	for _, importSpec := range current.Imports {
 		if _, ok := importSet[importSpec.Path.Value]; !ok {
 			return true
 		}
@@ -1021,7 +1058,7 @@ func (s *snapshot) buildBuiltinPackage(ctx context.Context, goFiles []string) er
 	h := s.view.session.cache.store.Bind(fh.FileIdentity(), func(ctx context.Context, arg memoize.Arg) interface{} {
 		snapshot := arg.(*snapshot)
 
-		pgh := snapshot.view.session.cache.parseGoHandle(ctx, fh, source.ParseFull)
+		pgh := snapshot.parseGoHandle(ctx, fh, source.ParseFull)
 		pgf, _, err := snapshot.parseGo(ctx, pgh)
 		if err != nil {
 			return &builtinPackageData{err: err}