Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 24 additions & 10 deletions src/runtime/shell/builtin/rm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,14 +134,16 @@
panic!("Invalid");
}
let argc = Builtin::of(interp, cmd).args_slice().len();
// No args / only flags → print usage and exit 1.
if (idx as usize) >= argc {
let usage = Kind::Rm.usage_string();
return Self::write_err_literal(interp, cmd, idx, usage);
}

let arg = Builtin::of(interp, cmd).arg_bytes(idx as usize).to_vec();
match Self::parse_flag(&mut Self::state_mut(interp, cmd).opts, &arg) {
// Running out of arguments while still parsing flags means
// the (empty) operand list starts here.
Comment thread
robobun marked this conversation as resolved.
Outdated
let (arg, parsed) = if (idx as usize) < argc {
let arg = Builtin::of(interp, cmd).arg_bytes(idx as usize).to_vec();
let parsed = Self::parse_flag(&mut Self::state_mut(interp, cmd).opts, &arg);
(arg, parsed)
} else {
(Vec::new(), RmParseFlag::Done)
};
match parsed {
RmParseFlag::ContinueParsing => {
if let RmState::ParseOpts { idx: i, .. } =
&mut Self::state_mut(interp, cmd).state
Expand All @@ -151,6 +153,20 @@
continue;
}
RmParseFlag::Done => {
let args_start = idx as usize;
// No operands. POSIX: `-f` suppresses both the diagnostic
// and the failure status in that case; otherwise it is a
// usage error. Decided before the prompt-flag rejection
// below so that `rm -i` is a usage error too, as in GNU
// and BSD rm.
Comment thread
robobun marked this conversation as resolved.
Outdated
if args_start >= argc {
if Self::state_mut(interp, cmd).opts.force {
return Builtin::done(interp, cmd, 0);
}

Check warning on line 165 in src/runtime/shell/builtin/rm.rs

View check run for this annotation

Claude / Claude Code Review

-i/-I/--interactive do not clear force, so rm -f -i exits 0 instead of usage error

The PR description says `-i`, `-I`, `--interactive=once` and `--interactive=always` "now clear `force`" so that `rm -f -i` with no operands is still a usage error, but `parse_flag()` was not modified — those four arms only set `opts.prompt_behaviour` and leave `opts.force` alone, so `rm -f -i` exits 0 silently. The description also claims 15 test cases including `rm -i -f` and "each prompting flag after -f still print usage and exit 1", but the diff only has 11 and none of the -f/-i ordering tes
Comment thread
robobun marked this conversation as resolved.
let usage = Kind::Rm.usage_string();
return Self::write_err_literal(interp, cmd, idx, usage);
}

// `-r` implies `-d`.
{
let opts = &mut Self::state_mut(interp, cmd).opts;
Expand All @@ -166,8 +182,6 @@
return Self::write_err_literal(interp, cmd, idx, buf);
}

let args_start = idx as usize;

// Check that none of the paths will delete the root.
{
let cwd = Builtin::shell(interp, cmd).cwd().to_vec();
Expand Down
28 changes: 28 additions & 0 deletions test/js/bun/shell/commands/rm.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,34 @@ describe.concurrent("bunshell rm", () => {
}
});

// POSIX rm: -f shall "not write diagnostic messages or modify the exit
// status in the case of no file operands". GNU and BSD rm exit 0 silently,
// which is what `rm -f ${files}` with an empty list relies on. Without -f,
// no operands is still a usage error.
describe("no operands", () => {
const none: string[] = [];
const usage = "usage: rm [-f | -i] [-dIPRrvWx] file ...\n unlink [--] file\n";

TestBuilder.command`rm -f`.stdout("").stderr("").exitCode(0).runAsTest("rm -f");
TestBuilder.command`rm -f`.quiet().stdout("").stderr("").exitCode(0).runAsTest("rm -f (quiet)");
TestBuilder.command`rm -f ${none}`.stdout("").stderr("").exitCode(0).runAsTest("rm -f with an empty list");
TestBuilder.command`rm -rf`.stdout("").stderr("").exitCode(0).runAsTest("rm -rf");
TestBuilder.command`rm -fv`.stdout("").stderr("").exitCode(0).runAsTest("rm -fv");
TestBuilder.command`rm -f ${none} && echo cleaned`
.stdout("cleaned\n")
.stderr("")
.exitCode(0)
.runAsTest("rm -f succeeds in a && chain");

TestBuilder.command`rm`.stdout("").stderr(usage).exitCode(1).runAsTest("rm");
TestBuilder.command`rm -r`.stdout("").stderr(usage).exitCode(1).runAsTest("rm -r");
TestBuilder.command`rm -rv`.quiet().stdout("").stderr(usage).exitCode(1).runAsTest("rm -rv (quiet)");
// Missing operands are reported before the unsupported prompting mode is.
TestBuilder.command`rm -i`.stdout("").stderr(usage).exitCode(1).runAsTest("rm -i");
// -f only covers the missing operands, not a bad flag.
TestBuilder.command`rm -f -x`.stdout("").stderr("rm: illegal option -- x\n").exitCode(1).runAsTest("rm -f -x");
});

test("recursive", async () => {
const files = {
"existent.txt": "",
Expand Down
Loading