Skip to content

Fix RegexpFs.Rename silently doing nothing for directories - #673

Open
januththedev wants to merge 1 commit into
spf13:masterfrom
januththedev:fix/regexpfs-rename-dir
Open

januththedev wants to merge 1 commit into
spf13:masterfrom
januththedev:fix/regexpfs-rename-dir

Conversation

@januththedev

Copy link
Copy Markdown

RegexpFs.Rename silently does nothing when the source is a directory

Description

func (r *RegexpFs) Rename(oldname, newname string) error {
	dir, err := IsDir(r.source, oldname)
	if err != nil {
		return err
	}
	if dir {
		return nil          // returns success, renames nothing
	}
	...

The directory branch returns nil instead of delegating to r.source.Rename.

Why that's wrong

RegexpFs's own doc comment states "The RegexpFs filters files (not directories) by regular expression." Directories are explicitly exempt from the filter: dirOrMatches, Open, Remove, RemoveAll and Stat all treat a directory as automatically passing. So Rename has to delegate.

Returning nil reports success to the caller while the directory is left untouched at the old path and never appears at the new one — a silent data-visibility failure, which is the worst shape for a filesystem wrapper. The name filter is correctly skipped for directories too, so a destination like /renameddir that does not match the regexp must not be rejected.

Note the sibling if dir { return nil } at regexpfs.go:43 is in dirOrMatches and is correct — there, nil means "passes the filter". That one is deliberately left alone.

Reproduction

Comparing RegexpFs against its unfiltered source:

PROBE regexpfs  renameErr=<nil> srcStillThere=true  dstExists=false
PROBE plain     renameErr=<nil> srcStillThere=false dstExists=true

The fix

 	if dir {
-		return nil
+		return r.source.Rename(oldname, newname)
 	}

Tests

TestRegexpFsRenameDir in regexpfs_test.go, run against both the MemMapFs and OsFs backends. It asserts the source is gone, the destination exists, and the contents are readable through the new path.

  • Before, on both backends:
    source directory /regexpdir still exists after Rename
    destination directory /renameddir does not exist after Rename
    ReadFile after Rename failed: file does not exist
    
    After: passes on both.
  • go test ./... -count=1 → 130 top-level PASS, 56 subtest PASS, 0 FAIL, exit 0. go vet ./... clean.
  • Baseline on unmodified HEAD was fully green (afero, mem, tarfs, zipfs all ok), so the suite introduced zero regressions.

regexpfs_test.go previously had zero coverage of Rename. I checked the 30 open PRs: none touch it (#632 RegexpFile is a different file, about WriterTo/ReaderFrom). Issue #327 is about file-handle names after rename and is unrelated.

gcsfs/ and sftpfs/ are separate Go modules and so are not covered by the root ./...; both are untouched.

@januththedev
januththedev force-pushed the fix/regexpfs-rename-dir branch from 5842571 to 41ab900 Compare October 3, 2026 10:31
@CLAassistant

CLAassistant commented Oct 3, 2026 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants