go/packages: handle case when go list returns absolute ImportPath

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.
This change modifies the go list driver 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.

Fixes golang/go#33157

Change-Id: I5ac032879884f52edbe45e00ed3949d84a71715e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/188764
Run-TryBot: Michael Matloob <[email protected]>
Reviewed-by: Rebecca Stambler <[email protected]>

Author: matloob

go/packages/golist.go

@@ -13,6 +13,7 @@ import (
 	"log"
 	"os"
 	"os/exec"
+	"path"
 	"path/filepath"
 	"reflect"
 	"regexp"
@@ -86,6 +87,23 @@ func goListDriver(cfg *Config, patterns ...string) (*driverResponse, error) {
 		}()
 	}
 
+	// start fetching rootDirs
+	var rootDirs map[string]string
+	var rootDirsReady = make(chan struct{})
+	go func() {
+		rootDirs = determineRootDirs(cfg)
+		close(rootDirsReady)
+	}()
+	getRootDirs := func() map[string]string {
+		<-rootDirsReady
+		return rootDirs
+	}
+
+	// always pass getRootDirs to golistDriver
+	golistDriver := func(cfg *Config, patterns ...string) (*driverResponse, error) {
+		return golistDriver(cfg, getRootDirs, patterns...)
+	}
+
 	// Determine files requested in contains patterns
 	var containFiles []string
 	var packagesNamed []string
@@ -147,7 +165,7 @@ extractQueries:
 	var containsCandidates []string
 
 	if len(containFiles) != 0 {
-		if err := runContainsQueries(cfg, golistDriver, response, containFiles); err != nil {
+		if err := runContainsQueries(cfg, golistDriver, response, containFiles, getRootDirs); err != nil {
 			return nil, err
 		}
 	}
@@ -158,15 +176,15 @@ extractQueries:
 		}
 	}
 
-	modifiedPkgs, needPkgs, err := processGolistOverlay(cfg, response)
+	modifiedPkgs, needPkgs, err := processGolistOverlay(cfg, response, getRootDirs)
 	if err != nil {
 		return nil, err
 	}
 	if len(containFiles) > 0 {
 		containsCandidates = append(containsCandidates, modifiedPkgs...)
 		containsCandidates = append(containsCandidates, needPkgs...)
 	}
-	if err := addNeededOverlayPackages(cfg, golistDriver, response, needPkgs); err != nil {
+	if err := addNeededOverlayPackages(cfg, golistDriver, response, needPkgs, getRootDirs); err != nil {
 		return nil, err
 	}
 	// Check candidate packages for containFiles.
@@ -198,7 +216,7 @@ extractQueries:
 	return response.dr, nil
 }
 
-func addNeededOverlayPackages(cfg *Config, driver driver, response *responseDeduper, pkgs []string) error {
+func addNeededOverlayPackages(cfg *Config, driver driver, response *responseDeduper, pkgs []string, getRootDirs func() map[string]string) error {
 	if len(pkgs) == 0 {
 		return nil
 	}
@@ -209,17 +227,17 @@ func addNeededOverlayPackages(cfg *Config, driver driver, response *responseDedu
 	for _, pkg := range dr.Packages {
 		response.addPackage(pkg)
 	}
-	_, needPkgs, err := processGolistOverlay(cfg, response)
+	_, needPkgs, err := processGolistOverlay(cfg, response, getRootDirs)
 	if err != nil {
 		return err
 	}
-	if err := addNeededOverlayPackages(cfg, driver, response, needPkgs); err != nil {
+	if err := addNeededOverlayPackages(cfg, driver, response, needPkgs, getRootDirs); err != nil {
 		return err
 	}
 	return nil
 }
 
-func runContainsQueries(cfg *Config, driver driver, response *responseDeduper, queries []string) error {
+func runContainsQueries(cfg *Config, driver driver, response *responseDeduper, queries []string, rootDirs func() map[string]string) error {
 	for _, query := range queries {
 		// TODO(matloob): Do only one query per directory.
 		fdir := filepath.Dir(query)
@@ -567,7 +585,7 @@ func otherFiles(p *jsonPackage) [][]string {
 // golistDriver uses the "go list" command to expand the pattern
 // words and return metadata for the specified packages. dir may be
 // "" and env may be nil, as per os/exec.Command.
-func golistDriver(cfg *Config, words ...string) (*driverResponse, error) {
+func golistDriver(cfg *Config, rootsDirs func() map[string]string, words ...string) (*driverResponse, error) {
 	// go list uses the following identifiers in ImportPath and Imports:
 	//
 	// 	"p"			-- importable package or main (command)
@@ -608,6 +626,20 @@ func golistDriver(cfg *Config, words ...string) (*driverResponse, error) {
 			return nil, fmt.Errorf("package missing import path: %+v", p)
 		}
 
+		// Work around https://golang.org/issue/33157:
+		// 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
+		// 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)
+			if ok {
+				p.ImportPath = pkgPath
+			}
+		}
+
 		if old, found := seen[p.ImportPath]; found {
 			if !reflect.DeepEqual(p, old) {
 				return nil, fmt.Errorf("internal error: go list gives conflicting information for package %v", p.ImportPath)
@@ -711,6 +743,27 @@ func golistDriver(cfg *Config, words ...string) (*driverResponse, error) {
 	return &response, nil
 }
 
+// getPkgPath finds the package path of a directory if it's relative to a root directory.
+func getPkgPath(dir string, rootDirs func() map[string]string) (string, bool) {
+	for rdir, rpath := range rootDirs() {
+		// 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
+			// 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
+			// result straight from golist.
+			// TODO(matloob): Implement module tiebreaking?
+			return path.Join(rpath, filepath.ToSlash(r)), true
+		}
+	}
+	return "", false
+}
+
 // absJoin absolutizes and flattens the lists of files.
 func absJoin(dir string, fileses ...[]string) (res []string) {
 	for _, files := range fileses {

go/packages/golist_overlay.go

@@ -10,15 +10,14 @@ import (
 	"path/filepath"
 	"strconv"
 	"strings"
-	"sync"
 )
 
 // processGolistOverlay provides rudimentary support for adding
 // files that don't exist on disk to an overlay. The results can be
 // sometimes incorrect.
 // TODO(matloob): Handle unsupported cases, including the following:
 // - determining the correct package to add given a new import path
-func processGolistOverlay(cfg *Config, response *responseDeduper) (modifiedPkgs, needPkgs []string, err error) {
+func processGolistOverlay(cfg *Config, response *responseDeduper, rootDirs func() map[string]string) (modifiedPkgs, needPkgs []string, err error) {
 	havePkgs := make(map[string]string) // importPath -> non-test package ID
 	needPkgsSet := make(map[string]bool)
 	modifiedPkgsSet := make(map[string]bool)
@@ -29,9 +28,6 @@ func processGolistOverlay(cfg *Config, response *responseDeduper) (modifiedPkgs,
 		havePkgs[pkg.PkgPath] = pkg.ID
 	}
 
-	var rootDirs map[string]string
-	var onceGetRootDirs sync.Once
-
 	// If no new imports are added, it is safe to avoid loading any needPkgs.
 	// Otherwise, it's hard to tell which package is actually being loaded
 	// (due to vendoring) and whether any modified package will show up
@@ -76,13 +72,10 @@ func processGolistOverlay(cfg *Config, response *responseDeduper) (modifiedPkgs,
 		}
 		// The overlay could have included an entirely new package.
 		if pkg == nil {
-			onceGetRootDirs.Do(func() {
-				rootDirs = determineRootDirs(cfg)
-			})
 			// 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 {
+			for rdir, rpath := range rootDirs() {
 				// TODO(matloob): This doesn't properly handle symlinks.
 				r, err := filepath.Rel(rdir, dir)
 				if err != nil {

go/packages/packages_test.go

@@ -930,12 +930,6 @@ func testNewPackagesInOverlay(t *testing.T, exporter packagestest.Exporter) {
 			"b/b.go": `package b; import "golang.org/fake/c"; const B = "b" + c.C`,
 			"c/c.go": `package c; const C = "c"`,
 			"d/d.go": `package d; const D = "d"`,
-
-			// TODO: Remove these temporary files when golang.org/issue/33157 is resolved.
-			filepath.Join("e/e_temp.go"): ``,
-			filepath.Join("f/f_temp.go"): ``,
-			filepath.Join("g/g_temp.go"): ``,
-			filepath.Join("h/h_temp.go"): ``,
 		}}})
 	defer exported.Cleanup()