Mirrors/clap
2
0
Fork 0
mirror of https://github.com/clap-rs/clap synced 2024-12-13 14:22:34 +00:00
Code Issues Projects Releases Packages Wiki Activity

revert: Default value behaviour with conflicts and requirement validation

Browse source 
This feature is too immature at this stage in the release.   See
clap-rs/clap Issue 3020 when bringing this feature back.

This reverts commit 301c6f765a.
This reverts commit 43a4c90c86.
This reverts commit 4e29777b21.
This reverts commit 69957c4ddd.
This reverts commit bdb1d324a5.
This reverts commit b102da0cd2.
This reverts commit 72429be14e.
This reverts commit 0b7def675b.
This reverts commit b86aa631be.
This reverts commit 6b458c602d.
This commit is contained in:
Ed Page 2021-11-17 15:40:57 -06:00
parent 58b0fe537e
commit 9753f68a76
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());
}