internal/lsp: push initialization tasks into one function

This change moves as much view initialization code into the
initialization function, instead of having it happen on view create.
Also, the `go env` variables that are collected at inconsistent times
are all collected on view creation. That is sufficient, since the view
is recreated if the environment changes.

I had originally hoped that the initial call to `go env` and the
-modfile detection could become part of this parallel initialization as
well, but you can't create a *packages.Config until the temporary
modfile has been set up, so it still makes sense to do that on view
create. This is, however, the reasoning behind the refactorings in
the -modfile detection in this CL. The main changes are a few renamings
and a split between snapshot.ModFiles and view.modFiles to maximize the
amount of work done in the view. I changed view.modfiles to moduleInformation
because I thought we might want to store additional information there at some
point. Rohan, please let me know if you disagree with any of the changes I made,
and I can revert them.

Fixes golang/go#36487

Change-Id: I504db5a4f41b79bee99ebd391e32e7b520a19569
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214417
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rohan Challa <rohan@golang.org>

internal/lsp/cache/builtin.go

@@ -12,6 +12,7 @@ import (
 	"golang.org/x/tools/go/packages"
 	"golang.org/x/tools/internal/lsp/source"
 	"golang.org/x/tools/internal/span"
+	errors "golang.org/x/xerrors"
 )
 
 type builtinPkg struct {
@@ -19,11 +20,15 @@ type builtinPkg struct {
 	files []source.ParseGoHandle
 }
 
-func (b *builtinPkg) Lookup(name string) *ast.Object {
-	if b == nil || b.pkg == nil || b.pkg.Scope == nil {
-		return nil
+func (v *view) LookupBuiltin(name string) (*ast.Object, error) {
+	if v.builtin == nil || v.builtin.pkg == nil || v.builtin.pkg.Scope == nil {
+		return nil, errors.Errorf("no builtin package")
 	}
-	return b.pkg.Scope.Lookup(name)
+	astObj := v.builtin.pkg.Scope.Lookup(name)
+	if astObj == nil {
+		return nil, errors.Errorf("no builtin object for %s", name)
+	}
+	return astObj, nil
 }
 
 func (b *builtinPkg) CompiledGoFiles() []source.ParseGoHandle {
@@ -34,6 +39,9 @@ func (b *builtinPkg) CompiledGoFiles() []source.ParseGoHandle {
 // 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) buildBuiltinPackage(ctx context.Context) error {
+	if v.builtin != nil {
+		return errors.Errorf("rebuilding builtin package")
+	}
 	cfg := v.Config(ctx)
 	pkgs, err := packages.Load(cfg, "builtin")
 	// If the error is related to a go.mod parse error, we want to continue loading.
@@ -44,15 +52,15 @@ func (v *view) buildBuiltinPackage(ctx context.Context) error {
 		return err
 	}
 	if len(pkgs) != 1 {
-		return err
+		return errors.Errorf("expected 1 (got %v) packages for builtin", len(pkgs))
 	}
-	pkg := pkgs[0]
 	files := make(map[string]*ast.File)
-	for _, filename := range pkg.GoFiles {
+	var pghs []source.ParseGoHandle
+	for _, filename := range pkgs[0].GoFiles {
 		fh := v.session.GetFile(span.FileURI(filename))
-		ph := v.session.cache.ParseGoHandle(fh, source.ParseFull)
-		v.builtin.files = append(v.builtin.files, ph)
-		file, _, _, err := ph.Parse(ctx)
+		pgh := v.session.cache.ParseGoHandle(fh, source.ParseFull)
+		pghs = append(pghs, pgh)
+		file, _, _, err := pgh.Parse(ctx)
 		if err != nil {
 			return err
 		}
@@ -62,6 +70,13 @@ func (v *view) buildBuiltinPackage(ctx context.Context) error {
 		v.ignoredURIs[span.NewURI(filename)] = struct{}{}
 		v.ignoredURIsMu.Unlock()
 	}
-	v.builtin.pkg, err = ast.NewPackage(cfg.Fset, files, nil, nil)
-	return err
+	pkg, err := ast.NewPackage(cfg.Fset, files, nil, nil)
+	if err != nil {
+		return err
+	}
+	v.builtin = &builtinPkg{
+		files: pghs,
+		pkg:   pkg,
+	}
+	return nil
 }

internal/lsp/cache/load.go

@@ -228,11 +228,7 @@ func (v *view) isTestMain(ctx context.Context, pkg *packages.Package) bool {
 	if len(pkg.GoFiles) > 1 {
 		return false
 	}
-	buildCachePath, err := v.getBuildCachePath(ctx)
-	if err != nil {
-		return false
-	}
-	if !strings.HasPrefix(pkg.GoFiles[0], buildCachePath) {
+	if !strings.HasPrefix(pkg.GoFiles[0], v.gocache) {
 		return false
 	}
 	return true

internal/lsp/cache/modfiles.go

@@ -9,21 +9,47 @@ import (
 	"io"
 	"io/ioutil"
 	"os"
+	"path/filepath"
 	"strings"
 
 	"golang.org/x/tools/internal/lsp/source"
 	"golang.org/x/tools/internal/lsp/telemetry"
+	"golang.org/x/tools/internal/span"
 	"golang.org/x/tools/internal/telemetry/log"
 	errors "golang.org/x/xerrors"
 )
 
+func (v *view) modFiles(ctx context.Context) (span.URI, span.URI, error) {
+	// Don't return errors if the view is not a module.
+	if v.mod == nil {
+		return "", "", nil
+	}
+	// Copy the real go.mod file content into the temp go.mod file.
+	origFile, err := os.Open(v.mod.realMod.Filename())
+	if err != nil {
+		return "", "", err
+	}
+	defer origFile.Close()
+
+	tempFile, err := os.OpenFile(v.mod.tempMod.Filename(), os.O_WRONLY, os.ModePerm)
+	if err != nil {
+		return "", "", err
+	}
+	defer tempFile.Close()
+
+	if _, err := io.Copy(tempFile, origFile); err != nil {
+		return "", "", err
+	}
+	return v.mod.realMod, v.mod.tempMod, nil
+}
+
 // This function will return the main go.mod file for this folder if it exists and whether the -modfile
 // flag exists for this version of go.
-func modfileFlagExists(ctx context.Context, folder string, env []string) (string, bool, error) {
-	// Check the Go version by running go list with GO111MODULE=off.
-	// If the output is anything other than "go1.14\n", assume -modfile is not supported.
-	// Borrowed from internal/imports/mod.go:620
+func (v *view) modfileFlagExists(ctx context.Context, env []string) (string, bool, error) {
+	// Check the go version by running "go list" with modules off.
+	// Borrowed from internal/imports/mod.go:620.
 	const format = `{{range context.ReleaseTags}}{{if eq . "go1.14"}}{{.}}{{end}}{{end}}`
+	folder := v.folder.Filename()
 	stdout, err := source.InvokeGo(ctx, folder, append(env, "GO111MODULE=off"), "list", "-e", "-f", format)
 	if err != nil {
 		return "", false, err
@@ -34,62 +60,71 @@ func modfileFlagExists(ctx context.Context, folder string, env []string) (string
 		log.Error(ctx, "unexpected stdout when checking for go1.14", errors.Errorf("%q", stdout), telemetry.Directory.Of(folder))
 		return "", false, nil
 	}
-	// Get the go.mod file associated with this module.
-	b, err := source.InvokeGo(ctx, folder, env, "env", "GOMOD")
-	if err != nil {
-		return "", false, err
-	}
-	modfile := strings.TrimSpace(b.String())
+	modfile := strings.TrimSpace(v.gomod)
 	if modfile == os.DevNull {
-		return "", false, errors.Errorf("go env GOMOD did not detect a go.mod file in this folder")
+		return "", false, errors.Errorf("unable to detect a go.mod file in %s", v.folder)
 	}
 	return modfile, lines[0] == "go1.14", nil
 }
 
-// The function getModfiles will return the go.mod files associated with the directory that is passed in.
-func getModfiles(ctx context.Context, folder string, options source.Options) (*modfiles, error) {
-	if !options.TempModfile {
-		log.Print(ctx, "using the -modfile flag is disabled", telemetry.Directory.Of(folder))
-		return nil, nil
+func (v *view) setModuleInformation(ctx context.Context, enabled bool) error {
+	// The user has disabled the use of the -modfile flag.
+	if !enabled {
+		log.Print(ctx, "using the -modfile flag is disabled", telemetry.Directory.Of(v.folder))
+		return nil
 	}
-	modfile, flagExists, err := modfileFlagExists(ctx, folder, options.Env)
+	modFile, flagExists, err := v.modfileFlagExists(ctx, v.Options().Env)
 	if err != nil {
-		return nil, err
+		return err
 	}
+	// The user's version of Go does not support the -modfile flag.
 	if !flagExists {
-		return nil, nil
+		return nil
 	}
-	if modfile == "" || modfile == os.DevNull {
-		return nil, errors.Errorf("go env GOMOD cannot detect a go.mod file in this folder")
+	if modFile == "" || modFile == os.DevNull {
+		return errors.Errorf("unable to detect a go.mod file in %s", v.folder)
 	}
 	// Copy the current go.mod file into the temporary go.mod file.
-	tempFile, err := ioutil.TempFile("", "go.*.mod")
+	// The file's name will be of the format go.1234.mod.
+	// It's temporary go.sum file should have the corresponding format of go.1234.sum.
+	tempModFile, err := ioutil.TempFile("", "go.*.mod")
 	if err != nil {
-		return nil, err
+		return err
 	}
-	defer tempFile.Close()
-	origFile, err := os.Open(modfile)
+	defer tempModFile.Close()
+
+	origFile, err := os.Open(modFile)
 	if err != nil {
-		return nil, err
+		return err
 	}
 	defer origFile.Close()
-	if _, err := io.Copy(tempFile, origFile); err != nil {
-		return nil, err
+
+	if _, err := io.Copy(tempModFile, origFile); err != nil {
+		return err
 	}
-	copySumFile(modfile, tempFile.Name())
-	return &modfiles{real: modfile, temp: tempFile.Name()}, nil
+	v.mod = &moduleInformation{
+		realMod: span.FileURI(modFile),
+		tempMod: span.FileURI(tempModFile.Name()),
+	}
+	// Copy go.sum file as well (if there is one).
+	sumFile := filepath.Join(filepath.Dir(modFile), "go.sum")
+	stat, err := os.Stat(sumFile)
+	if err != nil || !stat.Mode().IsRegular() {
+		return nil
+	}
+	contents, err := ioutil.ReadFile(sumFile)
+	if err != nil {
+		return err
+	}
+	if err := ioutil.WriteFile(v.mod.tempSumFile(), contents, stat.Mode()); err != nil {
+		return err
+	}
+	return nil
 }
 
-func copySumFile(realFile, tempFile string) {
-	realSum := realFile[0:len(realFile)-3] + "sum"
-	tempSum := tempFile[0:len(tempFile)-3] + "sum"
-	stat, err := os.Stat(realSum)
-	if err != nil || !stat.Mode().IsRegular() {
-		return
-	}
-	contents, err := ioutil.ReadFile(realSum)
-	if err != nil {
-		return
-	}
-	ioutil.WriteFile(tempSum, contents, stat.Mode())
+// tempSumFile returns the path to the copied temporary go.sum file.
+// It simply replaces the extension of the temporary go.mod file with "sum".
+func (mod *moduleInformation) tempSumFile() string {
+	tmp := mod.tempMod.Filename()
+	return tmp[:len(tmp)-len("mod")] + "sum"
 }

internal/lsp/cache/session.go

@@ -15,7 +15,6 @@ import (
 	"golang.org/x/tools/internal/lsp/source"
 	"golang.org/x/tools/internal/lsp/telemetry"
 	"golang.org/x/tools/internal/span"
-	"golang.org/x/tools/internal/telemetry/log"
 	"golang.org/x/tools/internal/telemetry/trace"
 	"golang.org/x/tools/internal/xcontext"
 	errors "golang.org/x/xerrors"
@@ -78,10 +77,6 @@ func (s *session) createView(ctx context.Context, name string, folder span.URI,
 	baseCtx := trace.Detach(xcontext.Detach(ctx))
 	backgroundCtx, cancel := context.WithCancel(baseCtx)
 
-	modfiles, err := getModfiles(ctx, folder.Filename(), options)
-	if err != nil {
-		log.Error(ctx, "error getting modfiles", err, telemetry.Directory.Of(folder))
-	}
 	v := &view{
 		session:       s,
 		initialized:   make(chan struct{}),
@@ -91,7 +86,6 @@ func (s *session) createView(ctx context.Context, name string, folder span.URI,
 		backgroundCtx: backgroundCtx,
 		cancel:        cancel,
 		name:          name,
-		modfiles:      modfiles,
 		folder:        folder,
 		filesByURI:    make(map[span.URI]*fileBase),
 		filesByBase:   make(map[string][]*fileBase),
@@ -105,7 +99,6 @@ func (s *session) createView(ctx context.Context, name string, folder span.URI,
 			workspacePackages: make(map[packageID]packagePath),
 		},
 		ignoredURIs: make(map[span.URI]struct{}),
-		builtin:     &builtinPkg{},
 	}
 	v.snapshot.view = v
 
@@ -113,10 +106,12 @@ func (s *session) createView(ctx context.Context, name string, folder span.URI,
 		v.session.cache.options(&v.options)
 	}
 
-	// Preemptively build the builtin package,
-	// so we immediately add builtin.go to the list of ignored files.
-	// TODO(rstambler): This could be part of the initialization process.
-	if err := v.buildBuiltinPackage(ctx); err != nil {
+	// Make sure to get the `go env` before continuing with initialization.
+	if err := v.setGoEnv(ctx, folder.Filename(), options.Env); err != nil {
+		return nil, nil, err
+	}
+	// Set the module-specific information.
+	if err := v.setModuleInformation(ctx, v.options.TempModfile); err != nil {
 		return nil, nil, err
 	}
 

internal/lsp/cache/snapshot.go

@@ -6,7 +6,6 @@ package cache
 
 import (
 	"context"
-	"io"
 	"os"
 	"path/filepath"
 	"strings"
@@ -68,32 +67,22 @@ func (s *snapshot) View() source.View {
 }
 
 func (s *snapshot) ModFiles(ctx context.Context) (source.FileHandle, source.FileHandle, error) {
-	if s.view.modfiles == nil {
+	r, t, err := s.view.modFiles(ctx)
+	if err != nil {
+		return nil, nil, err
+	}
+	if r == "" || t == "" {
 		return nil, nil, nil
 	}
-	realfh, err := s.GetFile(span.FileURI(s.view.modfiles.real))
+	// Get the real mod file's content through the snapshot,
+	// as it may be open in an overlay.
+	realfh, err := s.GetFile(r)
 	if err != nil {
 		return nil, nil, err
 	}
-	if realfh == nil {
-		return nil, nil, errors.Errorf("go.mod filehandle is nil")
-	}
-	// Copy the real go.mod file content into the temp go.mod file.
-	origFile, err := os.Open(s.view.modfiles.real)
-	if err != nil {
-		return nil, nil, err
-	}
-	defer origFile.Close()
-	tempFile, err := os.OpenFile(s.view.modfiles.temp, os.O_WRONLY, os.ModePerm)
-	if err != nil {
-		return nil, nil, err
-	}
-	defer tempFile.Close()
-	if _, err := io.Copy(tempFile, origFile); err != nil {
-		return nil, nil, err
-	}
-	// Go directly to disk to get the correct FileHandle, since we just copied the file without invalidating the snapshot.
-	tempfh := s.view.Session().Cache().GetFile(span.FileURI(s.view.modfiles.temp))
+	// Go directly to disk to get the temporary mod file,
+	// since it is always on disk.
+	tempfh := s.view.session.cache.GetFile(t)
 	if tempfh == nil {
 		return nil, nil, errors.Errorf("temporary go.mod filehandle is nil")
 	}

internal/lsp/cache/view.go

@@ -7,17 +7,18 @@ package cache
 
 import (
 	"context"
+	"encoding/json"
 	"fmt"
 	"go/ast"
 	"go/token"
 	"os"
-	"os/exec"
 	"path/filepath"
 	"reflect"
 	"strings"
 	"sync"
 	"time"
 
+	"golang.org/x/sync/errgroup"
 	"golang.org/x/tools/go/packages"
 	"golang.org/x/tools/internal/imports"
 	"golang.org/x/tools/internal/lsp/debug"
@@ -53,12 +54,12 @@ type view struct {
 	// Name is the user visible name of this view.
 	name string
 
-	// modfiles are the go.mod files attributed to this view.
-	modfiles *modfiles
-
 	// Folder is the root of this view.
 	folder span.URI
 
+	// mod is the module information for this view.
+	mod *moduleInformation
+
 	// process is the process env for this view.
 	// Note: this contains cached module and filesystem state.
 	//
@@ -81,21 +82,23 @@ type view struct {
 	snapshotMu sync.Mutex
 	snapshot   *snapshot
 
-	// builtin is used to resolve builtin types.
-	builtin *builtinPkg
-
 	// ignoredURIs is the set of URIs of files that we ignore.
 	ignoredURIsMu sync.Mutex
 	ignoredURIs   map[span.URI]struct{}
 
-	// initialized is closed when we have attempted to load the view's workspace packages.
-	// If we failed to load initially, we don't re-try to avoid too many go/packages calls.
+	// `go env` variables that need to be tracked by the view.
+	gopath, gomod, gocache string
+
+	// initialized is closed when the view has been fully initialized.
+	// On initialization, the view's workspace packages are loaded.
+	// All of the fields below are set as part of initialization.
+	// If we failed to load, we don't re-try to avoid too many go/packages calls.
 	initializeOnce      sync.Once
 	initialized         chan struct{}
 	initializationError error
 
-	// buildCachePath is the value of `go env GOCACHE`.
-	buildCachePath string
+	// builtin is used to resolve builtin types.
+	builtin *builtinPkg
 }
 
 // fileBase holds the common functionality for all files.
@@ -120,9 +123,10 @@ func (f *fileBase) addURI(uri span.URI) int {
 	return len(f.uris)
 }
 
-// modfiles holds the real and temporary go.mod files that are attributed to a view.
-type modfiles struct {
-	real, temp string
+// moduleInformation holds the real and temporary go.mod files
+// that are attributed to a view.
+type moduleInformation struct {
+	realMod, tempMod span.URI
 }
 
 func (v *view) Session() source.Session {
@@ -173,13 +177,12 @@ func (v *view) Config(ctx context.Context) *packages.Config {
 	// We want to run the go commands with the -modfile flag if the version of go
 	// that we are using supports it.
 	buildFlags := v.options.BuildFlags
-	if v.modfiles != nil {
-		buildFlags = append(buildFlags, fmt.Sprintf("-modfile=%s", v.modfiles.temp))
+	if v.mod != nil {
+		buildFlags = append(buildFlags, fmt.Sprintf("-modfile=%s", v.mod.tempMod.Filename()))
 	}
-	return &packages.Config{
+	cfg := &packages.Config{
 		Dir:        v.folder.Filename(),
 		Context:    ctx,
-		Env:        v.options.Env,
 		BuildFlags: buildFlags,
 		Mode: packages.NeedName |
 			packages.NeedFiles |
@@ -199,6 +202,9 @@ func (v *view) Config(ctx context.Context) *packages.Config {
 		},
 		Tests: true,
 	}
+	cfg.Env = append(cfg.Env, fmt.Sprintf("GOPATH=%s", v.gopath))
+	cfg.Env = append(cfg.Env, v.options.Env...)
+	return cfg
 }
 
 func (v *view) RunProcessEnvFunc(ctx context.Context, fn func(*imports.Options) error) error {
@@ -284,14 +290,6 @@ func (v *view) buildProcessEnv(ctx context.Context) (*imports.ProcessEnv, error)
 			env.GOSUMDB = split[1]
 		}
 	}
-
-	if env.GOPATH == "" {
-		gopath, err := getGoEnvVar(ctx, cfg, "GOPATH")
-		if err != nil {
-			return nil, err
-		}
-		env.GOPATH = gopath
-	}
 	return env, nil
 }
 
@@ -423,8 +421,9 @@ func (v *view) shutdown(context.Context) {
 		v.cancel()
 		v.cancel = nil
 	}
-	if v.modfiles != nil {
-		os.Remove(v.modfiles.temp)
+	if v.mod != nil {
+		os.Remove(v.mod.tempMod.Filename())
+		os.Remove(v.mod.tempSumFile())
 	}
 	debug.DropView(debugView{v})
 }
@@ -453,10 +452,6 @@ func (v *view) BackgroundContext() context.Context {
 	return v.backgroundCtx
 }
 
-func (v *view) BuiltinPackage() source.BuiltinPackage {
-	return v.builtin
-}
-
 func (v *view) Snapshot() source.Snapshot {
 	return v.getSnapshot()
 }
@@ -472,16 +467,32 @@ func (v *view) initialize(ctx context.Context, s *snapshot) {
 	v.initializeOnce.Do(func() {
 		defer close(v.initialized)
 
-		// Do not cancel the call to go/packages.Load for the entire workspace.
-		meta, err := s.load(ctx, directoryURI(v.folder))
-		if err != nil {
+		g, ctx := errgroup.WithContext(ctx)
+
+		// Load all of the packages in the workspace.
+		g.Go(func() error {
+			// Do not cancel the call to go/packages.Load for the entire workspace.
+			meta, err := s.load(ctx, directoryURI(v.folder))
+			if err != nil {
+				return err
+			}
+			// A test variant of a package can only be loaded directly by loading
+			// the non-test variant with -test. Track the import path of the non-test variant.
+			for _, m := range meta {
+				s.setWorkspacePackage(m.id, m.pkgPath)
+			}
+			return nil
+		})
+
+		// Build the builtin package on initialization.
+		g.Go(func() error {
+			return v.buildBuiltinPackage(ctx)
+		})
+
+		// Wait for all initialization tasks to complete.
+		if err := g.Wait(); err != nil {
 			v.initializationError = err
 		}
-		// A test variant of a package can only be loaded directly by loading
-		// the non-test variant with -test. Track the import path of the non-test variant.
-		for _, m := range meta {
-			s.setWorkspacePackage(m.id, m.pkgPath)
-		}
 	})
 }
 
@@ -532,42 +543,47 @@ func (v *view) cancelBackground() {
 	v.backgroundCtx, v.cancel = context.WithCancel(v.baseCtx)
 }
 
-func (v *view) getBuildCachePath(ctx context.Context) (string, error) {
-	v.mu.Lock()
-	defer v.mu.Unlock()
-
-	if v.buildCachePath == "" {
-		path, err := getGoEnvVar(ctx, v.Config(ctx), "GOCACHE")
-		if err != nil {
-			return "", err
-		}
-		v.buildCachePath = path
-	}
-	return v.buildCachePath, nil
-}
-
-func getGoEnvVar(ctx context.Context, cfg *packages.Config, value string) (string, error) {
-	var result string
-	for _, kv := range cfg.Env {
-		split := strings.Split(kv, "=")
-		if len(split) < 2 {
+func (v *view) setGoEnv(ctx context.Context, dir string, env []string) error {
+	var gocache, gopath bool
+	for _, e := range env {
+		split := strings.Split(e, "=")
+		if len(split) != 2 {
 			continue
 		}
-		if split[0] == value {
-			result = split[1]
+		switch split[0] {
+		case "GOCACHE":
+			v.gocache = split[1]
+			gocache = true
+		case "GOPATH":
+			v.gopath = split[1]
+			gopath = true
 		}
 	}
-	if result == "" {
-		cmd := exec.CommandContext(ctx, "go", "env", value)
-		cmd.Env = cfg.Env
-		out, err := cmd.CombinedOutput()
-		if err != nil {
-			return "", err
-		}
-		result = strings.TrimSpace(string(out))
-		if result == "" {
-			return "", errors.Errorf("no value for %s", value)
+	b, err := source.InvokeGo(ctx, dir, env, "env", "-json")
+	if err != nil {
+		return err
+	}
+	envMap := make(map[string]string)
+	decoder := json.NewDecoder(b)
+	if err := decoder.Decode(&envMap); err != nil {
+		return err
+	}
+	if !gopath {
+		if gopath, ok := envMap["GOPATH"]; ok {
+			v.gopath = gopath
+		} else {
+			return errors.New("unable to determine GOPATH")
 		}
 	}
-	return result, nil
+	if !gocache {
+		if gocache, ok := envMap["GOCACHE"]; ok {
+			v.gocache = gocache
+		} else {
+			return errors.New("unable to determine GOCACHE")
+		}
+	}
+	if gomod, ok := envMap["GOMOD"]; ok {
+		v.gomod = gomod
+	}
+	return nil
 }

internal/lsp/source/completion_builtin.go

@@ -13,11 +13,10 @@ import (
 // argument. It attempts to use the AST hints from builtin.go where
 // possible.
 func (c *completer) builtinArgKind(obj types.Object, call *ast.CallExpr) objKind {
-	astObj := c.snapshot.View().BuiltinPackage().Lookup(obj.Name())
-	if astObj == nil {
+	astObj, err := c.snapshot.View().LookupBuiltin(obj.Name())
+	if err != nil {
 		return 0
 	}
-
 	exprIdx := indexExprAtPos(c.pos, call.Args)
 
 	decl, ok := astObj.Decl.(*ast.FuncDecl)

internal/lsp/source/completion_format.go

@@ -245,11 +245,12 @@ func (c *completer) formatBuiltin(cand candidate) CompletionItem {
 		item.Kind = protocol.ConstantCompletion
 	case *types.Builtin:
 		item.Kind = protocol.FunctionCompletion
-		builtin := c.snapshot.View().BuiltinPackage().Lookup(obj.Name())
-		if obj == nil {
+		astObj, err := c.snapshot.View().LookupBuiltin(obj.Name())
+		if err != nil {
+			log.Error(c.ctx, "no builtin package", err)
 			break
 		}
-		decl, ok := builtin.Decl.(*ast.FuncDecl)
+		decl, ok := astObj.Decl.(*ast.FuncDecl)
 		if !ok {
 			break
 		}

internal/lsp/source/identifier.go

@@ -153,11 +153,11 @@ func identifier(s Snapshot, pkg Package, file *ast.File, pos token.Pos) (*Identi
 
 	// Handle builtins separately.
 	if result.Declaration.obj.Parent() == types.Universe {
-		obj := view.BuiltinPackage().Lookup(result.Name)
-		if obj == nil {
-			return result, nil
+		astObj, err := view.LookupBuiltin(result.Name)
+		if err != nil {
+			return nil, err
 		}
-		decl, ok := obj.Decl.(ast.Node)
+		decl, ok := astObj.Decl.(ast.Node)
 		if !ok {
 			return nil, errors.Errorf("no declaration for %s", result.Name)
 		}

internal/lsp/source/signature_help.go

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

internal/lsp/source/view.go

@@ -92,8 +92,8 @@ type View interface {
 	// Folder returns the root folder for this view.
 	Folder() span.URI
 
-	// BuiltinPackage returns the type information for the special "builtin" package.
-	BuiltinPackage() BuiltinPackage
+	// LookupBuiltin returns the go/ast.Object for the given name in the builtin package.
+	LookupBuiltin(name string) (*ast.Object, error)
 
 	// BackgroundContext returns a context used for all background processing
 	// on behalf of this view.
@@ -358,8 +358,3 @@ const (
 func (e *Error) Error() string {
 	return fmt.Sprintf("%s:%s: %s", e.URI, e.Range, e.Message)
 }
-
-type BuiltinPackage interface {
-	Lookup(name string) *ast.Object
-	CompiledGoFiles() []ParseGoHandle
-}