gopls/internal/cache/parsego: support lazy object resolution for Files

For the purposes of go/analysis, gopls could not skip syntactic object
resolution, as it is part of the go/analysis contract. This prevents
analysis from reusing existing type-checked packages, leading to
increased CPU and memory during diagnostics.

Fix this by making object resolution lazy, and ensuring that all
analysed files are resolved prior to analysis.

This could introduce a race if gopls were to read the fields set by
object resolution, for example if it was printing the tree using
ast.Fprint, so we include a test that these fields are only accessed
from packages or declarations that are verified to be safe.

Since the resolver is not separate from the parser, we fork the code and
use go generate to keep it in sync.

For golang/go#53275

Change-Id: I24ce94b5d8532c5e679789d2ec1f75376e9e9208
Reviewed-on: https://go-review.googlesource.com/c/tools/+/619516
Reviewed-by: Alan Donovan <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>

Author: findleyr

go/ast/astutil/imports.go

@@ -344,7 +344,12 @@ func RewriteImport(fset *token.FileSet, f *ast.File, oldPath, newPath string) (r
 }
 
 // UsesImport reports whether a given import is used.
+// The provided File must have been parsed with syntactic object resolution
+// (not using go/parser.SkipObjectResolution).
 func UsesImport(f *ast.File, path string) (used bool) {
+	if f.Scope == nil {
+		panic("file f was not parsed with syntactic object resolution")
+	}
 	spec := importSpec(f, path)
 	if spec == nil {
 		return

gopls/internal/cache/analysis.go

@@ -15,7 +15,6 @@ import (
 	"errors"
 	"fmt"
 	"go/ast"
-	"go/parser"
 	"go/token"
 	"go/types"
 	"log"
@@ -234,7 +233,10 @@ func (s *Snapshot) Analyze(ctx context.Context, pkgs map[PackageID]*metadata.Pac
 	facty = requiredAnalyzers(facty)
 
 	// File set for this batch (entire graph) of analysis.
-	fset := token.NewFileSet()
+	//
+	// Start at reservedForParsing so that cached parsed files can be inserted
+	// into the fileset retroactively.
+	fset := fileSetWithBase(reservedForParsing)
 
 	// Get the metadata graph once for lock-free access during analysis.
 	meta := s.MetadataGraph()
@@ -261,6 +263,7 @@ func (s *Snapshot) Analyze(ctx context.Context, pkgs map[PackageID]*metadata.Pac
 
 			an = &analysisNode{
 				allNodes:     nodes,
+				parseCache:   s.view.parseCache,
 				fset:         fset,
 				fsource:      s, // expose only ReadFile
 				viewType:     s.View().Type(),
@@ -548,6 +551,7 @@ func (an *analysisNode) decrefPreds() {
 // type-checking and analyzing syntax (miss).
 type analysisNode struct {
 	allNodes        map[PackageID]*analysisNode // all nodes, for lazy lookup of transitive dependencies
+	parseCache      *parseCache                 // shared parse cache
 	fset            *token.FileSet              // file set shared by entire batch (DAG)
 	fsource         file.Source                 // Snapshot.ReadFile, for use by Pass.ReadFile
 	viewType        ViewType                    // type of view
@@ -868,12 +872,13 @@ func (an *analysisNode) run(ctx context.Context) (*analyzeSummary, error) {
 		for i, fh := range an.files {
 			i, fh := i, fh
 			group.Go(func() error {
-				// Call parseGoImpl directly, not the caching wrapper,
-				// as cached ASTs require the global FileSet.
-				// ast.Object resolution is unfortunately an implied part of the
-				// go/analysis contract.
-				pgf, err := parseGoImpl(ctx, an.fset, fh, parsego.Full&^parser.SkipObjectResolution, false)
-				parsed[i] = pgf
+				// Files fetched from the cache must also have their ast.Ident.Objects
+				// resolved, as it is part of the analysis contract.
+				pgfs, err := an.parseCache.parseFiles(ctx, an.fset, parsego.Full, false, fh)
+				if err == nil {
+					pgfs[0].Resolve()
+				}
+				parsed[i] = pgfs[0]
 				return err
 			})
 		}

gopls/internal/cache/parsego/file.go

@@ -9,6 +9,7 @@ import (
 	"go/parser"
 	"go/scanner"
 	"go/token"
+	"sync"
 
 	"golang.org/x/tools/gopls/internal/protocol"
 	"golang.org/x/tools/gopls/internal/util/safetoken"
@@ -18,6 +19,10 @@ import (
 type File struct {
 	URI  protocol.DocumentURI
 	Mode parser.Mode
+
+	// File is the file resulting from parsing. Clients must not access the AST's
+	// legacy ast.Object-related fields without first ensuring that
+	// [File.Resolve] was already called.
 	File *ast.File
 	Tok  *token.File
 	// Source code used to build the AST. It may be different from the
@@ -39,13 +44,16 @@ type File struct {
 	fixedAST bool
 	Mapper   *protocol.Mapper // may map fixed Src, not file content
 	ParseErr scanner.ErrorList
+
+	// resolveOnce guards the lazy ast.Object resolution. See [File.Resolve].
+	resolveOnce sync.Once
 }
 
-func (pgf File) String() string { return string(pgf.URI) }
+func (pgf *File) String() string { return string(pgf.URI) }
 
 // Fixed reports whether p was "Fixed", meaning that its source or positions
 // may not correlate with the original file.
-func (pgf File) Fixed() bool {
+func (pgf *File) Fixed() bool {
 	return pgf.fixedSrc || pgf.fixedAST
 }
 
@@ -100,3 +108,28 @@ func (pgf *File) RangePos(r protocol.Range) (token.Pos, token.Pos, error) {
 	}
 	return pgf.Tok.Pos(start), pgf.Tok.Pos(end), nil
 }
+
+// Resolve lazily resolves ast.Ident.Objects in the enclosed syntax tree.
+//
+// Resolve must be called before accessing any of:
+//   - pgf.File.Scope
+//   - pgf.File.Unresolved
+//   - Ident.Obj, for any Ident in pgf.File
+func (pgf *File) Resolve() {
+	pgf.resolveOnce.Do(func() {
+		if pgf.File.Scope != nil {
+			return // already resolved by parsing without SkipObjectResolution.
+		}
+		defer func() {
+			// (panic handler duplicated from go/parser)
+			if e := recover(); e != nil {
+				// A bailout indicates the resolution stack has exceeded max depth.
+				if _, ok := e.(bailout); !ok {
+					panic(e)
+				}
+			}
+		}()
+		declErr := func(token.Pos, string) {}
+		resolveFile(pgf.File, pgf.Tok, declErr)
+	})
+}

gopls/internal/cache/parsego/parse.go

@@ -2,6 +2,14 @@
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
+//go:generate go run resolver_gen.go
+
+// The parsego package defines the [File] type, a wrapper around a go/ast.File
+// that is useful for answering LSP queries. Notably, it bundles the
+// *token.File and *protocol.Mapper necessary for token.Pos locations to and
+// from UTF-16 LSP positions.
+//
+// Run `go generate` to update resolver.go from GOROOT.
 package parsego
 
 import (