Merge pull request #539 from wking/windows-clean

filepath/clean: Add Windows support

filepath/abs.go

@@ -1,7 +1,6 @@
 package filepath
 
 import (
-	"errors"
 	"regexp"
 	"strings"
 )
@@ -11,9 +10,6 @@ var windowsAbs = regexp.MustCompile(`^[a-zA-Z]:\\.*$`)
 // Abs is a version of path/filepath's Abs with an explicit operating
 // system and current working directory.
 func Abs(os, path, cwd string) (_ string, err error) {
-	if os == "windows" {
-		return "", errors.New("Abs() does not support windows yet")
-	}
 	if IsAbs(os, path) {
 		return Clean(os, path), nil
 	}

filepath/abs_test.go

@@ -2,6 +2,8 @@ package filepath
 
 import (
 	"fmt"
+	"path/filepath"
+	"runtime"
 	"testing"
 )
 
@@ -66,6 +68,60 @@ func TestAbs(t *testing.T) {
 			cwd:      "/cwd",
 			expected: "/b",
 		},
+		{
+			os:       "windows",
+			path:     "c:\\",
+			cwd:      "/cwd",
+			expected: "c:\\",
+		},
+		{
+			os:       "windows",
+			path:     "c:\\a",
+			cwd:      "c:\\cwd",
+			expected: "c:\\a",
+		},
+		{
+			os:       "windows",
+			path:     "c:\\a\\",
+			cwd:      "c:\\cwd",
+			expected: "c:\\a",
+		},
+		{
+			os:       "windows",
+			path:     "c:\\\\a",
+			cwd:      "c:\\cwd",
+			expected: "c:\\a",
+		},
+		{
+			os:       "windows",
+			path:     ".",
+			cwd:      "c:\\cwd",
+			expected: "c:\\cwd",
+		},
+		{
+			os:       "windows",
+			path:     ".\\c",
+			cwd:      "c:\\a\\b",
+			expected: "c:\\a\\b\\c",
+		},
+		{
+			os:       "windows",
+			path:     ".\\\\c",
+			cwd:      "c:\\a\\b",
+			expected: "c:\\a\\b\\c",
+		},
+		{
+			os:       "windows",
+			path:     "..\\a",
+			cwd:      "c:\\cwd",
+			expected: "c:\\a",
+		},
+		{
+			os:       "windows",
+			path:     "..\\..\\b",
+			cwd:      "c:\\cwd",
+			expected: "c:\\b",
+		},
 	} {
 		t.Run(
 			fmt.Sprintf("Abs(%q,%q,%q)", test.os, test.path, test.cwd),
@@ -74,7 +130,7 @@ func TestAbs(t *testing.T) {
 				if err != nil {
 					t.Error(err)
 				} else if abs != test.expected {
-					t.Errorf("unexpected result: %q (expected %q)\n", abs, test.expected)
+					t.Errorf("unexpected result: %q (expected %q)", abs, test.expected)
 				}
 			},
 		)
@@ -163,7 +219,13 @@ func TestIsAbs(t *testing.T) {
 			func(t *testing.T) {
 				abs := IsAbs(test.os, test.path)
 				if abs != test.expected {
-					t.Errorf("unexpected result: %t (expected %t)\n", abs, test.expected)
+					t.Errorf("unexpected result: %t (expected %t)", abs, test.expected)
+				}
+				if runtime.GOOS == test.os {
+					stdAbs := filepath.IsAbs(test.path)
+					if abs != stdAbs {
+						t.Errorf("non-standard result: %t (%t is standard)", abs, stdAbs)
+					}
 				}
 			},
 		)

filepath/ancestor_test.go

@@ -97,6 +97,104 @@ func TestIsAncestor(t *testing.T) {
 			cwd:      "/cwd",
 			expected: true,
 		},
+		{
+			os:       "windows",
+			pathA:    "c:\\",
+			pathB:    "c:\\a",
+			cwd:      "c:\\cwd",
+			expected: true,
+		},
+		{
+			os:       "windows",
+			pathA:    "c:\\",
+			pathB:    "d:\\a",
+			cwd:      "c:\\cwd",
+			expected: false,
+		},
+		{
+			os:       "windows",
+			pathA:    "c:\\",
+			pathB:    ".",
+			cwd:      "d:\\cwd",
+			expected: false,
+		},
+		{
+			os:       "windows",
+			pathA:    "c:\\a",
+			pathB:    "c:\\a",
+			cwd:      "c:\\cwd",
+			expected: false,
+		},
+		{
+			os:       "windows",
+			pathA:    "c:\\a",
+			pathB:    "c:\\",
+			cwd:      "c:\\cwd",
+			expected: false,
+		},
+		{
+			os:       "windows",
+			pathA:    "c:\\a",
+			pathB:    "c:\\ab",
+			cwd:      "c:\\cwd",
+			expected: false,
+		},
+		{
+			os:       "windows",
+			pathA:    "c:\\a\\",
+			pathB:    "c:\\a",
+			cwd:      "c:\\cwd",
+			expected: false,
+		},
+		{
+			os:       "windows",
+			pathA:    "c:\\\\a",
+			pathB:    "c:\\a",
+			cwd:      "c:\\cwd",
+			expected: false,
+		},
+		{
+			os:       "windows",
+			pathA:    "c:\\\\a",
+			pathB:    "c:\\a\\b",
+			cwd:      "c:\\cwd",
+			expected: true,
+		},
+		{
+			os:       "windows",
+			pathA:    "c:\\a",
+			pathB:    "c:\\a\\",
+			cwd:      "c:\\cwd",
+			expected: false,
+		},
+		{
+			os:       "windows",
+			pathA:    "c:\\a",
+			pathB:    ".",
+			cwd:      "c:\\cwd",
+			expected: false,
+		},
+		{
+			os:       "windows",
+			pathA:    "c:\\a",
+			pathB:    "b",
+			cwd:      "c:\\a",
+			expected: true,
+		},
+		{
+			os:       "windows",
+			pathA:    "c:\\a",
+			pathB:    "..\\a",
+			cwd:      "c:\\cwd",
+			expected: false,
+		},
+		{
+			os:       "windows",
+			pathA:    "c:\\a",
+			pathB:    "..\\a\\b",
+			cwd:      "c:\\cwd",
+			expected: true,
+		},
 	} {
 		t.Run(
 			fmt.Sprintf("IsAncestor(%q,%q,%q,%q)", test.os, test.pathA, test.pathB, test.cwd),

filepath/clean.go

@@ -30,6 +30,9 @@ func Clean(os, path string) string {
 	// Eliminate each inner .. path name element (the parent directory)
 	// along with the non-.. element that precedes it.
 	for i := 1; i < len(elements); i++ {
+		if i == 1 && abs && sep == '\\' {
+			continue
+		}
 		if i > 0 && elements[i] == ".." {
 			elements = append(elements[:i-1], elements[i+1:]...)
 			i -= 2
@@ -39,16 +42,31 @@ func Clean(os, path string) string {
 	// Eliminate .. elements that begin a rooted path:
 	// that is, replace "/.." by "/" at the beginning of a path,
 	// assuming Separator is '/'.
-	if abs && len(elements) > 0 {
-		for elements[0] == ".." {
-			elements = elements[1:]
+	offset := 0
+	if sep == '\\' {
+		offset = 1
+	}
+	if abs {
+		for len(elements) > offset && elements[offset] == ".." {
+			elements = append(elements[:offset], elements[offset+1:]...)
 		}
 	}
 
 	cleaned := strings.Join(elements, string(sep))
 	if abs {
-		cleaned = fmt.Sprintf("%c%s", sep, cleaned)
+		if sep == '/' {
+			cleaned = fmt.Sprintf("%c%s", sep, cleaned)
+		} else if len(elements) == 1 {
+			cleaned = fmt.Sprintf("%s%c", cleaned, sep)
+		}
 	}
+
+	// If the result of this process is an empty string, Clean returns
+	// the string ".".
+	if len(cleaned) == 0 {
+		cleaned = "."
+	}
+
 	if cleaned == path {
 		return path
 	}

filepath/clean_test.go

@@ -2,6 +2,8 @@ package filepath
 
 import (
 	"fmt"
+	"path/filepath"
+	"runtime"
 	"testing"
 )
 
@@ -36,6 +38,16 @@ func TestClean(t *testing.T) {
 			path:     "//a",
 			expected: "/a",
 		},
+		{
+			os:       "linux",
+			path:     "/..",
+			expected: "/",
+		},
+		{
+			os:       "linux",
+			path:     "/../a",
+			expected: "/a",
+		},
 		{
 			os:       "linux",
 			path:     ".",
@@ -56,13 +68,84 @@ func TestClean(t *testing.T) {
 			path:     "a/../b",
 			expected: "b",
 		},
+		{
+			os:       "linux",
+			path:     "a/..",
+			expected: ".",
+		},
+		{
+			os:       "windows",
+			path:     "c:\\",
+			expected: "c:\\",
+		},
+		{
+			os:       "windows",
+			path:     "c:\\\\",
+			expected: "c:\\",
+		},
+		{
+			os:       "windows",
+			path:     "c:\\a",
+			expected: "c:\\a",
+		},
+		{
+			os:       "windows",
+			path:     "c:\\a\\",
+			expected: "c:\\a",
+		},
+		{
+			os:       "windows",
+			path:     "c:\\\\a",
+			expected: "c:\\a",
+		},
+		{
+			os:       "windows",
+			path:     "c:\\..",
+			expected: "c:\\",
+		},
+		{
+			os:       "windows",
+			path:     "c:\\..\\a",
+			expected: "c:\\a",
+		},
+		{
+			os:       "windows",
+			path:     ".",
+			expected: ".",
+		},
+		{
+			os:       "windows",
+			path:     ".\\c",
+			expected: "c",
+		},
+		{
+			os:       "windows",
+			path:     "..\\.\\a",
+			expected: "..\\a",
+		},
+		{
+			os:       "windows",
+			path:     "a\\..\\b",
+			expected: "b",
+		},
+		{
+			os:       "windows",
+			path:     "a\\..",
+			expected: ".",
+		},
 	} {
 		t.Run(
 			fmt.Sprintf("Clean(%q,%q)", test.os, test.path),
 			func(t *testing.T) {
 				clean := Clean(test.os, test.path)
 				if clean != test.expected {
-					t.Errorf("unexpected result: %q (expected %q)\n", clean, test.expected)
+					t.Errorf("unexpected result: %q (expected %q)", clean, test.expected)
+				}
+				if runtime.GOOS == test.os {
+					stdClean := filepath.Clean(test.path)
+					if clean != stdClean {
+						t.Errorf("non-standard result: %q (%q is standard)", clean, stdClean)
+					}
 				}
 			},
 		)