Merge pull request #3793 from epage/required

fix(validator): Ignore defaults for requireds

src/output/usage.rs

@@ -395,7 +395,13 @@ impl<'help, 'cmd> Usage<'help, 'cmd> {
             .filter(|name| !self.cmd.get_positionals().any(|p| &&p.id == name))
             .filter(|name| !self.cmd.get_groups().any(|g| &&g.id == name))
             .filter(|name| !args_in_groups.contains(name))
-            .filter(|name| !(matcher.is_some() && matcher.as_ref().unwrap().contains(name)))
+            .filter(|name| {
+                !(matcher.is_some()
+                    && matcher
+                        .as_ref()
+                        .unwrap()
+                        .check_explicit(name, ArgPredicate::IsPresent))
+            })
         {
             debug!("Usage::get_required_usage_from:iter:{:?}", a);
             let arg = self.cmd.find(a).expect(INTERNAL_ERROR_MSG).to_string();
@@ -412,7 +418,7 @@ impl<'help, 'cmd> Usage<'help, 'cmd> {
                     .cmd
                     .unroll_args_in_group(g)
                     .iter()
-                    .any(|arg| m.contains(arg));
+                    .any(|arg| m.check_explicit(arg, ArgPredicate::IsPresent));
                 if have_group_entry {
                     continue;
                 }
@@ -429,7 +435,9 @@ impl<'help, 'cmd> Usage<'help, 'cmd> {
             .iter()
             .chain(incls.iter())
             .filter(|a| self.cmd.get_positionals().any(|p| &&p.id == a))
-            .filter(|&pos| matcher.map_or(true, |m| !m.contains(pos)))
+            .filter(|&pos| {
+                matcher.map_or(true, |m| !m.check_explicit(pos, ArgPredicate::IsPresent))
+            })
             .filter_map(|pos| self.cmd.find(pos))
             .filter(|&pos| incl_last || !pos.is_last_set())
             .filter(|pos| !args_in_groups.contains(&pos.id))

src/parser/validator.rs

@@ -88,12 +88,7 @@ impl<'help, 'cmd> Validator<'help, 'cmd> {
         Ok(())
     }
 
-    fn validate_arg_values(
-        &self,
-        arg: &Arg,
-        ma: &MatchedArg,
-        matcher: &ArgMatcher,
-    ) -> ClapResult<()> {
+    fn validate_arg_values(&self, arg: &Arg, ma: &MatchedArg) -> ClapResult<()> {
         debug!("Validator::validate_arg_values: arg={:?}", arg.name);
         for val in ma.raw_vals_flatten() {
             if !arg.possible_vals.is_empty() {
@@ -121,7 +116,7 @@ impl<'help, 'cmd> Validator<'help, 'cmd> {
             }
             {
                 #![allow(deprecated)]
-                if arg.is_forbid_empty_values_set() && val.is_empty() && matcher.contains(&arg.id) {
+                if arg.is_forbid_empty_values_set() && val.is_empty() {
                     debug!("Validator::validate_arg_values: illegal empty val found");
                     return Err(Error::empty_value(
                         self.cmd,
@@ -327,7 +322,7 @@ impl<'help, 'cmd> Validator<'help, 'cmd> {
             );
             if let Some(arg) = self.cmd.find(name) {
                 self.validate_arg_num_vals(arg, ma)?;
-                self.validate_arg_values(arg, ma, matcher)?;
+                self.validate_arg_values(arg, ma)?;
                 self.validate_arg_num_occurs(arg, ma)?;
             }
             Ok(())
@@ -476,7 +471,11 @@ impl<'help, 'cmd> Validator<'help, 'cmd> {
             is_exclusive_present
         );
 
-        for arg_or_group in self.required.iter().filter(|r| !matcher.contains(r)) {
+        for arg_or_group in self
+            .required
+            .iter()
+            .filter(|r| !matcher.check_explicit(r, ArgPredicate::IsPresent))
+        {
             debug!("Validator::validate_required:iter:aog={:?}", arg_or_group);
             if let Some(arg) = self.cmd.find(arg_or_group) {
                 debug!("Validator::validate_required:iter: This is an arg");
@@ -489,7 +488,7 @@ impl<'help, 'cmd> Validator<'help, 'cmd> {
                     .cmd
                     .unroll_args_in_group(&group.id)
                     .iter()
-                    .any(|a| matcher.contains(a))
+                    .any(|a| matcher.check_explicit(a, ArgPredicate::IsPresent))
                 {
                     return self.missing_required_error(matcher, vec![]);
                 }
@@ -500,7 +499,7 @@ impl<'help, 'cmd> Validator<'help, 'cmd> {
         for a in self.cmd.get_arguments() {
             for (other, val) in &a.r_ifs {
                 if matcher.check_explicit(other, ArgPredicate::Equals(std::ffi::OsStr::new(*val)))
-                    && !matcher.contains(&a.id)
+                    && !matcher.check_explicit(&a.id, ArgPredicate::IsPresent)
                 {
                     return self.missing_required_error(matcher, vec![a.id.clone()]);
                 }
@@ -509,7 +508,10 @@ impl<'help, 'cmd> Validator<'help, 'cmd> {
             let match_all = a.r_ifs_all.iter().all(|(other, val)| {
                 matcher.check_explicit(other, ArgPredicate::Equals(std::ffi::OsStr::new(*val)))
             });
-            if match_all && !a.r_ifs_all.is_empty() && !matcher.contains(&a.id) {
+            if match_all
+                && !a.r_ifs_all.is_empty()
+                && !matcher.check_explicit(&a.id, ArgPredicate::IsPresent)
+            {
                 return self.missing_required_error(matcher, vec![a.id.clone()]);
             }
         }
@@ -537,7 +539,7 @@ impl<'help, 'cmd> Validator<'help, 'cmd> {
             .get_arguments()
             .filter(|&a| {
                 (!a.r_unless.is_empty() || !a.r_unless_all.is_empty())
-                    && !matcher.contains(&a.id)
+                    && !matcher.check_explicit(&a.id, ArgPredicate::IsPresent)
                     && self.fails_arg_required_unless(a, matcher)
             })
             .map(|a| a.id.clone())

tests/builder/action.rs

@@ -202,9 +202,9 @@ fn set_true_with_required_if_eq() {
     assert_eq!(*matches.get_one::<bool>("dog").unwrap(), false);
     assert_eq!(*matches.get_one::<bool>("mammal").unwrap(), true);
 
-    let matches = cmd.clone().try_get_matches_from(["test", "--dog"]).unwrap();
-    assert_eq!(*matches.get_one::<bool>("dog").unwrap(), true);
-    assert_eq!(*matches.get_one::<bool>("mammal").unwrap(), false);
+    cmd.clone()
+        .try_get_matches_from(["test", "--dog"])
+        .unwrap_err();
 
     let matches = cmd
         .clone()