Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions argv/src/spec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -462,6 +462,12 @@ pub struct FlagMeta<'a> {
/// flag win, this reports it: the combination has no meaning, so honouring one
/// side silently would hide a mistake.
pub conflicts: &'a [&'a str],
/// Flags that must also be given when this one is.
///
/// The positive form of [`conflicts`](Self::conflicts), and the mirror image of
/// [`required_if`](Self::required_if): the same rule written on the flag that
/// imposes it rather than on the flag it lands on.
pub requires: &'a [&'a str],
/// Flags that make this one necessary.
pub required_if: &'a [&'a str],
/// Flags that make this one unnecessary.
Expand Down Expand Up @@ -491,6 +497,7 @@ impl FlagMeta<'_> {
var_max: None,
overrides: &[],
conflicts: &[],
requires: &[],
required_if: &[],
required_unless: &[],
help_heading: None,
Expand Down Expand Up @@ -911,6 +918,7 @@ fn write_flag(out: &mut String, meta: &FlagMeta<'_>, depth: usize) -> core::fmt:
write_single_default(out, meta.default)?;
write_single_list(out, "overrides", meta.overrides)?;
write_single_list(out, "conflicts", meta.conflicts)?;
write_single_list(out, "requires", meta.requires)?;
write_single_list(out, "required_if", meta.required_if)?;
write_single_list(out, "required_unless", meta.required_unless)?;

Expand All @@ -920,6 +928,7 @@ fn write_flag(out: &mut String, meta: &FlagMeta<'_>, depth: usize) -> core::fmt:
|| meta.default.len() > 1
|| meta.overrides.len() > 1
|| meta.conflicts.len() > 1
|| meta.requires.len() > 1
|| meta.required_if.len() > 1
|| meta.required_unless.len() > 1;
if !has_children {
Expand All @@ -936,6 +945,7 @@ fn write_flag(out: &mut String, meta: &FlagMeta<'_>, depth: usize) -> core::fmt:
write_many_defaults(out, meta.default, inner)?;
write_many_list(out, "overrides", meta.overrides, inner)?;
write_many_list(out, "conflicts", meta.conflicts, inner)?;
write_many_list(out, "requires", meta.requires, inner)?;
write_many_list(out, "required_if", meta.required_if, inner)?;
write_many_list(out, "required_unless", meta.required_unless, inner)?;
if meta.flag.takes_value {
Expand Down
25 changes: 25 additions & 0 deletions conformance/tests/derive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -730,6 +730,15 @@ struct Related {
/// Where to write
#[usage(long, required_if = "--stdin")]
out: Option<String>,
/// Sign the output
#[usage(long, requires("--key", "--identity"))]
sign: bool,
/// The signing key
#[usage(long)]
key: Option<String>,
/// Who is signing
#[usage(long)]
identity: Option<String>,
}

#[test]
Expand All @@ -749,6 +758,13 @@ fn flag_relationships_reach_the_spec() {
vec!["--file".to_string(), "--url".to_string()]
);
assert_eq!(flag("out").required_if, vec!["--stdin".to_string()]);
// Two selectors, so the emitted KDL takes the child-node spelling rather than the
// property one — the same pair of shapes `conflicts` has, and both have to survive
// being parsed back by usage-lib.
assert_eq!(
flag("sign").requires,
vec!["--key".to_string(), "--identity".to_string()]
);
assert_eq!(flag("color").overrides, vec!["--plain".to_string()]);
assert_eq!(
flag("file").required_unless,
Expand All @@ -762,12 +778,21 @@ fn flag_relationships_reach_the_spec() {
"{}",
Related::to_kdl()
);
assert!(
Related::to_kdl().contains(r#"requires "--key" "--identity""#),
"{}",
Related::to_kdl()
);

// And the same declaration still parses: a satisfied set of relationships is
// invisible, which is the point.
let a = argv(["--stdin", "--out", "o"]);
let rel = Related::parse_from(&a).expect("should parse");
assert!(rel.stdin);
// `--sign` was not given, so what it requires is not asked for.
assert!(!rel.sign);
assert!(rel.key.is_none());
assert!(rel.identity.is_none());
assert!(rel.color && !rel.plain);
assert_eq!(rel.out.as_deref(), Some("o"));
assert!(rel.file.is_none());
Expand Down
46 changes: 46 additions & 0 deletions conformance/tests/post_binding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,15 @@ struct Rel {
/// How many at once
#[usage(long, default = "4", required_if = "--stdin")]
jobs: Option<String>,
/// Sign the output
#[usage(long, requires("--key"))]
sign: bool,
/// Tag the output, which needs a job count — and one is always there
#[usage(long, requires("--jobs"))]
tag: bool,
/// The key to sign with
#[usage(long)]
key: Option<String>,
}

#[test]
Expand Down Expand Up @@ -258,6 +267,43 @@ fn conflicting_flags_cannot_both_be_given() {
);
}

#[test]
fn a_requirement_names_the_flag_that_was_not_given() {
// Reported as the *other* flag missing rather than as something wrong with
// `--sign`, which is what clap says for an unmet `requires` and the thing a user
// can act on: the fix is to type `--key`, not to delete `--sign`.
let a = argv(["--file", "f", "--sign"]);
assert!(matches!(
Rel::parse_from(&a),
Err(Error::MissingRequired { name: "key" })
));

// Satisfied.
let a = argv(["--file", "f", "--sign", "--key", "k"]);
let rel = Rel::parse_from(&a).expect("should parse");
assert!(rel.sign);
assert_eq!(rel.key.as_deref(), Some("k"));

// Nothing is imposed when the flag that declares the requirement is absent, which
// is what makes this different from plain required-ness: `--key` is optional until
// `--sign` asks for it.
let a = argv(["--file", "f"]);
let rel = Rel::parse_from(&a).expect("should parse");
assert!(!rel.sign);
assert!(rel.key.is_none());
}

#[test]
fn a_default_on_the_required_flag_satisfies_the_requirement() {
// The check is not emitted at all when the flag a requirement names has a default,
// because such a flag can never be missing — the same rule plain required-ness
// follows, and usage-lib agrees.
let a = argv(["--file", "f", "--tag"]);
let rel = Rel::parse_from(&a).expect("the defaulted flag is not missing");
assert!(rel.tag);
assert_eq!(rel.jobs.as_deref(), Some("4"));
}

#[test]
fn a_conflict_is_reported_before_what_it_left_unfilled() {
// `--stdin` without `--out` is also a missing-required error. The conflict is the
Expand Down
37 changes: 37 additions & 0 deletions derive/src/codegen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -741,6 +741,7 @@ fn flag_meta(i: usize, field: &Field, owner: &syn::Ident) -> TokenStream {
// the struct says.
let overrides = &field.overrides;
let conflicts = &field.conflicts;
let requires = &field.requires;
let required_if = &field.required_if;
let required_unless = &field.required_unless;

Expand Down Expand Up @@ -773,6 +774,7 @@ fn flag_meta(i: usize, field: &Field, owner: &syn::Ident) -> TokenStream {
var_max: #var_max,
overrides: &[#(#overrides),*],
conflicts: &[#(#conflicts),*],
requires: &[#(#requires),*],
required_if: &[#(#required_if),*],
required_unless: &[#(#required_unless),*],
..FlagMeta::EMPTY
Expand Down Expand Up @@ -2819,6 +2821,40 @@ fn post_binding(cli: &Cli) -> TokenStream {
})
});

// The positive form, and the same `__given_*` reading for the same reasons: a `bool`
// given as `false` is still a flag the user asked for, and env fallback has already
// run, so a requirement satisfied from the environment is satisfied.
//
// Reported as the *other* flag being missing rather than as something wrong with the
// one that named it, which is what clap says: an unmet `requires` is a required
// argument that was not provided. That also means no new `Error` variant — the hot
// path's `Result` does not grow to carry a check that only ever fires on the cold one.
//
// A target with a default is skipped outright, since it can never be missing.
let requirement_checks = cli.fields.iter().flat_map(move |f| {
let given = format_ident!("__given_{}", f.ident);
f.requires.iter().filter_map(move |selector| {
// Resolved in the model, which rejects a selector naming nothing.
let other = cli.field_for_selector(selector)?;
// A flag with a default always has a value, so the requirement it satisfies
// can never fail — the same reason plain required-ness skips such a field.
// Decided at compile time, so the check is not merely always-true at run
// time, it is not there.
if !other.default.is_empty() {
return None;
}
let other_given = format_ident!("__given_{}", other.ident);
let other_name = &other.name;
Some(quote! {
if partial.#given && !partial.#other_given {
return ::std::result::Result::Err(
::usage_argv::Error::MissingRequired { name: #other_name },
);
}
})
})
});

// `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,
Expand Down Expand Up @@ -2883,6 +2919,7 @@ 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)*
#(#requirement_checks)*
#(#flattened_checks)*
#(#required_checks)*
#(#relationship_required_checks)*
Expand Down
1 change: 1 addition & 0 deletions derive/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,7 @@
//! | `arg` | force a field to be positional |
//! | `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 |
//! | `required_if = "--other"` | a flag whose presence makes this one necessary |
//! | `required_unless = "--other"` | a flag whose presence makes this one unnecessary |
//!
Expand Down
17 changes: 16 additions & 1 deletion derive/src/model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,14 @@ pub struct Field {
/// Flags this one cannot be given with. Checked after the parse: whether a flag
/// is unwelcome depends on the whole command line, not on the token itself.
pub conflicts: Vec<String>,
/// Flags that must also be given when this one is. Checked after the parse for the
/// same reason `conflicts` is: the flag that satisfies the requirement may still be
/// ahead of the one that imposes it.
///
/// The same rule `required_if` states from the other end, and worth having both: this
/// 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<String>,
/// Flags whose presence makes this one necessary.
pub required_if: Vec<String>,
/// Flags whose presence makes this one unnecessary.
Expand Down Expand Up @@ -712,6 +720,7 @@ impl Cli {
for (option, selectors) in [
("overrides", &field.overrides),
("conflicts", &field.conflicts),
("requires", &field.requires),
("required_if", &field.required_if),
("required_unless", &field.required_unless),
] {
Expand Down Expand Up @@ -834,6 +843,7 @@ impl Field {
var_max: None,
overrides: Vec::new(),
conflicts: Vec::new(),
requires: Vec::new(),
required_if: Vec::new(),
required_unless: Vec::new(),
hide: false,
Expand Down Expand Up @@ -925,6 +935,7 @@ impl Field {
var_max: None,
overrides: Vec::new(),
conflicts: Vec::new(),
requires: Vec::new(),
required_if: Vec::new(),
required_unless: Vec::new(),
hide: false,
Expand Down Expand Up @@ -980,6 +991,7 @@ impl Field {
let mut var_max: Option<usize> = None;
let mut overrides: Vec<String> = Vec::new();
let mut conflicts: Vec<String> = Vec::new();
let mut requires: Vec<String> = Vec::new();
let mut required_if: Vec<String> = Vec::new();
let mut required_unless: Vec<String> = Vec::new();

Expand Down Expand Up @@ -1066,6 +1078,7 @@ impl Field {
// there is nothing to lose by accepting the shorter form.
"overrides" => overrides = selectors(&meta)?,
"conflicts" => conflicts = selectors(&meta)?,
"requires" => requires = selectors(&meta)?,
"required_if" => required_if = selectors(&meta)?,
"required_unless" => required_unless = selectors(&meta)?,
"value_enum" => value_enum = flag_value(&meta)?,
Expand Down Expand Up @@ -1108,7 +1121,7 @@ impl Field {
`short`, `negate`, `global`, `var`, `variadic`, \
`count`, `hide`, `arg`, `env`, `default`, `choices`, \
`var_min`, `var_max`, `value_enum`, `overrides`, \
`conflicts`, `required_if`, \
`conflicts`, `requires`, `required_if`, \
`required_unless`, `help_heading`, `value_name`, \
`required`, and `double_dash`"
),
Expand Down Expand Up @@ -1334,6 +1347,7 @@ impl Field {
for (option, selectors) in [
("overrides", &overrides),
("conflicts", &conflicts),
("requires", &requires),
("required_if", &required_if),
("required_unless", &required_unless),
] {
Expand Down Expand Up @@ -1614,6 +1628,7 @@ impl Field {
var_max,
overrides,
conflicts,
requires,
required_if,
required_unless,
hide,
Expand Down
12 changes: 12 additions & 0 deletions docs/spec/integrations/clap.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,18 @@ mycli --usage-spec | usage generate md --out-file docs.md
mycli --usage-spec | usage generate manpage --out-file mycli.1
```

## What a generated spec cannot carry

The spec is produced by reading a `clap::Command` back, so it can only carry what clap
exposes a getter for. [`requires`](/spec/reference/flag#requires) is the notable one it
does not: `Arg::requires`, `requires_if`, `requires_ifs` and `requires_all` are setters
with no reader, so a flag declared with them arrives in the spec with no requirement on
it, and everything downstream — help, docs, completions — describes a CLI without that
constraint.

Declaring it in the spec is the way to have it. A CLI that keeps its source of truth in
clap can add the node to a hand-written overlay spec merged with the generated one.

## Links

- [crate on crates.io](https://crates.io/crates/clap_usage)
Expand Down
27 changes: 27 additions & 0 deletions docs/spec/reference/flag.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,16 @@ flag "--file <file>" required_if="--dir" // if --dir is set, --file must als
flag "--file <file>" required_unless="--dir" // either --file or --dir must be present
flag "--file <file>" overrides="--stdin" // --file and --stdin override each other; the last one wins
flag "--file <file>" conflicts="--stdin" // --file and --stdin cannot be given together
flag "--out <path>" requires="--format" // giving --out means --format must be given too

flag "--stdin" {
conflicts "--file" "--url" // several, one per argument
}

flag "--sign" {
requires "--key" "--identity" // several, and all of them are needed
}

flag "--shell <shell>" {
choices "bash" "zsh" "fish" // <shell> must be one of the choices
}
Expand Down Expand Up @@ -69,6 +74,28 @@ mistake, and silently honouring one of them hides it.
A conflict holds in either direction, so declaring it once is enough; it applies to flags
that were actually given, not to defaults.

## `requires`

The positive form: giving this flag means the flags it names must be given too. Every
selector has to be satisfied, so `requires "--key" "--identity"` means both. Nothing
happens when the declaring flag is absent — a requirement is a consequence of using the
flag, not a rule about the command line as a whole.

`required_if` says the same thing from the other end, and both exist because they are
written in different places. `--out` needing `--format` is `requires="--format"` on
`--out`, or `required_if="--out"` on `--format`; the first keeps the rule on the flag it
is about, which is usually where a reader looks for it.

A value from the environment or a default satisfies a requirement, on the same principle
`conflicts` follows: the question is whether the other flag ended up with a value, not
how it got one.

::: warning
A spec generated from a clap command never carries this. clap has `Arg::requires` as a
setter with no getter, so [the clap integration](/spec/integrations/clap) cannot read it
back out — a CLI that wants the constraint in its spec has to declare it here.
:::

## `global`

A `global` flag is recognized by the command that declares it and by everything below it, so
Expand Down
Loading
Loading