shell: name the rejected flag in the builtins' illegal option errors

src/runtime/shell/builtin/cat.rs

@@ -3,7 +3,8 @@ use std::sync::Arc;
 use crate::shell::ExitCode;
 use crate::shell::builtin::{Builtin, BuiltinIO, BuiltinInput, BuiltinState, IoKind, Kind};
 use crate::shell::interpreter::{
-    FlagParser, Interpreter, NodeId, ParseFlagResult, parse_flags, shell_openat, unsupported_flag,
+    FlagParser, Interpreter, NodeId, ParseFlagResult, illegal_flag, parse_flags, shell_openat,
+    unsupported_flag,
 };
 use crate::shell::io_reader::{ChildPtr as ReaderChildPtr, IOReader, ReaderTag};
 use crate::shell::io_writer::{ChildPtr, WriterTag};
@@ -419,9 +420,7 @@ impl FlagParser for Opts {
             b't' => Some(ParseFlagResult::Unsupported(unsupported_flag(b"-t"))),
             b'u' => Some(ParseFlagResult::Unsupported(unsupported_flag(b"-u"))),
             b'v' => Some(ParseFlagResult::Unsupported(unsupported_flag(b"-v"))),
-            _ => Some(ParseFlagResult::IllegalOption(
-                &raw const smallflags[1 + i..],
-            )),
+            _ => Some(ParseFlagResult::IllegalOption(illegal_flag(smallflags, i))),
         }
     }
 }

src/runtime/shell/builtin/cp.rs

@@ -3,7 +3,7 @@ use bun_paths::resolve_path;
 use crate::shell::builtin::{Builtin, BuiltinState, IoKind, Kind};
 use crate::shell::interpreter::{
     EventLoopHandle, FlagParser, Interpreter, NodeId, OutputSrc, OutputTask, OutputTaskVTable,
-    ParseFlagResult, ShellTask, parse_flags, unsupported_flag,
+    ParseFlagResult, ShellTask, illegal_flag, parse_flags, unsupported_flag,
 };
 use crate::shell::io_writer::{ChildPtr, WriterTag};
 use crate::shell::yield_::Yield;
@@ -809,7 +809,7 @@ impl FlagParser for Opts {
                 Some(ParseFlagResult::ContinueParsing)
             }
             b'n' => Some(ParseFlagResult::ContinueParsing),
-            _ => Some(ParseFlagResult::IllegalOption(&raw const smallflags[i..])),
+            _ => Some(ParseFlagResult::IllegalOption(illegal_flag(smallflags, i))),
         }
     }
 }

src/runtime/shell/builtin/ls.rs

@@ -259,7 +259,7 @@ impl Ls {
                 | b'h' | b'H' | b'i' | b'I' | b'k' | b'L' | b'm' | b'n' | b'N' | b'o' | b'p'
                 | b'q' | b'Q' | b's' | b'S' | b't' | b'T' | b'u' | b'U' | b'v' | b'w' | b'x'
                 | b'X' | b'Z' => {}
-                _ => return ParseFlag::IllegalOption(Box::from(&flag[1..2])),
+                _ => return ParseFlag::IllegalOption(Box::from([ch])),
             }
         }
         ParseFlag::ContinueParsing

src/runtime/shell/builtin/mkdir.rs

@@ -4,7 +4,7 @@ use crate::shell::ExitCode;
 use crate::shell::builtin::{Builtin, BuiltinState, IoKind, Kind};
 use crate::shell::interpreter::{
     EventLoopHandle, FlagParser, Interpreter, NodeId, OutputSrc, OutputTask, OutputTaskVTable,
-    ParseFlagResult, ShellTask, parse_flags, unsupported_flag,
+    ParseFlagResult, ShellTask, illegal_flag, parse_flags, unsupported_flag,
 };
 use crate::shell::io_writer::{ChildPtr, WriterTag};
 use crate::shell::yield_::Yield;
@@ -454,9 +454,7 @@ impl FlagParser for Opts {
                 self.verbose = true;
                 None
             }
-            _ => Some(ParseFlagResult::IllegalOption(
-                &raw const smallflags[1 + i..],
-            )),
+            _ => Some(ParseFlagResult::IllegalOption(illegal_flag(smallflags, i))),
         }
     }
 }

src/runtime/shell/builtin/mv.rs

@@ -46,7 +46,8 @@ pub enum MvState {
 
 /// mv uses its own simpler parser.
 enum MvParseError {
-    IllegalOption(&'static [u8]),
+    /// The rejected option byte.
+    IllegalOption(u8),
     ShowUsage,
 }
 
@@ -94,11 +95,11 @@ impl Mv {
             Tag::Idle => {
                 if let Err(e) = Self::parse_opts(interp, cmd) {
                     let buf: Vec<u8> = match e {
-                        MvParseError::IllegalOption(s) => Builtin::fmt_error_arena(
+                        MvParseError::IllegalOption(ch) => Builtin::fmt_error_arena(
                             interp,
                             cmd,
                             Some(Kind::Mv),
-                            format_args!("illegal option -- {}\n", bstr::BStr::new(s)),
+                            format_args!("illegal option -- {}\n", bstr::BStr::new(&[ch])),
                         )
                         .to_vec(),
                         MvParseError::ShowUsage => Kind::Mv.usage_string().to_vec(),
@@ -360,7 +361,7 @@ impl Mv {
                     return Ok(());
                 }
                 MvFlag::ContinueParsing => {}
-                MvFlag::IllegalOption(s) => return Err(MvParseError::IllegalOption(s)),
+                MvFlag::IllegalOption(ch) => return Err(MvParseError::IllegalOption(ch)),
             }
             idx += 1;
         }
@@ -374,7 +375,7 @@ impl Mv {
         for &ch in &flag[1..] {
             match ch {
                 b'f' | b'h' | b'i' | b'n' | b'v' => {}
-                _ => return MvFlag::IllegalOption(b"-"),
+                _ => return MvFlag::IllegalOption(ch),
             }
         }
         MvFlag::ContinueParsing
@@ -396,7 +397,8 @@ impl Drop for Mv {
 enum MvFlag {
     ContinueParsing,
     Done,
-    IllegalOption(&'static [u8]),
+    /// The rejected option byte.
+    IllegalOption(u8),
 }
 
 /// `openat(target, O_RDONLY|O_DIRECTORY)`

src/runtime/shell/builtin/rm.rs

@@ -92,8 +92,10 @@ pub enum PromptBehaviour {
 enum RmParseFlag {
     ContinueParsing,
     Done,
+    /// Unknown `--long` option.
     IllegalOption,
-    IllegalOptionWithFlag,
+    /// Unknown short option: the rejected byte.
+    IllegalOptionWithFlag(u8),
 }
 
 impl Rm {
@@ -248,7 +250,7 @@ impl Rm {
                                 b"rm: illegal option -- -\n",
                             );
                         }
-                        RmParseFlag::IllegalOptionWithFlag => {
+                        RmParseFlag::IllegalOptionWithFlag(ch) => {
                             if let Some(safeguard) = Builtin::of(interp, cmd).stderr.needs_io() {
                                 Self::state_mut(interp, cmd).state = RmState::ParseOpts {
                                     idx,
@@ -258,18 +260,15 @@ impl Rm {
                                 return Builtin::of_mut(interp, cmd).stderr.enqueue_fmt(
                                     child,
                                     Some(Kind::Rm),
-                                    format_args!(
-                                        "illegal option -- {}\n",
-                                        bstr::BStr::new(&arg[1..])
-                                    ),
+                                    format_args!("illegal option -- {}\n", bstr::BStr::new(&[ch])),
                                     safeguard,
                                 );
                             }
                             let buf = Builtin::fmt_error_arena(
                                 interp,
                                 cmd,
                                 Some(Kind::Rm),
-                                format_args!("illegal option -- {}\n", bstr::BStr::new(&arg[1..])),
+                                format_args!("illegal option -- {}\n", bstr::BStr::new(&[ch])),
                             )
                             .to_vec();
                             let _ = Builtin::write_no_io(interp, cmd, IoKind::Stderr, &buf);
@@ -532,7 +531,7 @@ impl Rm {
                 b'd' => opts.remove_empty_dirs = true,
                 b'i' => opts.prompt_behaviour = PromptBehaviour::Once { removed_count: 0 },
                 b'I' => opts.prompt_behaviour = PromptBehaviour::Always,
-                _ => return RmParseFlag::IllegalOptionWithFlag,
+                _ => return RmParseFlag::IllegalOptionWithFlag(ch),
             }
         }
         RmParseFlag::ContinueParsing

src/runtime/shell/builtin/touch.rs

@@ -2,7 +2,7 @@ use crate::shell::ExitCode;
 use crate::shell::builtin::{Builtin, BuiltinState, IoKind, Kind};
 use crate::shell::interpreter::{
     EventLoopHandle, FlagParser, Interpreter, NodeId, OutputSrc, OutputTask, OutputTaskVTable,
-    ParseFlagResult, ShellTask, parse_flags, unsupported_flag,
+    ParseFlagResult, ShellTask, illegal_flag, parse_flags, unsupported_flag,
 };
 use crate::shell::io_writer::{ChildPtr, WriterTag};
 use crate::shell::yield_::Yield;
@@ -373,9 +373,7 @@ impl FlagParser for Opts {
             b'm' => Some(ParseFlagResult::Unsupported(unsupported_flag(b"-m"))),
             b'r' => Some(ParseFlagResult::Unsupported(unsupported_flag(b"-r"))),
             b't' => Some(ParseFlagResult::Unsupported(unsupported_flag(b"-t"))),
-            _ => Some(ParseFlagResult::IllegalOption(
-                &raw const smallflags[1 + i..],
-            )),
+            _ => Some(ParseFlagResult::IllegalOption(illegal_flag(smallflags, i))),
         }
     }
 }

src/runtime/shell/interpreter.rs

@@ -2343,11 +2343,19 @@ pub(crate) const fn unsupported_flag(name: &'static [u8]) -> *const [u8] {
     std::ptr::from_ref::<[u8]>(name)
 }
 
+/// `IllegalOption` payload for the byte `FlagParser::parse_short` is rejecting:
+/// that one byte (all getopt(3) names either), borrowed from argv.
+#[inline]
+pub(crate) fn illegal_flag(smallflags: &[u8], i: usize) -> *const [u8] {
+    &raw const smallflags[i..=i]
+}
+
 /// Per-builtin opts type implements this to plug into `FlagParser::parse_flags`.
 pub trait FlagParser {
     /// Handle a `--long` flag. Return `None` to fall through to short parsing.
     fn parse_long(&mut self, flag: &[u8]) -> Option<ParseFlagResult>;
-    /// Handle one byte of a `-abc` cluster. Return `None` to keep iterating.
+    /// Handle one byte of a `-abc` cluster (`ch == smallflags[i]`). Return `None`
+    /// to keep iterating; reject an unknown byte with [`illegal_flag`].
     fn parse_short(&mut self, ch: u8, smallflags: &[u8], i: usize) -> Option<ParseFlagResult>;
 }
 

test/js/bun/shell/commands/ls.test.ts

@@ -288,17 +288,11 @@ describe.concurrent("bunshell ls", () => {
     });
 
     test("invalid flag", async () => {
-      await TestBuilder.command`ls -z`
-        .exitCode(1)
-        .stderr(s => expect(s).toContain("illegal option"))
-        .run();
+      await TestBuilder.command`ls -z`.exitCode(1).stderr("ls: illegal option -- z\n").run();
     });
 
     test("invalid combined flags", async () => {
-      await TestBuilder.command`ls -az`
-        .exitCode(1)
-        .stderr(s => expect(s).toContain("illegal option"))
-        .run();
+      await TestBuilder.command`ls -az`.exitCode(1).stderr("ls: illegal option -- z\n").run();
     });
 
     test.if(isPosix)("permission denied directory", async () => {

test/js/bun/shell/exec.test.ts

@@ -46,8 +46,8 @@ describe("bun exec", () => {
     // prettier-ignore
     const programs = [
       // ["cat",    1, "", ""],
-      ["touch",  1, "touch: illegal option -- help\n", ""],
-      ["mkdir",  1, "mkdir: illegal option -- help\n", ""],
+      ["touch",  1, "touch: illegal option -- -\n", ""],
+      ["mkdir",  1, "mkdir: illegal option -- -\n", ""],
       // ["cd",     1, "cd: no such file or directory: --help\n", ""],
       ["echo",   0, "", "--help\n"],
       ["pwd",    1, "pwd: too many arguments\n", ""],
@@ -71,6 +71,33 @@ describe("bun exec", () => {
     }
   });
 
+  // Like getopt(3), the message names the one byte that was rejected, wherever
+  // it sits in a cluster of short flags. An unknown --long option is rejected
+  // at its second `-`, so it is reported as `-` (what BSD getopt prints too).
+  describe.concurrent("illegal option names the rejected flag", () => {
+    // prettier-ignore
+    const programs: [program: string, cases: [args: string, rejected: string][]][] = [
+      ["cat",   [["-z", "z"], ["-zb", "z"],                 ["--bogus", "-"]]],
+      ["touch", [["-z", "z"], ["-za", "z"],                 ["--bogus", "-"]]],
+      ["mkdir", [["-z", "z"], ["-pz", "z"], ["-zp", "z"],   ["--bogus", "-"]]],
+      ["cp",    [["-z", "z"], ["-zR", "z"],                 ["--bogus", "-"]]],
+      ["ls",    [["-z", "z"], ["-az", "z"], ["-za", "z"],   ["--bogus", "-"]]],
+      ["rm",    [["-z", "z"], ["-rz", "z"], ["-zr", "z"],   ["--bogus", "-"]]],
+      ["mv",    [["-z", "z"], ["-fz", "z"], ["-zf", "z"],   ["--bogus", "-"]]],
+    ];
+    for (const [program, cases] of programs) {
+      for (const [args, rejected] of cases) {
+        TestBuilder.command`${BUN} exec ${`${program} ${args}`}`
+          // cat and cp are builtins only on Windows unless this flag is set.
+          .env({ ...bunEnv, BUN_ENABLE_EXPERIMENTAL_SHELL_BUILTINS: "1" })
+          .exitCode(1)
+          .stderr(`${program}: illegal option -- ${rejected}\n`)
+          .stdout("")
+          .runAsTest(`${program} ${args}`);
+      }
+    }
+  });
+
   TestBuilder.command`${BUN} exec cd`
     .env(bunEnv)
     .exitCode(0)