fix: MemMapFs.Remove returns error for non-empty directories

[Yanhu007]

Fixes #232

Problem

MemMapFs.Remove() silently removes a non-empty directory and all its contents. This violates the os.Remove contract, which returns ENOTEMPTY for non-empty directories.

fs := afero.NewMemMapFs()
fs.MkdirAll("/dirname", 0755)
afero.WriteFile(fs, "/dirname/file", []byte("data"), 0644)
fs.Remove("/dirname") // succeeds, should fail with ENOTEMPTY

Fix

Before removing a directory, check if any entries have the directory path as a prefix. If children exist, return &os.PathError{Op: "remove", Path: name, Err: syscall.ENOTEMPTY}.

This matches the behavior of os.Remove on real filesystems.

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[afurm]

The prefix check strings.HasPrefix(p, prefix) with prefix = name + FilePathSeparator correctly identifies children at one level. However, does the in-memory file system also support deep nesting like /dir/subdir/file.txt? If so, the check would also need to handle that the child path itself could have arbitrary depth. For a MemMapFs, the key question is whether a child at any depth under name should cause the remove to fail, or only direct children. The POSIX ENOTEMPTY typically applies to direct children only — but the current check also catches grandchildren since strings.HasPrefix("/dir/subdir", "/dir/") is true. Is that the intended behavior, or should the check be more precise?

[spf13]

@Yanhu007 the fix is correct and the downstream impact analysis looks good — we checked the major consumers and they all either pre-check emptiness or use RemoveAll for trees. Two things blocking merge:

1. CLA not signed. Please sign at https://cla-assistant.io/spf13/afero — takes a minute.
2. No test. Please add a test that: creates a non-empty directory, calls Remove, expects ENOTEMPTY; and a second case that removes an empty directory, expects success.

The implementation (prefix scan, syscall.ENOTEMPTY, PathError wrapper) is correct as-is.

[spf13]

The fix is correct — os.Remove returns ENOTEMPTY for non-empty directories, and MemMapFs should match. But this is a real behavioral break: code currently using Remove on a non-empty directory as a lazy recursive delete will go from silently succeeding to getting an error. That's a meaningful change for anyone using MemMapFs as a test double.

Before merging, we need to think through the impact:

1. RemoveAll is the right tool for recursive deletion — but are users actually calling it, or are they leaning on Remove working permissively in MemMapFs?
2. This warrants at minimum a clear changelog entry and likely a minor version bump.
3. The O(n) linear scan of all keys on every Remove call (while holding the write lock) is also a performance concern for large in-memory filesystems — that should be addressed before this lands.

The intent is right. The implementation needs the performance issue resolved, and we need to be deliberate about how we communicate this change to users.