diff --git a/argv/src/diagnostic.rs b/argv/src/diagnostic.rs index a6709fd2a..b79f6b214 100644 --- a/argv/src/diagnostic.rs +++ b/argv/src/diagnostic.rs @@ -288,6 +288,41 @@ fn shown<'a>(meta: Option<&'a CommandMeta<'a>>, name: &str) -> String { name.to_string() } +/// A group member is stored as a selector (`--file` or `-f`), not a field name. +fn group_member_shown(meta: Option<&CommandMeta<'_>>, selector: &str) -> String { + let Some(meta) = meta else { + return selector.to_string(); + }; + let found = meta.flags.iter().find(|flag| { + flag.flag + .longs + .iter() + .any(|long| selector == format!("--{long}")) + || flag + .flag + .shorts + .iter() + .any(|short| selector == format!("-{}", *short as char)) + || flag + .flag + .negate + .is_some_and(|negate| selector == format!("--{negate}")) + }); + found + .map(|flag| { + let mut shown = crate::help::flag_spelling(flag); + if flag.flag.takes_value { + let name = flag.value_name.unwrap_or(flag.flag.name); + let _ = write!(shown, " <{name}>"); + if flag.flag.variadic { + shown.push('…'); + } + } + shown + }) + .unwrap_or_else(|| selector.to_string()) +} + /// The word that was bound to a named argument, recovered from argv. /// /// The parse itself does not carry it: an error that owned the offending text would allocate on @@ -591,6 +626,20 @@ pub fn render( invalid.reason ); } + Error::MissingGroup { group, members } => { + with_usage = true; + // clap's own shape for a required group, which is the required-arguments + // message with the members listed under it. The group's name goes on the + // first line rather than into the list, since it is not something to type. + let _ = writeln!( + out, + "{} one of the following required arguments was not provided ({group}):", + style.error("error:") + ); + for member in *members { + let _ = writeln!(out, " {}", style.valid(&group_member_shown(here, member))); + } + } Error::ConflictingFlags { name, other } => { // Spelled by `help`, like every other name in this module — and like clap, which // writes `the argument '--force' cannot be used with '--jobs '`. @@ -818,6 +867,46 @@ mod tests { assert_eq!(line, crate::help::usage_line(&["ex", "use"], &USE_META)); } + #[test] + fn a_missing_group_lists_value_taking_members_completely() { + static FILE: Flag = Flag { + name: "file", + longs: &["file"], + takes_value: true, + ..Flag::BOOL + }; + static ROOT: Command = Command { + name: "grouped", + flags: &[&FILE], + ..Command::EMPTY + }; + static META: CommandMeta = CommandMeta { + cmd: &ROOT, + flags: &[FlagMeta { + flag: &FILE, + value_name: Some("PATH"), + ..FlagMeta::EMPTY + }], + ..CommandMeta::EMPTY + }; + static SPEC: Spec = Spec { + name: "grouped", + bin: Some("grouped"), + root: &META, + ..Spec::EMPTY + }; + let message = render( + &SPEC, + &[], + &Error::MissingGroup { + group: "input", + members: &["--file"], + }, + Style::PLAIN, + ); + assert!(message.contains(" --file "), "{message}"); + } + #[test] fn a_missing_subcommand_prints_the_choices() { let message = rendered(&[], Error::MissingSubcommand); diff --git a/argv/src/lib.rs b/argv/src/lib.rs index 904fb0cf6..277fa1d54 100644 --- a/argv/src/lib.rs +++ b/argv/src/lib.rs @@ -454,6 +454,18 @@ pub enum Error<'t, 'v> { /// does not grow. A value that will not convert has already failed, and a message /// worth reading is worth one allocation. InvalidValue(::std::boxed::Box>), + /// A required group had none of its members given. + /// + /// Carries the members as members rather than as a rendered sentence: the caller + /// decides how to say it, and a completion asking what would satisfy this needs the + /// list rather than the prose. + MissingGroup { + /// The group's declared name, which appears in the message so a command with + /// several groups does not report the same sentence twice. + group: &'t str, + /// The flags that would satisfy it, as the declaration spells them. + members: &'t [&'t str], + }, /// A subcommand was required, and none was given. MissingSubcommand, /// `--help` or `-h` was given, and `cmd` is what it was asked about. diff --git a/argv/src/spec.rs b/argv/src/spec.rs index 2576009d3..e12981c60 100644 --- a/argv/src/spec.rs +++ b/argv/src/spec.rs @@ -78,6 +78,27 @@ fn duplicate_flag_form(cmd: &Command<'_>) -> Option { .find_map(|sub| duplicate_flag_form(sub)) } +/// A group name that two declarations on the same command both claim, if any. +/// +/// Within one struct the derive catches this, and `#[usage(flatten)]` joins declarations +/// from two expansions that cannot see each other — so a parent and the struct it +/// flattens can each declare `input`, and each then checks only its own members. One +/// member from either side would satisfy neither exclusion, and the emitted KDL would +/// carry two `group "input"` nodes saying different things. +/// +/// Checked here for the same reason duplicate flag forms are: this is where the joined +/// tables are visible. +fn duplicate_group_name(meta: &CommandMeta<'_>) -> Option { + let mut names: std::vec::Vec<&str> = meta.groups.iter().map(|g| g.name).collect(); + names.sort_unstable(); + if let Some(pair) = names.windows(2).find(|pair| pair[0] == pair[1]) { + return Some(pair[0].to_string()); + } + meta.subcommands + .iter() + .find_map(|sub| duplicate_group_name(sub)) +} + /// An argument that no word could ever reach, if any. /// /// An unbounded variadic takes every remaining word, so what follows it can never be filled — @@ -390,6 +411,79 @@ pub const fn concat_aliases(groups: &[&[&'static str]]) -> [&'st out } +/// A set of one command's flags that relate to one another as a set. +/// +/// Pairwise `conflicts` can say "at most one of these", once per pair; what it cannot +/// say is that one of them is *needed*, which is a statement about the set. +#[derive(Debug, Clone, Copy)] +pub struct GroupMeta<'a> { + /// What the group is called. It appears in the message a failed check produces, and + /// it is how a reader tells two groups on one command apart. + pub name: &'a str, + /// The flags in the group, as selectors — `--long` or `-s`, the way every other + /// relationship names a flag. + pub members: &'a [&'a str], + /// Whether at least one member has to be given. + pub required: bool, + /// Whether more than one member may be given. False is what makes a bare group + /// mutual exclusion, as it does in clap. + pub multiple: bool, +} + +impl GroupMeta<'_> { + /// A group with nothing in it, for the array initialiser a const concat needs. + pub const EMPTY: GroupMeta<'static> = GroupMeta { + name: "", + members: &[], + required: false, + multiple: false, + }; +} + +/// Join groups of group metadata into one, at compile time. +/// +/// The same shape as [`concat_flag_metas`], and needed for the same reason: a flattened +/// struct's groups describe flags that are now in the parent's table, so they belong in +/// the parent's emitted spec. Without this a group declared on a flattened struct would +/// be enforced — the child's own `check` runs — and invisible to the KDL, which is +/// exactly the drift the spec-as-definition rule exists to prevent. +/// +/// `N` must be [`table_len`](crate::table_len) of the same groups. +pub const fn concat_group_metas( + groups: &[&[GroupMeta<'static>]], +) -> [GroupMeta<'static>; N] { + let mut out = [GroupMeta::EMPTY; N]; + let mut at = 0; + let mut g = 0; + while g < groups.len() { + let group = groups[g]; + let mut i = 0; + while i < group.len() { + // This function initialises a generated `static`, so a collision across a parent + // and a flattened child is rejected while the adopter compiles. Leaving this to + // `to_kdl` let direct parsing enforce two independent groups with the same name. + let mut seen = 0; + while seen < at { + assert!( + !crate::str_eq(out[seen].name, group[i].name), + "two flattened groups on one command have the same name" + ); + seen += 1; + } + out[at] = group[i]; + at += 1; + i += 1; + } + g += 1; + } + assert!( + at == N, + "`N` must be `table_len` of the same groups, or the metadata would describe a \ + group that does not exist" + ); + out +} + /// What a command knows about itself beyond how it parses. #[derive(Debug, Clone, Copy)] pub struct CommandMeta<'a> { @@ -441,6 +535,11 @@ pub struct CommandMeta<'a> { pub args: &'a [ArgMeta<'a>], /// Metadata for `cmd.subcommands`, in the same order. pub subcommands: &'a [&'a CommandMeta<'a>], + /// Sets of this command's flags that relate to one another as a set. + /// + /// Cold like everything else here: a group is checked once the last token has been + /// read, by code the derive generates, and a successful parse never reads this. + pub groups: &'a [GroupMeta<'a>], } impl CommandMeta<'_> { @@ -460,6 +559,7 @@ impl CommandMeta<'_> { after_help: None, after_long_help: None, examples: &[], + groups: &[], flags: &[], args: &[], subcommands: &[], @@ -647,6 +747,14 @@ impl Spec<'_> { the parent and the struct it flattens each declared it.", duplicate_flag_form(self.root.cmd) ); + assert!( + duplicate_group_name(self.root).is_none(), + "two groups on the same command are called {:?}, so each would enforce only \ + its own members and one from either side would satisfy neither. With \ + `flatten` this is the collision neither expansion can see: the parent and \ + the struct it flattens each declared it. Give one of them another name.", + duplicate_group_name(self.root) + ); debug_assert!( unfillable_arg(self.root.cmd).is_none(), "no word could ever reach the argument {:?}, because an unbounded variadic before \ @@ -807,6 +915,11 @@ fn write_body( write_arg(out, arg, depth)?; } write_completion_types(out, meta, depth)?; + // After the flags and arguments they name, so a reader meets the members before the + // rule about them — the order usage-lib writes, so a round trip reads the same way. + for group in meta.groups { + write_group(out, group, depth)?; + } #[cfg(feature = "complete")] write_completers(out, meta, bin, depth)?; for sub in meta.subcommands { @@ -928,6 +1041,22 @@ fn write_command( Ok(()) } +fn write_group(out: &mut String, group: &GroupMeta<'_>, depth: usize) -> core::fmt::Result { + indent(out, depth)?; + write!(out, "group {}", quoted(group.name))?; + for member in group.members { + write!(out, " {}", quoted(member))?; + } + if group.required { + out.push_str(" required=#true"); + } + if group.multiple { + out.push_str(" multiple=#true"); + } + out.push('\n'); + Ok(()) +} + fn write_example(out: &mut String, example: &Example<'_>, depth: usize) -> core::fmt::Result { indent(out, depth)?; write!(out, "example {}", quoted(example.code))?; @@ -1735,3 +1864,20 @@ mod tests { assert_eq!(placeholder("BUMP", false, true), "[BUMP]"); } } +#[test] +#[should_panic(expected = "two flattened groups on one command have the same name")] +fn concatenating_group_metadata_rejects_duplicate_names() { + static LEFT: [GroupMeta; 1] = [GroupMeta { + name: "input", + members: &["--file", "--url"], + required: false, + multiple: false, + }]; + static RIGHT: [GroupMeta; 1] = [GroupMeta { + name: "input", + members: &["--json", "--yaml"], + required: false, + multiple: false, + }]; + let _ = concat_group_metas::<2>(&[&LEFT, &RIGHT]); +} diff --git a/conformance/src/tables.rs b/conformance/src/tables.rs index 6a31926a2..6714d19cf 100644 --- a/conformance/src/tables.rs +++ b/conformance/src/tables.rs @@ -24,8 +24,8 @@ //! turned up a rule the two implementations disagree about that nothing had recorded. use usage::spec::cmd::SpecExample; -use usage::{Spec, SpecArg, SpecCommand, SpecComplete, SpecFlag}; -use usage_argv::spec::{ArgMeta, CommandMeta, Effect, Example, FlagMeta}; +use usage::{Spec, SpecArg, SpecCommand, SpecComplete, SpecFlag, SpecGroup}; +use usage_argv::spec::{ArgMeta, CommandMeta, Effect, Example, FlagMeta, GroupMeta}; use usage_argv::{Arg, Command, DoubleDash, Flag, UnknownFlags as ArgvUnknownFlags}; /// A command's two tables, built together so the metadata can borrow the parse table. @@ -144,6 +144,7 @@ pub fn build( after_help: opt(&cmd.after_help), after_long_help: opt(&cmd.after_help_long), examples: examples(&cmd.examples), + groups: groups(&cmd.groups), flags: Box::leak(flag_metas.into_boxed_slice()), args: Box::leak(arg_metas.into_boxed_slice()), subcommands: Box::leak( @@ -413,6 +414,30 @@ fn example(e: &SpecExample) -> Example<'static> { } } +/// The command's groups, as usage-argv's cold model of them. +/// +/// Selectors are leaked one at a time rather than joined: a group names flags the way every +/// other relationship does, and the metadata holds them in that form. +fn groups(list: &[SpecGroup]) -> &'static [GroupMeta<'static>] { + Box::leak( + list.iter() + .map(|g| GroupMeta { + name: leak(&g.name), + members: Box::leak( + g.members + .iter() + .map(|m| leak(m)) + .collect::>() + .into_boxed_slice(), + ), + required: g.required, + multiple: g.multiple, + }) + .collect::>() + .into_boxed_slice(), + ) +} + fn examples(list: &[SpecExample]) -> &'static [Example<'static>] { Box::leak( list.iter() diff --git a/conformance/tests/flatten.rs b/conformance/tests/flatten.rs index f043d03f7..fd1dce109 100644 --- a/conformance/tests/flatten.rs +++ b/conformance/tests/flatten.rs @@ -257,3 +257,120 @@ fn flattening_nests() { assert!(nested.outer.inner.listing.no_header); assert_eq!(nested.outer.inner.listing.what.as_deref(), Some("keys")); } + +/// A group declared inside a struct that gets flattened somewhere else. +#[derive(Args)] +#[usage(group("output", required))] +struct Emitting { + /// Emit JSON + #[usage(long, group = "output")] + json: bool, + /// Emit YAML + #[usage(long, group = "output")] + yaml: bool, +} + +#[derive(Cli)] +#[usage(bin = "fl")] +struct Flattened { + #[usage(flatten)] + emitting: Emitting, + /// Where to write + #[usage(long)] + out: Option, +} + +#[test] +fn a_flattened_structs_group_is_enforced_and_emitted() { + // Enforced: the child's own `check` runs, so the group holds on the command that + // flattened it. + let a = ["--json", "--out", "o"].map(OsStr::new); + let fl = Flattened::parse_from(&a).expect("one member"); + assert!(fl.emitting.json && !fl.emitting.yaml); + assert_eq!(fl.out.as_deref(), Some("o")); + + let a = ["--json", "--yaml"].map(OsStr::new); + assert!(matches!( + Flattened::parse_from(&a), + Err(Error::ConflictingFlags { .. }) + )); + + let a: [&OsStr; 0] = []; + assert!(matches!( + Flattened::parse_from(&a), + Err(Error::MissingGroup { + group: "output", + .. + }) + )); + + // And emitted, which is the half that can silently rot: the flags are joined into + // the parent's tables, so the group describing them has to be joined too, or the + // spec would describe a CLI without a rule the CLI enforces. + let kdl = Flattened::to_kdl(); + assert!( + kdl.contains(r#"group "output" "--json" "--yaml" required=#true"#), + "{kdl}" + ); + let spec: LibSpec = kdl.parse().expect("the emitted spec should parse"); + assert_eq!(spec.cmd.groups.len(), 1); + assert!(spec.cmd.groups[0].required); +} + +/// A second group to flatten, so a struct can hold one on each side of one. +#[allow(dead_code)] +#[derive(Args)] +#[usage(group("format"))] +struct Formatting { + /// Compact output + #[usage(long, group = "format")] + compact: bool, + /// Pretty output + #[usage(long, group = "format")] + pretty: bool, +} + +/// A group before the flattened field, and another after it. +#[allow(dead_code)] +#[derive(Cli)] +#[usage(bin = "ord", group("source"), group("sink"))] +struct GroupOrder { + /// Read from a file + #[usage(long, group = "source")] + file: Option, + /// Read from a URL + #[usage(long, group = "source")] + url: Option, + #[usage(flatten)] + formatting: Formatting, + /// Write to a file + #[usage(long, group = "sink")] + out: Option, + /// Write to stdout + #[usage(long, group = "sink")] + stdout: bool, +} + +#[test] +fn a_flattened_structs_groups_land_where_the_field_was_written() { + // The order the flag and argument tables already promise, on the groups beside them: + // a flattened struct's groups splice in at the field, rather than after everything this + // struct declares. Emitting all the local ones first put `sink` — written below the + // flattened field — above the group that field brought in. + let kdl = GroupOrder::to_kdl(); + let at = |name: &str| kdl.find(name).unwrap_or_else(|| panic!("{name} in {kdl}")); + assert!( + at("\"source\"") < at("\"format\"") && at("\"format\"") < at("\"sink\""), + "groups follow the fields that declare them: {kdl}" + ); + + let spec: usage::Spec = kdl.parse().expect("the emitted spec should parse"); + assert_eq!( + spec.cmd + .groups + .iter() + .map(|g| g.name.as_str()) + .collect::>(), + ["source", "format", "sink"], + ); +} diff --git a/conformance/tests/post_binding.rs b/conformance/tests/post_binding.rs index cc1e3a111..3532cdafd 100644 --- a/conformance/tests/post_binding.rs +++ b/conformance/tests/post_binding.rs @@ -651,3 +651,148 @@ fn an_optional_collection_with_defaults_is_never_none() { // And one that declares no default still tells "never given" from "given nothing". assert_eq!(d.plain, None); } + +/// A CLI whose flags are grouped. +/// +/// `--file`/`--url`/`--stdin` are one exclusive, required group — exactly one source — +/// and `--json`/`--yaml` are an ordinary exclusive one, where saying nothing is fine. +#[derive(Cli)] +#[usage(bin = "grp")] +#[usage(group("input", required))] +struct Grp { + /// Read from a file + #[usage(long, group = "input")] + file: Option, + /// Read from a URL + #[usage(long, group = "input")] + url: Option, + /// Read from standard input + #[usage(short = 's', long, group = "input")] + stdin: bool, + /// Emit JSON + #[usage(long, group = "format")] + json: bool, + /// Emit YAML + #[usage(long, group = "format")] + yaml: bool, +} + +#[test] +fn a_required_group_needs_one_of_its_members() { + let a = argv([]); + assert!(matches!( + Grp::parse_from(&a), + Err(Error::MissingGroup { + group: "input", + members: ["--file", "--url", "--stdin"] + }) + )); + + let a = argv(["--url", "u"]); + assert_eq!( + Grp::parse_from(&a).expect("one member").url.as_deref(), + Some("u") + ); +} + +#[test] +fn two_members_of_an_exclusive_group_cannot_both_be_given() { + let a = argv(["--file", "f", "--stdin"]); + assert!(matches!( + Grp::parse_from(&a), + Err(Error::ConflictingFlags { + name: "stdin", + other: "file" + }) + )); + + // By its short form too, since the group is between flags rather than spellings. + let a = argv(["--url", "u", "-s"]); + assert!(matches!( + Grp::parse_from(&a), + Err(Error::ConflictingFlags { name: "stdin", .. }) + )); + + // One member alone still parses, and lands where it was declared. + let a = argv(["--file", "f"]); + let grp = Grp::parse_from(&a).expect("one member"); + assert_eq!(grp.file.as_deref(), Some("f")); + assert!(!grp.stdin); +} + +#[test] +fn a_group_that_is_not_required_may_be_left_alone() { + // `--json`/`--yaml` exclude each other and neither is needed. + let a = argv(["--url", "u"]); + let grp = Grp::parse_from(&a).expect("saying nothing about format is fine"); + assert!(!grp.json && !grp.yaml); + + let a = argv(["--url", "u", "--json"]); + assert!(Grp::parse_from(&a).expect("one of them").json); + + let a = argv(["--url", "u", "--json", "--yaml"]); + assert!(matches!( + Grp::parse_from(&a), + Err(Error::ConflictingFlags { .. }) + )); +} + +#[test] +fn a_group_reaches_the_emitted_spec_and_usage_lib_agrees() { + let kdl = Grp::to_kdl(); + assert!( + kdl.contains(r#"group "input" "--file" "--url" "--stdin" required=#true"#), + "{kdl}" + ); + assert!(kdl.contains(r#"group "format" "--json" "--yaml""#), "{kdl}"); + + // The reference implementation reads what the derive wrote, and enforces the same + // rule — which is the point of the spec being the definition rather than a summary. + let spec: usage::Spec = kdl.parse().expect("the emitted spec should parse"); + let group = spec.cmd.groups.iter().find(|g| g.name == "input").unwrap(); + assert!(group.required); + assert_eq!(group.members.len(), 3); +} + +/// Two groups on one command, one required and one exclusive. +#[derive(Cli)] +#[usage(bin = "ex4")] +#[usage(group("input", required))] +struct TwoGroups { + #[usage(long, group = "input")] + file: Option, + #[usage(long, group = "input")] + url: Option, + #[usage(long, group = "format")] + json: bool, + #[usage(long, group = "format")] + yaml: bool, +} + +#[test] +fn a_conflict_answers_before_an_unsatisfied_group_does() { + // Both are wrong here: `input` has no member, and `format` has two. The conflict is + // the more useful answer — it says which flag not to have typed, where the other + // asks for one more — and it is the order the rest of the checks already follow. + let a = argv(["--json", "--yaml"]); + assert!( + matches!( + TwoGroups::parse_from(&a), + Err(Error::ConflictingFlags { .. }) + ), + "the exclusivity of a later group should answer before an earlier group's requiredness" + ); + + // With the conflict gone, the unsatisfied group is what is left to say. + let a = argv(["--json"]); + assert!(matches!( + TwoGroups::parse_from(&a), + Err(Error::MissingGroup { group: "input", .. }) + )); + + // And with both satisfied, the values land where they were declared. + let a = argv(["--file", "f", "--yaml"]); + let two = TwoGroups::parse_from(&a).expect("one from each group"); + assert_eq!(two.file.as_deref(), Some("f")); + assert!(two.url.is_none() && two.yaml && !two.json); +} diff --git a/derive/src/codegen.rs b/derive/src/codegen.rs index cb5e2de50..d88e6e7ab 100644 --- a/derive/src/codegen.rs +++ b/derive/src/codegen.rs @@ -123,6 +123,7 @@ pub fn emit(cli: &Cli) -> TokenStream { let tables = tables(cli); let table_decls = &tables.decls; let meta_table_decls = &tables.meta_decls; + let (group_meta_decl, group_meta_table_ref) = group_meta_table(cli); let flag_table_ref = &tables.flags; let arg_table_ref = &tables.args; let flag_meta_table_ref = &tables.flag_metas; @@ -274,6 +275,8 @@ pub fn emit(cli: &Cli) -> TokenStream { #(#arg_metas)* #meta_table_decls + #group_meta_decl + pub static ROOT_META: usage_argv::spec::CommandMeta = usage_argv::spec::CommandMeta { cmd: &ROOT, about: #about, @@ -287,6 +290,7 @@ pub fn emit(cli: &Cli) -> TokenStream { after_long_help: #after_long_help, flags: #flag_meta_table_ref, args: #arg_meta_table_ref, + groups: #group_meta_table_ref, #sub_metas ..usage_argv::spec::CommandMeta::EMPTY }; @@ -2339,6 +2343,7 @@ pub fn emit_args(cli: &Cli) -> TokenStream { let tables = tables(cli); let table_decls = &tables.decls; let meta_table_decls = &tables.meta_decls; + let (group_meta_decl, group_meta_table_ref) = group_meta_table(cli); let flag_table_ref = &tables.flags; let arg_table_ref = &tables.args; let flag_meta_table_ref = &tables.flag_metas; @@ -2410,6 +2415,8 @@ pub fn emit_args(cli: &Cli) -> TokenStream { #(#arg_metas)* #meta_table_decls + #group_meta_decl + pub static COMMAND_META: usage_argv::spec::CommandMeta = usage_argv::spec::CommandMeta { cmd: &COMMAND, effect: #effect, @@ -2425,6 +2432,7 @@ pub fn emit_args(cli: &Cli) -> TokenStream { after_long_help: #after_long_help, flags: #flag_meta_table_ref, args: #arg_meta_table_ref, + groups: #group_meta_table_ref, #sub_metas ..usage_argv::spec::CommandMeta::EMPTY }; @@ -2859,6 +2867,135 @@ fn displaced_guard(cli: &Cli, field: &Field) -> TokenStream { quote!(&& !partial.#overridden) } +/// The groups a command declares, in the order their first member is written. +/// +/// Membership lives on the fields and properties on the struct, so this is where the two +/// are joined — and it is the only place both are visible, which is why the emitted +/// metadata is built here rather than in the model. +fn declared_groups(cli: &Cli) -> Vec<(String, bool, bool, Vec)> { + let mut groups: Vec<(String, bool, bool, Vec)> = Vec::new(); + for field in &cli.fields { + let Some(name) = field.group.as_deref() else { + continue; + }; + let Some(selector) = Cli::selector_for_field(field) else { + continue; + }; + match groups.iter_mut().find(|(n, _, _, _)| n == name) { + Some((_, _, _, members)) => members.push(selector), + None => { + // An undeclared group takes the defaults, which is the common case: "at + // most one of these" needs no properties, and making it say so anyway + // would be ceremony. + let decl = cli.groups.iter().find(|d| d.name == name); + groups.push(( + name.to_string(), + decl.is_some_and(|d| d.required), + decl.is_some_and(|d| d.multiple), + vec![selector], + )); + } + } + } + groups +} + +/// The `static` array of group metadata, and the expression referring to it. +/// +/// A flattened struct's groups are joined in, the way its flags and their metadata are: +/// the child enforces them through its own `check`, and its flags are in *this* command's +/// table, so its groups describe this command and belong in this command's emitted KDL. +/// Leaving them out would enforce a rule the spec does not mention — the drift the +/// spec-as-definition rule exists to prevent. +fn group_meta_table(cli: &Cli) -> (TokenStream, TokenStream) { + let groups = declared_groups(cli); + // Where each group's first member was written, which is the position `declared_groups` + // already orders them by. A group whose members straddle a flattened field still belongs + // where it *starts*, so it keeps its whole member list rather than being split in two. + let first_member_at: Vec = groups + .iter() + .map(|(name, _, _, _)| { + cli.fields + .iter() + .position(|f| { + f.group.as_deref() == Some(name.as_str()) + && Cli::selector_for_field(f).is_some() + }) + .unwrap_or(usize::MAX) + }) + .collect(); + let entry = |(name, required, multiple, members): &(String, bool, bool, Vec)| { + quote! { + usage_argv::spec::GroupMeta { + name: #name, + members: &[#(#members),*], + required: #required, + multiple: #multiple, + } + } + }; + + // One walk over the fields, so a flattened struct's groups land where the field was + // written rather than after everything this struct declares — the same interleaving the + // flag and argument tables are built with, and visible in the same places their order is. + let mut parts: Vec = Vec::new(); + let mut run: Vec = Vec::new(); + let mut emitted = vec![false; groups.len()]; + let mut any_flattened = false; + for (i, field) in cli.fields.iter().enumerate() { + let Kind::Flatten { ty } = &field.kind else { + continue; + }; + any_flattened = true; + for (g, group) in groups.iter().enumerate() { + if !emitted[g] && first_member_at[g] < i { + emitted[g] = true; + run.push(entry(group)); + } + } + if !run.is_empty() { + let entries = std::mem::take(&mut run); + parts.push(quote!(&[#(#entries),*])); + } + // Named directly, as the flag and argument tables beside this one are: the + // generated items live in the user's own scope now rather than in a module + // above it, so there is no path to rewrite. + parts.push(quote!(<#ty as usage_argv::spec::CommandArgs>::META.groups)); + } + for (g, group) in groups.iter().enumerate() { + if !emitted[g] { + run.push(entry(group)); + } + } + if !run.is_empty() { + parts.push(quote!(&[#(#run),*])); + } + + if parts.is_empty() { + return (quote!(), quote!(&[])); + } + if !any_flattened { + let len = groups.len(); + let entries = groups.iter().map(entry); + return ( + quote! { + pub static GROUP_METAS: [usage_argv::spec::GroupMeta; #len] = [#(#entries),*]; + }, + quote!(&GROUP_METAS), + ); + } + ( + quote! { + const GROUP_META_GROUPS: &[&[usage_argv::spec::GroupMeta<'static>]] = + &[#(#parts),*]; + static GROUP_METAS: [usage_argv::spec::GroupMeta<'static>; + usage_argv::table_len(GROUP_META_GROUPS)] = + usage_argv::spec::concat_group_metas(GROUP_META_GROUPS); + }, + quote!(&GROUP_METAS), + ) +} + /// Everything decided once the last token has been read. /// /// Ordered deliberately. The environment fills what argv left out, so it runs @@ -3167,6 +3304,84 @@ fn post_binding(cli: &Cli) -> TokenStream { }) }); + // Groups, checked once per group rather than per member: both questions a group asks + // — how many members were given, and whether that is enough — are about the set. + // + // The two halves read a default differently, deliberately, and the same way + // usage-lib does. Exclusivity counts what was supplied, or a defaulted member would + // collide with the sibling the user typed; requiredness asks whether a member ended + // up with a value, and a default is a value. + let group_checks = declared_groups(cli) + .into_iter() + .map(|(name, required, multiple, members)| { + let fields: Vec<&Field> = members + .iter() + .filter_map(|selector| cli.field_for_selector(selector)) + .collect(); + let given: Vec = fields + .iter() + .map(|f| { + let given = format_ident!("__given_{}", f.ident); + quote!(partial.#given) + }) + .collect(); + // A member with a default always has a value, so the group can never be + // unsatisfied. Decided here rather than at run time, as `requires` is. + let always_filled = fields.iter().any(|f| !f.default.is_empty()); + let exclusivity = (!multiple).then(|| { + // Reported as the first two that were given, which is the pair the user has + // to choose between. `ConflictingFlags` rather than a group-shaped error: + // what went wrong is that two flags were given together, which is exactly + // what that error says. + let names: Vec<&String> = fields.iter().map(|f| &f.name).collect(); + let pairs = (0..fields.len()).flat_map(|i| { + let (later, earlier) = (given.clone(), given.clone()); + let (later_names, earlier_names) = (names.clone(), names.clone()); + ((i + 1)..fields.len()) + .map(move |j| { + let (a, b) = (&earlier[i], &later[j]); + let (name_a, name_b) = (earlier_names[i], later_names[j]); + quote! { + if #a && #b { + return ::std::result::Result::Err( + usage_argv::Error::ConflictingFlags { + name: #name_b, + other: #name_a, + }, + ); + } + } + }) + .collect::>() + }); + quote!(#(#pairs)*) + }); + let requiredness = (required && !always_filled).then(|| { + let selectors = &members; + quote! { + if !(#(#given)||*) { + return ::std::result::Result::Err( + usage_argv::Error::MissingGroup { + group: #name, + members: &[#(#selectors),*], + }, + ); + } + } + }); + (exclusivity, requiredness) + }) + .collect::>(); + // Two passes rather than one block per group, because the order between *kinds* of + // check is the one this function promises: what the user typed wrong before what + // they left out. Emitted together, an earlier group's `MissingGroup` would answer + // before a later group's `ConflictingFlags` — and before a flattened child's, since + // those run later still. + let group_exclusivity_checks: Vec = + group_checks.iter().filter_map(|(e, _)| e.clone()).collect(); + let group_required_checks: Vec = + group_checks.iter().filter_map(|(_, r)| r.clone()).collect(); + // `required_if` and `required_unless` are the same question asked two ways: which // other flags decide whether this one had to be given. Neither needs to know the // order they arrived in — only whether they arrived — so both are answered here, @@ -3232,9 +3447,11 @@ fn post_binding(cli: &Cli) -> TokenStream { // more useful of the two answers when a conflict has also left something // unfilled, and it is the one usage-lib reports. #(#conflict_checks)* + #(#group_exclusivity_checks)* #(#requirement_checks)* #(#flattened_checks)* #(#required_checks)* + #(#group_required_checks)* #(#relationship_required_checks)* #(#choice_checks)* #(#bound_checks)* diff --git a/derive/src/lib.rs b/derive/src/lib.rs index 1dba460a5..320a5daac 100644 --- a/derive/src/lib.rs +++ b/derive/src/lib.rs @@ -213,6 +213,7 @@ //! | `overrides = "--other"` | a flag this one displaces, the last given winning | //! | `conflicts = "--other"` | a flag this one cannot be given with | //! | `requires = "--other"` | a flag that must also be given when this one is | +//! | `group = "input"` | the group this flag is one of; see below | //! | `required_if = "--other"` | a flag whose presence makes this one necessary | //! | `required_unless = "--other"` | a flag whose presence makes this one unnecessary | //! @@ -221,6 +222,28 @@ //! is a compile error, which is the advantage of declaring a relationship in code: in a //! hand-written spec a typo'd selector is a relationship that quietly does not hold. //! +//! A **group** is the one relationship that is not written flag-to-flag, because what it +//! says is about the set: `required` means one of them is needed, and no rule on an +//! individual flag expresses that. Membership goes on the fields and the properties on the +//! struct, which may be left out entirely when the group is a plain "at most one": +//! +//! ```ignore +//! #[derive(Cli)] +//! #[usage(bin = "ex")] +//! #[usage(group("input", required))] +//! struct Ex { +//! #[usage(long, group = "input")] +//! file: Option, +//! #[usage(long, group = "input")] +//! url: Option, +//! } +//! ``` +//! +//! `required` means at least one member is needed and `multiple` means more than one may +//! be given, so a bare group is "at most one", `required` alone is "exactly one", and the +//! two together are "at least one" — clap's two properties, read the same way. A group +//! with one member, or a declaration no field joins, is a compile error. +//! //! They describe relationships *between flags*, so a positional cannot declare one — //! the spec records them on a flag and has nowhere to put them on an argument, and a //! check the emitted spec cannot describe would leave docs and completions saying diff --git a/derive/src/model.rs b/derive/src/model.rs index e56e689ba..14941c398 100644 --- a/derive/src/model.rs +++ b/derive/src/model.rs @@ -96,9 +96,23 @@ pub struct Cli { /// Carried into the spec and nowhere else. The parser never runs it: a mount costs a /// subprocess, and completions are the cold path where that is affordable. pub mount: Option, + /// Groups declared on this command, with their properties. + /// + /// Membership is on the field — `#[usage(group = "input")]` — and only the two + /// properties live here, because a group that says nothing but "these three are + /// exclusive" should not need declaring twice. + pub groups: Vec, pub fields: Vec, } +/// A `#[usage(group("input", required))]` on the struct. +pub struct GroupDecl { + pub name: String, + pub required: bool, + pub multiple: bool, + pub span: Span, +} + /// One field, resolved to the thing it declares. pub struct Field { pub ident: syn::Ident, @@ -191,6 +205,10 @@ pub struct Field { /// one lives on the flag the rule is about, which is where clap puts it and where a /// reader looks for it. pub requires: Vec, + /// The group this flag belongs to, if any. Properties live on the group's own + /// declaration; membership lives here, because a field is where a reader looks to + /// see what a flag is part of. + pub group: Option, /// Flags whose presence makes this one necessary. pub required_if: Vec, /// Flags whose presence makes this one unnecessary. @@ -407,6 +425,7 @@ impl Cli { after_long_help: None, restart_token: None, mount: None, + groups: Vec::new(), fields: Vec::new(), }; @@ -474,14 +493,16 @@ impl Cli { } "restart_token" => cli.restart_token = Some(string_value(&meta)?), "mount" => cli.mount = Some(string_value(&meta)?), + "group" => cli.groups.push(group_decl(&meta)?), other => { return Err(syn::Error::new_spanned( path, format!( "unknown option `{other}` on a struct; usage::Cli takes \ `name`, `bin`, `version`, `usage`, `verbatim_doc_comment`, `unknown_flags`, \ - `default_subcommand`, `restart_token`, and `mount` here, \ - and the description comes from the doc comment" + `default_subcommand`, `restart_token`, `mount` and \ + `group` here, and the description comes from the doc \ + comment" ), )); } @@ -668,6 +689,20 @@ impl Cli { Ok(()) } + /// How a group names one of its member fields in the emitted spec. + /// + /// The long form when there is one, since that is how a spec refers to a flag + /// everywhere else; a short form otherwise, which selectors accept just as readily. + pub fn selector_for_field(field: &Field) -> Option { + let Kind::Flag { longs, shorts, .. } = &field.kind else { + return None; + }; + longs + .first() + .map(|long| format!("--{long}")) + .or_else(|| shorts.first().map(|short| format!("-{short}"))) + } + pub fn field_for_selector(&self, selector: &str) -> Option<&Field> { self.fields.iter().find(|field| { let Kind::Flag { @@ -817,6 +852,73 @@ impl Cli { } } + // Groups: every member is a flag, every declared group has members, and a group + // holds at least two of them — the same floor the spec enforces, checked here so + // it fails where it is written rather than when the spec is emitted. + let mut group_members: Vec<(&str, Vec<&Field>)> = Vec::new(); + for field in &self.fields { + let Some(name) = field.group.as_deref() else { + continue; + }; + if !matches!(field.kind, Kind::Flag { .. }) { + return Err(syn::Error::new( + field.span, + "`group` describes a relationship between flags, so the field needs \ + a `long` or a `short`", + )); + } + // `group("")` on the struct is refused as nameless; two fields saying + // `group = ""` would otherwise form the same nameless group by the back + // door, and it would be emitted and reported with nothing to call it. + if name.is_empty() { + return Err(syn::Error::new( + field.span, + "a group with no name answers to nothing; give it one, as \ + `group = \"input\"`", + )); + } + match group_members.iter_mut().find(|(n, _)| *n == name) { + Some((_, members)) => members.push(field), + None => group_members.push((name, vec![field])), + } + } + for (name, members) in &group_members { + if members.len() < 2 { + return Err(syn::Error::new( + members[0].span, + format!( + "group `{name}` has one flag in it; a rule about a single flag \ + belongs on that flag, as `required` or `requires`" + ), + )); + } + } + for (i, decl) in self.groups.iter().enumerate() { + // Two declarations of one group would be read first-match-wins, so the second + // one's properties would be silently dropped — a `required` written and not + // enforced, which is worse than not being able to write it. + if self.groups[..i].iter().any(|d| d.name == decl.name) { + return Err(syn::Error::new( + decl.span, + format!( + "group `{}` is declared twice; one declaration carries all of \ + its properties", + decl.name + ), + )); + } + if !group_members.iter().any(|(n, _)| *n == decl.name) { + return Err(syn::Error::new( + decl.span, + format!( + "group `{}` is declared and no field is in it; a field joins a \ + group with `#[usage(group = \"{}\")]`", + decl.name, decl.name + ), + )); + } + } + // Every relationship names a flag that exists. Resolving these at compile time // is the advantage of declaring them in code: a spec written by hand can only // find a typo'd selector at parse time, or never, since a selector naming @@ -951,6 +1053,7 @@ impl Field { overrides: Vec::new(), conflicts: Vec::new(), requires: Vec::new(), + group: None, required_if: Vec::new(), required_unless: Vec::new(), hide: false, @@ -1045,6 +1148,7 @@ impl Field { overrides: Vec::new(), conflicts: Vec::new(), requires: Vec::new(), + group: None, required_if: Vec::new(), required_unless: Vec::new(), hide: false, @@ -1102,6 +1206,7 @@ impl Field { let mut overrides: Vec = Vec::new(); let mut conflicts: Vec = Vec::new(); let mut requires: Vec = Vec::new(); + let mut group: Option = None; let mut required_if: Vec = Vec::new(); let mut required_unless: Vec = Vec::new(); @@ -1194,6 +1299,7 @@ impl Field { "overrides" => overrides = selectors(&meta)?, "conflicts" => conflicts = selectors(&meta)?, "requires" => requires = selectors(&meta)?, + "group" => group = Some(string_value(&meta)?), "required_if" => required_if = selectors(&meta)?, "required_unless" => required_unless = selectors(&meta)?, "value_enum" => value_enum = flag_value(&meta)?, @@ -1237,7 +1343,7 @@ impl Field { `short`, `negate`, `global`, `var`, `variadic`, \ `count`, `hide`, `arg`, `env`, `default`, `choices`, \ `var_min`, `var_max`, `value_enum`, `value_hint`, `overrides`, \ - `conflicts`, `requires`, `required_if`, \ + `conflicts`, `requires`, `group`, `required_if`, \ `required_unless`, `help_heading`, `value_name`, \ `verbatim_doc_comment`, \ `required`, and `double_dash`" @@ -1784,6 +1890,7 @@ impl Field { overrides, conflicts, requires, + group, required_if, required_unless, hide, @@ -1976,6 +2083,80 @@ fn string_value(meta: &Meta) -> syn::Result { /// than as field names, so a declaration reads the same in Rust as it does in KDL. /// Which flag each one names is resolved in [`Cli::check`], where every field is in /// view. +/// `group("input", required, multiple)` — a name, then any of the two properties. +/// +/// Hand-parsed rather than reusing [`selectors`], because the list is mixed: a string +/// literal for the name and bare idents for the properties. Spelling the properties as +/// idents rather than as `required = true` matches how `long`, `global` and `count` are +/// already written on a field — a property that is only ever on or off is said by naming +/// it. +fn group_decl(meta: &Meta) -> syn::Result { + let span = meta.path().span(); + let Meta::List(list) = meta else { + return Err(syn::Error::new_spanned( + meta.path(), + "a group is declared as `group(\"name\")`, with `required` and `multiple` \ + after the name if it needs them", + )); + }; + let mut name: Option = None; + let mut decl = GroupDecl { + name: String::new(), + required: false, + multiple: false, + span, + }; + list.parse_args_with(|input: syn::parse::ParseStream| { + while !input.is_empty() { + if input.peek(syn::LitStr) { + let lit: syn::LitStr = input.parse()?; + if name.is_some() { + return Err(syn::Error::new_spanned( + &lit, + "a group takes one name; its members are declared on the fields, \ + with `#[usage(group = \"…\")]`", + )); + } + name = Some(lit.value()); + } else { + let ident: syn::Ident = input.parse()?; + match ident.to_string().as_str() { + "required" => decl.required = true, + "multiple" => decl.multiple = true, + other => { + return Err(syn::Error::new_spanned( + &ident, + format!( + "unknown group property `{other}`; a group takes \ + `required` and `multiple`" + ), + )); + } + } + } + if input.is_empty() { + break; + } + input.parse::()?; + } + Ok(()) + })?; + let Some(name) = name else { + return Err(syn::Error::new( + span, + "a group needs a name, as in `group(\"input\", required)`", + )); + }; + if name.is_empty() { + return Err(syn::Error::new( + span, + "a group with no name answers to nothing", + )); + } + decl.name = name; + Ok(decl) +} + fn selectors(meta: &Meta) -> syn::Result> { let Meta::List(list) = meta else { return Ok(vec![string_value(meta)?]); @@ -3362,6 +3543,77 @@ mod tests { assert!(err.contains("takes no value"), "unhelpful message: {err}"); } + #[test] + fn a_group_is_declared_once_and_joined_by_at_least_two_flags() { + // Two declarations would be read first-match-wins, so the second one's + // properties would be silently dropped — a `required` written and not enforced. + let err = rejection( + r#" + #[usage(group("input", required))] + #[usage(group("input", multiple))] + struct Ex { + #[usage(long, group = "input")] + file: Option, + #[usage(long, group = "input")] + url: Option, + } + "#, + ); + assert!(err.contains("declared twice"), "unhelpful message: {err}"); + + // A group of one is a statement about that flag. + let err = rejection( + r#" + struct Ex { + #[usage(long, group = "input")] + file: Option, + } + "#, + ); + assert!(err.contains("one flag in it"), "unhelpful message: {err}"); + + // And a declaration nothing joins holds for nothing. + let err = rejection( + r#" + #[usage(group("input", required))] + struct Ex { + #[usage(long)] + file: Option, + } + "#, + ); + assert!( + err.contains("no field is in it"), + "unhelpful message: {err}" + ); + + // A positional cannot be in one, as it cannot hold any other relationship. + let err = rejection( + r#" + struct Ex { + #[usage(group = "input")] + target: String, + #[usage(long, group = "input")] + url: Option, + } + "#, + ); + assert!(err.contains("between flags"), "unhelpful message: {err}"); + + // A group with no name answers to nothing, whichever way it is written. + let err = rejection( + r#" + struct Ex { + #[usage(long, group = "")] + file: Option, + #[usage(long, group = "")] + url: Option, + } + "#, + ); + assert!(err.contains("no name"), "unhelpful message: {err}"); + } + #[test] fn an_alias_cannot_name_a_sibling() { // The parser takes the first table entry that matches, so a name claimed twice diff --git a/docs/spec/reference/group.md b/docs/spec/reference/group.md index cee334f66..f5653cd81 100644 --- a/docs/spec/reference/group.md +++ b/docs/spec/reference/group.md @@ -84,6 +84,25 @@ Members are counted by the flag they name, not by the selector, so a group listi `-f` and `--file` holds one member and not two. Listing both is redundant rather than wrong, and a flag is never in conflict with itself. +## From the derive + +`#[derive(usage::Cli)]` writes the same group with membership on the fields and the +properties on the struct: + +```rust +#[usage(group("input", required))] +struct Ex { + #[usage(long, group = "input")] + file: Option, + #[usage(long, group = "input")] + url: Option, +} +``` + +The `#[usage(group(...))]` line can be left out when the group is a plain "at most one". +A group with fewer than two members, or a declaration no field joins, is a compile error +rather than a rule that quietly holds for nothing. + ## Coming from clap `ArgGroup` carries across, with `required` and `multiple` read the same way. A group that diff --git a/xtask/src/shadow.rs b/xtask/src/shadow.rs index dfb570bdb..4d098574f 100644 --- a/xtask/src/shadow.rs +++ b/xtask/src/shadow.rs @@ -314,6 +314,14 @@ fn emit_command(out: &mut String, cmd: &SpecCommand, ty: &Type, is_root: bool, r run.skipped.note("a command's second and later mounts"); } } + // Counted rather than emitted, in *both* dialects. Both can express a group — the + // derive with `group(…)` and clap with `ArgGroup` — so this is a gap in the shadow + // generator rather than in either target, and no spec in the fleet declares one yet + // for it to matter to. It is counted so the report cannot claim the shadow expressed + // a whole spec that it did not. + if !cmd.groups.is_empty() { + run.skipped.note("a `group` on a command"); + } for (_, sub, sub_ty) in &children { // `run` travels down unchanged; `default_subcommand` is read under an `is_root` // guard, so a child cannot pick up the root's.