path/filepath: fixes for windows paths

- Clean and IsAbs to handle paths with drive letter properly.
- Clean to replace / with \.

R=golang-dev, adg
CC=golang-dev, mattn.jp
http://codereview.appspot.com/4758051

3 files changed, 58 insertions, 20 deletions

diff --git a/src/pkg/path/filepath/path.go b/src/pkg/path/filepath/path.go
index a5e6a22ae..28ad676c2 100644
--- a/src/pkg/path/filepath/path.go
+++ b/src/pkg/path/filepath/path.go
@@ -38,19 +38,19 @@ const (
 // Getting Dot-Dot right,''
 // http://plan9.bell-labs.com/sys/doc/lexnames.html
 func Clean(path string) string {
+	vol := volumeName(path)
+	path = path[len(vol):]
 	if path == "" {
-		return "."
+		return vol + "."
 	}
 
-	rooted := IsAbs(path)
+	rooted := os.IsPathSeparator(path[0])
 
 	// Invariants:
 	//	reading from path; r is index of next byte to process.
 	//	writing to buf; w is index of next byte to write.
 	//	dotdot is index in buf where .. must stop, either because
 	//		it is the leading slash or it is a leading ../../.. prefix.
-	prefix := volumeName(path)
-	path = path[len(prefix):]
 	n := len(path)
 	buf := []byte(path)
 	r, w, dotdot := 0, 0, 0
@@ -110,7 +110,7 @@ func Clean(path string) string {
 		w++
 	}
 
-	return prefix + string(buf[0:w])
+	return FromSlash(vol + string(buf[0:w]))
 }
 
 // ToSlash returns the result of replacing each separator character
diff --git a/src/pkg/path/filepath/path_test.go b/src/pkg/path/filepath/path_test.go
index 58c4c0301..d2a10698e 100644
--- a/src/pkg/path/filepath/path_test.go
+++ b/src/pkg/path/filepath/path_test.go
@@ -67,9 +67,27 @@ var cleantests = []PathTest{
 	{"abc/../../././../def", "../../def"},
 }
 
+var wincleantests = []PathTest{
+	{`c:`, `c:.`},
+	{`c:\`, `c:\`},
+	{`c:\abc`, `c:\abc`},
+	{`c:abc\..\..\.\.\..\def`, `c:..\..\def`},
+	{`c:\abc\def\..\..`, `c:\`},
+	{`c:..\abc`, `c:..\abc`},
+	{`\`, `\`},
+	{`/`, `\`},
+}
+
 func TestClean(t *testing.T) {
-	for _, test := range cleantests {
-		if s := filepath.ToSlash(filepath.Clean(test.path)); s != test.result {
+	tests := cleantests
+	if runtime.GOOS == "windows" {
+		for i, _ := range tests {
+			tests[i].result = filepath.FromSlash(tests[i].result)
+		}
+		tests = append(tests, wincleantests...)
+	}
+	for _, test := range tests {
+		if s := filepath.Clean(test.path); s != test.result {
 			t.Errorf("Clean(%q) = %q, want %q", test.path, s, test.result)
 		}
 	}
@@ -399,16 +417,30 @@ var winisabstests = []IsAbsTest{
 	{`C:\`, true},
 	{`c\`, false},
 	{`c::`, false},
-	{`/`, true},
-	{`\`, true},
-	{`\Windows`, true},
+	{`c:`, false},
+	{`/`, false},
+	{`\`, false},
+	{`\Windows`, false},
+	{`c:a\b`, false},
 }
 
 func TestIsAbs(t *testing.T) {
+	var tests []IsAbsTest
 	if runtime.GOOS == "windows" {
-		isabstests = append(isabstests, winisabstests...)
+		tests = append(tests, winisabstests...)
+		// All non-windows tests should fail, because they have no volume letter.
+		for _, test := range isabstests {
+			tests = append(tests, IsAbsTest{test.path, false})
+		}
+		// All non-windows test should work as intended if prefixed with volume letter.
+		for _, test := range isabstests {
+			tests = append(tests, IsAbsTest{"c:" + test.path, test.isAbs})
+		}
+	} else {
+		tests = isabstests
 	}
-	for _, test := range isabstests {
+
+	for _, test := range tests {
 		if r := filepath.IsAbs(test.path); r != test.isAbs {
 			t.Errorf("IsAbs(%q) = %v, want %v", test.path, r, test.isAbs)
 		}
diff --git a/src/pkg/path/filepath/path_windows.go b/src/pkg/path/filepath/path_windows.go
index 35302eb1a..b7d18ee5a 100644
--- a/src/pkg/path/filepath/path_windows.go
+++ b/src/pkg/path/filepath/path_windows.go
@@ -4,25 +4,31 @@
 
 package filepath
 
-import "os"
-
 // IsAbs returns true if the path is absolute.
-func IsAbs(path string) bool {
-	return path != "" && (volumeName(path) != "" || os.IsPathSeparator(path[0]))
+func IsAbs(path string) (b bool) {
+	v := volumeName(path)
+	if v == "" {
+		return false
+	}
+	path = path[len(v):]
+	if path == "" {
+		return false
+	}
+	return path[0] == '/' || path[0] == '\\'
 }
 
 // volumeName return leading volume name.  
 // If given "C:\foo\bar", return "C:" on windows.
-func volumeName(path string) string {
-	if path == "" {
+func volumeName(path string) (v string) {
+	if len(path) < 2 {
 		return ""
 	}
 	// with drive letter
 	c := path[0]
-	if len(path) > 2 && path[1] == ':' && os.IsPathSeparator(path[2]) &&
+	if path[1] == ':' &&
 		('0' <= c && c <= '9' || 'a' <= c && c <= 'z' ||
 			'A' <= c && c <= 'Z') {
-		return path[0:2]
+		return path[:2]
 	}
 	return ""
 }