internal/imports: fix crash when adding stdlib imports

We failed to initialize the ProcessEnv when adding stdlib imports.
Somehow this went unnnoticed for a month or something. Initialize on
demand there too, and add a stronger check so there's a better error
message.

Fixes golang/go#40670.

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

internal/gopathwalk/walk.go

@@ -10,7 +10,6 @@ import (
 	"bufio"
 	"bytes"
 	"fmt"
-	"go/build"
 	"io/ioutil"
 	"log"
 	"os"
@@ -47,16 +46,6 @@ type Root struct {
 	Type RootType
 }
 
-// SrcDirsRoots returns the roots from build.Default.SrcDirs(). Not modules-compatible.
-func SrcDirsRoots(ctx *build.Context) []Root {
-	var roots []Root
-	roots = append(roots, Root{filepath.Join(ctx.GOROOT, "src"), RootGOROOT})
-	for _, p := range filepath.SplitList(ctx.GOPATH) {
-		roots = append(roots, Root{filepath.Join(p, "src"), RootGOPATH})
-	}
-	return roots
-}
-
 // Walk walks Go source directories ($GOROOT, $GOPATH, etc) to find packages.
 // For each package found, add will be called (concurrently) with the absolute
 // paths of the containing source directory and the package directory.

internal/imports/fix.go

@@ -573,7 +573,9 @@ func getFixes(fset *token.FileSet, f *ast.File, filename string, env *ProcessEnv
 		return fixes, nil
 	}
 
-	addStdlibCandidates(p, p.missingRefs)
+	if err := addStdlibCandidates(p, p.missingRefs); err != nil {
+		return nil, err
+	}
 	p.assumeSiblingImportsValid()
 	if fixes, done := p.fix(); done {
 		return fixes, nil
@@ -601,10 +603,14 @@ func getCandidatePkgs(ctx context.Context, wrappedCallback *scanCallback, filena
 	notSelf := func(p *pkg) bool {
 		return p.packageName != filePkg || p.dir != filepath.Dir(filename)
 	}
+	goenv, err := env.goEnv()
+	if err != nil {
+		return err
+	}
 	// Start off with the standard library.
 	for importPath, exports := range stdlib {
 		p := &pkg{
-			dir:             filepath.Join(env.goroot(), "src", importPath),
+			dir:             filepath.Join(goenv["GOROOT"], "src", importPath),
 			importPathShort: importPath,
 			packageName:     path.Base(importPath),
 			relevance:       MaxRelevance,
@@ -779,33 +785,44 @@ type ProcessEnv struct {
 	// If Logf is non-nil, debug logging is enabled through this function.
 	Logf func(format string, args ...interface{})
 
+	initialized bool
+
 	resolver Resolver
 }
 
-func (e *ProcessEnv) goroot() string {
-	return e.mustGetEnv("GOROOT")
-}
-
-func (e *ProcessEnv) gopath() string {
-	return e.mustGetEnv("GOPATH")
-}
-
-func (e *ProcessEnv) mustGetEnv(k string) string {
-	v, ok := e.Env[k]
-	if !ok {
-		panic(fmt.Sprintf("%v not set in evaluated environment", k))
+func (e *ProcessEnv) goEnv() (map[string]string, error) {
+	if err := e.init(); err != nil {
+		return nil, err
 	}
-	return v
+	return e.Env, nil
+}
+
+func (e *ProcessEnv) matchFile(dir, name string) (bool, error) {
+	return build.Default.MatchFile(dir, name)
 }
 
 // CopyConfig copies the env's configuration into a new env.
 func (e *ProcessEnv) CopyConfig() *ProcessEnv {
-	copy := *e
-	copy.resolver = nil
-	return &copy
+	copy := &ProcessEnv{
+		GocmdRunner: e.GocmdRunner,
+		initialized: e.initialized,
+		BuildFlags:  e.BuildFlags,
+		Logf:        e.Logf,
+		WorkingDir:  e.WorkingDir,
+		resolver:    nil,
+		Env:         map[string]string{},
+	}
+	for k, v := range e.Env {
+		copy.Env[k] = v
+	}
+	return copy
 }
 
 func (e *ProcessEnv) init() error {
+	if e.initialized {
+		return nil
+	}
+
 	foundAllRequired := true
 	for _, k := range RequiredGoEnvVars {
 		if _, ok := e.Env[k]; !ok {
@@ -814,6 +831,7 @@ func (e *ProcessEnv) init() error {
 		}
 	}
 	if foundAllRequired {
+		e.initialized = true
 		return nil
 	}
 
@@ -832,6 +850,7 @@ func (e *ProcessEnv) init() error {
 	for k, v := range goEnv {
 		e.Env[k] = v
 	}
+	e.initialized = true
 	return nil
 }
 
@@ -858,10 +877,14 @@ func (e *ProcessEnv) GetResolver() (Resolver, error) {
 	return e.resolver, nil
 }
 
-func (e *ProcessEnv) buildContext() *build.Context {
+func (e *ProcessEnv) buildContext() (*build.Context, error) {
 	ctx := build.Default
-	ctx.GOROOT = e.goroot()
-	ctx.GOPATH = e.gopath()
+	goenv, err := e.goEnv()
+	if err != nil {
+		return nil, err
+	}
+	ctx.GOROOT = goenv["GOROOT"]
+	ctx.GOPATH = goenv["GOPATH"]
 
 	// As of Go 1.14, build.Context has a Dir field
 	// (see golang.org/issue/34860).
@@ -877,7 +900,7 @@ func (e *ProcessEnv) buildContext() *build.Context {
 		dir.SetString(e.WorkingDir)
 	}
 
-	return &ctx
+	return &ctx, nil
 }
 
 func (e *ProcessEnv) invokeGo(ctx context.Context, verb string, args ...string) (*bytes.Buffer, error) {
@@ -892,10 +915,14 @@ func (e *ProcessEnv) invokeGo(ctx context.Context, verb string, args ...string)
 	return e.GocmdRunner.Run(ctx, inv)
 }
 
-func addStdlibCandidates(pass *pass, refs references) {
+func addStdlibCandidates(pass *pass, refs references) error {
+	goenv, err := pass.env.goEnv()
+	if err != nil {
+		return err
+	}
 	add := func(pkg string) {
 		// Prevent self-imports.
-		if path.Base(pkg) == pass.f.Name.Name && filepath.Join(pass.env.goroot(), "src", pkg) == pass.srcDir {
+		if path.Base(pkg) == pass.f.Name.Name && filepath.Join(goenv["GOROOT"], "src", pkg) == pass.srcDir {
 			return
 		}
 		exports := copyExports(stdlib[pkg])
@@ -916,6 +943,7 @@ func addStdlibCandidates(pass *pass, refs references) {
 			}
 		}
 	}
+	return nil
 }
 
 // A Resolver does the build-system-specific parts of goimports.
@@ -1112,21 +1140,24 @@ func (r *gopathResolver) ClearForNewScan() {
 
 func (r *gopathResolver) loadPackageNames(importPaths []string, srcDir string) (map[string]string, error) {
 	names := map[string]string{}
+	bctx, err := r.env.buildContext()
+	if err != nil {
+		return nil, err
+	}
 	for _, path := range importPaths {
-		names[path] = importPathToName(r.env, path, srcDir)
+		names[path] = importPathToName(bctx, path, srcDir)
 	}
 	return names, nil
 }
 
 // importPathToName finds out the actual package name, as declared in its .go files.
-// If there's a problem, it returns "".
-func importPathToName(env *ProcessEnv, importPath, srcDir string) (packageName string) {
+func importPathToName(bctx *build.Context, importPath, srcDir string) string {
 	// Fast path for standard library without going to disk.
 	if _, ok := stdlib[importPath]; ok {
 		return path.Base(importPath) // stdlib packages always match their paths.
 	}
 
-	buildPkg, err := env.buildContext().Import(importPath, srcDir, build.FindOnly)
+	buildPkg, err := bctx.Import(importPath, srcDir, build.FindOnly)
 	if err != nil {
 		return ""
 	}
@@ -1287,8 +1318,18 @@ func (r *gopathResolver) scan(ctx context.Context, callback *scanCallback) error
 	}
 	stop := r.cache.ScanAndListen(ctx, processDir)
 	defer stop()
+
+	goenv, err := r.env.goEnv()
+	if err != nil {
+		return err
+	}
+	var roots []gopathwalk.Root
+	roots = append(roots, gopathwalk.Root{filepath.Join(goenv["GOROOT"], "src"), gopathwalk.RootGOROOT})
+	for _, p := range filepath.SplitList(goenv["GOPATH"]) {
+		roots = append(roots, gopathwalk.Root{filepath.Join(p, "src"), gopathwalk.RootGOPATH})
+	}
 	// The callback is not necessarily safe to use in the goroutine below. Process roots eagerly.
-	roots := filterRoots(gopathwalk.SrcDirsRoots(r.env.buildContext()), callback.rootFound)
+	roots = filterRoots(roots, callback.rootFound)
 	// We can't cancel walks, because we need them to finish to have a usable
 	// cache. Instead, run them in a separate goroutine and detach.
 	scanDone := make(chan struct{})
@@ -1348,8 +1389,6 @@ func VendorlessPath(ipath string) string {
 }
 
 func loadExportsFromFiles(ctx context.Context, env *ProcessEnv, dir string, includeTest bool) (string, []string, error) {
-	var exports []string
-
 	// Look for non-test, buildable .go files which could provide exports.
 	all, err := ioutil.ReadDir(dir)
 	if err != nil {
@@ -1361,7 +1400,7 @@ func loadExportsFromFiles(ctx context.Context, env *ProcessEnv, dir string, incl
 		if !strings.HasSuffix(name, ".go") || (!includeTest && strings.HasSuffix(name, "_test.go")) {
 			continue
 		}
-		match, err := env.buildContext().MatchFile(dir, fi.Name())
+		match, err := env.matchFile(dir, fi.Name())
 		if err != nil || !match {
 			continue
 		}
@@ -1373,6 +1412,7 @@ func loadExportsFromFiles(ctx context.Context, env *ProcessEnv, dir string, incl
 	}
 
 	var pkgName string
+	var exports []string
 	fset := token.NewFileSet()
 	for _, fi := range files {
 		select {

internal/imports/fix_test.go

@@ -1568,8 +1568,8 @@ var _ = bytes.Buffer
 		},
 	}.test(t, func(t *goimportTest) {
 		// Run in GOROOT/src so that the std module shows up in go list -m all.
-		t.env.WorkingDir = filepath.Join(t.env.goroot(), "src")
-		got, err := t.processNonModule(filepath.Join(t.env.goroot(), "src/x.go"), []byte(input), nil)
+		t.env.WorkingDir = filepath.Join(t.goroot, "src")
+		got, err := t.processNonModule(filepath.Join(t.goroot, "src/x.go"), []byte(input), nil)
 		if err != nil {
 			t.Fatalf("Process() = %v", err)
 		}
@@ -1591,7 +1591,7 @@ var _ = ecdsa.GenerateKey
 			Files: fm{"x.go": "package x"},
 		},
 	}.test(t, func(t *goimportTest) {
-		got, err := t.processNonModule(filepath.Join(t.env.goroot(), "src/crypto/ecdsa/foo.go"), []byte(input), nil)
+		got, err := t.processNonModule(filepath.Join(t.goroot, "src/crypto/ecdsa/foo.go"), []byte(input), nil)
 		if err != nil {
 			t.Fatalf("Process() = %v", err)
 		}
@@ -1646,6 +1646,7 @@ func (c testConfig) test(t *testing.T, fn func(*goimportTest)) {
 			// packagestest clears out GOROOT to work around golang/go#32849,
 			// which isn't relevant here. Fill it back in so we can find the standard library.
 			it.env.Env["GOROOT"] = build.Default.GOROOT
+			it.goroot = build.Default.GOROOT
 
 			fn(it)
 		})
@@ -1662,6 +1663,7 @@ func (c testConfig) processTest(t *testing.T, module, file string, contents []by
 
 type goimportTest struct {
 	*testing.T
+	goroot   string
 	env      *ProcessEnv
 	exported *packagestest.Exported
 }

internal/imports/mod.go

@@ -53,6 +53,10 @@ func (r *ModuleResolver) init() error {
 		return nil
 	}
 
+	goenv, err := r.env.goEnv()
+	if err != nil {
+		return err
+	}
 	inv := gocommand.Invocation{
 		BuildFlags: r.env.BuildFlags,
 		Env:        r.env.env(),
@@ -82,7 +86,7 @@ func (r *ModuleResolver) init() error {
 	if gmc := r.env.Env["GOMODCACHE"]; gmc != "" {
 		r.moduleCacheDir = gmc
 	} else {
-		r.moduleCacheDir = filepath.Join(filepath.SplitList(r.env.gopath())[0], "/pkg/mod")
+		r.moduleCacheDir = filepath.Join(filepath.SplitList(goenv["GOPATH"])[0], "/pkg/mod")
 	}
 
 	sort.Slice(r.modsByModPath, func(i, j int) bool {
@@ -99,7 +103,7 @@ func (r *ModuleResolver) init() error {
 	})
 
 	r.roots = []gopathwalk.Root{
-		{filepath.Join(r.env.goroot(), "/src"), gopathwalk.RootGOROOT},
+		{filepath.Join(goenv["GOROOT"], "/src"), gopathwalk.RootGOROOT},
 	}
 	if r.main != nil {
 		r.roots = append(r.roots, gopathwalk.Root{r.main.Dir, gopathwalk.RootCurrentModule})
@@ -240,7 +244,7 @@ func (r *ModuleResolver) findPackage(importPath string) (*gocommand.ModuleJSON,
 		// files in that directory. If not, it could be provided by an
 		// outer module. See #29736.
 		for _, fi := range pkgFiles {
-			if ok, _ := r.env.buildContext().MatchFile(pkgDir, fi.Name()); ok {
+			if ok, _ := r.env.matchFile(pkgDir, fi.Name()); ok {
 				return m, pkgDir
 			}
 		}

internal/imports/mod_test.go

@@ -652,6 +652,7 @@ var proxyDir string
 type modTest struct {
 	*testing.T
 	env      *ProcessEnv
+	gopath   string
 	resolver *ModuleResolver
 	cleanup  func()
 }
@@ -714,6 +715,7 @@ func setup(t *testing.T, main, wd string) *modTest {
 	}
 	return &modTest{
 		T:        t,
+		gopath:   env.Env["GOPATH"],
 		env:      env,
 		resolver: resolver.(*ModuleResolver),
 		cleanup:  func() { removeDir(dir) },
@@ -841,7 +843,7 @@ package x
 import _ "rsc.io/quote"
 `, "")
 	defer mt.cleanup()
-	want := filepath.Join(mt.resolver.env.gopath(), "pkg/mod", "rsc.io/quote@v1.5.2")
+	want := filepath.Join(mt.gopath, "pkg/mod", "rsc.io/quote@v1.5.2")
 
 	found := mt.assertScanFinds("rsc.io/quote", "quote")
 	modDir, _ := mt.resolver.modInfo(found.dir)