diff --git a/PLAN.md b/PLAN.md index 4c962dad4..e81f25d1c 100644 --- a/PLAN.md +++ b/PLAN.md @@ -507,8 +507,9 @@ Groups are the opposite case: `Command::get_groups`, `ArgGroup::get_args` and requirement declared by its parent while conflicts and the child's own requirements still apply. KDL, usage-lib, typed Rust, generated Go, and the clap bridge agree. - [ ] **Remaining command parsing policy** — - `args_conflicts_with_subcommands`, `subcommand_precedence_over_arg`, and - `allow_missing_positional`. + `args_conflicts_with_subcommands` is now portable across KDL, typed Rust, + usage-lib, generated Go, and the clap bridge. Remaining: + `subcommand_precedence_over_arg` and `allow_missing_positional`. - [x] **`args_override_self`.** Repeated scalar flags are corrections by default; the later value wins. `args_override_self=false` opts a command into duplicate errors. KDL, usage-lib, the typed derive/static metadata, generated Go, and the diff --git a/argv/src/diagnostic.rs b/argv/src/diagnostic.rs index ecd83bdbe..dc77d8fd5 100644 --- a/argv/src/diagnostic.rs +++ b/argv/src/diagnostic.rs @@ -524,6 +524,15 @@ pub fn render( )); } } + Error::SubcommandConflict { subcommand } => { + with_usage = true; + let _ = writeln!( + out, + "{} the subcommand '{}' cannot be used with arguments on its parent command", + style.error("error:"), + style.invalid(subcommand.name) + ); + } Error::MissingRequired { name } => { with_usage = true; let _ = writeln!( diff --git a/argv/src/lib.rs b/argv/src/lib.rs index e63930bfe..aeb15d44c 100644 --- a/argv/src/lib.rs +++ b/argv/src/lib.rs @@ -191,6 +191,9 @@ pub struct Command<'a> { pub arg_required_else_help: bool, /// Selecting a subcommand suppresses this command's required arguments. pub subcommand_negates_reqs: bool, + /// Once this command binds a flag or positional, selecting one of its + /// subcommands is an error. + pub args_conflicts_with_subcommands: bool, /// Disable delimiter splitting for positional values after `--` or on an /// automatic trailing argument. Inherited by subcommands. pub dont_delimit_trailing_values: bool, @@ -237,6 +240,7 @@ impl Command<'_> { external_subcommand: false, arg_required_else_help: false, subcommand_negates_reqs: false, + args_conflicts_with_subcommands: false, dont_delimit_trailing_values: false, unknown_flags: ::core::option::Option::None, version: false, @@ -517,6 +521,8 @@ pub enum Error<'t, 'v> { /// A word was offered to a `double_dash = "required"` argument before any /// `--` had been seen. ArgRequiresDoubleDash { arg: &'t Arg<'t> }, + /// A subcommand was selected after this command had already bound an argument. + SubcommandConflict { subcommand: &'t Command<'t> }, /// The command tree is deeper than [`MAX_DEPTH`]. TooDeep, @@ -1060,6 +1066,10 @@ pub struct Parser<'t, 'a, 'v> { /// Whether any word has been bound to a positional of `cmd`. Once one has, /// no further word can select a subcommand. arg_filled: bool, + /// Whether this command has bound any flag or positional. Unlike + /// `arg_filled`, flags count because clap's command policy treats both as + /// arguments that exclude a later subcommand. + command_arg_found: bool, /// Whether flag interpretation has stopped. A `--` does this, and so does an /// `automatic` argument taking a value. flags_stopped: bool, @@ -1114,6 +1124,7 @@ impl<'t: 'v, 'a, 'v> Parser<'t, 'a, 'v> { arg_pos: 0, arg_taken: 0, arg_filled: false, + command_arg_found: false, flags_stopped: false, separator_seen: false, default_taken: false, @@ -1223,6 +1234,9 @@ impl<'t: 'v, 'a, 'v> Parser<'t, 'a, 'v> { return None; } let event = self.step(); + if matches!(event, Some(Ok(Event::Flag { .. } | Event::Arg { .. }))) { + self.command_arg_found = true; + } if let Some(Err(_)) = event { self.done = true; } @@ -1524,6 +1538,9 @@ impl<'t: 'v, 'a, 'v> Parser<'t, 'a, 'v> { // to equal a subcommand name is just a value. if !self.arg_filled && !self.flags_stopped { if let Some(sub) = self.find_subcommand(token) { + if self.cmd.args_conflicts_with_subcommands && self.command_arg_found { + return Err(Error::SubcommandConflict { subcommand: sub }); + } self.descend(sub)?; return Ok(Event::Command(sub)); } @@ -1678,6 +1695,7 @@ impl<'t: 'v, 'a, 'v> Parser<'t, 'a, 'v> { self.arg_pos = 0; self.arg_taken = 0; self.arg_filled = false; + self.command_arg_found = false; Ok(()) } @@ -1912,6 +1930,13 @@ mod tests { subcommands: &[&INSTALL], ..Command::EMPTY }; + static ARGUMENT_CONFLICT: Command = Command { + name: "ex", + flags: &[&FORCE], + subcommands: &[&INSTALL], + args_conflicts_with_subcommands: true, + ..Command::EMPTY + }; // A CLI shaped exactly like mise's root: a default subcommand, a positional of its own, // and a subcommand under the default — which is the arrangement that tells routing from @@ -2256,6 +2281,15 @@ mod tests { } } + #[test] + fn a_parent_argument_can_exclude_a_later_subcommand() { + let a = argv(["--force", "install"]); + assert!(matches!( + parse(&ARGUMENT_CONFLICT, &a), + Err(Error::SubcommandConflict { subcommand }) if subcommand.name == "install" + )); + } + #[test] fn subcommand_only_routes_before_a_positional_is_filled() { let a = argv(["other", "install"]); diff --git a/argv/src/spec.rs b/argv/src/spec.rs index b7319e5fb..e8600b222 100644 --- a/argv/src/spec.rs +++ b/argv/src/spec.rs @@ -1137,6 +1137,9 @@ impl Spec<'_> { if self.root.cmd.subcommand_negates_reqs { writeln!(out, "subcommand_negates_reqs #true")?; } + if self.root.cmd.args_conflicts_with_subcommands { + writeln!(out, "args_conflicts_with_subcommands #true")?; + } // A `complete` block for every completer this CLI declares, naming the command that // asks the binary itself. Written rather than declared, so there is one place a // completer is said to exist: the Rust function. Everything that reads a spec — the @@ -1389,6 +1392,9 @@ fn write_command<'a>( if meta.cmd.subcommand_negates_reqs { out.push_str(" subcommand_negates_reqs=#true"); } + if meta.cmd.args_conflicts_with_subcommands { + out.push_str(" args_conflicts_with_subcommands=#true"); + } out.push_str(" {\n"); let inner = depth + 1; diff --git a/clap_usage/src/report.rs b/clap_usage/src/report.rs index 9d255b14c..1ebed153c 100644 --- a/clap_usage/src/report.rs +++ b/clap_usage/src/report.rs @@ -14,7 +14,6 @@ pub enum FidelityFeature { DistinctValueNames, GranularHide, AllowMissingPositional, - ArgsConflictWithSubcommands, SubcommandPrecedenceOverArg, FlattenHelp, NextLineHelp, @@ -86,11 +85,6 @@ fn visit(cmd: &Command, ancestors: &[String], losses: &mut BTreeSet TokenStream { let dont_delimit_trailing_values = cli.dont_delimit_trailing_values; let args_override_self = cli.args_override_self; let subcommand_negates_reqs = cli.subcommand_negates_reqs; + let args_conflicts_with_subcommands = cli.args_conflicts_with_subcommands; let usage = option_str(cli.usage.as_deref()); let restart_token = option_str(cli.restart_token.as_deref()); let mount = option_str(cli.mount.as_deref()); @@ -424,6 +425,7 @@ pub fn emit(cli: &Cli) -> TokenStream { unknown_flags: #unknown_flags, arg_required_else_help: #arg_required_else_help, subcommand_negates_reqs: #subcommand_negates_reqs, + args_conflicts_with_subcommands: #args_conflicts_with_subcommands, dont_delimit_trailing_values: #dont_delimit_trailing_values, name: #name, key: #root_key, @@ -3448,6 +3450,7 @@ pub fn emit_args(cli: &Cli) -> TokenStream { let dont_delimit_trailing_values = cli.dont_delimit_trailing_values; let args_override_self = cli.args_override_self; let subcommand_negates_reqs = cli.subcommand_negates_reqs; + let args_conflicts_with_subcommands = cli.args_conflicts_with_subcommands; let before_help = option_expr(cli.before_help.as_ref()); let before_long_help = option_expr(cli.before_long_help.as_ref()); let after_help = option_expr(cli.after_help.as_ref()); @@ -3559,6 +3562,7 @@ pub fn emit_args(cli: &Cli) -> TokenStream { unknown_flags: #unknown_flags, arg_required_else_help: #arg_required_else_help, subcommand_negates_reqs: #subcommand_negates_reqs, + args_conflicts_with_subcommands: #args_conflicts_with_subcommands, dont_delimit_trailing_values: #dont_delimit_trailing_values, flags: #flag_table_ref, args: #arg_table_ref, diff --git a/derive/src/model.rs b/derive/src/model.rs index 0d69b7c98..2eafc5868 100644 --- a/derive/src/model.rs +++ b/derive/src/model.rs @@ -112,6 +112,7 @@ pub struct Cli { pub dont_delimit_trailing_values: bool, pub args_override_self: bool, pub subcommand_negates_reqs: bool, + pub args_conflicts_with_subcommands: bool, /// Declared descriptions, for the case a doc comment cannot express: a long form that does /// not contain the short one. pub about_attr: Option, @@ -558,6 +559,7 @@ impl Cli { dont_delimit_trailing_values: false, args_override_self: true, subcommand_negates_reqs: false, + args_conflicts_with_subcommands: false, about_attr: None, long_about_attr: None, before_help: None, @@ -701,6 +703,9 @@ impl Cli { } "args_override_self" => cli.args_override_self = flag_value(&meta)?, "subcommand_negates_reqs" => cli.subcommand_negates_reqs = flag_value(&meta)?, + "args_conflicts_with_subcommands" => { + cli.args_conflicts_with_subcommands = flag_value(&meta)? + } "restart_token" => cli.restart_token = Some(string_value(&meta)?), "mount" => cli.mount = Some(string_value(&meta)?), "group" => cli.groups.push(group_decl(&meta)?), @@ -712,7 +717,7 @@ impl Cli { format!( "unknown option `{other}` on a struct; usage::Cli takes \ `name`, `name_spec`, `bin`, `bin_spec`, `version`, `version_spec`, `usage`, `verbatim_doc_comment`, `unknown_flags`, \ - `default_subcommand`, `multicall`, `no_binary_name`, `arg_required_else_help`, `dont_delimit_trailing_values`, `args_override_self`, `subcommand_negates_reqs`, \ + `default_subcommand`, `multicall`, `no_binary_name`, `arg_required_else_help`, `dont_delimit_trailing_values`, `args_override_self`, `subcommand_negates_reqs`, `args_conflicts_with_subcommands`, \ `next_help_heading`, `restart_token`, `mount` and \ `group` here, and the description comes from the doc \ comment" diff --git a/docs/rust/clap-compatibility.md b/docs/rust/clap-compatibility.md index b6bc79f55..455fd5930 100644 --- a/docs/rust/clap-compatibility.md +++ b/docs/rust/clap-compatibility.md @@ -98,7 +98,8 @@ the Rust declaration, not only from generated KDL, wherever the bridge column sa | `arg_required_else_help` | yes | yes | yes | yes | yes | yes | Bare selected commands request short help; defaults and environment values do not count as argv. | | `args_override_self` | yes | yes | yes | yes | yes | yes | Usage defaults to permissive last-one-wins behavior; set false for strict duplicate checking. | | `subcommand_negates_reqs` | yes | yes | yes | yes | yes | yes | A selected child suppresses its parent's positive requirements, not conflicts or the child's requirements. | -| remaining subcommand/argument policies | no | no | no | no | no | no | `args_conflicts_with_subcommands`, precedence, and missing positionals are not represented. | +| `args_conflicts_with_subcommands` | yes | yes | yes | yes | yes | yes | Parent flags or positionals exclude a later child subcommand. | +| remaining subcommand/argument policies | no | no | no | no | no | no | Subcommand precedence and missing positionals are not represented. | | unknown flags | different | different | different | different | yes | different | usage is permissive by default; `unknown_flags = "error"` opts into strict parsing. | ## Help, version, and generated artifacts diff --git a/docs/rust/migrating-from-clap.md b/docs/rust/migrating-from-clap.md index 916a09bf6..cd1149e66 100644 --- a/docs/rust/migrating-from-clap.md +++ b/docs/rust/migrating-from-clap.md @@ -76,6 +76,9 @@ command received an argv token; environment and default fallbacks do not count. `#[command(subcommand_negates_reqs)]` also migrates in place. Selecting a child suppresses the parent's positive requirements while leaving conflicts and the child's requirements active. +`#[command(args_conflicts_with_subcommands)]` migrates in place as well. A flag +or positional bound on the parent prevents selecting a later child command. + Container casing also migrates in place. `#[command(rename_all = "snake_case")]` controls inferred field or subcommand names, and `rename_all_env` controls names generated by bare `#[arg(env)]`; an explicit `long`, `name`, or `env = "NAME"` still wins. diff --git a/docs/spec/reference/cmd.md b/docs/spec/reference/cmd.md index 369129dc9..cf1644adc 100644 --- a/docs/spec/reference/cmd.md +++ b/docs/spec/reference/cmd.md @@ -183,6 +183,17 @@ cmd "inspect" `ex inspect` is valid, while bare `ex` still requires `input`. Conflicts continue to apply, and the selected child must still satisfy its own requirements. +### Make parent arguments conflict with subcommands + +`args_conflicts_with_subcommands` makes arguments on a command and its child +subcommands mutually exclusive. Once that command binds a flag or positional, a +later child name is rejected; arguments after a selected child belong to the +child as usual: + +```kdl +args_conflicts_with_subcommands #true +``` + ### Preserve delimiters in trailing values `dont_delimit_trailing_values` keeps a positional token whole after `--`, or once a diff --git a/go/argv/argv.go b/go/argv/argv.go index 423905092..3514a42d4 100644 --- a/go/argv/argv.go +++ b/go/argv/argv.go @@ -72,6 +72,9 @@ type Command struct { ArgRequiredElseHelp bool // SubcommandNegatesReqs lets a selected child satisfy this command's requirements. SubcommandNegatesReqs bool + // ArgsConflictWithSubcommands rejects selecting a child after this command + // has already bound a flag or positional. + ArgsConflictWithSubcommands bool // DontDelimitTrailingValues disables delimiter splitting after -- and for // automatic trailing arguments. It is inherited by subcommands. DontDelimitTrailingValues bool @@ -316,6 +319,8 @@ const ( CodeMissingFlagValue // CodeUnexpectedArg means a word arrived with no argument left to hold it. CodeUnexpectedArg + // CodeSubcommandConflict means an argument was bound before a subcommand. + CodeSubcommandConflict // CodeArgRequiresDoubleDash means a double_dash="required" argument was // offered a word before any -- had been seen. CodeArgRequiresDoubleDash @@ -355,6 +360,7 @@ var codeNames = [...]string{ CodeUnknownFlag: "unknown_flag", CodeMissingFlagValue: "missing_flag_value", CodeUnexpectedArg: "unexpected_arg", + CodeSubcommandConflict: "subcommand_conflict", CodeArgRequiresDoubleDash: "arg_requires_double_dash", CodeTooDeep: "too_deep", CodeHelp: "help", @@ -450,6 +456,8 @@ func (e *Error) Error() string { return "missing value for flag: " + e.Flag.Name case CodeUnexpectedArg: return "unexpected argument: " + safe(e.Token) + case CodeSubcommandConflict: + return "subcommand conflicts with an earlier argument: " + safe(e.Token) case CodeArgRequiresDoubleDash: return "argument requires a -- separator: " + e.Arg.Name case CodeTooDeep: diff --git a/go/argv/parser.go b/go/argv/parser.go index 66e960687..06e4ea11d 100644 --- a/go/argv/parser.go +++ b/go/argv/parser.go @@ -62,6 +62,9 @@ type Parser struct { // argFilled records whether any word has been bound to a positional of cmd. // Once one has, no further word can select a subcommand. argFilled bool + // commandArgFound includes flags as well as positionals for the command + // policy that makes either exclude a later subcommand. + commandArgFound bool // flagsStopped records whether flag interpretation has stopped. A -- does // this, and so does an automatic argument taking a value. flagsStopped bool @@ -187,6 +190,9 @@ func (p *Parser) fail(e Error) bool { // emit records an event and reports that iteration continues. func (p *Parser) emit(e Event) bool { p.event = e + if e.Kind == KindFlag || e.Kind == KindArg { + p.commandArgFound = true + } return true } @@ -474,6 +480,9 @@ func (p *Parser) word(token string) bool { // equal a subcommand name is just a value. if !p.argFilled && !p.flagsStopped { if sub := p.findSubcommand(token); sub != nil { + if p.cmd.ArgsConflictWithSubcommands && p.commandArgFound { + return p.fail(Error{Code: CodeSubcommandConflict, Token: token, Cmd: p.cmd}) + } if !p.descend(sub) { return false } @@ -596,6 +605,7 @@ func (p *Parser) descend(sub *Command) bool { p.argPos = 0 p.argTaken = 0 p.argFilled = false + p.commandArgFound = false return true } diff --git a/go/argv/parser_test.go b/go/argv/parser_test.go index ff8cbe411..b7f965da4 100644 --- a/go/argv/parser_test.go +++ b/go/argv/parser_test.go @@ -31,6 +31,12 @@ var ( Args: []*Arg{file, rest}, Subcommands: []*Command{install}, } + argumentConflict = &Command{ + Name: "ex", + Flags: []*Flag{force}, + Subcommands: []*Command{install}, + ArgsConflictWithSubcommands: true, + } // The same CLI, but one that owns all of its flags and wants typo detection. strictInstall = &Command{ @@ -104,6 +110,7 @@ func TestBinding(t *testing.T) { {"an alias descends", root, []string{"i"}, "cmd:install"}, {"a global reaches a subcommand", root, []string{"install", "--verbose"}, "cmd:install flag:verbose"}, {"a subcommand flag is not in scope above it", root, []string{"--force", "install"}, "flag:force cmd:install"}, + {"a parent argument excludes a later subcommand", argumentConflict, []string{"--force", "install"}, "flag:force err:subcommand_conflict"}, {"only the descent position routes", root, []string{"other", "install"}, "arg:file=other arg:rest=install"}, {"a separator makes values", root, []string{"--", "--force"}, "arg:file=--force"}, {"a second separator is a value", root, []string{"--", "a", "--"}, "arg:file=a arg:rest=--"}, diff --git a/go/internal/spec/spec.go b/go/internal/spec/spec.go index dc56f9d42..916fc942f 100644 --- a/go/internal/spec/spec.go +++ b/go/internal/spec/spec.go @@ -81,27 +81,28 @@ type Completer struct { // Cmd is one command in the lowered spec. type Cmd struct { - Name string `json:"name"` - Hide bool `json:"hide"` - Help string `json:"help"` - HelpLong string `json:"help_long"` - Usage string `json:"usage"` - BeforeHelp string `json:"before_help"` - AfterHelp string `json:"after_help"` - BeforeHelpLong string `json:"before_help_long"` - AfterHelpLong string `json:"after_help_long"` - Examples []Example `json:"examples"` - Aliases []string `json:"aliases"` - HiddenAliases []string `json:"hidden_aliases"` - Subcommands Subcommands `json:"subcommands"` - Args []Arg `json:"args"` - Flags []Flag `json:"flags"` - UnknownFlags *string `json:"unknown_flags"` - ExternalSubcommand bool `json:"external_subcommand"` - ArgRequiredElseHelp bool `json:"arg_required_else_help"` - DontDelimitTrailingValues bool `json:"dont_delimit_trailing_values"` - ArgsOverrideSelf bool `json:"args_override_self"` - SubcommandNegatesReqs bool `json:"subcommand_negates_reqs"` + Name string `json:"name"` + Hide bool `json:"hide"` + Help string `json:"help"` + HelpLong string `json:"help_long"` + Usage string `json:"usage"` + BeforeHelp string `json:"before_help"` + AfterHelp string `json:"after_help"` + BeforeHelpLong string `json:"before_help_long"` + AfterHelpLong string `json:"after_help_long"` + Examples []Example `json:"examples"` + Aliases []string `json:"aliases"` + HiddenAliases []string `json:"hidden_aliases"` + Subcommands Subcommands `json:"subcommands"` + Args []Arg `json:"args"` + Flags []Flag `json:"flags"` + UnknownFlags *string `json:"unknown_flags"` + ExternalSubcommand bool `json:"external_subcommand"` + ArgRequiredElseHelp bool `json:"arg_required_else_help"` + DontDelimitTrailingValues bool `json:"dont_delimit_trailing_values"` + ArgsOverrideSelf bool `json:"args_override_self"` + SubcommandNegatesReqs bool `json:"subcommand_negates_reqs"` + ArgsConflictWithSubcommands bool `json:"args_conflicts_with_subcommands"` } // Subcommands is a command's children, in the order the spec declared them. @@ -603,13 +604,14 @@ func (b *builder) command(c *Cmd, inherited argv.UnknownFlags) *argv.Command { } out := &argv.Command{ - Name: c.Name, - UnknownFlags: unknown, - ExternalSubcommand: c.ExternalSubcommand, - ArgRequiredElseHelp: c.ArgRequiredElseHelp, - SubcommandNegatesReqs: c.SubcommandNegatesReqs, - DontDelimitTrailingValues: c.DontDelimitTrailingValues, - Key: b.next(), + Name: c.Name, + UnknownFlags: unknown, + ExternalSubcommand: c.ExternalSubcommand, + ArgRequiredElseHelp: c.ArgRequiredElseHelp, + SubcommandNegatesReqs: c.SubcommandNegatesReqs, + ArgsConflictWithSubcommands: c.ArgsConflictWithSubcommands, + DontDelimitTrailingValues: c.DontDelimitTrailingValues, + Key: b.next(), } examples := make([]argv.Example, 0, len(c.Examples)) for _, e := range c.Examples { diff --git a/lib/src/docs/models.rs b/lib/src/docs/models.rs index 4de469e03..5c164530b 100644 --- a/lib/src/docs/models.rs +++ b/lib/src/docs/models.rs @@ -486,6 +486,7 @@ impl From<&crate::SpecCommand> for SpecCommand { dont_delimit_trailing_values: _, args_override_self: _, subcommand_negates_reqs: _, + args_conflicts_with_subcommands: _, // Rendered above, or deliberately absent from the docs model. args: _, flags: _, diff --git a/lib/src/go/mod.rs b/lib/src/go/mod.rs index 03a838b2b..077037cb9 100644 --- a/lib/src/go/mod.rs +++ b/lib/src/go/mod.rs @@ -380,6 +380,12 @@ impl<'a> Emitter<'a> { if e.cmd.subcommand_negates_reqs { lines.push(Line::Field("SubcommandNegatesReqs".into(), "true".into())); } + if e.cmd.args_conflicts_with_subcommands { + lines.push(Line::Field( + "ArgsConflictWithSubcommands".into(), + "true".into(), + )); + } if e.cmd.dont_delimit_trailing_values { lines.push(Line::Field( "DontDelimitTrailingValues".into(), @@ -1889,6 +1895,14 @@ cmd "run" arg_required_else_help=#true { ); } + #[test] + fn argument_subcommand_conflicts_reach_generated_go() { + let out = go( + "name \"ex\"\nbin \"ex\"\nargs_conflicts_with_subcommands #true\nflag \"--verbose\"\ncmd \"run\"\n", + ); + assert!(out.contains("ArgsConflictWithSubcommands: true"), "{out}"); + } + #[test] fn strict_duplicate_policy_reaches_metadata() { let permissive = go("name \"ex\"\nbin \"ex\"\nflag \"--jobs \"\n"); diff --git a/lib/src/parse.rs b/lib/src/parse.rs index d6863b9be..c94b32a9c 100644 --- a/lib/src/parse.rs +++ b/lib/src/parse.rs @@ -722,6 +722,7 @@ fn parse_partial_with_env( // drops the parent's non-global flags and a mounted command may declare the same name as // a global seen here. Recording the owner keeps a word bound to the flag it was read as. let mut prefix_bindings: VecDeque, usize)>> = VecDeque::new(); + let mut command_arg_found = false; let mut idx = 0; // Track whether we've already applied the default_subcommand to prevent // multiple switches (e.g., if default is "run" and there's a task named "run") @@ -784,6 +785,12 @@ fn parse_partial_with_env( out.cmd = mounted; } if let Some(subcommand) = out.cmd.find_subcommand(&input[idx]) { + if out.cmd.args_conflicts_with_subcommands && command_arg_found { + bail!( + "subcommand '{}' cannot be used with arguments on its parent command", + input[idx] + ); + } let mut subcommand = subcommand.clone(); // Pass prefix words (global flags before this subcommand) to mount subcommand.mount(&mount_prefix_words(&prefix_flags))?; @@ -803,6 +810,7 @@ fn parse_partial_with_env( // A descent already ran the new command's mounts, above. mounts_resolved = true; prefix_flags.clear(); + command_arg_found = false; // Continue from current position (don't reset to 0) // After remove(), idx now points to the next element } else if !is_command_word(&input[idx]) { @@ -823,6 +831,7 @@ fn parse_partial_with_env( .cloned() .filter(|_| is_bundle) { + command_arg_found = true; // Skip the flag and keep scanning. Both global and non-global flags may precede // a subcommand (`mycli --verbose run task`, `mycli run --force task`), and // stopping at one would hide the subcommand — and any mount on it — from the @@ -874,6 +883,12 @@ fn parse_partial_with_env( .find_subcommand(default_name) .filter(|_| default_accepts_word(&out.cmd, default_name, &input[idx])) { + if out.cmd.args_conflicts_with_subcommands && command_arg_found { + bail!( + "subcommand '{}' cannot be used with arguments on its parent command", + subcommand.name + ); + } let mut subcommand = subcommand.clone(); // Pass prefix words (global flags before this) to mount subcommand.mount(&mount_prefix_words(&prefix_flags))?; @@ -887,6 +902,7 @@ fn parse_partial_with_env( out.cmd = subcommand.clone(); command_has_argv = true; prefix_flags.clear(); + command_arg_found = false; // This descent ran the new command's mounts, so lazy // discovery must not run them a second time. mounts_resolved = true; @@ -4965,6 +4981,26 @@ cmd "run" { flag "--child" required=#true } .expect("parent requires relationships are negated too"); } + #[test] + fn a_parent_argument_can_conflict_with_a_later_subcommand() { + let spec: Spec = r#"name "ex" +bin "ex" +args_conflicts_with_subcommands #true +flag "--verbose" +cmd "run" +"# + .parse() + .unwrap(); + + parse(&spec, &input(&["ex", "run"])) + .expect("the subcommand is valid without a parent argument"); + let err = parse(&spec, &input(&["ex", "--verbose", "run"])).unwrap_err(); + assert!( + err.to_string().contains("cannot be used with arguments"), + "{err}" + ); + } + #[test] fn unknown_flags_can_be_rejected_for_the_whole_cli() { let spec: Spec = diff --git a/lib/src/spec/builder.rs b/lib/src/spec/builder.rs index 3d88904b3..7d568f6b1 100644 --- a/lib/src/spec/builder.rs +++ b/lib/src/spec/builder.rs @@ -849,6 +849,12 @@ impl SpecCommandBuilder { self } + /// Set whether arguments on this command exclude a later subcommand. + pub fn args_conflicts_with_subcommands(mut self, enabled: bool) -> Self { + self.inner.args_conflicts_with_subcommands = enabled; + self + } + /// Set what running this command does to the world pub fn effect(mut self, effect: SpecCommandEffect) -> Self { self.inner.effect = Some(effect); diff --git a/lib/src/spec/cmd.rs b/lib/src/spec/cmd.rs index d13215548..c88af35ff 100644 --- a/lib/src/spec/cmd.rs +++ b/lib/src/spec/cmd.rs @@ -114,6 +114,9 @@ pub struct SpecCommand { /// Whether selecting a subcommand satisfies this command's required arguments. #[serde(skip_serializing_if = "is_false")] pub subcommand_negates_reqs: bool, + /// Whether binding an argument prevents selecting a later subcommand. + #[serde(skip_serializing_if = "is_false")] + pub args_conflicts_with_subcommands: bool, /// Token that resets argument parsing, allowing multiple command invocations. /// e.g., `mise run lint ::: test ::: check` with restart_token=":::" #[serde(skip_serializing_if = "Option::is_none")] @@ -187,6 +190,7 @@ impl Default for SpecCommand { dont_delimit_trailing_values: false, args_override_self: true, subcommand_negates_reqs: false, + args_conflicts_with_subcommands: false, restart_token: None, help: None, help_long: None, @@ -299,6 +303,9 @@ impl SpecCommand { } "args_override_self" => cmd.args_override_self = v.ensure_bool()?, "subcommand_negates_reqs" => cmd.subcommand_negates_reqs = v.ensure_bool()?, + "args_conflicts_with_subcommands" => { + cmd.args_conflicts_with_subcommands = v.ensure_bool()? + } "hide" => cmd.hide = v.ensure_bool()?, "unknown_flags" => { let raw = v.ensure_string()?; @@ -438,6 +445,10 @@ impl SpecCommand { cmd.subcommand_negates_reqs = child.ensure_arg_len(1..=1)?.arg(0)?.ensure_bool()? } + "args_conflicts_with_subcommands" => { + cmd.args_conflicts_with_subcommands = + child.ensure_arg_len(1..=1)?.arg(0)?.ensure_bool()? + } "hide" => cmd.hide = child.ensure_arg_len(1..=1)?.arg(0)?.ensure_bool()?, "effect" => { let arg = child.ensure_arg_len(1..=1)?.arg(0)?; @@ -552,6 +563,7 @@ impl SpecCommand { dont_delimit_trailing_values, args_override_self, subcommand_negates_reqs, + args_conflicts_with_subcommands, restart_token, subcommands, complete, @@ -629,6 +641,7 @@ impl SpecCommand { self.dont_delimit_trailing_values = dont_delimit_trailing_values; self.args_override_self = args_override_self; self.subcommand_negates_reqs = subcommand_negates_reqs; + self.args_conflicts_with_subcommands = args_conflicts_with_subcommands; if effect.is_some() { self.effect = effect; } @@ -736,6 +749,7 @@ impl From<&SpecCommand> for KdlNode { dont_delimit_trailing_values, args_override_self, subcommand_negates_reqs, + args_conflicts_with_subcommands, restart_token, unknown_flags, aliases, @@ -792,6 +806,9 @@ impl From<&SpecCommand> for KdlNode { if *subcommand_negates_reqs { node.push(KdlEntry::new_prop("subcommand_negates_reqs", true)); } + if *args_conflicts_with_subcommands { + node.push(KdlEntry::new_prop("args_conflicts_with_subcommands", true)); + } if let Some(restart_token) = &restart_token { node.entries_mut() .push(KdlEntry::new_prop("restart_token", restart_token.clone())); @@ -1043,6 +1060,7 @@ impl From<&clap::Command> for SpecCommand { spec.dont_delimit_trailing_values = cmd.is_dont_delimit_trailing_values_set(); spec.args_override_self = cmd.is_args_override_self(); spec.subcommand_negates_reqs = cmd.is_subcommand_negates_reqs_set(); + spec.args_conflicts_with_subcommands = cmd.is_args_conflicts_with_subcommands_set(); for subcmd in cmd.get_subcommands() { let mut scmd: SpecCommand = subcmd.into(); scmd.name = subcmd.get_name().to_string(); @@ -1420,4 +1438,18 @@ cmd "hidden" hide=#true let node: kdl::KdlNode = (&spec).into(); assert!(node.to_string().contains("subcommand_negates_reqs=#true")); } + + #[cfg(feature = "clap")] + #[test] + fn the_clap_bridge_preserves_argument_subcommand_conflicts() { + use super::SpecCommand; + + let spec: SpecCommand = + (&clap::Command::new("ex").args_conflicts_with_subcommands(true)).into(); + assert!(spec.args_conflicts_with_subcommands); + let node: kdl::KdlNode = (&spec).into(); + assert!(node + .to_string() + .contains("args_conflicts_with_subcommands=#true")); + } } diff --git a/lib/src/spec/mod.rs b/lib/src/spec/mod.rs index 34ea8d2a4..e67532ac6 100644 --- a/lib/src/spec/mod.rs +++ b/lib/src/spec/mod.rs @@ -310,6 +310,9 @@ impl Spec { "subcommand_negates_reqs" => { schema.cmd.subcommand_negates_reqs = node.arg(0)?.ensure_bool()?; } + "args_conflicts_with_subcommands" => { + schema.cmd.args_conflicts_with_subcommands = node.arg(0)?.ensure_bool()?; + } "example" => { let code = node.ensure_arg_len(1..=1)?.arg(0)?.ensure_string()?; let mut example = SpecExample::new(code.trim().to_string()); @@ -608,6 +611,11 @@ impl Display for Spec { node.push(true); nodes.push(node); } + if self.cmd.args_conflicts_with_subcommands { + let mut node = KdlNode::new("args_conflicts_with_subcommands"); + node.push(true); + nodes.push(node); + } if !self.usage.is_empty() { let mut node = KdlNode::new("usage"); node.push(string_entry(None, &self.usage)); diff --git a/usage-rs/tests/facade.rs b/usage-rs/tests/facade.rs index 49f329f9d..4e23e5aaa 100644 --- a/usage-rs/tests/facade.rs +++ b/usage-rs/tests/facade.rs @@ -43,6 +43,22 @@ enum NegatedCommand { Show, } +#[derive(Cli)] +#[command(bin = "argument-conflict", args_conflicts_with_subcommands)] +#[allow(dead_code)] +struct ArgumentConflict { + #[arg(long)] + verbose: bool, + #[command(subcommand)] + command: Option, +} + +#[derive(Subcommands)] +#[allow(dead_code)] +enum ArgumentConflictCommand { + Run, +} + #[derive(Subcommands, serde::Deserialize)] enum InlineCommand { /// Run a named benchmark. @@ -952,6 +968,22 @@ fn typed_subcommands_negate_only_parent_requirements() { assert!(kdl.contains("subcommand_negates_reqs #true"), "{kdl}"); } +#[test] +fn typed_parent_arguments_exclude_a_later_subcommand() { + ArgumentConflict::parse_from(&[OsStr::new("run")]) + .expect("a subcommand without parent arguments remains valid"); + assert!( + ArgumentConflict::parse_from(&[OsStr::new("--verbose"), OsStr::new("run")]).is_err(), + "a parent flag must exclude a later subcommand" + ); + + let kdl = ArgumentConflict::to_kdl(); + assert!( + kdl.contains("args_conflicts_with_subcommands #true"), + "{kdl}" + ); +} + #[test] fn emitted_parser_settings_are_portable_spec_metadata() { let Err(_) = StrictEx::parse_from(&[OsStr::new("--wat")]) else {