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

src/runtime/shell/Builtin.rs

@@ -1077,22 +1077,22 @@ impl Builtin {
         e: &ParseError,
         set_wait_err: impl FnOnce(),
     ) -> Yield {
-        let buf: Vec<u8> = match e {
-            ParseError::IllegalOption(_) => Self::fmt_error_arena(
+        let buf: Vec<u8> = match *e {
+            ParseError::IllegalOption(ch) => Self::fmt_error_arena(
                 interp,
                 cmd,
                 Some(kind),
-                format_args!("illegal option -- {}\n", bstr::BStr::new(e.opt())),
+                format_args!("illegal option -- {}\n", bstr::BStr::new(&[ch])),
             )
             .to_vec(),
             ParseError::ShowUsage => kind.usage_string().to_vec(),
-            ParseError::Unsupported(_) => Self::fmt_error_arena(
+            ParseError::Unsupported(name) => Self::fmt_error_arena(
                 interp,
                 cmd,
                 Some(kind),
                 format_args!(
                     "unsupported option, please open a GitHub issue -- {}\n",
-                    bstr::BStr::new(e.opt())
+                    bstr::BStr::new(name)
                 ),
             )
             .to_vec(),

src/runtime/shell/builtin/cat.rs

@@ -410,7 +410,7 @@ impl FlagParser for Opts {
         None
     }
 
-    fn parse_short(&mut self, ch: u8, smallflags: &[u8], i: usize) -> Option<ParseFlagResult> {
+    fn parse_short(&mut self, ch: u8) -> Option<ParseFlagResult> {
         match ch {
             b'b' => Some(ParseFlagResult::Unsupported(unsupported_flag(b"-b"))),
             b'e' => Some(ParseFlagResult::Unsupported(unsupported_flag(b"-e"))),
@@ -419,9 +419,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(ch)),
         }
     }
 }

src/runtime/shell/builtin/cp.rs

@@ -792,7 +792,7 @@ impl FlagParser for Opts {
         None
     }
 
-    fn parse_short(&mut self, ch: u8, smallflags: &[u8], i: usize) -> Option<ParseFlagResult> {
+    fn parse_short(&mut self, ch: u8) -> Option<ParseFlagResult> {
         match ch {
             b'f' => Some(ParseFlagResult::Unsupported(unsupported_flag(b"-f"))),
             b'H' => Some(ParseFlagResult::Unsupported(unsupported_flag(b"-H"))),
@@ -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(ch)),
         }
     }
 }

src/runtime/shell/builtin/ls.rs

@@ -44,7 +44,8 @@ pub struct ExecState {
 enum ParseFlag {
     ContinueParsing,
     Done,
-    IllegalOption(Box<[u8]>),
+    /// The rejected option byte, as getopt(3) reports it.
+    IllegalOption(u8),
 }
 
 impl Ls {
@@ -72,12 +73,12 @@ impl Ls {
                 // which case we run once with ".".
                 let paths_start = match Self::parse_opts(interp, cmd) {
                     Ok(p) => p,
-                    Err(opt) => {
+                    Err(ch) => {
                         let buf: Vec<u8> = Builtin::fmt_error_arena(
                             interp,
                             cmd,
                             Some(Kind::Ls),
-                            format_args!("illegal option -- {}\n", bstr::BStr::new(&opt[..])),
+                            format_args!("illegal option -- {}\n", bstr::BStr::new(&[ch])),
                         )
                         .to_vec();
                         Self::state_mut(interp, cmd).state = State::WaitingWriteErr;
@@ -221,7 +222,7 @@ impl Ls {
 
     /// Returns the index of the first non-flag arg, or `None` if there are no
     /// positional args. `Err` carries the offending flag byte.
-    fn parse_opts(interp: &Interpreter, cmd: NodeId) -> Result<Option<usize>, Box<[u8]>> {
+    fn parse_opts(interp: &Interpreter, cmd: NodeId) -> Result<Option<usize>, u8> {
         let argc = Builtin::of(interp, cmd).args_slice().len();
         if argc == 0 {
             return Ok(None);
@@ -232,7 +233,7 @@ impl Ls {
             match Self::parse_flag(&mut Self::state_mut(interp, cmd).opts, flag) {
                 ParseFlag::Done => return Ok(Some(idx)),
                 ParseFlag::ContinueParsing => {}
-                ParseFlag::IllegalOption(s) => return Err(s),
+                ParseFlag::IllegalOption(ch) => return Err(ch),
             }
             idx += 1;
         }
@@ -245,7 +246,7 @@ impl Ls {
         }
         // FIXME windows
         if flag.len() == 1 {
-            return ParseFlag::IllegalOption(Box::from(&b"-"[..]));
+            return ParseFlag::IllegalOption(b'-');
         }
         for &ch in &flag[1..] {
             match ch {
@@ -259,7 +260,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(ch),
             }
         }
         ParseFlag::ContinueParsing

src/runtime/shell/builtin/mkdir.rs

@@ -443,7 +443,7 @@ impl FlagParser for Opts {
         None
     }
 
-    fn parse_short(&mut self, ch: u8, smallflags: &[u8], i: usize) -> Option<ParseFlagResult> {
+    fn parse_short(&mut self, ch: u8) -> Option<ParseFlagResult> {
         match ch {
             b'm' => Some(ParseFlagResult::Unsupported(unsupported_flag(b"-m "))),
             b'p' => {
@@ -454,9 +454,7 @@ impl FlagParser for Opts {
                 self.verbose = true;
                 None
             }
-            _ => Some(ParseFlagResult::IllegalOption(
-                &raw const smallflags[1 + i..],
-            )),
+            _ => Some(ParseFlagResult::IllegalOption(ch)),
         }
     }
 }

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,8 @@ pub enum PromptBehaviour {
 enum RmParseFlag {
     ContinueParsing,
     Done,
-    IllegalOption,
-    IllegalOptionWithFlag,
+    /// The rejected option byte, as getopt(3) reports it (`-` for an unknown `--long` option).
+    IllegalOption(u8),
 }
 
 impl Rm {
@@ -240,40 +240,15 @@ impl Rm {
                             });
                             continue;
                         }
-                        RmParseFlag::IllegalOption => {
-                            return Self::write_err_literal(
-                                interp,
-                                cmd,
-                                idx,
-                                b"rm: illegal option -- -\n",
-                            );
-                        }
-                        RmParseFlag::IllegalOptionWithFlag => {
-                            if let Some(safeguard) = Builtin::of(interp, cmd).stderr.needs_io() {
-                                Self::state_mut(interp, cmd).state = RmState::ParseOpts {
-                                    idx,
-                                    wait_write_err: true,
-                                };
-                                let child = ChildPtr::new(cmd, WriterTag::Builtin);
-                                return Builtin::of_mut(interp, cmd).stderr.enqueue_fmt(
-                                    child,
-                                    Some(Kind::Rm),
-                                    format_args!(
-                                        "illegal option -- {}\n",
-                                        bstr::BStr::new(&arg[1..])
-                                    ),
-                                    safeguard,
-                                );
-                            }
+                        RmParseFlag::IllegalOption(ch) => {
                             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);
-                            return Builtin::done(interp, cmd, 1);
+                            return Self::write_err_literal(interp, cmd, idx, &buf);
                         }
                     }
                 }
@@ -518,7 +493,7 @@ impl Rm {
                     opts.prompt_behaviour = PromptBehaviour::Always;
                     RmParseFlag::ContinueParsing
                 }
-                _ => RmParseFlag::IllegalOption,
+                _ => RmParseFlag::IllegalOption(b'-'),
             };
         }
         for &ch in &flag[1..] {
@@ -532,7 +507,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::IllegalOption(ch),
             }
         }
         RmParseFlag::ContinueParsing

src/runtime/shell/builtin/touch.rs

@@ -364,7 +364,7 @@ impl FlagParser for Opts {
         }
     }
 
-    fn parse_short(&mut self, ch: u8, smallflags: &[u8], i: usize) -> Option<ParseFlagResult> {
+    fn parse_short(&mut self, ch: u8) -> Option<ParseFlagResult> {
         match ch {
             b'a' => Some(ParseFlagResult::Unsupported(unsupported_flag(b"-a"))),
             b'c' => Some(ParseFlagResult::Unsupported(unsupported_flag(b"-c"))),
@@ -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(ch)),
         }
     }
 }

src/runtime/shell/interpreter.rs

@@ -2303,52 +2303,34 @@
 // ────────────────────────────────────────────────────────────────────────────
 
 /// Custom parse error for invalid options.
-///
-/// Payload slices borrow from the builtin's argv (NUL-terminated arena strings)
-/// or are `'static` literals; the builtin formats them into an error message
-/// before the next argv mutation, so a raw fat pointer is safe.
 pub(crate) enum ParseError {
-    IllegalOption(*const [u8]),
-    Unsupported(*const [u8]),
+    /// The rejected option byte, as getopt(3) reports it.
+    IllegalOption(u8),
+    Unsupported(&'static [u8]),
     ShowUsage,
 }
 
-impl ParseError {
-    /// Borrow the option-name payload. The pointer borrows either a `'static`
-    /// literal (e.g. `b"-"`) or the owning `Builtin`'s argv storage
-    /// (NUL-terminated `Vec<u8>` in `Cmd::args`, live for the `Builtin`'s
-    /// lifetime — see [`Builtin::arg_bytes`](crate::shell::builtin::Builtin::arg_bytes)).
-    /// Builtins format the error before any argv mutation.
-    #[inline]
-    pub(crate) fn opt(&self) -> &[u8] {
-        match self {
-            // SAFETY: see doc comment.
-            ParseError::IllegalOption(s) | ParseError::Unsupported(s) => unsafe { &**s },
-            ParseError::ShowUsage => b"",
-        }
-    }
-}
-
 pub enum ParseFlagResult {
     ContinueParsing,
     Done,
-    IllegalOption(*const [u8]),
-    Unsupported(*const [u8]),
+    /// The rejected option byte, as getopt(3) reports it.
+    IllegalOption(u8),
+    Unsupported(&'static [u8]),
 }
 
 /// Returns just `name` and lets the caller's `fmt_error_arena` add the
 /// "unsupported option" prefix once.
 #[inline]
-pub(crate) const fn unsupported_flag(name: &'static [u8]) -> *const [u8] {
-    std::ptr::from_ref::<[u8]>(name)
+pub(crate) const fn unsupported_flag(name: &'static [u8]) -> &'static [u8] {
+    name
 }
 
 /// 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.
-    fn parse_short(&mut self, ch: u8, smallflags: &[u8], i: usize) -> Option<ParseFlagResult>;
+    fn parse_short(&mut self, ch: u8) -> Option<ParseFlagResult>;
 }
 
 /// Returns the trailing non-flag args (`args[idx..]`) on success.
@@ -2379,16 +2361,15 @@
         return ParseFlagResult::Done;
     }
     if flag.len() == 1 {
-        return ParseFlagResult::IllegalOption(std::ptr::from_ref::<[u8]>(b"-"));
+        return ParseFlagResult::IllegalOption(b'-');
     }
     if flag.len() > 2 && flag[1] == b'-' {
         if let Some(r) = opts.parse_long(flag) {
             return r;
         }
     }
-    let small_flags = &flag[1..];
-    for (i, &ch) in small_flags.iter().enumerate() {
-        if let Some(r) = opts.parse_short(ch, small_flags, i) {
+    for &ch in &flag[1..] {
+        if let Some(r) = opts.parse_short(ch) {
             return r;
         }
     }

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 () => {