Skip to content

Commit

Permalink
fix: don't bypass conflicts when using requires
Browse files Browse the repository at this point in the history
The intent for `is_missing_required_ok` is to allow a situation where
two args are required, but they're part of the same group, or one
overrides the other, which is equivalent, so having either of them is
good enough.

The way it was implemented, by reusing `gather_conflicts` introduced a
loophole where `required` could be bypassed when it shouldn't.

Fixes clap-rs#4520
  • Loading branch information
pierrechevalier83 committed Nov 29, 2022
1 parent 2c51d8f commit 26a8c18
Showing 1 changed file with 48 additions and 2 deletions.
50 changes: 48 additions & 2 deletions src/parser/validator.rs
Expand Up @@ -381,8 +381,8 @@ impl<'cmd> Validator<'cmd> {
conflicts: &mut Conflicts,
) -> bool {
debug!("Validator::is_missing_required_ok: {}", a.get_id());
let conflicts = conflicts.gather_conflicts(self.cmd, matcher, a.get_id());
!conflicts.is_empty()
let overrides = conflicts.gather_overrides(self.cmd, matcher, a.get_id());
!overrides.is_empty()
}

// Failing a required unless means, the arg's "unless" wasn't present, and neither were they
Expand Down Expand Up @@ -530,6 +530,52 @@ impl Conflicts {
conf
})
}

fn gather_overrides(&mut self, cmd: &Command, matcher: &ArgMatcher, arg_id: &Id) -> Vec<Id> {
debug!("Conflicts::gather_overrides: arg={:?}", arg_id);
let mut overrides = Vec::new();
for other_arg_id in matcher
.arg_ids()
.filter(|arg_id| matcher.check_explicit(arg_id, &ArgPredicate::IsPresent))
{
if arg_id == other_arg_id {
continue;
}

if self
.gather_direct_overrides(cmd, arg_id)
.contains(other_arg_id)
{
overrides.push(other_arg_id.clone());
}
if self
.gather_direct_overrides(cmd, other_arg_id)
.contains(arg_id)
{
overrides.push(other_arg_id.clone());
}
}
debug!("Conflicts::gather_overrides: overrides={:?}", conflicts);
overrides
}

fn gather_direct_overrides(&mut self, cmd: &Command, arg_id: &Id) -> &[Id] {
self.potential.entry(arg_id.clone()).or_insert_with(|| {
let conf = if let Some(arg) = cmd.find(arg_id) {
arg.overrides.clone()
} else if let Some(group) = cmd.find_group(arg_id) {
group.conflicts.clone()
} else {
debug_assert!(false, "id={:?} is unknown", arg_id);
Vec::new()
};
debug!(
"Conflicts::gather_direct_overrides id={:?}, overrides={:?}",
arg_id, conf
);
conf
})
}
}

pub(crate) fn get_possible_values_cli(a: &Arg) -> Vec<PossibleValue> {
Expand Down

0 comments on commit 26a8c18

Please sign in to comment.