internal/lsp: use ParseGoHandles for the builtin package

This change allows to remove some of the special handling for the
builtin package.

Change-Id: I105fcefd8812af2d42ff42edca954824c98db429
Reviewed-on: https://go-review.googlesource.com/c/tools/+/195758
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>

internal/lsp/cache/builtin.go

@@ -0,0 +1,54 @@
+package cache
+
+import (
+	"context"
+	"go/ast"
+
+	"golang.org/x/tools/go/packages"
+	"golang.org/x/tools/internal/lsp/source"
+	"golang.org/x/tools/internal/span"
+)
+
+type builtinPkg struct {
+	pkg   *ast.Package
+	files []source.ParseGoHandle
+}
+
+func (b *builtinPkg) Lookup(name string) *ast.Object {
+	if b == nil || b.pkg == nil || b.pkg.Scope == nil {
+		return nil
+	}
+	return b.pkg.Scope.Lookup(name)
+}
+
+// buildBuiltinPkg builds the view's builtin package.
+// It assumes that the view is not active yet,
+// i.e. it has not been added to the session's list of views.
+func (view *view) buildBuiltinPackage(ctx context.Context) error {
+	cfg := view.Config(ctx)
+	pkgs, err := packages.Load(cfg, "builtin")
+	if err != nil {
+		return err
+	}
+	if len(pkgs) != 1 {
+		return err
+	}
+	pkg := pkgs[0]
+	files := make(map[string]*ast.File)
+	for _, filename := range pkg.GoFiles {
+		fh := view.session.GetFile(span.FileURI(filename))
+		ph := view.session.cache.ParseGoHandle(fh, source.ParseFull)
+		view.builtin.files = append(view.builtin.files, ph)
+		file, _, err := ph.Parse(ctx)
+		if file == nil {
+			return err
+		}
+		files[filename] = file
+
+		view.ignoredURIsMu.Lock()
+		view.ignoredURIs[span.NewURI(filename)] = struct{}{}
+		view.ignoredURIsMu.Unlock()
+	}
+	view.builtin.pkg, err = ast.NewPackage(cfg.Fset, files, nil, nil)
+	return err
+}

internal/lsp/cache/gofile.go

@@ -194,16 +194,6 @@ func (f *goFile) wrongParseMode(ctx context.Context, fh source.FileHandle, mode
 	return true
 }
 
-func (f *goFile) Builtin() (*ast.File, bool) {
-	builtinPkg := f.View().BuiltinPackage()
-	for filename, file := range builtinPkg.Files {
-		if filename == f.URI().Filename() {
-			return file, true
-		}
-	}
-	return nil, false
-}
-
 // isDirty is true if the file needs to be type-checked.
 // It assumes that the file's view's mutex is held by the caller.
 func (f *goFile) isDirty(ctx context.Context, fh source.FileHandle) bool {

internal/lsp/cache/session.go

@@ -102,10 +102,11 @@ func (s *session) NewView(ctx context.Context, name string, folder span.URI, opt
 			packages: make(map[packageID]*metadata),
 		},
 		ignoredURIs: make(map[span.URI]struct{}),
+		builtin:     &builtinPkg{},
 	}
 	// Preemptively build the builtin package,
 	// so we immediately add builtin.go to the list of ignored files.
-	v.buildBuiltinPkg(ctx)
+	v.buildBuiltinPackage(ctx)
 
 	s.views = append(s.views, v)
 	// we always need to drop the view map

internal/lsp/cache/view.go

@@ -73,8 +73,8 @@ type view struct {
 	// mcache caches metadata for the packages of the opened files in a view.
 	mcache *metadataCache
 
-	// builtinPkg is the AST package used to resolve builtin types.
-	builtinPkg *ast.Package
+	// builtin is used to resolve builtin types.
+	builtin *builtinPkg
 
 	// ignoredURIs is the set of URIs of files that we ignore.
 	ignoredURIsMu sync.Mutex
@@ -289,41 +289,8 @@ func (v *view) BackgroundContext() context.Context {
 	return v.backgroundCtx
 }
 
-func (v *view) BuiltinPackage() *ast.Package {
-	return v.builtinPkg
-}
-
-// buildBuiltinPkg builds the view's builtin package.
-// It assumes that the view is not active yet,
-// i.e. it has not been added to the session's list of views.
-func (v *view) buildBuiltinPkg(ctx context.Context) {
-	cfg := *v.Config(ctx)
-	pkgs, err := packages.Load(&cfg, "builtin")
-	if err != nil {
-		log.Error(ctx, "error getting package metadata for \"builtin\" package", err)
-	}
-	if len(pkgs) != 1 {
-		v.builtinPkg, _ = ast.NewPackage(cfg.Fset, nil, nil, nil)
-		return
-	}
-	pkg := pkgs[0]
-	files := make(map[string]*ast.File)
-	for _, filename := range pkg.GoFiles {
-		fh := v.session.GetFile(span.FileURI(filename))
-		ph := v.session.cache.ParseGoHandle(fh, source.ParseFull)
-		file, _, err := ph.Parse(ctx)
-		if file == nil {
-			log.Error(ctx, "failed to parse builtin", err, telemetry.File.Of(filename))
-			v.builtinPkg, _ = ast.NewPackage(cfg.Fset, nil, nil, nil)
-			return
-		}
-		files[filename] = file
-
-		v.ignoredURIsMu.Lock()
-		v.ignoredURIs[span.NewURI(filename)] = struct{}{}
-		v.ignoredURIsMu.Unlock()
-	}
-	v.builtinPkg, _ = ast.NewPackage(cfg.Fset, files, nil, nil)
+func (v *view) BuiltinPackage() source.BuiltinPackage {
+	return v.builtin
 }
 
 // SetContent sets the overlay contents for a file.

internal/lsp/source/completion_format.go

@@ -173,7 +173,11 @@ func (c *completer) formatBuiltin(cand candidate) CompletionItem {
 		item.Kind = ConstantCompletionItem
 	case *types.Builtin:
 		item.Kind = FunctionCompletionItem
-		decl, ok := lookupBuiltinDecl(c.view, obj.Name()).(*ast.FuncDecl)
+		builtin := c.view.BuiltinPackage().Lookup(obj.Name())
+		if obj == nil {
+			break
+		}
+		decl, ok := builtin.Decl.(*ast.FuncDecl)
 		if !ok {
 			break
 		}

internal/lsp/source/identifier.go

@@ -143,7 +143,11 @@ func identifier(ctx context.Context, view View, pkgs []Package, file *ast.File,
 
 	// Handle builtins separately.
 	if result.Declaration.obj.Parent() == types.Universe {
-		decl, ok := lookupBuiltinDecl(view, result.Name).(ast.Node)
+		obj := view.BuiltinPackage().Lookup(result.Name)
+		if obj == nil {
+			return result, nil
+		}
+		decl, ok := obj.Decl.(ast.Node)
 		if !ok {
 			return nil, errors.Errorf("no declaration for %s", result.Name)
 		}

internal/lsp/source/signature_help.go

@@ -135,7 +135,11 @@ FindCall:
 }
 
 func builtinSignature(ctx context.Context, v View, callExpr *ast.CallExpr, name string, pos token.Pos) (*SignatureInformation, error) {
-	decl, ok := lookupBuiltinDecl(v, name).(*ast.FuncDecl)
+	obj := v.BuiltinPackage().Lookup(name)
+	if obj == nil {
+		return nil, errors.Errorf("no object for %s", name)
+	}
+	decl, ok := obj.Decl.(*ast.FuncDecl)
 	if !ok {
 		return nil, errors.Errorf("no function declaration for builtin: %s", name)
 	}

internal/lsp/source/util.go

@@ -102,15 +102,18 @@ func cachedFileToMapper(ctx context.Context, view View, uri span.URI) (*ast.File
 	if !ok {
 		return nil, nil, errors.Errorf("%s is not a Go file", f.URI())
 	}
-	if file, ok := gof.Builtin(); ok {
-		return builtinFileToMapper(ctx, view, gof, file)
-	}
 	pkg, err := gof.GetCachedPackage(ctx)
+	if err == nil {
+		file, m, err := pkgToMapper(ctx, view, pkg, f.URI())
 		if err != nil {
 			return nil, nil, err
 		}
-	file, m, err := pkgToMapper(ctx, view, pkg, f.URI())
-	if err != nil {
+		return file, m, nil
+	}
+	// Fallback to just looking for the AST.
+	ph := view.Session().Cache().ParseGoHandle(gof.Handle(ctx), ParseFull)
+	file, m, err := ph.Cached(ctx)
+	if file == nil {
 		return nil, nil, err
 	}
 	return file, m, nil
@@ -133,21 +136,6 @@ func pkgToMapper(ctx context.Context, view View, pkg Package, uri span.URI) (*as
 	return file, m, nil
 }
 
-func builtinFileToMapper(ctx context.Context, view View, f GoFile, file *ast.File) (*ast.File, *protocol.ColumnMapper, error) {
-	fh := f.Handle(ctx)
-	data, _, err := fh.Read(ctx)
-	if err != nil {
-		return nil, nil, err
-	}
-	converter := span.NewContentConverter(fh.Identity().URI.Filename(), data)
-	m := &protocol.ColumnMapper{
-		URI:       fh.Identity().URI,
-		Content:   data,
-		Converter: converter,
-	}
-	return nil, m, nil
-}
-
 func IsGenerated(ctx context.Context, view View, uri span.URI) bool {
 	f, err := view.GetFile(ctx, uri)
 	if err != nil {
@@ -355,18 +343,6 @@ func resolveInvalid(obj types.Object, node ast.Node, info *types.Info) types.Obj
 	return formatResult(resultExpr)
 }
 
-func lookupBuiltinDecl(v View, name string) interface{} {
-	builtinPkg := v.BuiltinPackage()
-	if builtinPkg == nil || builtinPkg.Scope == nil {
-		return nil
-	}
-	obj := builtinPkg.Scope.Lookup(name)
-	if obj == nil {
-		return nil
-	}
-	return obj.Decl
-}
-
 func isPointer(T types.Type) bool {
 	_, ok := T.(*types.Pointer)
 	return ok

internal/lsp/source/view.go

@@ -215,8 +215,8 @@ type View interface {
 	// Folder returns the root folder for this view.
 	Folder() span.URI
 
-	// BuiltinPackage returns the ast for the special "builtin" package.
-	BuiltinPackage() *ast.Package
+	// BuiltinPackage returns the type information for the special "builtin" package.
+	BuiltinPackage() BuiltinPackage
 
 	// GetFile returns the file object for a given URI, initializing it
 	// if it is not already part of the view.
@@ -265,8 +265,6 @@ type File interface {
 type GoFile interface {
 	File
 
-	Builtin() (*ast.File, bool)
-
 	// GetCachedPackage returns the cached package for the file, if any.
 	GetCachedPackage(ctx context.Context) (Package, error)
 
@@ -323,3 +321,7 @@ type Package interface {
 	// belong to or be part of a dependency of the given package.
 	FindFile(ctx context.Context, uri span.URI) (ParseGoHandle, *ast.File, Package, error)
 }
+
+type BuiltinPackage interface {
+	Lookup(name string) *ast.Object
+}