Merge #2957

2957: Allow r_unless_all to be used along with r_unless_any r=ldm0 a=pksunkara



Co-authored-by: Pavan Kumar Sunkara <pavan.sss1991@gmail.com>

src/build/arg/mod.rs

@@ -96,6 +96,7 @@ pub struct Arg<'help> {
     pub(crate) r_ifs: Vec<(Id, &'help str)>,
     pub(crate) r_ifs_all: Vec<(Id, &'help str)>,
     pub(crate) r_unless: Vec<Id>,
+    pub(crate) r_unless_all: Vec<Id>,
     pub(crate) short: Option<char>,
     pub(crate) long: Option<&'help str>,
     pub(crate) aliases: Vec<(&'help str, bool)>, // (name, visible)
@@ -925,8 +926,8 @@ impl<'help> Arg<'help> {
         I: IntoIterator<Item = T>,
         T: Key,
     {
-        self.r_unless.extend(names.into_iter().map(Id::from));
-        self.setting(ArgSettings::RequiredUnlessAll)
+        self.r_unless_all.extend(names.into_iter().map(Id::from));
+        self
     }
 
     /// Deprecated, see [`Arg::required_unless_present_all`]

src/build/arg/settings.rs

@@ -14,7 +14,6 @@ bitflags! {
         const TAKES_VAL        = 1 << 5;
         const USE_DELIM        = 1 << 6;
         const NEXT_LINE_HELP   = 1 << 7;
-        const R_UNLESS_ALL     = 1 << 8;
         const REQ_DELIM        = 1 << 9;
         const DELIM_NOT_SET    = 1 << 10;
         const HIDE_POS_VALS    = 1 << 11;
@@ -54,7 +53,6 @@ impl_settings! { ArgSettings, ArgFlags,
     TakesValue("takesvalue") => Flags::TAKES_VAL,
     UseValueDelimiter("usevaluedelimiter") => Flags::USE_DELIM,
     NextLineHelp("nextlinehelp") => Flags::NEXT_LINE_HELP,
-    RequiredUnlessAll("requiredunlessall") => Flags::R_UNLESS_ALL,
     RequireDelimiter("requiredelimiter") => Flags::REQ_DELIM,
     HidePossibleValues("hidepossiblevalues") => Flags::HIDE_POS_VALS,
     AllowHyphenValues("allowhyphenvalues") => Flags::ALLOW_TAC_VALS,
@@ -126,9 +124,6 @@ pub enum ArgSettings {
     HiddenLongHelp,
     /// Specifies that option values that are invalid UTF-8 should *not* be treated as an error.
     AllowInvalidUtf8,
-
-    #[doc(hidden)]
-    RequiredUnlessAll,
 }
 
 #[cfg(test)]
@@ -157,10 +152,6 @@ mod test {
             "nextlinehelp".parse::<ArgSettings>().unwrap(),
             ArgSettings::NextLineHelp
         );
-        assert_eq!(
-            "requiredunlessall".parse::<ArgSettings>().unwrap(),
-            ArgSettings::RequiredUnlessAll
-        );
         assert_eq!(
             "requiredelimiter".parse::<ArgSettings>().unwrap(),
             ArgSettings::RequireDelimiter

src/parse/validator.rs

@@ -617,7 +617,7 @@ impl<'help, 'app, 'parser> Validator<'help, 'app, 'parser> {
             .args
             .args()
             .filter(|&a| {
-                !a.r_unless.is_empty()
+                (!a.r_unless.is_empty() || !a.r_unless_all.is_empty())
                     && !matcher.contains(&a.id)
                     && self.fails_arg_required_unless(a, matcher)
             })
@@ -634,17 +634,10 @@ 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);
-        if a.is_set(ArgSettings::RequiredUnlessAll) {
-            debug!("Validator::fails_arg_required_unless:{}:All", a.name);
-            !a.r_unless
-                .iter()
-                .all(|id| matcher.contains(id) && !matcher.is_default_value(id))
-        } else {
-            debug!("Validator::fails_arg_required_unless:{}:Any", a.name);
-            !a.r_unless
-                .iter()
-                .any(|id| matcher.contains(id) && !matcher.is_default_value(id))
-        }
+        let exists = |id| matcher.contains(id) && !matcher.is_default_value(id);
+
+        (a.r_unless_all.is_empty() || !a.r_unless_all.iter().all(exists))
+            && !a.r_unless.iter().any(exists)
     }
 
     // `incl`: an arg to include in the error even if not used

tests/require.rs

@@ -1060,6 +1060,40 @@ fn issue_2624() {
     assert!(matches.is_present("unique"));
 }
 
+#[test]
+fn required_unless_all_with_any() {
+    let app = App::new("prog")
+        .arg(Arg::new("foo").long("foo"))
+        .arg(Arg::new("bar").long("bar"))
+        .arg(Arg::new("baz").long("baz"))
+        .arg(
+            Arg::new("flag")
+                .long("flag")
+                .required_unless_present_any(&["foo"])
+                .required_unless_present_all(&["bar", "baz"]),
+        );
+
+    let result = app.clone().try_get_matches_from(vec!["myprog"]);
+
+    assert!(result.is_err(), "{:?}", result.unwrap());
+
+    let result = app.clone().try_get_matches_from(vec!["myprog", "--foo"]);
+
+    assert!(result.is_ok(), "{:?}", result.unwrap());
+    assert!(!result.unwrap().is_present("flag"));
+
+    let result = app
+        .clone()
+        .try_get_matches_from(vec!["myprog", "--bar", "--baz"]);
+
+    assert!(result.is_ok(), "{:?}", result.unwrap());
+    assert!(!result.unwrap().is_present("flag"));
+
+    let result = app.try_get_matches_from(vec!["myprog", "--bar"]);
+
+    assert!(result.is_err(), "{:?}", result.unwrap());
+}
+
 #[cfg(debug_assertions)]
 #[test]
 #[should_panic = "Argument or group 'extra' specified in 'requires*' for 'config' does not exist"]