diff --git a/file_handling.go b/file_handling.go index 9844c3a..7c0b1a7 100644 --- a/file_handling.go +++ b/file_handling.go @@ -1,6 +1,7 @@ package main import ( + "fmt" "io" "log" "os" @@ -15,12 +16,15 @@ type File struct { info os.FileInfo } -func NewFile(path string) *File { +// NewFile resolves path to an absolute path and wraps it in a *File. It +// returns an error if the working directory cannot be determined (the only +// failure mode of filepath.Abs). +func NewFile(path string) (*File, error) { absPath, err := filepath.Abs(path) if err != nil { - log.Fatalf("Unable to resolve absolute path of %v: %v", path, err) + return nil, fmt.Errorf("resolve absolute path of %v: %w", path, err) } - return &File{Path: absPath} + return &File{Path: absPath}, nil } func (f *File) Base() string { @@ -31,26 +35,37 @@ func (f *File) Dir() string { return filepath.Dir(f.Path) } -func (f *File) Info() os.FileInfo { +// Info lazily stats the file and caches the result. It returns an error if +// the underlying os.Stat fails. +func (f *File) Info() (os.FileInfo, error) { if f.info == nil { stat, err := os.Stat(f.Path) if err != nil { - log.Fatalf("Failed to stat %v: %v", f.Path, err) + return nil, fmt.Errorf("stat %v: %w", f.Path, err) } f.info = stat } - return f.info + return f.info, nil } -func (f *File) Mode() os.FileMode { - return f.Info().Mode() +// Mode returns the cached mode bits. It is only safe to call after Info() has +// succeeded; callers that have a *File handed to them by the walker can rely +// on that precondition because the walker calls Info() before dispatching. +func (f *File) Mode() (os.FileMode, error) { + info, err := f.Info() + if err != nil { + return 0, err + } + return info.Mode(), nil } -// Read the file into a string. -func (f *File) Read() string { +// Read reads the file into a string, or returns the empty string for binary +// files. An error indicates the file could not be opened or fully read; the +// caller should log-and-skip rather than abort. +func (f *File) Read() (string, error) { handle, err := os.Open(f.Path) if err != nil { - log.Fatalf("Unable to open %v: %v", f.Path, err) + return "", fmt.Errorf("open %v: %w", f.Path, err) } defer handle.Close() @@ -58,31 +73,43 @@ func (f *File) Read() string { var buf [1024]byte n, err := handle.Read(buf[0:]) if err != nil || !util.IsText(buf[0:n]) { - return "" + return "", nil } // Reset file handle so we can read the entire file. if _, err := handle.Seek(0, io.SeekStart); err != nil { - log.Fatalf("Failed to seek back to beginning of %v: %v", f.Path, err) + return "", fmt.Errorf("seek to start of %v: %w", f.Path, err) } builder := new(strings.Builder) if _, err := io.Copy(builder, handle); err != nil { - log.Fatalf("Failed to read %v to a string: %v", f.Path, err) + return "", fmt.Errorf("read %v: %w", f.Path, err) } - return builder.String() + return builder.String(), nil } -// Write content to file atomically, by writing it to a temporary file first, -// and then moving it to the destination, overwriting the original. -func (f *File) Write(content string) { +// Write atomically replaces the file with content, via a temp file + rename. +// A deferred os.Remove(tempName) ensures the temp file is cleaned up if any +// step after its creation fails (including the rename); on success the remove +// is a no-op because the file has already been renamed away. +func (f *File) Write(content string) error { + mode, err := f.Mode() + if err != nil { + return err + } + tempName := filepath.Join(f.Dir(), RandomString(20)) - if err := os.WriteFile(tempName, []byte(content), f.Mode()); err != nil { - log.Fatalf("Error creating tempfile in %v: %v", f.Dir(), err) + if err := os.WriteFile(tempName, []byte(content), mode); err != nil { + return fmt.Errorf("create tempfile in %v: %w", f.Dir(), err) } + // Make sure the temp file is removed if the rename below fails. On + // success, the rename has already moved the file to f.Path so this is + // a no-op (we deliberately ignore the not-exist error). + defer os.Remove(tempName) log.Printf("Rewriting %v", f.Path) if err := os.Rename(tempName, f.Path); err != nil { - log.Fatalf("Unable to atomically move temp file %v to %v: %v", tempName, f.Path, err) + return fmt.Errorf("atomically move temp file %v to %v: %w", tempName, f.Path, err) } + return nil } diff --git a/file_handling_test.go b/file_handling_test.go index 0296964..91ee538 100644 --- a/file_handling_test.go +++ b/file_handling_test.go @@ -5,11 +5,7 @@ import ( "testing" ) -// TestNewFile exercises NewFile's path-resolution behavior. It does NOT cover -// the filepath.Abs error path: NewFile currently calls log.Fatalf on that -// failure, which would kill the test binary, and that surface is not reachable -// on most platforms in any case. The error path will become testable when -// NewFile is refactored to return an error (see issue #6). +// TestNewFile exercises NewFile's path-resolution behavior. func TestNewFile(t *testing.T) { tmp := t.TempDir() @@ -65,7 +61,10 @@ func TestNewFile(t *testing.T) { want = abs } - got := NewFile(tc.input) + got, err := NewFile(tc.input) + if err != nil { + t.Fatalf("NewFile(%q) returned unexpected error: %v", tc.input, err) + } if got == nil { t.Fatalf("NewFile(%q) returned nil", tc.input) } diff --git a/find_replace.go b/find_replace.go index a2f2c65..9bdbe79 100644 --- a/find_replace.go +++ b/find_replace.go @@ -2,13 +2,13 @@ package main import ( "errors" + "fmt" + "io" "log" - "math/rand" "os" "path/filepath" "strings" "sync" - "time" ) // findReplace is a struct used to provide context to all find & replace @@ -16,6 +16,45 @@ import ( type findReplace struct { find string replace string + + // errs accumulates non-fatal errors that occurred during a walk. The + // walker logs each error at the point of failure (preserving the + // operator-visible UX) and appends it here so main can surface a + // non-zero exit code at the end. + errs errAccumulator +} + +// errAccumulator is a tiny thread-safe collector for errors that occur in +// concurrent walker goroutines. It is intentionally small: just enough to +// preserve "log everything, exit non-zero if anything failed" semantics +// without committing the codebase to a particular concurrency primitive +// (see issue #7 for the eventual bounded worker pool). +type errAccumulator struct { + mu sync.Mutex + errs []error +} + +// add records err. A nil err is ignored so callers can write +// `acc.add(fn())` without a guard. +func (a *errAccumulator) add(err error) { + if err == nil { + return + } + a.mu.Lock() + defer a.mu.Unlock() + a.errs = append(a.errs, err) +} + +// err returns the accumulated errors joined with errors.Join, or nil if +// nothing was recorded. The returned error is safe to unwrap with errors.Is +// / errors.As over every accumulated error. +func (a *errAccumulator) err() error { + a.mu.Lock() + defer a.mu.Unlock() + if len(a.errs) == 0 { + return nil + } + return errors.Join(a.errs...) } // main processes command line arguments, builds the context struct, and begins @@ -27,45 +66,78 @@ type findReplace struct { // • baseName: the relative name of a file, without a directory // • path: the relative path to a specific file or directory, including both dirName and baseName func main() { - // Remove date/time from logging output + os.Exit(run(os.Args, os.Stderr)) +} + +// run is the testable body of main. It returns the process exit code: 0 on +// clean success, 1 if argument parsing failed or any traversal error was +// recorded. Output documented in the README (Renaming/Rewriting lines) still +// goes to log.Default(); usage and aggregated error summaries go to stderr. +func run(args []string, stderr io.Writer) int { + // Remove date/time from logging output. log.SetFlags(0) - rand.Seed(time.Now().UnixNano()) - if len(os.Args) != 3 { - log.Fatal("Usage: find-replace FIND REPLACE") + if len(args) != 3 { + fmt.Fprintln(stderr, "Usage: find-replace FIND REPLACE") + return 1 } - find := os.Args[1] - replace := os.Args[2] - - fr := findReplace{find: find, replace: replace} + fr := findReplace{find: args[1], replace: args[2]} // Recursively explore the hierarchy depth first, rewrite files as needed, // and rename files last (after we don't have to revisit them). - // path.filepath.WalkDir() won't work here because it walks files - // alphabetically, breadth-first (and you'd be renaming files that you - // haven't explored yet). - - fr.WalkDir(NewFile(".")) + // filepath.WalkDir won't work here because it walks files alphabetically, + // breadth-first (and you'd be renaming files that you haven't explored + // yet). + root, err := NewFile(".") + if err != nil { + fmt.Fprintln(stderr, err) + return 1 + } + fr.WalkDir(root) + + if err := fr.errs.err(); err != nil { + // Each individual error has already been printed at the point of + // failure; the join here is for completeness in case a caller is + // scraping stderr. + fmt.Fprintln(stderr, err) + return 1 + } + return 0 } -// Walks files in the directory given by dirName, which is a relative path to a -// directory. Calls HandleFile for each file it finds, if it's not ignored. +// WalkDir lists files in the directory given by f and dispatches each child +// to HandleFile in its own goroutine. Per-child errors are logged at their +// failure site and recorded on fr so main can surface a non-zero exit code. +// A failure to read the directory itself is recorded and returned to the +// caller, but does not abort the rest of the walk in any other subtree. func (fr *findReplace) WalkDir(f *File) { var wg sync.WaitGroup // List the files in this directory. files, err := os.ReadDir(f.Path) if err != nil { - log.Fatalf("Unable to read directory: %v", err) + wrapped := fmt.Errorf("read directory %v: %w", f.Path, err) + log.Print(wrapped) + fr.errs.add(wrapped) + return } for _, file := range files { - childFile := NewFile(filepath.Join(f.Path, file.Name())) + childPath := filepath.Join(f.Path, file.Name()) + childFile, err := NewFile(childPath) + if err != nil { + log.Print(err) + fr.errs.add(err) + continue + } wg.Add(1) go func() { defer wg.Done() - fr.HandleFile(childFile) + if err := fr.HandleFile(childFile); err != nil { + log.Print(err) + fr.errs.add(err) + } }() } @@ -75,50 +147,66 @@ func (fr *findReplace) WalkDir(f *File) { // HandleFile immediately recurses depth-first into directories it finds, // otherwise calls ReplaceContents for regular files. When either operation is // complete, the file is renamed (if necessary) since no subsequent operations -// will need to access it again. -func (fr *findReplace) HandleFile(f *File) { +// will need to access it again. Errors from ReplaceContents are not fatal to +// the rename step; the failure is returned so the walker can log it and +// continue with siblings. +func (fr *findReplace) HandleFile(f *File) error { + info, err := f.Info() + if err != nil { + return err + } + // If file is a directory, recurse immediately (depth-first). - if f.Info().IsDir() { + if info.IsDir() { // Ignore certain directories if f.Base() == ".git" { - return + return nil } fr.WalkDir(f) } else { - // Replace the contents of regular files - fr.ReplaceContents(f) + // Replace the contents of regular files. + if err := fr.ReplaceContents(f); err != nil { + return err + } } - // Rename the file now that we're otherwise done with it - fr.RenameFile(f) + // Rename the file now that we're otherwise done with it. + return fr.RenameFile(f) } -// RenameFile renames a file if the destination file name does not already -// exist. -func (fr *findReplace) RenameFile(f *File) { - newBaseName := strings.Replace(f.Base(), fr.find, fr.replace, -1) +// RenameFile renames f to its post-replacement name if (a) the name actually +// changes and (b) no file already exists at the destination. It returns an +// error if the destination is occupied or if the os.Rename itself fails. +func (fr *findReplace) RenameFile(f *File) error { + newBaseName := strings.ReplaceAll(f.Base(), fr.find, fr.replace) + if f.Base() == newBaseName { + return nil + } + newPath := filepath.Join(f.Dir(), newBaseName) + if _, err := os.Stat(newPath); err == nil { + return fmt.Errorf("refusing to rename %v to %v: %v already exists", f.Path, newBaseName, newPath) + } else if !errors.Is(err, os.ErrNotExist) { + return fmt.Errorf("stat rename destination %v: %w", newPath, err) + } - if f.Base() != newBaseName { - if _, err := os.Stat(newPath); errors.Is(err, os.ErrNotExist) { - log.Printf("Renaming %v to %v", f.Path, newBaseName) - if err := os.Rename(f.Path, newPath); err != nil { - log.Fatalf("Unable to rename %v to %v: %v", f.Path, newBaseName, err) - } - } else { - log.Fatalf("Refusing to rename %v to %v: %v already exists", f.Path, newBaseName, newPath) - } + log.Printf("Renaming %v to %v", f.Path, newBaseName) + if err := os.Rename(f.Path, newPath); err != nil { + return fmt.Errorf("rename %v to %v: %w", f.Path, newBaseName, err) } + return nil } -// Replaces the contents of the given file, using the find & replace values in -// context. -func (fr *findReplace) ReplaceContents(f *File) { - // Find & replace the contents of text files. Binary-looking files return - // an empty string and will be skipped here. - content := f.Read() - if strings.Contains(content, fr.find) { - newContent := strings.Replace(content, fr.find, fr.replace, -1) - f.Write(newContent) +// ReplaceContents rewrites the file at f if its contents contain the find +// string. Binary-looking files (where Read returns "") are skipped silently. +func (fr *findReplace) ReplaceContents(f *File) error { + content, err := f.Read() + if err != nil { + return err + } + if !strings.Contains(content, fr.find) { + return nil } + newContent := strings.ReplaceAll(content, fr.find, fr.replace) + return f.Write(newContent) } diff --git a/find_replace_test.go b/find_replace_test.go index e52014e..9a54564 100644 --- a/find_replace_test.go +++ b/find_replace_test.go @@ -1,10 +1,13 @@ package main import ( + "bytes" "errors" + "io/fs" "os" "os/exec" "path/filepath" + "runtime" "strings" "testing" ) @@ -33,7 +36,7 @@ func newTestFile(tb testing.TB, path string, baseName string, content string) *F tb.Fatalf("close %v: %v", f.Name(), err) } - return NewFile(f.Name()) + return newFileOrFatal(tb, f.Name()) } // newTestDir creates a directory in the given directory path, with the given @@ -47,11 +50,32 @@ func newTestDir(tb testing.TB, path string, baseName string) *File { if err != nil { tb.Fatalf("MkdirTemp(%q, %q): %v", path, baseName, err) } - return NewFile(dirPath) + return newFileOrFatal(tb, dirPath) +} + +// newFileOrFatal wraps NewFile for tests that should never see the +// (vanishingly rare) filepath.Abs error. +func newFileOrFatal(tb testing.TB, path string) *File { + tb.Helper() + f, err := NewFile(path) + if err != nil { + tb.Fatalf("NewFile(%q): %v", path, err) + } + return f +} + +// readOrFatal returns the contents of f or fails the test. +func readOrFatal(tb testing.TB, f *File) string { + tb.Helper() + s, err := f.Read() + if err != nil { + tb.Fatalf("Read(%q): %v", f.Path, err) + } + return s } func expectedPathAfterRename(f *File, fr *findReplace) string { - return filepath.Join(f.Dir(), strings.Replace(f.Base(), fr.find, fr.replace, -1)) + return filepath.Join(f.Dir(), strings.ReplaceAll(f.Base(), fr.find, fr.replace)) } /* @@ -81,7 +105,7 @@ func assertFileNonexistent(t *testing.T, f *File) { func assertPathExistsAfterRename(t *testing.T, f *File, expectedPath string) *File { t.Helper() assertFileNonexistent(t, f) - newFile := NewFile(expectedPath) + newFile := newFileOrFatal(t, expectedPath) assertFileExists(t, newFile) return newFile } @@ -147,17 +171,20 @@ func TestWalkDir(t *testing.T) { fr := findReplace{find: find, replace: replace} fr.WalkDir(d) + if err := fr.errs.err(); err != nil { + t.Fatalf("WalkDir reported errors: %v", err) + } // d1: who/ > fo/ d1ExpectedPath := expectedPathAfterRename(d1, &fr) assertPathExistsAfterRename(t, d1, d1ExpectedPath) // d1d1: who/what/ > fo/foat/ - d1d1ExpectedPath := filepath.Join(d1ExpectedPath, strings.Replace(d1d1.Base(), fr.find, fr.replace, -1)) + d1d1ExpectedPath := filepath.Join(d1ExpectedPath, strings.ReplaceAll(d1d1.Base(), fr.find, fr.replace)) assertPathExistsAfterRename(t, d1d1, d1d1ExpectedPath) // d1d1f1: who/what/when > fo/fat/fen (contains "fere") - d1d1f1ExpectedPath := filepath.Join(d1d1ExpectedPath, strings.Replace(d1d1f1.Base(), fr.find, fr.replace, -1)) + d1d1f1ExpectedPath := filepath.Join(d1d1ExpectedPath, strings.ReplaceAll(d1d1f1.Base(), fr.find, fr.replace)) assertPathExistsAfterRename(t, d1d1f1, d1d1f1ExpectedPath) assertNewContentsOfFile(t, d1d1f1ExpectedPath, d1d1f1Contents, find, replace, "fere") @@ -166,11 +193,11 @@ func TestWalkDir(t *testing.T) { assertPathExistsAfterRename(t, d2, d2ExpectedPath) // d2d1: what/when/ - d2d1ExpectedPath := filepath.Join(d2ExpectedPath, strings.Replace(d2d1.Base(), fr.find, fr.replace, -1)) + d2d1ExpectedPath := filepath.Join(d2ExpectedPath, strings.ReplaceAll(d2d1.Base(), fr.find, fr.replace)) assertPathExistsAfterRename(t, d2d1, d2d1ExpectedPath) // d2d1d1: what/when/where (directories with no files) - d2d1d1ExpectedPath := filepath.Join(d2d1ExpectedPath, strings.Replace(d2d1d1.Base(), fr.find, fr.replace, -1)) + d2d1d1ExpectedPath := filepath.Join(d2d1ExpectedPath, strings.ReplaceAll(d2d1d1.Base(), fr.find, fr.replace)) assertPathExistsAfterRename(t, d2d1d1, d2d1d1ExpectedPath) // d3: when/ @@ -178,7 +205,7 @@ func TestWalkDir(t *testing.T) { assertPathExistsAfterRename(t, d3, d3ExpectedPath) // d3f1: when/where (contains "why") - d3f1ExpectedPath := filepath.Join(d3ExpectedPath, strings.Replace(d3f1.Base(), fr.find, fr.replace, -1)) + d3f1ExpectedPath := filepath.Join(d3ExpectedPath, strings.ReplaceAll(d3f1.Base(), fr.find, fr.replace)) assertPathExistsAfterRename(t, d3f1, d3f1ExpectedPath) assertNewContentsOfFile(t, d3f1ExpectedPath, d3f1Contents, find, replace, "fy") @@ -199,12 +226,14 @@ func TestHandleFileWithDir(t *testing.T) { f := newTestDir(t, "", initial) defer os.Remove(f.Path) - expectedPath := filepath.Join(f.Dir(), strings.Replace(f.Base(), find, replace, -1)) + expectedPath := filepath.Join(f.Dir(), strings.ReplaceAll(f.Base(), find, replace)) defer os.Remove(expectedPath) fr := findReplace{find: find, replace: replace} assertFileExists(t, f) - fr.HandleFile(f) + if err := fr.HandleFile(f); err != nil { + t.Fatalf("HandleFile(%q): %v", f.Path, err) + } assertPathExistsAfterRename(t, f, expectedPath) } @@ -213,21 +242,22 @@ func TestHandleFileWithIgnoredDir(t *testing.T) { find := "git" replace := "got" - dirPath := filepath.Join(os.TempDir(), initial) + dirPath := filepath.Join(t.TempDir(), initial) if err := os.Mkdir(dirPath, 0700); err != nil { t.Fatalf("Mkdir(%q): %v", dirPath, err) } - f := NewFile(dirPath) - defer os.Remove(f.Path) + f := newFileOrFatal(t, dirPath) // Just in case it's unexpectedly renamed, let's make sure we cleanup the // anticipated name. - unexpectedName := strings.Replace(f.Base(), find, replace, -1) + unexpectedName := strings.ReplaceAll(f.Base(), find, replace) unexpectedPath := filepath.Join(f.Dir(), unexpectedName) defer os.Remove(unexpectedPath) fr := findReplace{find: find, replace: replace} assertFileExists(t, f) - fr.HandleFile(f) + if err := fr.HandleFile(f); err != nil { + t.Fatalf("HandleFile(%q): %v", f.Path, err) + } assertFileExists(t, f) } @@ -239,16 +269,18 @@ func TestHandleFileWithFile(t *testing.T) { f := newTestFile(t, "", initial, initial) defer os.Remove(f.Path) - expectedName := strings.Replace(f.Base(), find, replace, -1) + expectedName := strings.ReplaceAll(f.Base(), find, replace) expectedPath := filepath.Join(f.Dir(), expectedName) defer os.Remove(expectedPath) fr := findReplace{find: find, replace: replace} assertFileExists(t, f) - fr.HandleFile(f) + if err := fr.HandleFile(f); err != nil { + t.Fatalf("HandleFile(%q): %v", f.Path, err) + } assertPathExistsAfterRename(t, f, expectedPath) - got := NewFile(expectedPath).Read() + got := readOrFatal(t, newFileOrFatal(t, expectedPath)) if got != want { t.Errorf("replace %v with %v in %v, but got %v; want %v", find, replace, initial, got, want) } @@ -261,13 +293,15 @@ func TestRenameFile(t *testing.T) { f := newTestFile(t, "", initial, "") defer os.Remove(f.Path) - expectedName := strings.Replace(f.Base(), find, replace, -1) + expectedName := strings.ReplaceAll(f.Base(), find, replace) expectedPath := filepath.Join(f.Dir(), expectedName) defer os.Remove(expectedPath) fr := findReplace{find: find, replace: replace} assertFileExists(t, f) - fr.RenameFile(f) + if err := fr.RenameFile(f); err != nil { + t.Fatalf("RenameFile(%q): %v", f.Path, err) + } assertPathExistsAfterRename(t, f, expectedPath) } @@ -275,7 +309,7 @@ func TestRenameFile(t *testing.T) { // path exactly match the desired string. func assertNewContentsOfFile(t *testing.T, path string, initial string, find string, replace string, want string) { t.Helper() - got := NewFile(path).Read() + got := readOrFatal(t, newFileOrFatal(t, path)) if got != want { t.Errorf("replace %v with %v in %v, but got %v; want %v", find, replace, initial, got, want) } @@ -290,7 +324,9 @@ func TestReplaceContents(t *testing.T) { f := newTestFile(t, "", "*", initial) defer os.Remove(f.Path) fr := findReplace{find: find, replace: replace} - fr.ReplaceContents(f) + if err := fr.ReplaceContents(f); err != nil { + t.Fatalf("ReplaceContents(%q): %v", f.Path, err) + } assertNewContentsOfFile(t, f.Path, initial, find, replace, want) } @@ -303,7 +339,9 @@ func TestReplaceContentsEntireFile(t *testing.T) { f := newTestFile(t, "", "*", initial) defer os.Remove(f.Path) fr := findReplace{find: find, replace: replace} - fr.ReplaceContents(f) + if err := fr.ReplaceContents(f); err != nil { + t.Fatalf("ReplaceContents(%q): %v", f.Path, err) + } assertNewContentsOfFile(t, f.Path, initial, find, replace, want) } @@ -316,7 +354,9 @@ func TestReplaceContentsMultipleMatchesSingleLine(t *testing.T) { f := newTestFile(t, "", "*", initial) defer os.Remove(f.Path) fr := findReplace{find: find, replace: replace} - fr.ReplaceContents(f) + if err := fr.ReplaceContents(f); err != nil { + t.Fatalf("ReplaceContents(%q): %v", f.Path, err) + } assertNewContentsOfFile(t, f.Path, initial, find, replace, want) } @@ -329,7 +369,9 @@ func TestReplaceContentsMultipleMatchesMultipleLines(t *testing.T) { f := newTestFile(t, "", "*", initial) defer os.Remove(f.Path) fr := findReplace{find: find, replace: replace} - fr.ReplaceContents(f) + if err := fr.ReplaceContents(f); err != nil { + t.Fatalf("ReplaceContents(%q): %v", f.Path, err) + } assertNewContentsOfFile(t, f.Path, initial, find, replace, want) } @@ -342,10 +384,282 @@ func TestReplaceContentsNoMatches(t *testing.T) { f := newTestFile(t, "", "*", initial) defer os.Remove(f.Path) fr := findReplace{find: find, replace: replace} - fr.ReplaceContents(f) + if err := fr.ReplaceContents(f); err != nil { + t.Fatalf("ReplaceContents(%q): %v", f.Path, err) + } assertNewContentsOfFile(t, f.Path, initial, find, replace, want) } +// TestWalkDir_PermissionDeniedSubdirContinues ensures that an unreadable +// subdirectory does not abort the walk. The sibling subtree must still be +// rewritten, and the walker must record an error referencing the failing +// subdirectory. +func TestWalkDir_PermissionDeniedSubdirContinues(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("permission semantics differ on Windows") + } + if os.Geteuid() == 0 { + t.Skip("test requires non-root: chmod 0 directories are still readable as root") + } + + root := t.TempDir() + + // Build root/a/inside.txt (unreadable parent) and root/b/inside.txt + // (normal). After WalkDir we expect b's file to be rewritten and a's + // directory to surface an error. + denied := filepath.Join(root, "a") + if err := os.Mkdir(denied, 0700); err != nil { + t.Fatalf("Mkdir(%q): %v", denied, err) + } + deniedChild := filepath.Join(denied, "inside.txt") + if err := os.WriteFile(deniedChild, []byte("alpha"), 0600); err != nil { + t.Fatalf("WriteFile(%q): %v", deniedChild, err) + } + + siblingDir := filepath.Join(root, "b") + if err := os.Mkdir(siblingDir, 0700); err != nil { + t.Fatalf("Mkdir(%q): %v", siblingDir, err) + } + siblingFile := filepath.Join(siblingDir, "inside.txt") + if err := os.WriteFile(siblingFile, []byte("alpha"), 0600); err != nil { + t.Fatalf("WriteFile(%q): %v", siblingFile, err) + } + + // Remove read+exec from the denied directory only. Restore the bit at + // cleanup so t.TempDir's RemoveAll can succeed. + if err := os.Chmod(denied, 0); err != nil { + t.Fatalf("Chmod(%q, 0): %v", denied, err) + } + t.Cleanup(func() { _ = os.Chmod(denied, 0700) }) + + rootFile := newFileOrFatal(t, root) + fr := findReplace{find: "alpha", replace: "beta"} + fr.WalkDir(rootFile) + + // The sibling file should have been rewritten despite the denied subtree. + got, err := os.ReadFile(siblingFile) + if err != nil { + t.Fatalf("ReadFile(%q): %v", siblingFile, err) + } + if string(got) != "beta" { + t.Errorf("sibling file contents = %q; want %q (denied sibling aborted the walk)", string(got), "beta") + } + + // The walker must have recorded an error referencing the denied subtree. + err = fr.errs.err() + if err == nil { + t.Fatalf("WalkDir succeeded; want error mentioning %q", denied) + } + if !strings.Contains(err.Error(), denied) { + t.Errorf("WalkDir error = %v; want one mentioning %q", err, denied) + } + // errors.Is should walk the joined chain and find the permission error. + if !errors.Is(err, fs.ErrPermission) { + t.Errorf("WalkDir error = %v; want errors.Is(_, fs.ErrPermission) == true", err) + } +} + +// TestRenameFile_ReturnsErrorOnExistingDestination ensures a clobbering +// rename is refused (returning an error) rather than crashing the process. +func TestRenameFile_ReturnsErrorOnExistingDestination(t *testing.T) { + tmp := t.TempDir() + src := filepath.Join(tmp, "alpha") + if err := os.WriteFile(src, nil, 0600); err != nil { + t.Fatalf("WriteFile(%q): %v", src, err) + } + dst := filepath.Join(tmp, "beta") + if err := os.WriteFile(dst, nil, 0600); err != nil { + t.Fatalf("WriteFile(%q): %v", dst, err) + } + + f := newFileOrFatal(t, src) + fr := findReplace{find: "alpha", replace: "beta"} + err := fr.RenameFile(f) + if err == nil { + t.Fatalf("RenameFile(%q): err = nil; want an error referencing the occupied destination", src) + } + if !strings.Contains(err.Error(), "beta") { + t.Errorf("RenameFile error = %v; want one mentioning %q", err, "beta") + } + // The source must still be present — RenameFile must not have clobbered + // the destination either. + if _, statErr := os.Stat(src); statErr != nil { + t.Errorf("Stat(%q) after refused rename: %v", src, statErr) + } + if _, statErr := os.Stat(dst); statErr != nil { + t.Errorf("Stat(%q) after refused rename: %v", dst, statErr) + } +} + +// TestWalkDir_BadRenameTargetDoesNotAbortSiblings sets up two sibling files +// whose post-rename names would collide with already-existing files. The +// walker must rename what it can, record errors for what it cannot, and not +// abort the rest of the tree. +func TestWalkDir_BadRenameTargetDoesNotAbortSiblings(t *testing.T) { + root := t.TempDir() + + // Files that will be renamed alpha -> beta. The "occupied" path already + // has a beta target so its rename must fail. The "free" path has a + // distinct prefix and should succeed. + occupied := filepath.Join(root, "occupied-alpha") + if err := os.WriteFile(occupied, nil, 0600); err != nil { + t.Fatalf("WriteFile(%q): %v", occupied, err) + } + occupiedTarget := filepath.Join(root, "occupied-beta") + if err := os.WriteFile(occupiedTarget, nil, 0600); err != nil { + t.Fatalf("WriteFile(%q): %v", occupiedTarget, err) + } + + free := filepath.Join(root, "free-alpha") + if err := os.WriteFile(free, nil, 0600); err != nil { + t.Fatalf("WriteFile(%q): %v", free, err) + } + + rootFile := newFileOrFatal(t, root) + fr := findReplace{find: "alpha", replace: "beta"} + fr.WalkDir(rootFile) + + // The free file should have been renamed. + freeRenamed := filepath.Join(root, "free-beta") + if _, err := os.Stat(freeRenamed); err != nil { + t.Errorf("Stat(%q) after walk: %v (free-alpha should have been renamed despite occupied-alpha's failure)", freeRenamed, err) + } + + // The walker must have recorded an error referencing the occupied target. + err := fr.errs.err() + if err == nil { + t.Fatalf("WalkDir succeeded; want a 'refusing to rename' error for occupied-alpha") + } + if !strings.Contains(err.Error(), "occupied-beta") { + t.Errorf("WalkDir error = %v; want one mentioning %q", err, "occupied-beta") + } +} + +// TestWriteCleansUpTempFileOnRenameFailure ensures that File.Write does not +// leak a temp file when the rename step fails. It forces the rename to fail +// (after the temp file has been created) by making the destination a +// non-empty directory; os.Rename of a regular file onto a non-empty +// directory returns ENOTEMPTY ("file exists") on Linux regardless of the +// running user, so this exercises the deferred-cleanup path under both root +// and non-root. +func TestWriteCleansUpTempFileOnRenameFailure(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("rename-over-directory semantics differ on Windows") + } + + dir := t.TempDir() + target := filepath.Join(dir, "target") + + // Create the target as a non-empty directory. Write will succeed in + // creating its tempfile next to it, then fail the rename step. + if err := os.Mkdir(target, 0700); err != nil { + t.Fatalf("Mkdir(%q): %v", target, err) + } + if err := os.WriteFile(filepath.Join(target, "sentinel"), nil, 0600); err != nil { + t.Fatalf("WriteFile sentinel: %v", err) + } + + // Snapshot the directory contents before the Write so we can confirm no + // stray files survive afterwards. + beforeEntries, err := os.ReadDir(dir) + if err != nil { + t.Fatalf("ReadDir(%q): %v", dir, err) + } + before := make(map[string]struct{}, len(beforeEntries)) + for _, e := range beforeEntries { + before[e.Name()] = struct{}{} + } + + // We need f.Mode() to succeed, so prime the cached info. + f := newFileOrFatal(t, target) + if _, err := f.Info(); err != nil { + t.Fatalf("Info(%q): %v", target, err) + } + + if err := f.Write("beta"); err == nil { + t.Fatalf("Write succeeded over a non-empty directory; expected an error") + } + + // Confirm no new entries (other than the existing target directory) + // linger in the parent. + afterEntries, err := os.ReadDir(dir) + if err != nil { + t.Fatalf("ReadDir(%q): %v", dir, err) + } + for _, e := range afterEntries { + if _, ok := before[e.Name()]; ok { + continue + } + t.Errorf("leftover entry %q in %q after Write failure (tempfile was not cleaned up)", e.Name(), dir) + } +} + +// TestRun_ExitsZeroOnSuccess confirms run() returns 0 for a clean walk. +func TestRun_ExitsZeroOnSuccess(t *testing.T) { + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, "alpha.txt"), []byte("alpha"), 0600); err != nil { + t.Fatalf("WriteFile: %v", err) + } + + withWorkingDir(t, dir) + + var stderr bytes.Buffer + got := run([]string{"find-replace", "alpha", "beta"}, &stderr) + if got != 0 { + t.Errorf("run = %d; want 0 (stderr: %q)", got, stderr.String()) + } +} + +// TestRun_ExitsNonZeroOnTraversalError confirms run() returns a non-zero +// exit code when any file failed to be processed. We force a failure by +// putting a file whose rename target is occupied. +func TestRun_ExitsNonZeroOnTraversalError(t *testing.T) { + dir := t.TempDir() + src := filepath.Join(dir, "occupied-alpha") + if err := os.WriteFile(src, nil, 0600); err != nil { + t.Fatalf("WriteFile(%q): %v", src, err) + } + dst := filepath.Join(dir, "occupied-beta") + if err := os.WriteFile(dst, nil, 0600); err != nil { + t.Fatalf("WriteFile(%q): %v", dst, err) + } + + withWorkingDir(t, dir) + + var stderr bytes.Buffer + got := run([]string{"find-replace", "alpha", "beta"}, &stderr) + if got == 0 { + t.Errorf("run = 0; want non-zero (stderr: %q)", stderr.String()) + } +} + +// TestRun_BadArgCountPrintsUsage confirms the usage message goes to stderr +// and the exit code is non-zero. +func TestRun_BadArgCountPrintsUsage(t *testing.T) { + var stderr bytes.Buffer + got := run([]string{"find-replace"}, &stderr) + if got == 0 { + t.Errorf("run = 0; want non-zero") + } + if !strings.Contains(stderr.String(), "Usage: find-replace") { + t.Errorf("stderr = %q; want it to contain a usage line", stderr.String()) + } +} + +// withWorkingDir chdirs to dir for the duration of the test and restores the +// previous working directory at cleanup. +func withWorkingDir(t *testing.T, dir string) { + t.Helper() + prev, err := os.Getwd() + if err != nil { + t.Fatalf("Getwd: %v", err) + } + if err := os.Chdir(dir); err != nil { + t.Fatalf("Chdir(%q): %v", dir, err) + } + t.Cleanup(func() { _ = os.Chdir(prev) }) +} + func CloneRepoToTestDir(b *testing.B, repoUrl string) *File { b.Helper() d := newTestDir(b, "", "*") diff --git a/go.mod b/go.mod index 96a1ddb..66fd9af 100644 --- a/go.mod +++ b/go.mod @@ -1,5 +1,5 @@ module github.com/dolph/find-replace -go 1.19 +go 1.20 require golang.org/x/tools v0.7.0