imports: prioritize closer packages

Prefer imports that are closer to the current package.

Fixes golang/go#17557

Change-Id: Iec55a294d396feac6234be307e08608b8559f65c
Reviewed-on: https://go-review.googlesource.com/37070
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

imports/fix.go

@@ -87,14 +87,6 @@ var dirPackageInfo = dirPackageInfoFile
 func dirPackageInfoFile(pkgName, srcDir, filename string) (*packageInfo, error) {
 	considerTests := strings.HasSuffix(filename, "_test.go")
 
-	// Handle file from stdin
-	if _, err := os.Stat(filename); err != nil {
-		if os.IsNotExist(err) {
-			return &packageInfo{}, nil
-		}
-		return nil, err
-	}
-
 	fileBase := filepath.Base(filename)
 	packageFileInfos, err := ioutil.ReadDir(srcDir)
 	if err != nil {
@@ -401,19 +393,44 @@ type pkg struct {
 	dir             string // absolute file path to pkg directory ("/usr/lib/go/src/net/http")
 	importPath      string // full pkg import path ("net/http", "foo/bar/vendor/a/b")
 	importPathShort string // vendorless import path ("net/http", "a/b")
+	distance        int    // relative distance to target
 }
 
-// byImportPathShortLength sorts by the short import path length, breaking ties on the
-// import string itself.
-type byImportPathShortLength []*pkg
+// byDistanceOrImportPathShortLength sorts by relative distance breaking ties
+// on the short import path length and then the import string itself.
+type byDistanceOrImportPathShortLength []*pkg
+
+func (s byDistanceOrImportPathShortLength) Len() int { return len(s) }
+func (s byDistanceOrImportPathShortLength) Less(i, j int) bool {
+	di, dj := s[i].distance, s[j].distance
+	if di == -1 {
+		return false
+	}
+	if dj == -1 {
+		return true
+	}
+	if di != dj {
+		return di < dj
+	}
 
-func (s byImportPathShortLength) Len() int { return len(s) }
-func (s byImportPathShortLength) Less(i, j int) bool {
 	vi, vj := s[i].importPathShort, s[j].importPathShort
-	return len(vi) < len(vj) || (len(vi) == len(vj) && vi < vj)
-
+	if len(vi) != len(vj) {
+		return len(vi) < len(vj)
+	}
+	return vi < vj
+}
+func (s byDistanceOrImportPathShortLength) Swap(i, j int) { s[i], s[j] = s[j], s[i] }
+
+func distance(basepath, targetpath string) int {
+	p, err := filepath.Rel(basepath, targetpath)
+	if err != nil {
+		return -1
+	}
+	if p == "." {
+		return 0
+	}
+	return strings.Count(p, string(filepath.Separator)) + 1
 }
-func (s byImportPathShortLength) Swap(i, j int) { s[i], s[j] = s[j], s[i] }
 
 // guarded by populateIgnoreOnce; populates ignoredDirs.
 func populateIgnore() {
@@ -724,6 +741,12 @@ func findImportGoPath(pkgName string, symbols map[string]bool, filename string)
 		defer testMu.RUnlock()
 	}
 
+	pkgDir, err := filepath.Abs(filename)
+	if err != nil {
+		return "", false, err
+	}
+	pkgDir = filepath.Dir(pkgDir)
+
 	// Fast path for the standard library.
 	// In the common case we hopefully never have to scan the GOPATH, which can
 	// be slow with moving disks.
@@ -765,6 +788,7 @@ func findImportGoPath(pkgName string, symbols map[string]bool, filename string)
 	var candidates []*pkg
 	for _, pkg := range dirScan {
 		if pkgIsCandidate(filename, pkgName, pkg) {
+			pkg.distance = distance(pkgDir, pkg.dir)
 			candidates = append(candidates, pkg)
 		}
 	}
@@ -773,7 +797,7 @@ func findImportGoPath(pkgName string, symbols map[string]bool, filename string)
 	// assuming that shorter package names are better than long
 	// ones.  Note that this sorts by the de-vendored name, so
 	// there's no "penalty" for vendoring.
-	sort.Sort(byImportPathShortLength(candidates))
+	sort.Sort(byDistanceOrImportPathShortLength(candidates))
 	if Debug {
 		for i, pkg := range candidates {
 			log.Printf("%s candidate %d/%d: %v in %v", pkgName, i+1, len(candidates), pkg.importPathShort, pkg.dir)

imports/fix_test.go

@@ -1878,3 +1878,82 @@ func TestProcessStdin(t *testing.T) {
 		t.Errorf("expected fmt import; got: %s", got)
 	}
 }
+
+// Tests LocalPackagePromotion when there is a local package that matches, it
+// should be the closest match.
+// https://golang.org/issues/17557
+func TestLocalPackagePromotion(t *testing.T) {
+	testConfig{
+		gopathFiles: map[string]string{
+			"config.net/config/config.go":         "package config\n type SystemConfig struct {}", // Will match but should not be first choice
+			"mycompany.net/config/config.go":      "package config\n type SystemConfig struct {}", // Will match but should not be first choice
+			"mycompany.net/tool/config/config.go": "package config\n type SystemConfig struct {}", // Local package should be promoted over shorter package
+		},
+	}.test(t, func(t *goimportTest) {
+		const in = "package main\n var c = &config.SystemConfig{}"
+		const want = `package main
+
+import "mycompany.net/tool/config"
+
+var c = &config.SystemConfig{}
+`
+		got, err := Process(filepath.Join(t.gopath, "src", "mycompany.net/tool/main.go"), []byte(in), nil)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if string(got) != want {
+			t.Errorf("Process = %q; want %q", got, want)
+		}
+	})
+}
+
+// Tests FindImportInLocalGoFiles looks at the import lines for other Go files in the
+// local directory, since the user is likely to import the same packages in the current
+// Go file.  If an import is found that satisfies the need, it should be used over the
+// standard library.
+// https://golang.org/issues/17557
+func TestFindImportInLocalGoFiles(t *testing.T) {
+	testConfig{
+		gopathFiles: map[string]string{
+			"bytes.net/bytes/bytes.go":  "package bytes\n type Buffer struct {}",                               // Should be selected over standard library
+			"mycompany.net/tool/io.go":  "package main\n import \"bytes.net/bytes\"\n var _ = &bytes.Buffer{}", // Contains package import that will cause stdlib to be ignored
+			"mycompany.net/tool/err.go": "package main\n import \"bogus.net/bytes\"\n var _ = &bytes.Buffer{}", // Contains import which is not resolved, so it is ignored
+		},
+	}.test(t, func(t *goimportTest) {
+		const in = "package main\n var _ = &bytes.Buffer{}"
+		const want = `package main
+
+import "bytes.net/bytes"
+
+var _ = &bytes.Buffer{}
+`
+		got, err := Process(filepath.Join(t.gopath, "src", "mycompany.net/tool/main.go"), []byte(in), nil)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if string(got) != want {
+			t.Errorf("Process = got %q; want %q", got, want)
+		}
+	})
+}
+
+func TestImportNoGoFiles(t *testing.T) {
+	testConfig{
+		gopathFiles: map[string]string{},
+	}.test(t, func(t *goimportTest) {
+		const in = "package main\n var _ = &bytes.Buffer{}"
+		const want = `package main
+
+import "bytes"
+
+var _ = &bytes.Buffer{}
+`
+		got, err := Process(filepath.Join(t.gopath, "src", "mycompany.net/tool/main.go"), []byte(in), nil)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if string(got) != want {
+			t.Errorf("Process = got %q; want %q", got, want)
+		}
+	})
+}