Skip to content

Commit

Permalink
fix(assert): Prevent arguments from requiring self
Browse files Browse the repository at this point in the history
It's non-sensical for an argument to require itself, so it must be a
mistake, and should be prevented.

This is arguably a breaking change, but of the spacebar heating kind.

Signed-off-by: Omer Tuchfeld <omer@tuchfeld.dev>
  • Loading branch information
omertuc committed Dec 3, 2024
1 parent 52aad0e commit 29d9e88
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 3 deletions.
6 changes: 6 additions & 0 deletions clap_builder/src/builder/debug_asserts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,12 @@ pub(crate) fn assert_app(cmd: &Command) {

// requires, r_if, r_unless
for (_predicate, req_id) in &arg.requires {
assert!(
&arg.id != req_id,
"Argument {} cannot require itself",
arg.get_id()
);

assert!(
cmd.id_exists(req_id),
"Command {}: Argument or group '{}' specified in 'requires*' for '{}' does not exist",
Expand Down
5 changes: 2 additions & 3 deletions tests/builder/require.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1480,10 +1480,9 @@ For more information, try '--help'.
}

#[test]
/// This test demonstrates existing broken behavior, ideally it should panic
#[should_panic = "Argument flag cannot require itself"]
fn requires_self() {
let result = Command::new("flag_required")
let _result = Command::new("flag_required")
.arg(arg!(-f --flag "some flag").requires("flag"))
.try_get_matches_from(vec![""]);
assert!(result.is_ok());
}

0 comments on commit 29d9e88

Please sign in to comment.