Mirrors/clap
2
0
Fork 0
mirror of https://github.com/clap-rs/clap synced 2024-11-11 07:14:15 +00:00
Code Issues Projects Releases Packages Wiki Activity

Merge pull request #7 from epage/revert

Browse source 
revert: Default value behaviour with conflicts and requirement validation
This commit is contained in:
Ed Page 2021-11-17 15:56:27 -06:00 committed by GitHub
commit 11537b24a5
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 44 additions and 270 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

20
src/build/app/mod.rs
View file

@ -27,7 +27,7 @@ use crate::{
build::{arg::ArgProvider, Arg, ArgGroup, ArgSettings},
mkeymap::MKeyMap,
output::{fmt::Colorizer, Help, HelpWriter, Usage},
parse::{ArgMatcher, ArgMatches, ArgPredicate, Input, Parser},
parse::{ArgMatcher, ArgMatches, Input, Parser},
util::{color::ColorChoice, Id, Key},
Error, ErrorKind, Result as ClapResult, INTERNAL_ERROR_MSG,
};
@ -2901,16 +2901,18 @@ impl<'help> App<'help> {
pub(crate) fn unroll_requirements_for_arg(&self, arg: &Id, matcher: &ArgMatcher) -> Vec<Id> {
let requires_if_or_not = |(val, req_arg): &(Option<&str>, Id)| -> Option<Id> {
let predicate = if let Some(val) = val {
ArgPredicate::Equals(*val)
if let Some(v) = val {
if matcher
.get(arg)
.map(|ma| ma.contains_val(v))
.unwrap_or(false)
{
Some(req_arg.clone())
} else {
None
}
} else {
ArgPredicate::IsPresent
};
if matcher.check_explicit(arg, predicate) {
Some(req_arg.clone())
} else {
None
}
};

6
src/parse/arg_matcher.rs
View file

@ -4,7 +4,7 @@ use std::{collections::HashMap, ffi::OsString, mem, ops::Deref};
// Internal
use crate::{
build::{App, Arg, ArgSettings},
parse::{ArgMatches, ArgPredicate, MatchedArg, SubCommand, ValueType},
parse::{ArgMatches, MatchedArg, SubCommand, ValueType},
util::Id,
};
@ -126,10 +126,6 @@ impl ArgMatcher {
self.0.args.iter()
}
pub(crate) fn check_explicit<'a>(&self, arg: &Id, predicate: ArgPredicate<'a>) -> bool {
self.get(arg).map_or(false, |a| a.check_explicit(predicate))
}
pub(crate) fn inc_occurrence_of(&mut self, arg: &Id, ci: bool) {
debug!("ArgMatcher::inc_occurrence_of: arg={:?}", arg);
let ma = self.entry(arg).or_insert(MatchedArg::new());

4
src/parse/arg_predicate.rs
View file

@ -1,4 +0,0 @@
pub(crate) enum ArgPredicate<'a> {
IsPresent,
Equals(&'a str),
}

28
src/parse/matches/matched_arg.rs
View file

@ -5,7 +5,8 @@ use std::{
slice::Iter,
};
use crate::{parse::ArgPredicate, util::eq_ignore_case, INTERNAL_ERROR_MSG};
use crate::util::eq_ignore_case;
use crate::INTERNAL_ERROR_MSG;
// TODO: Maybe make this public?
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
@ -124,22 +125,15 @@ impl MatchedArg {
}
}
pub(crate) fn check_explicit(&self, predicate: ArgPredicate) -> bool {
if self.ty == ValueType::DefaultValue {
return false;
}
match predicate {
ArgPredicate::Equals(val) => self.vals_flatten().any(|v| {
if self.case_insensitive {
// If `v` isn't utf8, it can't match `val`, so `OsStr::to_str` should be fine
v.to_str().map_or(false, |v| eq_ignore_case(v, val))
} else {
OsString::as_os_str(v) == OsStr::new(val)
}
}),
ArgPredicate::IsPresent => true,
}
pub(crate) fn contains_val(&self, val: &str) -> bool {
self.vals_flatten().any(|v| {
if self.case_insensitive {
// If `v` isn't utf8, it can't match `val`, so `OsStr::to_str` should be fine
v.to_str().map_or(false, |v| eq_ignore_case(v, val))
} else {
OsString::as_os_str(v) == OsStr::new(val)
}
})
}
pub(crate) fn set_ty(&mut self, ty: ValueType) {

2
src/parse/mod.rs
View file

@ -2,14 +2,12 @@ pub mod errors;
pub mod features;
mod arg_matcher;
mod arg_predicate;
pub mod matches;
mod parser;
mod validator;
pub(crate) use self::{
arg_matcher::ArgMatcher,
arg_predicate::ArgPredicate,
matches::{MatchedArg, SubCommand, ValueType},
parser::{Input, ParseState, Parser},
validator::Validator,

54
src/parse/validator.rs
View file

@ -4,7 +4,7 @@ use crate::{
output::Usage,
parse::{
errors::{Error, ErrorKind, Result as ClapResult},
ArgMatcher, ArgPredicate, MatchedArg, ParseState, Parser,
ArgMatcher, MatchedArg, ParseState, Parser, ValueType,
},
util::{ChildGraph, Id},
INTERNAL_ERROR_MSG, INVALID_UTF8,
@ -70,11 +70,10 @@ impl<'help, 'app, 'parser> Validator<'help, 'app, 'parser> {
));
}
self.validate_conflicts(matcher)?;
if !(self.p.is_set(AS::SubcommandsNegateReqs) && is_subcmd || reqs_validated) {
self.validate_required(matcher)?;
self.validate_required_unless(matcher)?;
}
self.validate_matched_args(matcher)?;
Ok(())
@ -279,25 +278,17 @@ impl<'help, 'app, 'parser> Validator<'help, 'app, 'parser> {
.app
.unroll_args_in_group(&g.id)
.iter()
.filter(|&a| matcher.check_explicit(a, ArgPredicate::IsPresent))
.filter(|&a| matcher.contains(a))
.count()
> 1
};
let conf_with_arg = || {
g.conflicts
.iter()
.any(|x| matcher.check_explicit(x, ArgPredicate::IsPresent))
};
let conf_with_arg = || g.conflicts.iter().any(|x| matcher.contains(x));
let arg_conf_with_gr = || {
matcher.check_explicit(&g.id, ArgPredicate::IsPresent)
&& matcher
.arg_names()
.filter_map(|x| self.p.app.find(x))
.any(|x| x.blacklist.iter().any(|c| *c == g.id))
matcher
.arg_names()
.filter_map(|x| self.p.app.find(x))
.any(|x| x.blacklist.iter().any(|c| *c == g.id))
};
conf_with_self() || conf_with_arg() || arg_conf_with_gr()
} else if let Some(ma) = matcher.get(name) {
debug!(
@ -347,9 +338,11 @@ impl<'help, 'app, 'parser> Validator<'help, 'app, 'parser> {
debug!("Validator::gather_conflicts:iter: id={:?}", name);
// if arg is "present" only because it got default value
// it doesn't conflict with anything and should be skipped
let skip = !matcher.check_explicit(name, ArgPredicate::IsPresent);
let skip = matcher
.get(name)
.map_or(false, |a| a.ty == ValueType::DefaultValue);
if skip {
debug!("Validator::gather_conflicts:iter: This is not an explicit value, skipping.",);
debug!("Validator::gather_conflicts:iter: This is default value, skipping.",);
}
!skip
})
@ -410,10 +403,8 @@ impl<'help, 'app, 'parser> Validator<'help, 'app, 'parser> {
}
} else if let Some(g) = self.p.app.groups.iter().find(|grp| grp.id == *name) {
debug!("Validator::gather_conflicts:iter:{:?}:group", name);
if matcher.check_explicit(&g.id, ArgPredicate::IsPresent) {
for r in &g.requires {
self.p.required.insert(r.clone());
}
for r in &g.requires {
self.p.required.insert(r.clone());
}
}
}
@ -572,25 +563,21 @@ impl<'help, 'app, 'parser> Validator<'help, 'app, 'parser> {
// Validate the conditionally required args
for a in self.p.app.args.args() {
for (other, val) in &a.r_ifs {
if matcher.check_explicit(other, ArgPredicate::Equals(*val))
&& !matcher.contains(&a.id)
{
return self.missing_required_error(matcher, vec![a.id.clone()]);
if let Some(ma) = matcher.get(other) {
if ma.contains_val(val) && !matcher.contains(&a.id) {
return self.missing_required_error(matcher, vec![a.id.clone()]);
}
}
}
let match_all = a
.r_ifs_all
.iter()
.all(|(other, val)| matcher.check_explicit(other, ArgPredicate::Equals(*val)));
.all(|(other, val)| matcher.get(other).map_or(false, |ma| ma.contains_val(val)));
if match_all && !a.r_ifs_all.is_empty() && !matcher.contains(&a.id) {
return self.missing_required_error(matcher, vec![a.id.clone()]);
}
}
self.validate_required_unless(matcher)?;
Ok(())
}
@ -627,7 +614,6 @@ impl<'help, 'app, 'parser> Validator<'help, 'app, 'parser> {
})
.map(|a| a.id.clone())
.collect();
if failed_args.is_empty() {
Ok(())
} else {
@ -638,7 +624,7 @@ impl<'help, 'app, 'parser> Validator<'help, 'app, 'parser> {
// Failing a required unless means, the arg's "unless" wasn't present, and neither were they
fn fails_arg_required_unless(&self, a: &Arg<'help>, matcher: &ArgMatcher) -> bool {
debug!("Validator::fails_arg_required_unless: a={:?}", a.name);
let exists = |id| matcher.check_explicit(id, ArgPredicate::IsPresent);
let exists = |id| matcher.contains(id);
(a.r_unless_all.is_empty() || !a.r_unless_all.iter().all(exists))
&& !a.r_unless.iter().any(exists)

65
tests/conflicts.rs
View file

@ -270,68 +270,3 @@ fn conflicts_with_alongside_default() {
assert_eq!(m.value_of("opt"), Some("default"));
assert!(m.is_present("flag"));
}
#[test]
fn group_in_conflicts_with() {
let result = App::new("conflict")
.arg(
Arg::new("opt")
.long("opt")
.default_value("default")
.group("one"),
)
.arg(Arg::new("flag").long("flag").conflicts_with("one"))
.try_get_matches_from(vec!["myprog", "--flag"]);
assert!(
result.is_ok(),
"conflicts_with on an arg group should ignore default_value: {:?}",
result.unwrap_err()
);
let m = result.unwrap();
assert_eq!(m.value_of("opt"), Some("default"));
assert!(m.is_present("flag"));
}
#[test]
fn group_conflicts_with_default_value() {
let result = App::new("conflict")
.arg(
Arg::new("opt")
.long("opt")
.default_value("default")
.group("one"),
)
.arg(Arg::new("flag").long("flag").group("one"))
.try_get_matches_from(vec!["myprog", "--flag"]);
assert!(
result.is_ok(),
"arg group count should ignore default_value: {:?}",
result.unwrap_err()
);
let m = result.unwrap();
assert_eq!(m.value_of("opt"), Some("default"));
assert!(m.is_present("flag"));
}
#[test]
fn group_conflicts_with_default_arg() {
let result = App::new("conflict")
.arg(Arg::new("opt").long("opt").default_value("default"))
.arg(Arg::new("flag").long("flag").group("one"))
.group(ArgGroup::new("one").conflicts_with("opt"))
.try_get_matches_from(vec!["myprog", "--flag"]);
assert!(
result.is_ok(),
"arg group conflicts_with should ignore default_value: {:?}",
result.unwrap_err()
);
let m = result.unwrap();
assert_eq!(m.value_of("opt"), Some("default"));
assert!(m.is_present("flag"));
}

135
tests/require.rs
View file

@ -238,7 +238,7 @@ fn required_unless_present() {
}
#[test]
fn required_unless_present_err() {
fn required_unless_err() {
let res = App::new("unlesstest")
.arg(
Arg::new("cfg")
@ -1141,136 +1141,3 @@ fn required_unless_invalid_arg() {
)
.try_get_matches_from(vec![""]);
}
#[test]
fn requires_with_default_value() {
let result = App::new("prog")
.arg(
Arg::new("opt")
.long("opt")
.default_value("default")
.requires("flag"),
)
.arg(Arg::new("flag").long("flag"))
.try_get_matches_from(vec!["myprog"]);
assert!(
result.is_ok(),
"requires should ignore default_value: {:?}",
result.unwrap_err()
);
let m = result.unwrap();
assert_eq!(m.value_of("opt"), Some("default"));
assert!(!m.is_present("flag"));
}
#[test]
fn requires_if_with_default_value() {
let result = App::new("prog")
.arg(
Arg::new("opt")
.long("opt")
.default_value("default")
.requires_if("default", "flag"),
)
.arg(Arg::new("flag").long("flag"))
.try_get_matches_from(vec!["myprog"]);
assert!(
result.is_ok(),
"requires_if should ignore default_value: {:?}",
result.unwrap_err()
);
let m = result.unwrap();
assert_eq!(m.value_of("opt"), Some("default"));
assert!(!m.is_present("flag"));
}
#[test]
fn group_requires_with_default_value() {
let result = App::new("prog")
.arg(Arg::new("opt").long("opt").default_value("default"))
.arg(Arg::new("flag").long("flag"))
.group(ArgGroup::new("one").arg("opt").requires("flag"))
.try_get_matches_from(vec!["myprog"]);
assert!(
result.is_ok(),
"arg group requires should ignore default_value: {:?}",
result.unwrap_err()
);
let m = result.unwrap();
assert_eq!(m.value_of("opt"), Some("default"));
assert!(!m.is_present("flag"));
}
#[test]
fn required_if_eq_on_default_value() {
let result = App::new("prog")
.arg(Arg::new("opt").long("opt").default_value("default"))
.arg(
Arg::new("flag")
.long("flag")
.required_if_eq("opt", "default"),
)
.try_get_matches_from(vec!["myprog"]);
assert!(
result.is_ok(),
"required_if_eq should ignore default_value: {:?}",
result.unwrap_err()
);
let m = result.unwrap();
assert_eq!(m.value_of("opt"), Some("default"));
assert!(!m.is_present("flag"));
}
#[test]
fn required_if_eq_all_on_default_value() {
let result = App::new("prog")
.arg(Arg::new("opt").long("opt").default_value("default"))
.arg(
Arg::new("flag")
.long("flag")
.required_if_eq_all(&[("opt", "default")]),
)
.try_get_matches_from(vec!["myprog"]);
assert!(
result.is_ok(),
"required_if_eq_all should ignore default_value: {:?}",
result.unwrap_err()
);
let m = result.unwrap();
assert_eq!(m.value_of("opt"), Some("default"));
assert!(!m.is_present("flag"));
}
#[test]
fn required_unless_on_default_value() {
let result = App::new("prog")
.arg(Arg::new("opt").long("opt").default_value("default"))
.arg(Arg::new("flag").long("flag").required_unless_present("opt"))
.try_get_matches_from(vec!["myprog"]);
assert!(result.is_err(), "{:?}", result.unwrap());
}
#[test]
fn required_unless_all_on_default_value() {
let result = App::new("prog")
.arg(Arg::new("opt").long("opt").default_value("default"))
.arg(
Arg::new("flag")
.long("flag")
.required_unless_present_all(&["opt"]),
)
.try_get_matches_from(vec!["myprog"]);
assert!(result.is_err(), "{:?}", result.unwrap());
}