feat(spec): make unknown flags configurable, and keep them as values

argv/src/lib.rs

@@ -101,6 +101,9 @@ pub struct Command<'a> {
     /// Positional arguments, in the order they are filled.
     pub args: &'a [&'a Arg<'a>],
     pub subcommands: &'a [&'a Command<'a>],
+    /// What an unrecognized flag-like token means here. Already resolved — see
+    /// [`UnknownFlags`].
+    pub unknown_flags: UnknownFlags,
     /// Caller-assigned identifier, echoed back in [`Event::Command`].
     pub key: u32,
 }
@@ -113,6 +116,7 @@ impl Command<'_> {
         flags: &[],
         args: &[],
         subcommands: &[],
+        unknown_flags: UnknownFlags::Value,
         key: 0,
     };
 }
@@ -198,6 +202,25 @@ impl Arg<'_> {
     };
 }
 
+/// What to do with a flag-like token that names no flag in scope.
+///
+/// The default is [`UnknownFlags::Value`]: the token carries on to the positional
+/// arguments, because a spec is often parsing a command line whose flags belong to
+/// something else — a wrapped tool, a task script. A CLI that owns all of its
+/// flags declares [`UnknownFlags::Error`] and gets typo detection instead.
+///
+/// Stored per command and already resolved: inheritance is a question for whoever
+/// builds the tables, and answering it at compile time keeps it out of the parse.
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)]
+pub enum UnknownFlags {
+    /// Offer the token to the positionals. If none can take it, it is an
+    /// unexpected argument.
+    #[default]
+    Value,
+    /// Reject the token.
+    Error,
+}
+
 /// How an argument relates to the `--` separator.
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Default)]
 pub enum DoubleDash {
@@ -422,8 +445,13 @@ impl<'t, 'v> Parser<'t, 'v> {
             // out one at a time, so discovering an unknown letter half way
             // through would mean the earlier letters had already been applied —
             // and the grammar rejects the entire token, not the tail of it.
-            if let Err(e) = self.check_bundle(token) {
-                return Some(Err(e));
+            match self.check_bundle(token) {
+                Ok(()) => {}
+                // Unrecognized, so it is a word unless this command wants it refused.
+                Err(e) if self.cmd.unknown_flags == UnknownFlags::Error => {
+                    return Some(Err(e));
+                }
+                Err(_) => return Some(self.word(token)),
             }
             self.bundle = &token[1..];
             self.bundle_token = token;
@@ -467,7 +495,11 @@ impl<'t, 'v> Parser<'t, 'v> {
             });
         }
 
-        Err(Error::UnknownFlag { token })
+        if self.cmd.unknown_flags == UnknownFlags::Error {
+            return Err(Error::UnknownFlag { token });
+        }
+        // Not a flag here, so it is a word like any other.
+        self.word(token)
     }
 
     /// Walk a short-flag token without binding anything, to find out whether all
@@ -647,11 +679,57 @@ fn bytes<'v>(s: &'v &'v OsStr) -> &'v [u8] {
 /// without which no CLI could accept `--offset -1`.
 fn is_flag_like(token: &[u8]) -> bool {
     match token {
-        [b'-', rest @ ..] if !rest.is_empty() => !rest[0].is_ascii_digit(),
+        [b'-', rest @ ..] if !rest.is_empty() => !is_number(rest),
         _ => false,
     }
 }
 
+/// Whether the text after a `-` is a number, so `-1`, `-2.5`, and `-1e5` are values
+/// while `-1x` is a flag-shaped token that names nothing.
+///
+/// Digits, at most one `.`, and an optional exponent. Deliberately narrower than
+/// `f64::from_str`, which also accepts `inf` and `NaN` — `-inf` is far likelier to be
+/// a misspelled flag than a number somebody meant to pass.
+///
+/// usage-lib applies the same rule, and the corpus pins the edges so the two cannot
+/// drift apart: they disagreed about `-1e5` when this was a hand-rolled scanner on
+/// one side and a float parse on the other.
+///
+/// Written out rather than deferred to `f64::from_str` because this runs on the hot
+/// path, and a parse would mean a UTF-8 check on a slice already decided by its
+/// bytes.
+fn is_number(rest: &[u8]) -> bool {
+    let (mantissa, exponent) = match rest.iter().position(|b| matches!(b, b'e' | b'E')) {
+        Some(at) => (&rest[..at], Some(&rest[at + 1..])),
+        None => (rest, None),
+    };
+
+    let mut seen_digit = false;
+    let mut seen_dot = false;
+    for &b in mantissa {
+        match b {
+            b'0'..=b'9' => seen_digit = true,
+            b'.' if !seen_dot => seen_dot = true,
+            _ => return false,
+        }
+    }
+    if !seen_digit {
+        return false;
+    }
+
+    match exponent {
+        None => true,
+        // An exponent needs digits of its own, and may carry a sign.
+        Some(exp) => {
+            let digits = exp
+                .strip_prefix(b"+")
+                .or_else(|| exp.strip_prefix(b"-"))
+                .unwrap_or(exp);
+            !digits.is_empty() && digits.iter().all(|b| b.is_ascii_digit())
+        }
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -698,6 +776,25 @@ mod tests {
         key: 100,
         ..Command::EMPTY
     };
+    /// Same shape as ROOT, but a CLI that owns all of its flags. The subcommand
+    /// carries the setting too: the tables hold it already resolved, because
+    /// inheritance is the table builder's job rather than the parser's.
+    static STRICT_INSTALL: Command = Command {
+        name: "install",
+        aliases: &["i"],
+        flags: &[&FORCE],
+        unknown_flags: UnknownFlags::Error,
+        key: 100,
+        ..Command::EMPTY
+    };
+    static STRICT: Command = Command {
+        name: "ex",
+        flags: &[&FORCE, &JOBS, &COLOR, &VERBOSE],
+        args: &[&FILE, &REST],
+        subcommands: &[&STRICT_INSTALL],
+        unknown_flags: UnknownFlags::Error,
+        ..Command::EMPTY
+    };
     static ROOT: Command = Command {
         name: "ex",
         flags: &[&FORCE, &JOBS, &COLOR, &VERBOSE],
@@ -790,13 +887,57 @@ mod tests {
 
     #[test]
     fn no_abbreviation() {
+        // A prefix names no flag, so by default it is a value like any other word.
         let a = argv(["--forc"]);
+        assert_eq!(
+            parse(&ROOT, &a).unwrap(),
+            vec![Event::Arg {
+                arg: &FILE,
+                value: b"--forc"
+            }]
+        );
+
+        // And a CLI that owns its flags hears about it, which is the whole reason
+        // the strict mode exists.
         assert!(matches!(
-            parse(&ROOT, &a),
+            parse(&STRICT, &a),
             Err(Error::UnknownFlag { token: b"--forc" })
         ));
     }
 
+    #[test]
+    fn an_unknown_flag_is_a_value_by_default() {
+        // The default, and the case it is for: a command line being forwarded to
+        // something whose flags this spec does not know.
+        let a = argv(["--wat", "keep"]);
+        assert_eq!(
+            parse(&ROOT, &a).unwrap(),
+            vec![
+                Event::Arg {
+                    arg: &FILE,
+                    value: b"--wat"
+                },
+                Event::Arg {
+                    arg: &REST,
+                    value: b"keep"
+                },
+            ]
+        );
+
+        // With nowhere to put it, it is an unexpected argument — the same error an
+        // extra word gets, rather than a special one about flags.
+        static ONE: Command = Command {
+            name: "ex",
+            args: &[&FILE],
+            ..Command::EMPTY
+        };
+        let a = argv(["a", "--wat"]);
+        assert_eq!(
+            parse(&ONE, &a),
+            Err(Error::UnexpectedArg { token: b"--wat" })
+        );
+    }
+
     #[test]
     fn negation() {
         let a = argv(["--no-color"]);
@@ -920,9 +1061,16 @@ mod tests {
             ]
         );
 
-        // `--jobs` belongs to the root and is not global.
+        // `--jobs` belongs to the root and is not global, so it is not a flag here.
+        // Strictly that is an unknown flag; leniently it is a word, and `install`
+        // declares no argument to hold one — either way it is never read as the
+        // root's flag, which is what this test is about.
         let a = argv(["install", "--jobs", "8"]);
-        assert!(matches!(parse(&ROOT, &a), Err(Error::UnknownFlag { .. })));
+        assert!(matches!(parse(&STRICT, &a), Err(Error::UnknownFlag { .. })));
+        assert!(matches!(
+            parse(&ROOT, &a),
+            Err(Error::UnexpectedArg { token: b"--jobs" })
+        ));
     }
 
     #[test]
@@ -1161,21 +1309,32 @@ mod tests {
         // flag event came out first, a caller would have applied `-f` from a
         // command line that was rejected.
         let a = argv(["-fz"]);
-        let mut parser = Parser::new(&ROOT, &a);
+        let mut parser = Parser::new(&STRICT, &a);
         assert_eq!(
             parser.next_event(),
             Some(Err(Error::UnknownFlag { token: b"-fz" })),
             "an unknown letter must reject the token before any of it is applied"
         );
         assert!(parser.next_event().is_none());
+
+        // Leniently, the same token is a value — and `-f` is *not* applied, since
+        // the token was never a bundle at all.
+        let a = argv(["-fz"]);
+        assert_eq!(
+            parse(&ROOT, &a).unwrap(),
+            vec![Event::Arg {
+                arg: &FILE,
+                value: b"-fz"
+            }]
+        );
     }
 
     #[test]
     fn unknown_short_error_names_the_whole_token() {
         for (tokens, want) in [(["-z"], &b"-z"[..]), (["-fz"], &b"-fz"[..])] {
             let a = argv(tokens);
             assert_eq!(
-                parse(&ROOT, &a),
+                parse(&STRICT, &a),
                 Err(Error::UnknownFlag { token: want }),
                 "{tokens:?}"
             );
@@ -1185,7 +1344,7 @@ mod tests {
     #[test]
     fn errors_are_terminal() {
         let a = argv(["--wat", "--force"]);
-        let mut parser = Parser::new(&ROOT, &a);
+        let mut parser = Parser::new(&STRICT, &a);
         assert!(parser.next_event().unwrap().is_err());
         assert!(parser.next_event().is_none());
     }