refactor/rename: fix spurious conflict report when renaming x/foo to y/foo

There is no need to check for import conflicts when the package name
doesn't change.  Add test.

Also, when reporting a (non-spurious) import conflict, make clear that
the error is just a warning.  Add a test for same.

Change-Id: Idde0483b502cd041fd893230fec06b8533f54f3c
Reviewed-on: https://go-review.googlesource.com/24917
Reviewed-by: Michael Matloob <matloob@golang.org>

refactor/rename/mvpkg_test.go

@@ -7,8 +7,10 @@ package rename
 import (
 	"fmt"
 	"go/build"
+	"go/token"
 	"io/ioutil"
 	"path/filepath"
+	"reflect"
 	"regexp"
 	"strings"
 	"testing"
@@ -115,6 +117,7 @@ func TestMoves(t *testing.T) {
 		ctxt         *build.Context
 		from, to     string
 		want         map[string]string
+		wantWarnings []string
 	}{
 		// Simple example.
 		{
@@ -267,6 +270,77 @@ var _ bar.T
 /* import " this is not an import comment */
 `},
 		},
+		// Import name conflict generates a warning, not an error.
+		{
+			ctxt: fakeContext(map[string][]string{
+				"x": {},
+				"a": {`package a; type A int`},
+				"b": {`package b; type B int`},
+				"conflict": {`package conflict
+
+import "a"
+import "b"
+var _ a.A
+var _ b.B
+`},
+				"ok": {`package ok
+import "b"
+var _ b.B
+`},
+			}),
+			from: "b", to: "x/a",
+			want: map[string]string{
+				"/go/src/a/0.go": `package a; type A int`,
+				"/go/src/ok/0.go": `package ok
+
+import "x/a"
+
+var _ a.B
+`,
+				"/go/src/conflict/0.go": `package conflict
+
+import "a"
+import "x/a"
+
+var _ a.A
+var _ b.B
+`,
+				"/go/src/x/a/0.go": `package a
+
+type B int
+`,
+			},
+			wantWarnings: []string{
+				`/go/src/conflict/0.go:4:8: renaming this imported package name "b" to "a"`,
+				`/go/src/conflict/0.go:3:8: 	conflicts with imported package name in same block`,
+				`/go/src/conflict/0.go:3:8: skipping update of this file`,
+			},
+		},
+		// Rename with same base name.
+		{
+			ctxt: fakeContext(map[string][]string{
+				"x": {},
+				"y": {},
+				"x/foo": {`package foo
+
+type T int
+`},
+				"main": {`package main; import "x/foo"; var _ foo.T`},
+			}),
+			from: "x/foo", to: "y/foo",
+			want: map[string]string{
+				"/go/src/y/foo/0.go": `package foo
+
+type T int
+`,
+				"/go/src/main/0.go": `package main
+
+import "y/foo"
+
+var _ foo.T
+`,
+			},
+		},
 	}
 
 	for _, test := range tests {
@@ -296,6 +370,10 @@ var _ bar.T
 			}
 			got[path] = string(bytes)
 		})
+		var warnings []string
+		reportError = func(posn token.Position, message string) {
+			warnings = append(warnings, posn.String()+": "+message)
+		}
 		writeFile = func(filename string, content []byte) error {
 			got[filename] = string(content)
 			return nil
@@ -318,6 +396,13 @@ var _ bar.T
 			continue
 		}
 
+		if !reflect.DeepEqual(warnings, test.wantWarnings) {
+			t.Errorf("%s: unexpected warnings:\n%s\nwant:\n%s",
+				prefix,
+				strings.Join(warnings, "\n"),
+				strings.Join(test.wantWarnings, "\n"))
+		}
+
 		for file, wantContent := range test.want {
 			k := filepath.FromSlash(file)
 			gotContent, ok := got[k]

refactor/rename/rename.go

@@ -174,11 +174,13 @@ var reportError = func(posn token.Position, message string) {
 	fmt.Fprintf(os.Stderr, "%s: %s\n", posn, message)
 }
 
-// importName renames imports of the package with the given path in
-// the given package.  If fromName is not empty, only imports as
-// fromName will be renamed.  If the renaming would lead to a conflict,
-// the file is left unchanged.
+// importName renames imports of fromPath within the package specified by info.
+// If fromName is not empty, importName renames only imports as fromName.
+// If the renaming would lead to a conflict, the file is left unchanged.
 func importName(iprog *loader.Program, info *loader.PackageInfo, fromPath, fromName, to string) error {
+	if fromName == to {
+		return nil // no-op (e.g. rename x/foo to y/foo)
+	}
 	for _, f := range info.Files {
 		var from types.Object
 		for _, imp := range f.Imports {
@@ -203,6 +205,8 @@ func importName(iprog *loader.Program, info *loader.PackageInfo, fromPath, fromN
 		}
 		r.check(from)
 		if r.hadConflicts {
+			reportError(iprog.Fset.Position(f.Imports[0].Pos()),
+				"skipping update of this file")
 			continue // ignore errors; leave the existing name
 		}
 		if err := r.update(); err != nil {