refactor/rename: re-enable TestMoves on Windows

This CL is based on the work and discussion in CL 24969 and CL 32392.
It simplifies the code further and makes trybots pass on Windows.

Also update the documentation and implementation of the subpackages
function slightly to make it more clear what it's doing.

Background

CL 24943 has made progress on fixing rename.Move behavior on Windows,
but hadn't re-enabled TestMoves at the time. CL 24969 attempted to do
so, but the tests weren't passing and it didn't get merged at the time.

Since that time, CL 65672 has fixed a bug in the subpackages function.
As a result, it's possible to simplify code in Move now.

The subpackages function returns a set of packages in the given srcDir
whose import path equals to root or has "root/" as the prefix.
After CL 65672, it correctly filters out ones that are not true
sub-packages of root. That's why it's not necessary to try to append
a trailing separator when doing the string replacement in Move now.

Given subpackages reports their import paths (always '/'-separated),
not filesystem paths, it's not necessary to use filepath.ToSlash either.

Updates golang/go#16384

Change-Id: Ia808fbaaeffe44ea1e51d7cc3ed9499bc2b438cf
Reviewed-on: https://go-review.googlesource.com/c/161777
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>

refactor/rename/mvpkg.go

@@ -9,7 +9,6 @@ package rename
 
 // TODO(matloob):
 // - think about what happens if the package is moving across version control systems.
-// - think about windows, which uses "\" as its directory separator.
 // - dot imports are not supported. Make sure it's clearly documented.
 
 import (
@@ -52,7 +51,6 @@ func Move(ctxt *build.Context, from, to, moveTmpl string) error {
 
 	// This should be the only place in the program that constructs
 	// file paths.
-	// TODO(matloob): test on Microsoft Windows.
 	fromDir := buildutil.JoinPath(ctxt, srcDir, filepath.FromSlash(from))
 	toDir := buildutil.JoinPath(ctxt, srcDir, filepath.FromSlash(to))
 	toParent := filepath.Dir(toDir)
@@ -79,12 +77,7 @@ func Move(ctxt *build.Context, from, to, moveTmpl string) error {
 		for r := range rev[pkg] {
 			affectedPackages[r] = true
 		}
-		// Ensure directories have a trailing separator.
-		dest := strings.Replace(pkg,
-			filepath.Join(from, ""),
-			filepath.Join(to, ""),
-			1)
-		destinations[pkg] = filepath.ToSlash(dest)
+		destinations[pkg] = strings.Replace(pkg, from, to, 1)
 	}
 
 	// Load all the affected packages.
@@ -135,19 +128,17 @@ func srcDir(ctxt *build.Context, pkg string) (string, error) {
 }
 
 // subpackages returns the set of packages in the given srcDir whose
-// import paths start with dir.
-func subpackages(ctxt *build.Context, srcDir string, dir string) map[string]bool {
-	subs := map[string]bool{dir: true}
-
-	// Find all packages under srcDir whose import paths start with dir.
+// import path equals to root, or has "root/" as the prefix.
+func subpackages(ctxt *build.Context, srcDir string, root string) map[string]bool {
+	var subs = make(map[string]bool)
 	buildutil.ForEachPackage(ctxt, func(pkg string, err error) {
 		if err != nil {
 			log.Fatalf("unexpected error in ForEachPackage: %v", err)
 		}
 
-		// Only process the package or a sub-package
-		if !(strings.HasPrefix(pkg, dir) &&
-			(len(pkg) == len(dir) || pkg[len(dir)] == '/')) {
+		// Only process the package root, or a sub-package of it.
+		if !(strings.HasPrefix(pkg, root) &&
+			(len(pkg) == len(root) || pkg[len(root)] == '/')) {
 			return
 		}
 
@@ -164,7 +155,6 @@ func subpackages(ctxt *build.Context, srcDir string, dir string) map[string]bool
 
 		subs[pkg] = true
 	})
-
 	return subs
 }
 

refactor/rename/mvpkg_test.go

@@ -12,7 +12,6 @@ import (
 	"path/filepath"
 	"reflect"
 	"regexp"
-	"runtime"
 	"strings"
 	"testing"
 
@@ -122,9 +121,6 @@ var _ foo.T
 }
 
 func TestMoves(t *testing.T) {
-	if runtime.GOOS == "windows" {
-		t.Skip("broken on Windows; see golang.org/issue/16384")
-	}
 	tests := []struct {
 		ctxt         *build.Context
 		from, to     string