imports: don't ignore GOPATH if GOROOT is a prefix of GOPATH

Fixes golang/go#16458

Change-Id: I1aaec5d115dd703dd702101f6bec37bf8b02a73d
Reviewed-on: https://go-review.googlesource.com/25192
Reviewed-by: Andrew Gerrand <adg@golang.org>

imports/fix.go

@@ -661,7 +661,7 @@ func findImportGoPath(pkgName string, symbols map[string]bool, filename string)
 	// TODO(bradfitz): run each $GOPATH entry async. But nobody
 	// really has more than one anyway, so low priority.
 	scanGoRootOnce.Do(scanGoRoot) // async
-	if !strings.HasPrefix(filename, build.Default.GOROOT) {
+	if !fileInDir(filename, build.Default.GOROOT) {
 		scanGoPathOnce.Do(scanGoPath) // blocking
 	}
 	<-scanGoRootDone
@@ -898,3 +898,15 @@ func findImportStdlib(shortPkg string, symbols map[string]bool) (importPath stri
 	}
 	return importPath, false, importPath != ""
 }
+
+// fileInDir reports whether the provided file path looks like
+// it's in dir. (without hitting the filesystem)
+func fileInDir(file, dir string) bool {
+	rest := strings.TrimPrefix(file, dir)
+	if len(rest) == len(file) {
+		// dir is not a prefix of file.
+		return false
+	}
+	// Check for boundary: either nothing (file == dir), or a slash.
+	return len(rest) == 0 || rest[0] == '/' || rest[0] == '\\'
+}

imports/fix_test.go

@@ -1153,6 +1153,11 @@ func TestFindImportStdlib(t *testing.T) {
 }
 
 type testConfig struct {
+	// goroot and gopath optionally specifies the path on disk
+	// to use for the GOROOT and GOPATH. If empty, a temp directory
+	// is made if needed.
+	goroot, gopath string
+
 	// gorootFiles optionally specifies the complete contents of GOROOT to use,
 	// If nil, the normal current $GOROOT is used.
 	gorootFiles map[string]string // paths relative to $GOROOT/src to contents
@@ -1190,23 +1195,24 @@ func mapToDir(destDir string, files map[string]string) error {
 }
 
 func (c testConfig) test(t *testing.T, fn func(*goimportTest)) {
-	var goroot string
+	goroot := c.goroot
-	var gopath string
+	gopath := c.gopath
 
-	if c.gorootFiles != nil {
+	if c.gorootFiles != nil && goroot == "" {
 		goroot = mustTempDir(t, "goroot-")
 		defer os.RemoveAll(goroot)
+	}
 	if err := mapToDir(goroot, c.gorootFiles); err != nil {
 		t.Fatal(err)
 	}
-	}
+
-	if c.gopathFiles != nil {
+	if c.gopathFiles != nil && gopath == "" {
 		gopath = mustTempDir(t, "gopath-")
 		defer os.RemoveAll(gopath)
+	}
 	if err := mapToDir(gopath, c.gopathFiles); err != nil {
 		t.Fatal(err)
 	}
-	}
 
 	withEmptyGoPath(func() {
 		if goroot != "" {
@@ -1398,6 +1404,30 @@ func TestSkipNodeModules(t *testing.T) {
 	})
 }
 
+// golang.org/issue/16458 -- if GOROOT is a prefix of GOPATH, GOPATH is ignored.
+func TestGoRootPrefixOfGoPath(t *testing.T) {
+	dir := mustTempDir(t, "importstest")
+	defer os.RemoveAll(dir)
+	testConfig{
+		goroot: filepath.Join(dir, "go"),
+		gopath: filepath.Join(dir, "gopath"),
+		gopathFiles: map[string]string{
+			"example.com/foo/pkg.go": "package foo\nconst X = 1",
+		},
+	}.test(t, func(t *goimportTest) {
+		const in = "package x\n\nconst _ = foo.X\n"
+		const want = "package x\n\nimport \"example.com/foo\"\n\nconst _ = foo.X\n"
+		buf, err := Process(t.gopath+"/src/x/x.go", []byte(in), nil)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if string(buf) != want {
+			t.Errorf("wrong output.\ngot:\n%q\nwant:\n%q\n", buf, want)
+		}
+	})
+
+}
+
 func strSet(ss []string) map[string]bool {
 	m := make(map[string]bool)
 	for _, s := range ss {