fix(argv): inherit unknown_flags, which reached one command out of a tree

PLAN.md

@@ -268,6 +268,16 @@ tasks --usage"`, so task names are meant to come from running that. usage-argv d
       flatten leaves behind while the group's `build` still demands one — and is a compile
       error now, asserted during const evaluation in the parent's expansion, where the group
       is only a type.
+- [x] **`unknown_flags`, which reached one command out of a tree** — usage-lib resolves it by
+      walking outward from the command that ran, so a root declaring `error` makes the whole
+      CLI strict. usage-argv held the effective value per command instead, on the theory that
+      whoever built the tables would resolve it — which a derive cannot, since it expands one
+      struct at a time and cannot see the command above. So the attribute reached the root
+      alone, and on an `Args` it parsed and was then ignored: a declaration that compiled and
+      did nothing. Now `None` means inherit, the parser carries the effective value down as it
+      descends, and the corpus's own table builder stops resolving it — one implementation of
+      the rule instead of two, and it was the second one that hid the parser not having it.
+      Costs **160 instructions per parse, 72,272 against 72,112** at mise's scale.
 - [ ] **`subcommand_required` on the root command** — the same restriction as the root
       mount, and found beside it: the spec accepts the property only inside a `cmd` block,
       so a CLI whose _root_ cannot be run alone has no way to say so. The clap bridge could

argv/src/lib.rs

@@ -153,9 +153,19 @@ pub struct Command<'a> {
     /// Resolve it with [`find_subcommand`], which turns a name that no subcommand answers to
     /// into a compile error.
     pub default_subcommand: ::core::option::Option<&'a Command<'a>>,
-    /// What an unrecognized flag-like token means here. Already resolved — see
-    /// [`UnknownFlags`].
-    pub unknown_flags: UnknownFlags,
+    /// What an unrecognized flag-like token means here, or `None` to keep whatever the
+    /// enclosing command said. See [`UnknownFlags`].
+    ///
+    /// Inherited rather than resolved per command, which is what usage-lib does — its
+    /// `effective_unknown_flags` walks outward from the command that ran and falls back to
+    /// the spec's. Resolving it in the tables instead was possible only for a builder that
+    /// can see the whole tree: a derive expands one struct at a time and cannot see its
+    /// parent, so `#[usage(unknown_flags = "error")]` on the root reached the root alone and
+    /// a subcommand had no way to say it at all.
+    ///
+    /// The parser carries the effective value down as it descends, so a command that states
+    /// nothing costs nothing.
+    pub unknown_flags: ::core::option::Option<UnknownFlags>,
     /// Whether this command answers to `--version` and `-V`.
     ///
     /// Set on the root, and only when the CLI declares a version: clap adds the flag exactly
@@ -183,7 +193,7 @@ impl Command<'_> {
         args: &[],
         subcommands: &[],
         default_subcommand: ::core::option::Option::None,
-        unknown_flags: UnknownFlags::Value,
+        unknown_flags: ::core::option::Option::None,
         version: false,
         key: 0,
     };
@@ -817,6 +827,12 @@ pub struct Parser<'t, 'v> {
     pos: usize,
     /// The command currently in scope.
     cmd: &'t Command<'t>,
+    /// What an unrecognized flag-like token means in the command currently in scope.
+    ///
+    /// Carried rather than looked up, because it is inherited: a command that states
+    /// nothing keeps what the enclosing one said, and walking back up the ancestors on
+    /// every unrecognized token would pay for the inheritance at the wrong moment.
+    unknown_flags: UnknownFlags,
     /// The chain above `cmd`, used to find inherited global flags. Fixed size so
     /// that nothing is allocated.
     ancestors: [Option<&'t Command<'t>>; MAX_DEPTH],
@@ -878,6 +894,11 @@ impl<'t, 'v> Parser<'t, 'v> {
             argv,
             pos: 0,
             cmd: root,
+            unknown_flags: match root.unknown_flags {
+                ::core::option::Option::Some(mode) => mode,
+                // Nothing above the root to inherit from, so the default stands.
+                ::core::option::Option::None => UnknownFlags::Value,
+            },
             ancestors: [None; MAX_DEPTH],
             depth: 0,
             bundle: &[],
@@ -1082,7 +1103,7 @@ impl<'t, 'v> Parser<'t, 'v> {
             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 => {
+                Err(e) if self.unknown_flags == UnknownFlags::Error => {
                     return Some(Err(e));
                 }
                 Err(_) => return Some(self.word(token)),
@@ -1149,7 +1170,7 @@ impl<'t, 'v> Parser<'t, 'v> {
             });
         }
 
-        if self.cmd.unknown_flags == UnknownFlags::Error {
+        if self.unknown_flags == UnknownFlags::Error {
             return Err(Error::UnknownFlag { token });
         }
         // Not a flag here, so it is a word like any other.
@@ -1330,6 +1351,10 @@ impl<'t, 'v> Parser<'t, 'v> {
         self.starts[self.depth] = self.cmd_start;
         self.depth += 1;
         self.cmd = sub;
+        // Only a command that says something changes it, which is what inheriting means.
+        if let ::core::option::Option::Some(mode) = sub.unknown_flags {
+            self.unknown_flags = mode;
+        }
         // Where this command's own words start, which is what lets a completion hand a callback
         // the half-parsed struct of the command it was declared on rather than of the root.
         self.cmd_start = self.pos;
@@ -1523,14 +1548,12 @@ 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.
+    /// Same shape as ROOT, but a CLI that owns all of its flags. The subcommand says
+    /// nothing and inherits it, which is the point: only the root declares the mode.
     static STRICT_INSTALL: Command = Command {
         name: "install",
         aliases: &["i"],
         flags: &[&FORCE],
-        unknown_flags: UnknownFlags::Error,
         key: 100,
         ..Command::EMPTY
     };
@@ -1539,7 +1562,7 @@ mod tests {
         flags: &[&FORCE, &JOBS, &COLOR, &VERBOSE],
         args: &[&FILE, &REST],
         subcommands: &[&STRICT_INSTALL],
-        unknown_flags: UnknownFlags::Error,
+        unknown_flags: Some(UnknownFlags::Error),
         ..Command::EMPTY
     };
     static ROOT: Command = Command {

argv/src/spec.rs

@@ -642,7 +642,7 @@ impl Spec<'_> {
         }
         // Written only when it is not the default, so an ordinary spec stays quiet
         // about it.
-        if self.root.cmd.unknown_flags == UnknownFlags::Error {
+        if self.root.cmd.unknown_flags == Some(UnknownFlags::Error) {
             prop(out, "unknown_flags", "error")?;
         }
         if let Some(default_subcommand) = self.default_subcommand {
@@ -695,7 +695,14 @@ impl Spec<'_> {
         for example in self.root.examples {
             write_example(out, example, 0)?;
         }
-        write_body(out, self.root, 0, self.bin.unwrap_or(self.name))
+        // Nothing above the root, so what it does not state is the default.
+        write_body(
+            out,
+            self.root,
+            0,
+            UnknownFlags::Value,
+            self.bin.unwrap_or(self.name),
+        )
     }
 }
 
@@ -707,9 +714,12 @@ fn write_body(
     out: &mut String,
     meta: &CommandMeta<'_>,
     depth: usize,
+    inherited_unknown_flags: UnknownFlags,
     bin: &str,
 ) -> core::fmt::Result {
-    let enclosing_unknown_flags = meta.cmd.unknown_flags;
+    // The effective setting for everything inside, which is this command's if it stated one
+    // and otherwise whatever it inherited.
+    let enclosing_unknown_flags = meta.cmd.unknown_flags.unwrap_or(inherited_unknown_flags);
     // Indexing by metadata position below cannot see a table entry with no
     // metadata, which would be silently unwritten. Check the lengths first.
     debug_assert_eq!(
@@ -775,15 +785,16 @@ fn write_command(
     if let Some(effect) = meta.effect {
         write!(out, " effect={}", quoted(effect.as_str()))?;
     }
-    // Written only where it changes, since the spec inherits it. The tables hold the
-    // effective value per command, so repeating the enclosing command's answer would
-    // say nothing — but a command that differs has to say so, or the setting is lost
-    // on the way out.
-    if meta.cmd.unknown_flags != inherited_unknown_flags {
+    // Written only where it changes, since the spec inherits it as the tables do: a command
+    // that states nothing has nothing to write, and one that restates what it inherited would
+    // be saying the same thing twice. A command that differs has to say so, or the setting is
+    // lost on the way out.
+    let effective_unknown_flags = meta.cmd.unknown_flags.unwrap_or(inherited_unknown_flags);
+    if effective_unknown_flags != inherited_unknown_flags {
         write!(
             out,
             " unknown_flags={}",
-            quoted(match meta.cmd.unknown_flags {
+            quoted(match effective_unknown_flags {
                 UnknownFlags::Value => "value",
                 UnknownFlags::Error => "error",
             })
@@ -832,7 +843,7 @@ fn write_command(
     for example in meta.examples {
         write_example(out, example, inner)?;
     }
-    write_body(out, meta, inner, bin)?;
+    write_body(out, meta, inner, effective_unknown_flags, bin)?;
 
     indent(out, depth)?;
     out.push_str("}\n");
@@ -1511,41 +1522,49 @@ mod tests {
 
     #[test]
     fn a_subcommand_writes_unknown_flags_only_where_it_differs() {
-        // The tables hold the effective value per command, so repeating the enclosing
-        // command's answer says nothing — but a command that differs has to say so, or
-        // the setting never reaches the spec.
+        // A command that restates what it inherited says nothing — and one that says
+        // nothing at all has nothing to write either — but a command that differs has to
+        // say so, or the setting never reaches the spec.
         static STRICT_SUB: Command = Command {
             name: "build",
-            unknown_flags: UnknownFlags::Error,
+            unknown_flags: Some(UnknownFlags::Error),
+            ..Command::EMPTY
+        };
+        static SILENT_SUB: Command = Command {
+            name: "test",
             ..Command::EMPTY
         };
         static LENIENT_SUB: Command = Command {
             name: "exec",
-            unknown_flags: UnknownFlags::Value,
+            unknown_flags: Some(UnknownFlags::Value),
             ..Command::EMPTY
         };
         static ROOT: Command = Command {
             name: "ex",
-            subcommands: &[&STRICT_SUB, &LENIENT_SUB],
-            unknown_flags: UnknownFlags::Error,
+            subcommands: &[&STRICT_SUB, &SILENT_SUB, &LENIENT_SUB],
+            unknown_flags: Some(UnknownFlags::Error),
             ..Command::EMPTY
         };
         static STRICT_META: CommandMeta = CommandMeta {
             cmd: &STRICT_SUB,
             ..CommandMeta::EMPTY
         };
+        static SILENT_META: CommandMeta = CommandMeta {
+            cmd: &SILENT_SUB,
+            ..CommandMeta::EMPTY
+        };
         static LENIENT_META: CommandMeta = CommandMeta {
             cmd: &LENIENT_SUB,
             ..CommandMeta::EMPTY
         };
         static ROOT_META: CommandMeta = CommandMeta {
             cmd: &ROOT,
-            subcommands: &[&STRICT_META, &LENIENT_META],
+            subcommands: &[&STRICT_META, &SILENT_META, &LENIENT_META],
             ..CommandMeta::EMPTY
         };
 
         let mut out = String::new();
-        write_body(&mut out, &ROOT_META, 0, "ex").unwrap();
+        write_body(&mut out, &ROOT_META, 0, UnknownFlags::Value, "ex").unwrap();
 
         // Counted rather than checked with `contains`, which is how a duplicated
         // write survived review: `unknown_flags="value" unknown_flags="value"` contains
@@ -1565,6 +1584,12 @@ mod tests {
             "a subcommand matching the enclosing command should not repeat it: {build}"
         );
 
+        let test = line("test");
+        assert!(
+            !test.contains("unknown_flags"),
+            "a subcommand that declares nothing inherits, and writes nothing: {test}"
+        );
+
         let exec = line("exec");
         assert_eq!(
             exec.matches(r#"unknown_flags="value""#).count(),

conformance/src/argv.rs

@@ -60,12 +60,11 @@ pub fn run(vector: &Vector) -> Outcome {
         return Outcome::OutOfScope(reason);
     }
 
-    // Inheritance is resolved here, not in the parser: usage-argv's tables hold the
-    // effective value per command, which is what a derive would emit.
-    let root = build(
-        &spec.cmd,
-        convert_unknown_flags(spec.unknown_flags.unwrap_or_default()),
-    );
+    // The spec's own setting belongs to the root command, which is where usage-argv's tables
+    // hold it. Everything below inherits it, which the parser now does itself rather than
+    // this flattening it on the way in — a second implementation of the same rule, and the
+    // one that hid the parser not having it.
+    let root = build(&spec.cmd, spec.unknown_flags.map(convert_unknown_flags));
     // `default_subcommand` is a property of the spec rather than of a command, so it is
     // resolved once, here, against the root's own subcommands. A name that answers to
     // nothing is left as None: the spec is what it is, and a vector that expects routing
@@ -225,16 +224,17 @@ fn out_of_scope(vector: &Vector) -> Option<&'static str> {
 
 /// Build leaked tables mirroring a spec command.
 ///
-/// `inherited_unknown_flags` is the effective setting from above, which a command
-/// that states nothing keeps and passes down.
+/// `unknown_flags` is carried through as the spec states it — `None` where a command says
+/// nothing — because the parser inherits it. The root takes the spec-level setting, since
+/// that is the command a spec's own property describes.
 fn build(
     cmd: &SpecCommand,
-    inherited_unknown_flags: ArgvUnknownFlags,
+    root_unknown_flags: Option<ArgvUnknownFlags>,
 ) -> &'static Command<'static> {
     let unknown_flags = cmd
         .unknown_flags
         .map(convert_unknown_flags)
-        .unwrap_or(inherited_unknown_flags);
+        .or(root_unknown_flags);
     let flags: Vec<&'static Flag<'static>> = cmd
         .flags
         .iter()
@@ -295,7 +295,9 @@ fn build(
     let subcommands: Vec<&'static Command<'static>> = cmd
         .subcommands
         .values()
-        .map(|sub| build(sub, unknown_flags))
+        // A subcommand states its own or says nothing; there is no spec-level setting to
+        // hand it, since the root has already taken that.
+        .map(|sub| build(sub, None))
         .collect();
 
     let aliases: Vec<&'static str> = cmd