internal/lsp: move builtin package to Snapshot

The builtin package was the one special case where we parsed Go outside
the context of a Snapshot. Move it up.

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

internal/lsp/cache/check.go

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

internal/lsp/cache/load.go

@@ -138,7 +138,7 @@ func (s *snapshot) load(ctx context.Context, scopes ...interface{}) error {
 		}
 		// Special case for the builtin package, as it has no dependencies.
 		if pkg.PkgPath == "builtin" {
-			if err := s.view.buildBuiltinPackage(ctx, pkg.GoFiles); err != nil {
+			if err := s.buildBuiltinPackage(ctx, pkg.GoFiles); err != nil {
 				return err
 			}
 			continue

internal/lsp/cache/parse.go

@@ -57,13 +57,13 @@ func (c *Cache) parseGoHandle(ctx context.Context, fh source.FileHandle, mode so
 		mode: mode,
 	}
 	parseHandle := c.store.Bind(key, func(ctx context.Context, arg memoize.Arg) interface{} {
-		view := arg.(*View)
-		return parseGo(ctx, view.session.cache.fset, fh, mode)
+		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{} {
-		view := arg.(*View)
-		return buildASTCache(ctx, view, parseHandle)
+		snapshot := arg.(*snapshot)
+		return buildASTCache(ctx, snapshot, parseHandle)
 	})
 
 	return &parseGoHandle{
@@ -88,12 +88,12 @@ 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)
-	pgf, _, err := s.view.parseGo(ctx, pgh)
+	pgf, _, err := s.parseGo(ctx, pgh)
 	return pgf, err
 }
 
-func (v *View) parseGo(ctx context.Context, pgh *parseGoHandle) (*source.ParsedGoFile, bool, error) {
-	d, err := pgh.handle.Get(ctx, v)
+func (s *snapshot) parseGo(ctx context.Context, pgh *parseGoHandle) (*source.ParsedGoFile, bool, error) {
+	d, err := pgh.handle.Get(ctx, s)
 	if err != nil {
 		return nil, false, err
 	}
@@ -108,7 +108,7 @@ func (s *snapshot) PosToDecl(ctx context.Context, pgf *source.ParsedGoFile) (map
 	}
 
 	pgh := s.view.session.cache.parseGoHandle(ctx, fh, pgf.Mode)
-	d, err := pgh.astCacheHandle.Get(ctx, s.view)
+	d, err := pgh.astCacheHandle.Get(ctx, s)
 	if err != nil {
 		return nil, err
 	}
@@ -128,7 +128,7 @@ func (s *snapshot) PosToField(ctx context.Context, pgf *source.ParsedGoFile) (ma
 	}
 
 	pgh := s.view.session.cache.parseGoHandle(ctx, fh, pgf.Mode)
-	d, err := pgh.astCacheHandle.Get(ctx, s.view)
+	d, err := pgh.astCacheHandle.Get(ctx, s)
 	if err != nil || d == nil {
 		return nil, err
 	}
@@ -151,7 +151,7 @@ type astCacheData struct {
 
 // buildASTCache builds caches to aid in quickly going from the typed
 // world to the syntactic world.
-func buildASTCache(ctx context.Context, view *View, parseHandle *memoize.Handle) *astCacheData {
+func buildASTCache(ctx context.Context, snapshot *snapshot, parseHandle *memoize.Handle) *astCacheData {
 	var (
 		// path contains all ancestors, including n.
 		path []ast.Node
@@ -159,7 +159,7 @@ func buildASTCache(ctx context.Context, view *View, parseHandle *memoize.Handle)
 		decls []ast.Decl
 	)
 
-	v, err := parseHandle.Get(ctx, view)
+	v, err := parseHandle.Get(ctx, snapshot)
 	if err != nil {
 		return &astCacheData{err: err}
 	}

internal/lsp/cache/snapshot.go

@@ -28,6 +28,7 @@ import (
 	"golang.org/x/tools/internal/packagesinternal"
 	"golang.org/x/tools/internal/span"
 	"golang.org/x/tools/internal/typesinternal"
+	errors "golang.org/x/xerrors"
 )
 
 type snapshot struct {
@@ -38,6 +39,9 @@ type snapshot struct {
 
 	active sync.WaitGroup
 
+	// builtin pins the AST and package for builtin.go in memory.
+	builtin *builtinPackageHandle
+
 	// mu guards all of the maps in the snapshot.
 	mu sync.Mutex
 
@@ -751,6 +755,7 @@ func (s *snapshot) clone(ctx context.Context, withoutURIs map[span.URI]source.Fi
 	result := &snapshot{
 		id:                s.id + 1,
 		view:              s.view,
+		builtin:           s.builtin,
 		ids:               make(map[span.URI][]packageID),
 		importedBy:        make(map[packageID][]packageID),
 		metadata:          make(map[packageID]*metadata),
@@ -985,3 +990,54 @@ func (s *snapshot) shouldInvalidateMetadata(ctx context.Context, originalFH, cur
 	}
 	return false
 }
+
+func (s *snapshot) BuiltinPackage(ctx context.Context) (*source.BuiltinPackage, error) {
+	s.view.awaitInitialized(ctx)
+
+	if s.builtin == nil {
+		return nil, errors.Errorf("no builtin package for view %s", s.view.name)
+	}
+	d, err := s.builtin.handle.Get(ctx, s)
+	if err != nil {
+		return nil, err
+	}
+	data := d.(*builtinPackageData)
+	return data.parsed, data.err
+}
+
+func (s *snapshot) buildBuiltinPackage(ctx context.Context, goFiles []string) error {
+	if len(goFiles) != 1 {
+		return errors.Errorf("only expected 1 file, got %v", len(goFiles))
+	}
+	uri := span.URIFromPath(goFiles[0])
+
+	// Get the FileHandle through the cache to avoid adding it to the snapshot
+	// and to get the file content from disk.
+	fh, err := s.view.session.cache.getFile(ctx, uri)
+	if err != nil {
+		return err
+	}
+	h := s.view.session.cache.store.Bind(fh.Identity(), func(ctx context.Context, arg memoize.Arg) interface{} {
+		snapshot := arg.(*snapshot)
+
+		pgh := snapshot.view.session.cache.parseGoHandle(ctx, fh, source.ParseFull)
+		pgf, _, err := snapshot.parseGo(ctx, pgh)
+		if err != nil {
+			return &builtinPackageData{err: err}
+		}
+		pkg, err := ast.NewPackage(snapshot.view.session.cache.fset, map[string]*ast.File{
+			pgf.URI.Filename(): pgf.File,
+		}, nil, nil)
+		if err != nil {
+			return &builtinPackageData{err: err}
+		}
+		return &builtinPackageData{
+			parsed: &source.BuiltinPackage{
+				ParsedFile: pgf,
+				Package:    pkg,
+			},
+		}
+	})
+	s.builtin = &builtinPackageHandle{handle: h}
+	return nil
+}

internal/lsp/cache/view.go

@@ -9,7 +9,6 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
-	"go/ast"
 	"io"
 	"io/ioutil"
 	"os"
@@ -33,8 +32,6 @@ import (
 )
 
 type View struct {
-	memoize.Arg // allow as a memoize.Function arg
-
 	session *Session
 	id      string
 
@@ -117,9 +114,6 @@ type View struct {
 	initializeOnce     *sync.Once
 	initializedErr     error
 
-	// builtin pins the AST and package for builtin.go in memory.
-	builtin *builtinPackageHandle
-
 	// True if the view is either in GOPATH, a module, or some other
 	// non go command build system.
 	hasValidBuildConfiguration bool
@@ -150,17 +144,8 @@ type builtinPackageHandle struct {
 type builtinPackageData struct {
 	memoize.NoCopy
 
-	pkg *ast.Package
-	pgf *source.ParsedGoFile
-	err error
-}
-
-func (d *builtinPackageData) Package() *ast.Package {
-	return d.pkg
-}
-
-func (d *builtinPackageData) ParsedFile() *source.ParsedGoFile {
-	return d.pgf
+	parsed *source.BuiltinPackage
+	err    error
 }
 
 // fileBase holds the common functionality for all files.
@@ -299,69 +284,6 @@ func (v *View) Rebuild(ctx context.Context) (source.Snapshot, func(), error) {
 	return snapshot, release, nil
 }
 
-func (v *View) BuiltinPackage(ctx context.Context) (source.BuiltinPackage, error) {
-	v.awaitInitialized(ctx)
-
-	if v.builtin == nil {
-		return nil, errors.Errorf("no builtin package for view %s", v.name)
-	}
-	data, err := v.builtin.handle.Get(ctx, v)
-	if err != nil {
-		return nil, err
-	}
-	if data == nil {
-		return nil, errors.Errorf("unexpected nil builtin package")
-	}
-	d, ok := data.(*builtinPackageData)
-	if !ok {
-		return nil, errors.Errorf("unexpected type %T", data)
-	}
-	if d.err != nil {
-		return nil, d.err
-	}
-	if d.pkg == nil || d.pkg.Scope == nil {
-		return nil, errors.Errorf("no builtin package")
-	}
-	return d, nil
-}
-
-func (v *View) buildBuiltinPackage(ctx context.Context, goFiles []string) error {
-	if len(goFiles) != 1 {
-		return errors.Errorf("only expected 1 file, got %v", len(goFiles))
-	}
-	uri := span.URIFromPath(goFiles[0])
-
-	// Get the FileHandle through the cache to avoid adding it to the snapshot
-	// and to get the file content from disk.
-	fh, err := v.session.cache.getFile(ctx, uri)
-	if err != nil {
-		return err
-	}
-	h := v.session.cache.store.Bind(fh.Identity(), func(ctx context.Context, arg memoize.Arg) interface{} {
-		view := arg.(*View)
-
-		pgh := view.session.cache.parseGoHandle(ctx, fh, source.ParseFull)
-		pgf, _, err := view.parseGo(ctx, pgh)
-		if err != nil {
-			return &builtinPackageData{err: err}
-		}
-		pkg, err := ast.NewPackage(view.session.cache.fset, map[string]*ast.File{
-			pgf.URI.Filename(): pgf.File,
-		}, nil, nil)
-		if err != nil {
-			return &builtinPackageData{err: err}
-		}
-		return &builtinPackageData{
-			pgf: pgf,
-			pkg: pkg,
-		}
-	})
-	v.builtin = &builtinPackageHandle{
-		handle: h,
-	}
-	return nil
-}
-
 func (v *View) WriteEnv(ctx context.Context, w io.Writer) error {
 	v.optionsMu.Lock()
 	env, buildFlags := v.envLocked()

internal/lsp/source/completion_builtin.go

@@ -14,13 +14,17 @@ import (
 // argument. It attempts to use the AST hints from builtin.go where
 // possible.
 func (c *completer) builtinArgKind(ctx context.Context, obj types.Object, call *ast.CallExpr) objKind {
-	builtin, err := c.snapshot.View().BuiltinPackage(ctx)
+	builtin, err := c.snapshot.BuiltinPackage(ctx)
 	if err != nil {
 		return 0
 	}
 	exprIdx := exprAtPos(c.pos, call.Args)
 
-	decl, ok := builtin.Package().Scope.Lookup(obj.Name()).Decl.(*ast.FuncDecl)
+	builtinObj := builtin.Package.Scope.Lookup(obj.Name())
+	if builtinObj == nil {
+		return 0
+	}
+	decl, ok := builtinObj.Decl.(*ast.FuncDecl)
 	if !ok || exprIdx >= len(decl.Type.Params.List) {
 		return 0
 	}

internal/lsp/source/completion_format.go

@@ -246,7 +246,7 @@ func (c *completer) formatBuiltin(ctx context.Context, cand candidate) (Completi
 		item.Kind = protocol.ConstantCompletion
 	case *types.Builtin:
 		item.Kind = protocol.FunctionCompletion
-		sig, err := newBuiltinSignature(ctx, c.snapshot.View(), obj.Name())
+		sig, err := newBuiltinSignature(ctx, c.snapshot, obj.Name())
 		if err != nil {
 			return CompletionItem{}, err
 		}

internal/lsp/source/identifier.go

@@ -171,11 +171,15 @@ func findIdentifier(ctx context.Context, s Snapshot, pkg Package, file *ast.File
 
 	// Handle builtins separately.
 	if result.Declaration.obj.Parent() == types.Universe {
-		builtin, err := view.BuiltinPackage(ctx)
+		builtin, err := s.BuiltinPackage(ctx)
 		if err != nil {
 			return nil, err
 		}
-		decl, ok := builtin.Package().Scope.Lookup(result.Name).Decl.(ast.Node)
+		builtinObj := builtin.Package.Scope.Lookup(result.Name)
+		if builtinObj == nil {
+			return nil, fmt.Errorf("no builtin object for %s", result.Name)
+		}
+		decl, ok := builtinObj.Decl.(ast.Node)
 		if !ok {
 			return nil, errors.Errorf("no declaration for %s", result.Name)
 		}
@@ -183,7 +187,7 @@ func findIdentifier(ctx context.Context, s Snapshot, pkg Package, file *ast.File
 
 		// The builtin package isn't in the dependency graph, so the usual utilities
 		// won't work here.
-		rng := newMappedRange(view.Session().Cache().FileSet(), builtin.ParsedFile().Mapper, decl.Pos(), decl.Pos()+token.Pos(len(result.Name)))
+		rng := newMappedRange(view.Session().Cache().FileSet(), builtin.ParsedFile.Mapper, decl.Pos(), decl.Pos()+token.Pos(len(result.Name)))
 		result.Declaration.MappedRange = append(result.Declaration.MappedRange, rng)
 
 		return result, nil

internal/lsp/source/signature_help.go

@@ -74,7 +74,7 @@ FindCall:
 
 	// Handle builtin functions separately.
 	if obj, ok := obj.(*types.Builtin); ok {
-		return builtinSignature(ctx, snapshot.View(), callExpr, obj.Name(), rng.Start)
+		return builtinSignature(ctx, snapshot, callExpr, obj.Name(), rng.Start)
 	}
 
 	// Get the type information for the function being called.
@@ -132,8 +132,8 @@ FindCall:
 	}, activeParam, nil
 }
 
-func builtinSignature(ctx context.Context, view View, callExpr *ast.CallExpr, name string, pos token.Pos) (*protocol.SignatureInformation, int, error) {
-	sig, err := newBuiltinSignature(ctx, view, name)
+func builtinSignature(ctx context.Context, snapshot Snapshot, callExpr *ast.CallExpr, name string, pos token.Pos) (*protocol.SignatureInformation, int, error) {
+	sig, err := newBuiltinSignature(ctx, snapshot, name)
 	if err != nil {
 		return nil, 0, err
 	}

internal/lsp/source/types_format.go

@@ -74,12 +74,12 @@ func (s *signature) format() string {
 	return b.String()
 }
 
-func newBuiltinSignature(ctx context.Context, view View, name string) (*signature, error) {
-	builtin, err := view.BuiltinPackage(ctx)
+func newBuiltinSignature(ctx context.Context, snapshot Snapshot, name string) (*signature, error) {
+	builtin, err := snapshot.BuiltinPackage(ctx)
 	if err != nil {
 		return nil, err
 	}
-	obj := builtin.Package().Scope.Lookup(name)
+	obj := builtin.Package.Scope.Lookup(name)
 	if obj == nil {
 		return nil, fmt.Errorf("no builtin object for %s", name)
 	}
@@ -98,8 +98,8 @@ func newBuiltinSignature(ctx context.Context, view View, name string) (*signatur
 			variadic = true
 		}
 	}
-	params, _ := formatFieldList(ctx, view, decl.Type.Params, variadic)
-	results, needResultParens := formatFieldList(ctx, view, decl.Type.Results, false)
+	params, _ := formatFieldList(ctx, snapshot.View(), decl.Type.Params, variadic)
+	results, needResultParens := formatFieldList(ctx, snapshot.View(), decl.Type.Results, false)
 	return &signature{
 		doc:              decl.Doc.Text(),
 		name:             name,

internal/lsp/source/view.go

@@ -86,6 +86,9 @@ type Snapshot interface {
 	// ModTidy returns the results of `go mod tidy` for the snapshot's module.
 	ModTidy(ctx context.Context) (*TidiedModule, error)
 
+	// BuiltinPackage returns information about the special builtin package.
+	BuiltinPackage(ctx context.Context) (*BuiltinPackage, error)
+
 	// PackagesForFile returns the packages that this file belongs to.
 	PackagesForFile(ctx context.Context, uri span.URI) ([]Package, error)
 
@@ -122,9 +125,6 @@ type View interface {
 	// ModFile is the go.mod file at the root of this view. It may not exist.
 	ModFile() span.URI
 
-	// BuiltinPackage returns the go/ast.Object for the given name in the builtin package.
-	BuiltinPackage(ctx context.Context) (BuiltinPackage, error)
-
 	// BackgroundContext returns a context used for all background processing
 	// on behalf of this view.
 	BackgroundContext() context.Context
@@ -172,9 +172,9 @@ type View interface {
 	WorkspaceDirectories(ctx context.Context) ([]string, error)
 }
 
-type BuiltinPackage interface {
-	Package() *ast.Package
-	ParsedFile() *ParsedGoFile
+type BuiltinPackage struct {
+	Package    *ast.Package
+	ParsedFile *ParsedGoFile
 }
 
 // A ParsedGoFile contains the results of parsing a Go file.