Fix the intermittent TestGPGGit failures (#9360)

* Fix the intermittent TestGPGGit failures

Reattempt to open the listener if the port is busy with a delay up to a second
Switch from generating a private key each time, just use a known good key

vendor/golang.org/x/tools/go/packages/doc.go

@@ -60,8 +60,7 @@ causes Load to run in LoadFiles mode, collecting minimal information.
 See the documentation for type Config for details.
 
 As noted earlier, the Config.Mode controls the amount of detail
-reported about the loaded packages, with each mode returning all the data of the
-previous mode with some extra added. See the documentation for type LoadMode
+reported about the loaded packages. See the documentation for type LoadMode
 for details.
 
 Most tools should pass their command-line arguments (after any flags)

vendor/golang.org/x/tools/go/packages/external.go

@@ -12,6 +12,7 @@ import (
 	"bytes"
 	"encoding/json"
 	"fmt"
+	"os"
 	"os/exec"
 	"strings"
 )
@@ -76,15 +77,21 @@ func findExternalDriver(cfg *Config) driver {
 		}
 
 		buf := new(bytes.Buffer)
+		stderr := new(bytes.Buffer)
 		cmd := exec.CommandContext(cfg.Context, tool, words...)
 		cmd.Dir = cfg.Dir
 		cmd.Env = cfg.Env
 		cmd.Stdin = bytes.NewReader(req)
 		cmd.Stdout = buf
-		cmd.Stderr = new(bytes.Buffer)
+		cmd.Stderr = stderr
+
 		if err := cmd.Run(); err != nil {
 			return nil, fmt.Errorf("%v: %v: %s", tool, err, cmd.Stderr)
 		}
+		if len(stderr.Bytes()) != 0 && os.Getenv("GOPACKAGESPRINTDRIVERERRORS") != "" {
+			fmt.Fprintf(os.Stderr, "%s stderr: <<%s>>\n", cmdDebugStr(cmd, words...), stderr)
+		}
+
 		var response driverResponse
 		if err := json.Unmarshal(buf.Bytes(), &response); err != nil {
 			return nil, err

vendor/golang.org/x/tools/go/packages/golist.go

@@ -109,6 +109,7 @@ func goListDriver(cfg *Config, patterns ...string) (*driverResponse, error) {
 			sizeswg.Done()
 		}()
 	}
+	defer sizeswg.Wait()
 
 	// start fetching rootDirs
 	var info goInfo
@@ -127,6 +128,10 @@ func goListDriver(cfg *Config, patterns ...string) (*driverResponse, error) {
 		return &info
 	}
 
+	// Ensure that we don't leak goroutines: Load is synchronous, so callers will
+	// not expect it to access the fields of cfg after the call returns.
+	defer getGoInfo()
+
 	// always pass getGoInfo to golistDriver
 	golistDriver := func(cfg *Config, patterns ...string) (*driverResponse, error) {
 		return golistDriver(cfg, getGoInfo, patterns...)
@@ -248,12 +253,7 @@ func addNeededOverlayPackages(cfg *Config, driver driver, response *responseDedu
 	if len(pkgs) == 0 {
 		return nil
 	}
-	drivercfg := *cfg
-	if getGoInfo().env.modulesOn {
-		drivercfg.BuildFlags = append(drivercfg.BuildFlags, "-mod=readonly")
-	}
-	dr, err := driver(&drivercfg, pkgs...)
-
+	dr, err := driver(cfg, pkgs...)
 	if err != nil {
 		return err
 	}
@@ -264,10 +264,7 @@ func addNeededOverlayPackages(cfg *Config, driver driver, response *responseDedu
 	if err != nil {
 		return err
 	}
-	if err := addNeededOverlayPackages(cfg, driver, response, needPkgs, getGoInfo); err != nil {
-		return err
-	}
-	return nil
+	return addNeededOverlayPackages(cfg, driver, response, needPkgs, getGoInfo)
 }
 
 func runContainsQueries(cfg *Config, driver driver, response *responseDeduper, queries []string, goInfo func() *goInfo) error {
@@ -291,18 +288,31 @@ func runContainsQueries(cfg *Config, driver driver, response *responseDeduper, q
 				// Return the original error if the attempt to fall back failed.
 				return err
 			}
+			// If we get nothing back from `go list`, try to make this file into its own ad-hoc package.
+			if len(dirResponse.Packages) == 0 && queryErr == nil {
+				dirResponse.Packages = append(dirResponse.Packages, &Package{
+					ID:              "command-line-arguments",
+					PkgPath:         query,
+					GoFiles:         []string{query},
+					CompiledGoFiles: []string{query},
+					Imports:         make(map[string]*Package),
+				})
+				dirResponse.Roots = append(dirResponse.Roots, "command-line-arguments")
+			}
 			// Special case to handle issue #33482:
 			// If this is a file= query for ad-hoc packages where the file only exists on an overlay,
 			// and exists outside of a module, add the file in for the package.
-			if len(dirResponse.Packages) == 1 && len(dirResponse.Packages) == 1 &&
-				dirResponse.Packages[0].ID == "command-line-arguments" && len(dirResponse.Packages[0].GoFiles) == 0 {
-				filename := filepath.Join(pattern, filepath.Base(query)) // avoid recomputing abspath
-				// TODO(matloob): check if the file is outside of a root dir?
-				for path := range cfg.Overlay {
-					if path == filename {
-						dirResponse.Packages[0].Errors = nil
-						dirResponse.Packages[0].GoFiles = []string{path}
-						dirResponse.Packages[0].CompiledGoFiles = []string{path}
+			if len(dirResponse.Packages) == 1 && (dirResponse.Packages[0].ID == "command-line-arguments" ||
+				filepath.ToSlash(dirResponse.Packages[0].PkgPath) == filepath.ToSlash(query)) {
+				if len(dirResponse.Packages[0].GoFiles) == 0 {
+					filename := filepath.Join(pattern, filepath.Base(query)) // avoid recomputing abspath
+					// TODO(matloob): check if the file is outside of a root dir?
+					for path := range cfg.Overlay {
+						if path == filename {
+							dirResponse.Packages[0].Errors = nil
+							dirResponse.Packages[0].GoFiles = []string{path}
+							dirResponse.Packages[0].CompiledGoFiles = []string{path}
+						}
 					}
 				}
 			}
@@ -395,6 +405,10 @@ func runNamedQueries(cfg *Config, driver driver, response *responseDeduper, quer
 		}
 
 		files, err := ioutil.ReadDir(modRoot)
+		if err != nil {
+			panic(err) // See above.
+		}
+
 		for _, f := range files {
 			if strings.HasSuffix(f.Name(), ".go") {
 				simpleMatches = append(simpleMatches, rel)
@@ -462,7 +476,7 @@ func runNamedQueries(cfg *Config, driver driver, response *responseDeduper, quer
 		// We're only trying to look at stuff in the module cache, so
 		// disable the network. This should speed things up, and has
 		// prevented errors in at least one case, #28518.
-		tmpCfg.Env = append(append([]string{"GOPROXY=off"}, cfg.Env...))
+		tmpCfg.Env = append([]string{"GOPROXY=off"}, cfg.Env...)
 
 		var err error
 		tmpCfg.Dir, err = ioutil.TempDir("", "gopackages-modquery")
@@ -510,17 +524,29 @@ func roots(cfg *Config) ([]gopathwalk.Root, string, error) {
 
 	var roots []gopathwalk.Root
 	// Always add GOROOT.
-	roots = append(roots, gopathwalk.Root{filepath.Join(goroot, "/src"), gopathwalk.RootGOROOT})
+	roots = append(roots, gopathwalk.Root{
+		Path: filepath.Join(goroot, "/src"),
+		Type: gopathwalk.RootGOROOT,
+	})
 	// If modules are enabled, scan the module dir.
 	if modDir != "" {
-		roots = append(roots, gopathwalk.Root{modDir, gopathwalk.RootCurrentModule})
+		roots = append(roots, gopathwalk.Root{
+			Path: modDir,
+			Type: gopathwalk.RootCurrentModule,
+		})
 	}
 	// Add either GOPATH/src or GOPATH/pkg/mod, depending on module mode.
 	for _, p := range gopath {
 		if modDir != "" {
-			roots = append(roots, gopathwalk.Root{filepath.Join(p, "/pkg/mod"), gopathwalk.RootModuleCache})
+			roots = append(roots, gopathwalk.Root{
+				Path: filepath.Join(p, "/pkg/mod"),
+				Type: gopathwalk.RootModuleCache,
+			})
 		} else {
-			roots = append(roots, gopathwalk.Root{filepath.Join(p, "/src"), gopathwalk.RootGOPATH})
+			roots = append(roots, gopathwalk.Root{
+				Path: filepath.Join(p, "/src"),
+				Type: gopathwalk.RootGOPATH,
+			})
 		}
 	}
 
@@ -637,9 +663,9 @@ func golistDriver(cfg *Config, rootsDirs func() *goInfo, words ...string) (*driv
 	// go list uses the following identifiers in ImportPath and Imports:
 	//
 	// 	"p"			-- importable package or main (command)
-	//      "q.test"		-- q's test executable
+	// 	"q.test"		-- q's test executable
 	// 	"p [q.test]"		-- variant of p as built for q's test executable
-	//	"q_test [q.test]"	-- q's external test package
+	// 	"q_test [q.test]"	-- q's external test package
 	//
 	// The packages p that are built differently for a test q.test
 	// are q itself, plus any helpers used by the external test q_test,
@@ -678,11 +704,11 @@ func golistDriver(cfg *Config, rootsDirs func() *goInfo, words ...string) (*driv
 		// go list -e, when given an absolute path, will find the package contained at
 		// that directory. But when no package exists there, it will return a fake package
 		// with an error and the ImportPath set to the absolute path provided to go list.
-		// Try toto convert that absolute path to what its package path would be if it's
+		// Try to convert that absolute path to what its package path would be if it's
 		// contained in a known module or GOPATH entry. This will allow the package to be
 		// properly "reclaimed" when overlays are processed.
 		if filepath.IsAbs(p.ImportPath) && p.Error != nil {
-			pkgPath, ok := getPkgPath(p.ImportPath, rootsDirs)
+			pkgPath, ok := getPkgPath(cfg, p.ImportPath, rootsDirs)
 			if ok {
 				p.ImportPath = pkgPath
 			}
@@ -779,9 +805,14 @@ func golistDriver(cfg *Config, rootsDirs func() *goInfo, words ...string) (*driv
 		}
 
 		if p.Error != nil {
+			msg := strings.TrimSpace(p.Error.Err) // Trim to work around golang.org/issue/32363.
+			// Address golang.org/issue/35964 by appending import stack to error message.
+			if msg == "import cycle not allowed" && len(p.Error.ImportStack) != 0 {
+				msg += fmt.Sprintf(": import stack: %v", p.Error.ImportStack)
+			}
 			pkg.Errors = append(pkg.Errors, Error{
 				Pos: p.Error.Pos,
-				Msg: strings.TrimSpace(p.Error.Err), // Trim to work around golang.org/issue/32363.
+				Msg: msg,
 			})
 		}
 
@@ -792,15 +823,31 @@ func golistDriver(cfg *Config, rootsDirs func() *goInfo, words ...string) (*driv
 }
 
 // getPkgPath finds the package path of a directory if it's relative to a root directory.
-func getPkgPath(dir string, goInfo func() *goInfo) (string, bool) {
+func getPkgPath(cfg *Config, dir string, goInfo func() *goInfo) (string, bool) {
+	absDir, err := filepath.Abs(dir)
+	if err != nil {
+		cfg.Logf("error getting absolute path of %s: %v", dir, err)
+		return "", false
+	}
 	for rdir, rpath := range goInfo().rootDirs {
+		absRdir, err := filepath.Abs(rdir)
+		if err != nil {
+			cfg.Logf("error getting absolute path of %s: %v", rdir, err)
+			continue
+		}
+		// Make sure that the directory is in the module,
+		// to avoid creating a path relative to another module.
+		if !strings.HasPrefix(absDir, absRdir) {
+			cfg.Logf("%s does not have prefix %s", absDir, absRdir)
+			continue
+		}
 		// TODO(matloob): This doesn't properly handle symlinks.
 		r, err := filepath.Rel(rdir, dir)
 		if err != nil {
 			continue
 		}
 		if rpath != "" {
-			// We choose only ore root even though the directory even it can belong in multiple modules
+			// We choose only one root even though the directory even it can belong in multiple modules
 			// or GOPATH entries. This is okay because we only need to work with absolute dirs when a
 			// file is missing from disk, for instance when gopls calls go/packages in an overlay.
 			// Once the file is saved, gopls, or the next invocation of the tool will get the correct
@@ -808,6 +855,7 @@ func getPkgPath(dir string, goInfo func() *goInfo) (string, bool) {
 			// TODO(matloob): Implement module tiebreaking?
 			return path.Join(rpath, filepath.ToSlash(r)), true
 		}
+		return filepath.ToSlash(r), true
 	}
 	return "", false
 }
@@ -832,8 +880,7 @@ func golistargs(cfg *Config, words []string) []string {
 		fmt.Sprintf("-compiled=%t", cfg.Mode&(NeedCompiledGoFiles|NeedSyntax|NeedTypesInfo|NeedTypesSizes) != 0),
 		fmt.Sprintf("-test=%t", cfg.Tests),
 		fmt.Sprintf("-export=%t", usesExportData(cfg)),
-		fmt.Sprintf("-deps=%t", cfg.Mode&NeedDeps != 0 ||
-			cfg.Mode&NeedTypesInfo != 0), // Dependencies are required to do typechecking on sources, which is required for the TypesInfo.
+		fmt.Sprintf("-deps=%t", cfg.Mode&NeedImports != 0),
 		// go list doesn't let you pass -test and -find together,
 		// probably because you'd just get the TestMain.
 		fmt.Sprintf("-find=%t", !cfg.Tests && cfg.Mode&findFlags == 0),
@@ -860,7 +907,7 @@ func invokeGo(cfg *Config, args ...string) (*bytes.Buffer, error) {
 	cmd.Stdout = stdout
 	cmd.Stderr = stderr
 	defer func(start time.Time) {
-		cfg.Logf("%s for %v, stderr: <<%s>>\n", time.Since(start), cmdDebugStr(cmd, args...), stderr)
+		cfg.Logf("%s for %v, stderr: <<%s>> stdout: <<%s>>\n", time.Since(start), cmdDebugStr(cmd, args...), stderr, stdout)
 	}(time.Now())
 
 	if err := cmd.Run(); err != nil {
@@ -897,7 +944,7 @@ func invokeGo(cfg *Config, args ...string) (*bytes.Buffer, error) {
 			//    (the Graphic characters without spaces) and may also exclude the
 			//    characters !"#$%&'()*,:;<=>?[\]^`{|} and the Unicode replacement character U+FFFD.
 			return unicode.IsOneOf([]*unicode.RangeTable{unicode.L, unicode.M, unicode.N, unicode.P, unicode.S}, r) &&
-				strings.IndexRune("!\"#$%&'()*,:;<=>?[\\]^`{|}\uFFFD", r) == -1
+				!strings.ContainsRune("!\"#$%&'()*,:;<=>?[\\]^`{|}\uFFFD", r)
 		}
 		if len(stderr.String()) > 0 && strings.HasPrefix(stderr.String(), "# ") {
 			if strings.HasPrefix(strings.TrimLeftFunc(stderr.String()[len("# "):], isPkgPathRune), "\n") {
@@ -941,10 +988,31 @@ func invokeGo(cfg *Config, args ...string) (*bytes.Buffer, error) {
 			return bytes.NewBufferString(output), nil
 		}
 
+		// Workaround for #34273. go list -e with GO111MODULE=on has incorrect behavior when listing a
+		// directory outside any module.
+		if len(stderr.String()) > 0 && strings.Contains(stderr.String(), "outside available modules") {
+			output := fmt.Sprintf(`{"ImportPath": %q,"Incomplete": true,"Error": {"Pos": "","Err": %q}}`,
+				// TODO(matloob): command-line-arguments isn't correct here.
+				"command-line-arguments", strings.Trim(stderr.String(), "\n"))
+			return bytes.NewBufferString(output), nil
+		}
+
+		// Another variation of the previous error
+		if len(stderr.String()) > 0 && strings.Contains(stderr.String(), "outside module root") {
+			output := fmt.Sprintf(`{"ImportPath": %q,"Incomplete": true,"Error": {"Pos": "","Err": %q}}`,
+				// TODO(matloob): command-line-arguments isn't correct here.
+				"command-line-arguments", strings.Trim(stderr.String(), "\n"))
+			return bytes.NewBufferString(output), nil
+		}
+
 		// Workaround for an instance of golang.org/issue/26755: go list -e  will return a non-zero exit
 		// status if there's a dependency on a package that doesn't exist. But it should return
 		// a zero exit status and set an error on that package.
 		if len(stderr.String()) > 0 && strings.Contains(stderr.String(), "no Go files in") {
+			// Don't clobber stdout if `go list` actually returned something.
+			if len(stdout.String()) > 0 {
+				return stdout, nil
+			}
 			// try to extract package name from string
 			stderrStr := stderr.String()
 			var importPath string
@@ -978,12 +1046,6 @@ func invokeGo(cfg *Config, args ...string) (*bytes.Buffer, error) {
 	if len(stderr.Bytes()) != 0 && os.Getenv("GOPACKAGESPRINTGOLISTERRORS") != "" {
 		fmt.Fprintf(os.Stderr, "%s stderr: <<%s>>\n", cmdDebugStr(cmd, args...), stderr)
 	}
-
-	// debugging
-	if false {
-		fmt.Fprintf(os.Stderr, "%s stdout: <<%s>>\n", cmdDebugStr(cmd, args...), stdout)
-	}
-
 	return stdout, nil
 }
 

vendor/golang.org/x/tools/go/packages/golist_overlay.go

@@ -6,7 +6,6 @@ import (
 	"fmt"
 	"go/parser"
 	"go/token"
-	"path"
 	"path/filepath"
 	"strconv"
 	"strings"
@@ -38,10 +37,10 @@ func processGolistOverlay(cfg *Config, response *responseDeduper, rootDirs func(
 	for opath, contents := range cfg.Overlay {
 		base := filepath.Base(opath)
 		dir := filepath.Dir(opath)
-		var pkg *Package
+		var pkg *Package           // if opath belongs to both a package and its test variant, this will be the test variant
 		var testVariantOf *Package // if opath is a test file, this is the package it is testing
 		var fileExists bool
-		isTest := strings.HasSuffix(opath, "_test.go")
+		isTestFile := strings.HasSuffix(opath, "_test.go")
 		pkgName, ok := extractPackageName(opath, contents)
 		if !ok {
 			// Don't bother adding a file that doesn't even have a parsable package statement
@@ -57,13 +56,26 @@ func processGolistOverlay(cfg *Config, response *responseDeduper, rootDirs func(
 				if !sameFile(filepath.Dir(f), dir) {
 					continue
 				}
-				if isTest && !hasTestFiles(p) {
+				// Make sure to capture information on the package's test variant, if needed.
+				if isTestFile && !hasTestFiles(p) {
 					// TODO(matloob): Are there packages other than the 'production' variant
 					// of a package that this can match? This shouldn't match the test main package
 					// because the file is generated in another directory.
 					testVariantOf = p
 					continue nextPackage
 				}
+				if pkg != nil && p != pkg && pkg.PkgPath == p.PkgPath {
+					// If we've already seen the test variant,
+					// make sure to label which package it is a test variant of.
+					if hasTestFiles(pkg) {
+						testVariantOf = p
+						continue nextPackage
+					}
+					// If we have already seen the package of which this is a test variant.
+					if hasTestFiles(p) {
+						testVariantOf = pkg
+					}
+				}
 				pkg = p
 				if filepath.Base(f) == base {
 					fileExists = true
@@ -74,32 +86,16 @@ func processGolistOverlay(cfg *Config, response *responseDeduper, rootDirs func(
 		if pkg == nil {
 			// Try to find the module or gopath dir the file is contained in.
 			// Then for modules, add the module opath to the beginning.
-			var pkgPath string
-			for rdir, rpath := range rootDirs().rootDirs {
-				// TODO(matloob): This doesn't properly handle symlinks.
-				r, err := filepath.Rel(rdir, dir)
-				if err != nil {
-					continue
-				}
-				pkgPath = filepath.ToSlash(r)
-				if rpath != "" {
-					pkgPath = path.Join(rpath, pkgPath)
-				}
-				// We only create one new package even it can belong in multiple modules or GOPATH entries.
-				// This is okay because tools (such as the LSP) that use overlays will recompute the overlay
-				// once the file is saved, and golist will do the right thing.
-				// TODO(matloob): Implement module tiebreaking?
+			pkgPath, ok := getPkgPath(cfg, dir, rootDirs)
+			if !ok {
 				break
 			}
-			if pkgPath == "" {
-				continue
-			}
 			isXTest := strings.HasSuffix(pkgName, "_test")
 			if isXTest {
 				pkgPath += "_test"
 			}
 			id := pkgPath
-			if isTest && !isXTest {
+			if isTestFile && !isXTest {
 				id = fmt.Sprintf("%s [%s.test]", pkgPath, pkgPath)
 			}
 			// Try to reclaim a package with the same id if it exists in the response.
@@ -115,7 +111,7 @@ func processGolistOverlay(cfg *Config, response *responseDeduper, rootDirs func(
 				response.addPackage(pkg)
 				havePkgs[pkg.PkgPath] = id
 				// Add the production package's sources for a test variant.
-				if isTest && !isXTest && testVariantOf != nil {
+				if isTestFile && !isXTest && testVariantOf != nil {
 					pkg.GoFiles = append(pkg.GoFiles, testVariantOf.GoFiles...)
 					pkg.CompiledGoFiles = append(pkg.CompiledGoFiles, testVariantOf.CompiledGoFiles...)
 				}
@@ -138,12 +134,16 @@ func processGolistOverlay(cfg *Config, response *responseDeduper, rootDirs func(
 			if !found {
 				overlayAddsImports = true
 				// TODO(matloob): Handle cases when the following block isn't correct.
-				// These include imports of test variants, imports of vendored packages, etc.
+				// These include imports of vendored packages, etc.
 				id, ok := havePkgs[imp]
 				if !ok {
 					id = imp
 				}
 				pkg.Imports[imp] = &Package{ID: id}
+				// Add dependencies to the non-test variant version of this package as wel.
+				if testVariantOf != nil {
+					testVariantOf.Imports[imp] = &Package{ID: id}
+				}
 			}
 		}
 		continue

vendor/golang.org/x/tools/go/packages/loadmode_string.go

@@ -0,0 +1,57 @@
+// Copyright 2019 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package packages
+
+import (
+	"fmt"
+	"strings"
+)
+
+var allModes = []LoadMode{
+	NeedName,
+	NeedFiles,
+	NeedCompiledGoFiles,
+	NeedImports,
+	NeedDeps,
+	NeedExportsFile,
+	NeedTypes,
+	NeedSyntax,
+	NeedTypesInfo,
+	NeedTypesSizes,
+}
+
+var modeStrings = []string{
+	"NeedName",
+	"NeedFiles",
+	"NeedCompiledGoFiles",
+	"NeedImports",
+	"NeedDeps",
+	"NeedExportsFile",
+	"NeedTypes",
+	"NeedSyntax",
+	"NeedTypesInfo",
+	"NeedTypesSizes",
+}
+
+func (mod LoadMode) String() string {
+	m := mod
+	if m == 0 {
+		return fmt.Sprintf("LoadMode(0)")
+	}
+	var out []string
+	for i, x := range allModes {
+		if x > m {
+			break
+		}
+		if (m & x) != 0 {
+			out = append(out, modeStrings[i])
+			m = m ^ x
+		}
+	}
+	if m != 0 {
+		out = append(out, "Unknown")
+	}
+	return fmt.Sprintf("LoadMode(%s)", strings.Join(out, "|"))
+}

vendor/golang.org/x/tools/go/packages/packages.go

@@ -48,8 +48,7 @@ const (
 	// "placeholder" Packages with only the ID set.
 	NeedImports
 
-	// NeedDeps adds the fields requested by the LoadMode in the packages in Imports. If NeedImports
-	// is not set NeedDeps has no effect.
+	// NeedDeps adds the fields requested by the LoadMode in the packages in Imports.
 	NeedDeps
 
 	// NeedExportsFile adds ExportsFile.
@@ -75,7 +74,7 @@ const (
 
 	// Deprecated: LoadImports exists for historical compatibility
 	// and should not be used. Please directly specify the needed fields using the Need values.
-	LoadImports = LoadFiles | NeedImports | NeedDeps
+	LoadImports = LoadFiles | NeedImports
 
 	// Deprecated: LoadTypes exists for historical compatibility
 	// and should not be used. Please directly specify the needed fields using the Need values.
@@ -87,7 +86,7 @@ const (
 
 	// Deprecated: LoadAllSyntax exists for historical compatibility
 	// and should not be used. Please directly specify the needed fields using the Need values.
-	LoadAllSyntax = LoadSyntax
+	LoadAllSyntax = LoadSyntax | NeedDeps
 )
 
 // A Config specifies details about how packages should be loaded.
@@ -416,11 +415,13 @@ type loader struct {
 	parseCacheMu sync.Mutex
 	exportMu     sync.Mutex // enforces mutual exclusion of exportdata operations
 
-	// TODO(matloob): Add an implied mode here and use that instead of mode.
-	// Implied mode would contain all the fields we need the data for so we can
-	// get the actually requested fields. We'll zero them out before returning
-	// packages to the user. This will make it easier for us to get the conditions
-	// where we need certain modes right.
+	// Config.Mode contains the implied mode (see impliedLoadMode).
+	// Implied mode contains all the fields we need the data for.
+	// In requestedMode there are the actually requested fields.
+	// We'll zero them out before returning packages to the user.
+	// This makes it easier for us to get the conditions where
+	// we need certain modes right.
+	requestedMode LoadMode
 }
 
 type parseValue struct {
@@ -462,7 +463,11 @@ func newLoader(cfg *Config) *loader {
 		}
 	}
 
-	if ld.Mode&NeedTypes != 0 {
+	// Save the actually requested fields. We'll zero them out before returning packages to the user.
+	ld.requestedMode = ld.Mode
+	ld.Mode = impliedLoadMode(ld.Mode)
+
+	if ld.Mode&NeedTypes != 0 || ld.Mode&NeedSyntax != 0 {
 		if ld.Fset == nil {
 			ld.Fset = token.NewFileSet()
 		}
@@ -476,6 +481,7 @@ func newLoader(cfg *Config) *loader {
 			}
 		}
 	}
+
 	return ld
 }
 
@@ -496,7 +502,7 @@ func (ld *loader) refine(roots []string, list ...*Package) ([]*Package, error) {
 		}
 		lpkg := &loaderPackage{
 			Package:   pkg,
-			needtypes: (ld.Mode&(NeedTypes|NeedTypesInfo) != 0 && rootIndex < 0) || rootIndex >= 0,
+			needtypes: (ld.Mode&(NeedTypes|NeedTypesInfo) != 0 && ld.Mode&NeedDeps != 0 && rootIndex < 0) || rootIndex >= 0,
 			needsrc: (ld.Mode&(NeedSyntax|NeedTypesInfo) != 0 && ld.Mode&NeedDeps != 0 && rootIndex < 0) || rootIndex >= 0 ||
 				len(ld.Overlay) > 0 || // Overlays can invalidate export data. TODO(matloob): make this check fine-grained based on dependencies on overlaid files
 				pkg.ExportFile == "" && pkg.PkgPath != "unsafe",
@@ -544,10 +550,8 @@ func (ld *loader) refine(roots []string, list ...*Package) ([]*Package, error) {
 		lpkg.color = grey
 		stack = append(stack, lpkg) // push
 		stubs := lpkg.Imports       // the structure form has only stubs with the ID in the Imports
-		// If NeedTypesInfo we need dependencies (at least for the roots) to typecheck the package.
 		// If NeedImports isn't set, the imports fields will all be zeroed out.
-		// If NeedDeps isn't also set we want to keep the stubs.
-		if ld.Mode&NeedTypesInfo != 0 || (ld.Mode&NeedImports != 0 && ld.Mode&NeedDeps != 0) {
+		if ld.Mode&NeedImports != 0 {
 			lpkg.Imports = make(map[string]*Package, len(stubs))
 			for importPath, ipkg := range stubs {
 				var importErr error
@@ -566,11 +570,8 @@ func (ld *loader) refine(roots []string, list ...*Package) ([]*Package, error) {
 					continue
 				}
 
-				// If !NeedDeps, just fill Imports for the root. No need to recurse further.
-				if ld.Mode&NeedDeps != 0 {
-					if visit(imp) {
-						lpkg.needsrc = true
-					}
+				if visit(imp) {
+					lpkg.needsrc = true
 				}
 				lpkg.Imports[importPath] = imp.Package
 			}
@@ -587,7 +588,7 @@ func (ld *loader) refine(roots []string, list ...*Package) ([]*Package, error) {
 		return lpkg.needsrc
 	}
 
-	if ld.Mode&(NeedImports|NeedDeps|NeedTypesInfo) == 0 {
+	if ld.Mode&NeedImports == 0 {
 		// We do this to drop the stub import packages that we are not even going to try to resolve.
 		for _, lpkg := range initial {
 			lpkg.Imports = nil
@@ -598,7 +599,7 @@ func (ld *loader) refine(roots []string, list ...*Package) ([]*Package, error) {
 			visit(lpkg)
 		}
 	}
-	if ld.Mode&NeedDeps != 0 { // TODO(matloob): This is only the case if NeedTypes is also set, right?
+	if ld.Mode&NeedImports != 0 && ld.Mode&NeedTypes != 0 {
 		for _, lpkg := range srcPkgs {
 			// Complete type information is required for the
 			// immediate dependencies of each source package.
@@ -608,9 +609,9 @@ func (ld *loader) refine(roots []string, list ...*Package) ([]*Package, error) {
 			}
 		}
 	}
-	// Load type data if needed, starting at
+	// Load type data and syntax if needed, starting at
 	// the initial packages (roots of the import DAG).
-	if ld.Mode&NeedTypes != 0 {
+	if ld.Mode&NeedTypes != 0 || ld.Mode&NeedSyntax != 0 {
 		var wg sync.WaitGroup
 		for _, lpkg := range initial {
 			wg.Add(1)
@@ -623,54 +624,44 @@ func (ld *loader) refine(roots []string, list ...*Package) ([]*Package, error) {
 	}
 
 	result := make([]*Package, len(initial))
-	importPlaceholders := make(map[string]*Package)
 	for i, lpkg := range initial {
 		result[i] = lpkg.Package
 	}
 	for i := range ld.pkgs {
 		// Clear all unrequested fields, for extra de-Hyrum-ization.
-		if ld.Mode&NeedName == 0 {
+		if ld.requestedMode&NeedName == 0 {
 			ld.pkgs[i].Name = ""
 			ld.pkgs[i].PkgPath = ""
 		}
-		if ld.Mode&NeedFiles == 0 {
+		if ld.requestedMode&NeedFiles == 0 {
 			ld.pkgs[i].GoFiles = nil
 			ld.pkgs[i].OtherFiles = nil
 		}
-		if ld.Mode&NeedCompiledGoFiles == 0 {
+		if ld.requestedMode&NeedCompiledGoFiles == 0 {
 			ld.pkgs[i].CompiledGoFiles = nil
 		}
-		if ld.Mode&NeedImports == 0 {
+		if ld.requestedMode&NeedImports == 0 {
 			ld.pkgs[i].Imports = nil
 		}
-		if ld.Mode&NeedExportsFile == 0 {
+		if ld.requestedMode&NeedExportsFile == 0 {
 			ld.pkgs[i].ExportFile = ""
 		}
-		if ld.Mode&NeedTypes == 0 {
+		if ld.requestedMode&NeedTypes == 0 {
 			ld.pkgs[i].Types = nil
 			ld.pkgs[i].Fset = nil
 			ld.pkgs[i].IllTyped = false
 		}
-		if ld.Mode&NeedSyntax == 0 {
+		if ld.requestedMode&NeedSyntax == 0 {
 			ld.pkgs[i].Syntax = nil
 		}
-		if ld.Mode&NeedTypesInfo == 0 {
+		if ld.requestedMode&NeedTypesInfo == 0 {
 			ld.pkgs[i].TypesInfo = nil
 		}
-		if ld.Mode&NeedTypesSizes == 0 {
+		if ld.requestedMode&NeedTypesSizes == 0 {
 			ld.pkgs[i].TypesSizes = nil
 		}
-		if ld.Mode&NeedDeps == 0 {
-			for j, pkg := range ld.pkgs[i].Imports {
-				ph, ok := importPlaceholders[pkg.ID]
-				if !ok {
-					ph = &Package{ID: pkg.ID}
-					importPlaceholders[pkg.ID] = ph
-				}
-				ld.pkgs[i].Imports[j] = ph
-			}
-		}
 	}
+
 	return result, nil
 }
 
@@ -691,7 +682,6 @@ func (ld *loader) loadRecursive(lpkg *loaderPackage) {
 			}(imp)
 		}
 		wg.Wait()
-
 		ld.loadPackage(lpkg)
 	})
 }
@@ -780,12 +770,23 @@ func (ld *loader) loadPackage(lpkg *loaderPackage) {
 		lpkg.Errors = append(lpkg.Errors, errs...)
 	}
 
+	if ld.Config.Mode&NeedTypes != 0 && len(lpkg.CompiledGoFiles) == 0 && lpkg.ExportFile != "" {
+		// The config requested loading sources and types, but sources are missing.
+		// Add an error to the package and fall back to loading from export data.
+		appendError(Error{"-", fmt.Sprintf("sources missing for package %s", lpkg.ID), ParseError})
+		ld.loadFromExportData(lpkg)
+		return // can't get syntax trees for this package
+	}
+
 	files, errs := ld.parseFiles(lpkg.CompiledGoFiles)
 	for _, err := range errs {
 		appendError(err)
 	}
 
 	lpkg.Syntax = files
+	if ld.Config.Mode&NeedTypes == 0 {
+		return
+	}
 
 	lpkg.TypesInfo = &types.Info{
 		Types:      make(map[ast.Expr]types.TypeAndValue),
@@ -818,7 +819,7 @@ func (ld *loader) loadPackage(lpkg *loaderPackage) {
 		if ipkg.Types != nil && ipkg.Types.Complete() {
 			return ipkg.Types, nil
 		}
-		log.Fatalf("internal error: nil Pkg importing %q from %q", path, lpkg)
+		log.Fatalf("internal error: package %q without types was imported from %q", path, lpkg)
 		panic("unreachable")
 	})
 
@@ -829,7 +830,7 @@ func (ld *loader) loadPackage(lpkg *loaderPackage) {
 		// Type-check bodies of functions only in non-initial packages.
 		// Example: for import graph A->B->C and initial packages {A,C},
 		// we can ignore function bodies in B.
-		IgnoreFuncBodies: (ld.Mode&(NeedDeps|NeedTypesInfo) == 0) && !lpkg.initial,
+		IgnoreFuncBodies: ld.Mode&NeedDeps == 0 && !lpkg.initial,
 
 		Error: appendError,
 		Sizes: ld.sizes,
@@ -1091,10 +1092,25 @@ func (ld *loader) loadFromExportData(lpkg *loaderPackage) (*types.Package, error
 	return tpkg, nil
 }
 
-func usesExportData(cfg *Config) bool {
-	return cfg.Mode&NeedExportsFile != 0 ||
-		// If NeedTypes but not NeedTypesInfo we won't typecheck using sources, so we need export data.
-		(cfg.Mode&NeedTypes != 0 && cfg.Mode&NeedTypesInfo == 0) ||
-		// If NeedTypesInfo but not NeedDeps, we're typechecking a package using its sources plus its dependencies' export data
-		(cfg.Mode&NeedTypesInfo != 0 && cfg.Mode&NeedDeps == 0)
+// impliedLoadMode returns loadMode with its dependencies.
+func impliedLoadMode(loadMode LoadMode) LoadMode {
+	if loadMode&NeedTypesInfo != 0 && loadMode&NeedImports == 0 {
+		// If NeedTypesInfo, go/packages needs to do typechecking itself so it can
+		// associate type info with the AST. To do so, we need the export data
+		// for dependencies, which means we need to ask for the direct dependencies.
+		// NeedImports is used to ask for the direct dependencies.
+		loadMode |= NeedImports
+	}
+
+	if loadMode&NeedDeps != 0 && loadMode&NeedImports == 0 {
+		// With NeedDeps we need to load at least direct dependencies.
+		// NeedImports is used to ask for the direct dependencies.
+		loadMode |= NeedImports
+	}
+
+	return loadMode
+}
+
+func usesExportData(cfg *Config) bool {
+	return cfg.Mode&NeedExportsFile != 0 || cfg.Mode&NeedTypes != 0 && cfg.Mode&NeedDeps == 0
 }