From 28bccc4ad0e4735200d07ec4d6168e063af71980 Mon Sep 17 00:00:00 2001 From: Steve Francia Date: Mon, 7 Dec 2015 09:29:41 -0500 Subject: [PATCH] reorder all utility functions to have fs as 1st param This is a BREAKING CHANGE to the Walk and ReadDir functions that existed prior to this branch addition. This change is not done without considerable thought. Having FS come first is done for two reasons. 1: It's more idiomatic go 2: It's more logical. It allows the function signature to read easier and flow from the broadest value to the most narrow. For example, WriteReader would read.. writeReader on this fs, to this path (on that fs), with this data (at that path, on that fs). I believe that when the first two were implemented it wasn't at all obvious that the order wasn't correct, nevertheless, permitting a lot of new functions defined in an incorrect or inconsistent order seems like the worst option. It was decided that a breaking change today would be best, even if it was mildly painful at the present. --- afero_test.go | 8 ++++---- ioutil.go | 20 ++++++++++---------- ioutil_test.go | 4 ++-- path.go | 20 ++++++++++---------- path_test.go | 2 +- util.go | 38 +++++++++++++++++++------------------- util_test.go | 30 +++++++++++++++--------------- 7 files changed, 61 insertions(+), 61 deletions(-) diff --git a/afero_test.go b/afero_test.go index 6f925cb..91503b5 100644 --- a/afero_test.go +++ b/afero_test.go @@ -143,7 +143,7 @@ func TestRename(t *testing.T) { if err != nil { t.Fatalf("rename %q, %q failed: %v", to, from, err) } - names, err := readDirNames(testDir, fs) + names, err := readDirNames(fs, testDir) if err != nil { t.Fatalf("readDirNames error: %v", err) } @@ -378,7 +378,7 @@ func TestReaddirnames(t *testing.T) { if err != nil { t.Fatal(err) } - findNames(t, namesRoot, namesSub, fs) + findNames(fs, t, namesRoot, namesSub) } } @@ -487,11 +487,11 @@ func TestReaddirAll(t *testing.T) { namesSub = append(namesSub, e.Name()) } - findNames(t, namesRoot, namesSub, fs) + findNames(fs, t, namesRoot, namesSub) } } -func findNames(t *testing.T, root, sub []string, fs Fs) { +func findNames(fs Fs, t *testing.T, root, sub []string) { var foundRoot bool for _, e := range root { _, err := fs.Open(path.Join(testDir, e)) diff --git a/ioutil.go b/ioutil.go index 2d3c68b..793e5c4 100644 --- a/ioutil.go +++ b/ioutil.go @@ -36,10 +36,10 @@ func (f byName) Swap(i, j int) { f[i], f[j] = f[j], f[i] } // ReadDir reads the directory named by dirname and returns // a list of sorted directory entries. func (a Afero) ReadDir(dirname string) ([]os.FileInfo, error) { - return ReadDir(dirname, a.fs) + return ReadDir(a.fs, dirname) } -func ReadDir(dirname string, fs Fs) ([]os.FileInfo, error) { +func ReadDir(fs Fs, dirname string) ([]os.FileInfo, error) { f, err := fs.Open(dirname) if err != nil { return nil, err @@ -58,10 +58,10 @@ func ReadDir(dirname string, fs Fs) ([]os.FileInfo, error) { // reads the whole file, it does not treat an EOF from Read as an error // to be reported. func (a Afero) ReadFile(filename string) ([]byte, error) { - return ReadFile(filename, a.fs) + return ReadFile(a.fs, filename) } -func ReadFile(filename string, fs Fs) ([]byte, error) { +func ReadFile(fs Fs, filename string) ([]byte, error) { f, err := fs.Open(filename) if err != nil { return nil, err @@ -118,10 +118,10 @@ func ReadAll(r io.Reader) ([]byte, error) { // If the file does not exist, WriteFile creates it with permissions perm; // otherwise WriteFile truncates it before writing. func (a Afero) WriteFile(filename string, data []byte, perm os.FileMode) error { - return WriteFile(filename, data, perm, a.fs) + return WriteFile(a.fs, filename, data, perm) } -func WriteFile(filename string, data []byte, perm os.FileMode, fs Fs) error { +func WriteFile(fs Fs, filename string, data []byte, perm os.FileMode) error { f, err := fs.OpenFile(filename, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, perm) if err != nil { return err @@ -169,10 +169,10 @@ func nextSuffix() string { // to find the pathname of the file. It is the caller's responsibility // to remove the file when no longer needed. func (a Afero) TempFile(dir, prefix string) (f File, err error) { - return TempFile(dir, prefix, a.fs) + return TempFile(a.fs, dir, prefix) } -func TempFile(dir, prefix string, fs Fs) (f File, err error) { +func TempFile(fs Fs, dir, prefix string) (f File, err error) { if dir == "" { dir = os.TempDir() } @@ -202,9 +202,9 @@ func TempFile(dir, prefix string, fs Fs) (f File, err error) { // will not choose the same directory. It is the caller's responsibility // to remove the directory when no longer needed. func (a Afero) TempDir(dir, prefix string) (name string, err error) { - return TempDir(dir, prefix, a.fs) + return TempDir(a.fs, dir, prefix) } -func TempDir(dir, prefix string, fs Fs) (name string, err error) { +func TempDir(fs Fs, dir, prefix string) (name string, err error) { if dir == "" { dir = os.TempDir() } diff --git a/ioutil_test.go b/ioutil_test.go index ed05977..894687a 100644 --- a/ioutil_test.go +++ b/ioutil_test.go @@ -82,13 +82,13 @@ func TestReadDir(t *testing.T) { testFS.Mkdir("/i-am-a-dir", 0777) testFS.Create("/this_exists.go") dirname := "rumpelstilzchen" - _, err := ReadDir(dirname, testFS) + _, err := ReadDir(testFS, dirname) if err == nil { t.Fatalf("ReadDir %s: error expected, none found", dirname) } dirname = ".." - list, err := ReadDir(dirname, testFS) + list, err := ReadDir(testFS, dirname) if err != nil { t.Fatalf("ReadDir %s: %v", dirname, err) } diff --git a/path.go b/path.go index 6ae81eb..54c5579 100644 --- a/path.go +++ b/path.go @@ -24,7 +24,7 @@ import ( // readDirNames reads the directory named by dirname and returns // a sorted list of directory entries. // adapted from https://golang.org/src/path/filepath/path.go -func readDirNames(dirname string, fs Fs) ([]string, error) { +func readDirNames(fs Fs, dirname string) ([]string, error) { f, err := fs.Open(dirname) if err != nil { return nil, err @@ -40,7 +40,7 @@ func readDirNames(dirname string, fs Fs) ([]string, error) { // walk recursively descends path, calling walkFn // adapted from https://golang.org/src/path/filepath/path.go -func walk(path string, info os.FileInfo, walkFn filepath.WalkFunc, fs Fs) error { +func walk(fs Fs, path string, info os.FileInfo, walkFn filepath.WalkFunc) error { err := walkFn(path, info, nil) if err != nil { if info.IsDir() && err == filepath.SkipDir { @@ -53,20 +53,20 @@ func walk(path string, info os.FileInfo, walkFn filepath.WalkFunc, fs Fs) error return nil } - names, err := readDirNames(path, fs) + names, err := readDirNames(fs, path) if err != nil { return walkFn(path, info, err) } for _, name := range names { filename := filepath.Join(path, name) - fileInfo, err := lstatIfOs(filename, fs) + fileInfo, err := lstatIfOs(fs, filename) if err != nil { if err := walkFn(filename, fileInfo, err); err != nil && err != filepath.SkipDir { return err } } else { - err = walk(filename, fileInfo, walkFn, fs) + err = walk(fs, filename, fileInfo, walkFn) if err != nil { if !fileInfo.IsDir() || err != filepath.SkipDir { return err @@ -78,7 +78,7 @@ func walk(path string, info os.FileInfo, walkFn filepath.WalkFunc, fs Fs) error } // if the filesystem is OsFs use Lstat, else use fs.Stat -func lstatIfOs(path string, fs Fs) (info os.FileInfo, err error) { +func lstatIfOs(fs Fs, path string) (info os.FileInfo, err error) { _, ok := fs.(*OsFs) if ok { info, err = os.Lstat(path) @@ -96,13 +96,13 @@ func lstatIfOs(path string, fs Fs) (info os.FileInfo, err error) { // Walk does not follow symbolic links. func (a Afero) Walk(root string, walkFn filepath.WalkFunc) error { - return Walk(root, walkFn, a.fs) + return Walk(a.fs, root, walkFn) } -func Walk(root string, walkFn filepath.WalkFunc, fs Fs) error { - info, err := lstatIfOs(root, fs) +func Walk(fs Fs, root string, walkFn filepath.WalkFunc) error { + info, err := lstatIfOs(fs, root) if err != nil { return walkFn(root, nil, err) } - return walk(root, info, walkFn, fs) + return walk(fs, root, info, walkFn) } diff --git a/path_test.go b/path_test.go index 3c62525..6a584a4 100644 --- a/path_test.go +++ b/path_test.go @@ -48,7 +48,7 @@ func TestWalk(t *testing.T) { outputs[i] += fmt.Sprintln(path, info.Name(), size, info.IsDir(), err) return nil } - err := Walk(testDir, walkFn, fs) + err := Walk(fs, testDir, walkFn) if err != nil { t.Error(err) } diff --git a/util.go b/util.go index d80f986..faed9d3 100644 --- a/util.go +++ b/util.go @@ -35,10 +35,10 @@ const FilePathSeparator = string(filepath.Separator) // Takes a reader and a path and writes the content func (a Afero) WriteReader(path string, r io.Reader) (err error) { - return WriteReader(path, r, a.fs) + return WriteReader(a.fs, path, r) } -func WriteReader(path string, r io.Reader, fs Fs) (err error) { +func WriteReader(fs Fs, path string, r io.Reader) (err error) { dir, _ := filepath.Split(path) ospath := filepath.FromSlash(dir) @@ -63,10 +63,10 @@ func WriteReader(path string, r io.Reader, fs Fs) (err error) { // Same as WriteReader but checks to see if file/directory already exists. func (a Afero) SafeWriteReader(path string, r io.Reader) (err error) { - return SafeWriteReader(path, r, a.fs) + return SafeWriteReader(a.fs, path, r) } -func SafeWriteReader(path string, r io.Reader, fs Fs) (err error) { +func SafeWriteReader(fs Fs, path string, r io.Reader) (err error) { dir, _ := filepath.Split(path) ospath := filepath.FromSlash(dir) @@ -77,7 +77,7 @@ func SafeWriteReader(path string, r io.Reader, fs Fs) (err error) { } } - exists, err := Exists(path, fs) + exists, err := Exists(fs, path) if err != nil { return } @@ -96,12 +96,12 @@ func SafeWriteReader(path string, r io.Reader, fs Fs) (err error) { } func (a Afero) GetTempDir(subPath string) string { - return GetTempDir(subPath, a.fs) + return GetTempDir(a.fs, subPath) } // GetTempDir returns the default temp directory with trailing slash // if subPath is not empty then it will be created recursively with mode 777 rwx rwx rwx -func GetTempDir(subPath string, fs Fs) string { +func GetTempDir(fs Fs, subPath string) string { addSlash := func(p string) string { if FilePathSeparator != p[len(p)-1:] { p = p + FilePathSeparator @@ -120,7 +120,7 @@ func GetTempDir(subPath string, fs Fs) string { dir = strings.Replace(dir, "____", "\\", -1) } - if exists, _ := Exists(dir, fs); exists { + if exists, _ := Exists(fs, dir); exists { return addSlash(dir) } @@ -170,11 +170,11 @@ func isMn(r rune) bool { } func (a Afero) FileContainsBytes(filename string, subslice []byte) (bool, error) { - return FileContainsBytes(filename, subslice, a.fs) + return FileContainsBytes(a.fs, filename, subslice) } // Check if a file contains a specified string. -func FileContainsBytes(filename string, subslice []byte, fs Fs) (bool, error) { +func FileContainsBytes(fs Fs, filename string, subslice []byte) (bool, error) { f, err := fs.Open(filename) if err != nil { return false, err @@ -221,11 +221,11 @@ func readerContains(r io.Reader, subslice []byte) bool { } func (a Afero) DirExists(path string) (bool, error) { - return DirExists(path, a.fs) + return DirExists(a.fs, path) } // DirExists checks if a path exists and is a directory. -func DirExists(path string, fs Fs) (bool, error) { +func DirExists(fs Fs, path string) (bool, error) { fi, err := fs.Stat(path) if err == nil && fi.IsDir() { return true, nil @@ -237,11 +237,11 @@ func DirExists(path string, fs Fs) (bool, error) { } func (a Afero) IsDir(path string) (bool, error) { - return IsDir(path, a.fs) + return IsDir(a.fs, path) } // IsDir checks if a given path is a directory. -func IsDir(path string, fs Fs) (bool, error) { +func IsDir(fs Fs, path string) (bool, error) { fi, err := fs.Stat(path) if err != nil { return false, err @@ -250,12 +250,12 @@ func IsDir(path string, fs Fs) (bool, error) { } func (a Afero) IsEmpty(path string) (bool, error) { - return IsEmpty(path, a.fs) + return IsEmpty(a.fs, path) } // IsEmpty checks if a given file or directory is empty. -func IsEmpty(path string, fs Fs) (bool, error) { - if b, _ := Exists(path, fs); !b { +func IsEmpty(fs Fs, path string) (bool, error) { + if b, _ := Exists(fs, path); !b { return false, fmt.Errorf("%q path does not exist", path) } fi, err := fs.Stat(path) @@ -275,11 +275,11 @@ func IsEmpty(path string, fs Fs) (bool, error) { } func (a Afero) Exists(path string) (bool, error) { - return Exists(path, a.fs) + return Exists(a.fs, path) } // Check if a file or directory exists. -func Exists(path string, fs Fs) (bool, error) { +func Exists(fs Fs, path string) (bool, error) { _, err := fs.Stat(path) if err == nil { return true, nil diff --git a/util_test.go b/util_test.go index 7ff3f84..46432c6 100644 --- a/util_test.go +++ b/util_test.go @@ -58,7 +58,7 @@ func TestDirExists(t *testing.T) { } for i, d := range data { - exists, _ := DirExists(filepath.FromSlash(d.input), testFS) + exists, _ := DirExists(testFS, filepath.FromSlash(d.input)) if d.expected != exists { t.Errorf("Test %d %q failed. Expected %t got %t", i, d.input, d.expected, exists) } @@ -81,7 +81,7 @@ func TestIsDir(t *testing.T) { for i, d := range data { - exists, _ := IsDir(d.input, testFS) + exists, _ := IsDir(testFS, d.input) if d.expected != exists { t.Errorf("Test %d failed. Expected %t got %t", i, d.expected, exists) } @@ -123,7 +123,7 @@ func TestIsEmpty(t *testing.T) { {nonExistentDir, false, dirDoesNotExist}, } for i, d := range data { - exists, err := IsEmpty(d.input, testFS) + exists, err := IsEmpty(testFS, d.input) if d.expectedResult != exists { t.Errorf("Test %d %q failed exists. Expected result %t got %t", i, d.input, d.expectedResult, exists) } @@ -141,7 +141,7 @@ func TestIsEmpty(t *testing.T) { func createZeroSizedFileInTempDir() (File, error) { filePrefix := "_path_test_" - f, e := TempFile("", filePrefix, testFS) // dir is os.TempDir() + f, e := TempFile(testFS, "", filePrefix) // dir is os.TempDir() if e != nil { // if there was an error no file was created. // => no requirement to delete the file @@ -156,7 +156,7 @@ func createNonZeroSizedFileInTempDir() (File, error) { // no file ?? } byteString := []byte("byteString") - err = WriteFile(f.Name(), byteString, 0644, testFS) + err = WriteFile(testFS, f.Name(), byteString, 0644) if err != nil { // delete the file deleteFileInTempDir(f) @@ -174,7 +174,7 @@ func deleteFileInTempDir(f File) { func createEmptyTempDir() (string, error) { dirPrefix := "_dir_prefix_" - d, e := TempDir("", dirPrefix, testFS) // will be in os.TempDir() + d, e := TempDir(testFS, "", dirPrefix) // will be in os.TempDir() if e != nil { // no directory to delete - it was never created return "", e @@ -188,7 +188,7 @@ func createTempDirWithZeroLengthFiles() (string, error) { //now what? } filePrefix := "_path_test_" - _, fileErr := TempFile(d, filePrefix, testFS) // dir is os.TempDir() + _, fileErr := TempFile(testFS, d, filePrefix) // dir is os.TempDir() if fileErr != nil { // if there was an error no file was created. // but we need to remove the directory to clean-up @@ -206,7 +206,7 @@ func createTempDirWithNonZeroLengthFiles() (string, error) { //now what? } filePrefix := "_path_test_" - f, fileErr := TempFile(d, filePrefix, testFS) // dir is os.TempDir() + f, fileErr := TempFile(testFS, d, filePrefix) // dir is os.TempDir() if fileErr != nil { // if there was an error no file was created. // but we need to remove the directory to clean-up @@ -214,7 +214,7 @@ func createTempDirWithNonZeroLengthFiles() (string, error) { return "", fileErr } byteString := []byte("byteString") - fileErr = WriteFile(f.Name(), byteString, 0644, testFS) + fileErr = WriteFile(testFS, f.Name(), byteString, 0644) if fileErr != nil { // delete the file deleteFileInTempDir(f) @@ -252,7 +252,7 @@ func TestExists(t *testing.T) { {nonExistentDir, false, nil}, } for i, d := range data { - exists, err := Exists(d.input, testFS) + exists, err := Exists(testFS, d.input) if d.expectedResult != exists { t.Errorf("Test %d failed. Expected result %t got %t", i, d.expectedResult, exists) } @@ -287,7 +287,7 @@ func TestSafeWriteToDisk(t *testing.T) { } for i, d := range data { - e := SafeWriteReader(d.filename, reader, testFS) + e := SafeWriteReader(testFS, d.filename, reader) if d.expectedErr != nil { if d.expectedErr.Error() != e.Error() { t.Errorf("Test %d failed. Expected error %q but got %q", i, d.expectedErr.Error(), e.Error()) @@ -296,7 +296,7 @@ func TestSafeWriteToDisk(t *testing.T) { if d.expectedErr != e { t.Errorf("Test %d failed. Expected %q but got %q", i, d.expectedErr, e) } - contents, _ := ReadFile(d.filename, testFS) + contents, _ := ReadFile(testFS, d.filename) if randomString != string(contents) { t.Errorf("Test %d failed. Expected contents %q but got %q", i, randomString, string(contents)) } @@ -327,11 +327,11 @@ func TestWriteToDisk(t *testing.T) { } for i, d := range data { - e := WriteReader(d.filename, reader, testFS) + e := WriteReader(testFS, d.filename, reader) if d.expectedErr != e { t.Errorf("Test %d failed. WriteToDisk Error Expected %q but got %q", i, d.expectedErr, e) } - contents, e := ReadFile(d.filename, testFS) + contents, e := ReadFile(testFS, d.filename) if e != nil { t.Errorf("Test %d failed. Could not read file %s. Reason: %s\n", i, d.filename, e) } @@ -363,7 +363,7 @@ func TestGetTempDir(t *testing.T) { } for _, test := range tests { - output := GetTempDir(test.input, new(MemMapFs)) + output := GetTempDir(new(MemMapFs), test.input) if output != test.expected { t.Errorf("Expected %#v, got %#v\n", test.expected, output) }