fix(Required Unless): fixes a bug where having required_unless set doesn't work when conflicts are also set

Closes #753

Author: kbknapp

src/app/parser.rs

@@ -1608,13 +1608,10 @@ impl<'a, 'b> Parser<'a, 'b>
     fn validate_num_args(&self, matcher: &mut ArgMatcher) -> ClapResult<()> {
         debugln!("fn=validate_num_args;");
         for (name, ma) in matcher.iter() {
-            if self.groups.contains_key(&**name) {
-                continue;
-            } else if let Some(opt) = find_by_name!(self, name, opts, iter) {
+            debugln!("iter;name={}", name);
+            if let Some(opt) = find_by_name!(self, name, opts, iter) {
                 try!(self._validate_num_vals(opt, ma, matcher));
-            } else if let Some(pos) = self.positionals
-                .values()
-                .find(|p| &p.b.name == name) {
+            } else if let Some(pos) = find_by_name!(self, name, positionals, values) {
                 try!(self._validate_num_vals(pos, ma, matcher));
             }
         }
@@ -1685,7 +1682,9 @@ impl<'a, 'b> Parser<'a, 'b>
     }
 
     fn validate_required(&self, matcher: &ArgMatcher) -> ClapResult<()> {
+        debugln!("fn=validate_required;required={:?};", self.required);
         'outer: for name in &self.required {
+            debugln!("iter;name={}", name);
             if matcher.contains(name) {
                 continue 'outer;
             }
@@ -1735,27 +1734,36 @@ impl<'a, 'b> Parser<'a, 'b>
     fn is_missing_required_ok<A>(&self, a: &A, matcher: &ArgMatcher) -> bool
         where A: AnyArg<'a, 'b>
     {
+        debugln!("fn=is_missing_required_ok;a={}", a.name());
         if let Some(bl) = a.blacklist() {
+            debugln!("Conflicts found...{:?}", bl);
             for n in bl.iter() {
+                debugln!("iter;conflict={}", n);
                 if matcher.contains(n) ||
                    self.groups
                     .get(n)
                     .map_or(false, |g| g.args.iter().any(|an| matcher.contains(an))) {
                     return true;
                 }
             }
-        } else if let Some(ru) = a.required_unless() {
+        } 
+        if let Some(ru) = a.required_unless() {
+            debugln!("Required unless found...{:?}", ru);
             let mut found_any = false;
             for n in ru.iter() {
+                debugln!("iter;ru={}", n);
                 if matcher.contains(n) ||
                    self.groups
                     .get(n)
                     .map_or(false, |g| g.args.iter().any(|an| matcher.contains(an))) {
                     if !a.is_set(ArgSettings::RequiredUnlessAll) {
+                        debugln!("Doesn't require all...returning true");
                         return true;
                     }
+                    debugln!("Requires all...next");
                     found_any = true;
                 } else if a.is_set(ArgSettings::RequiredUnlessAll) {
+                    debugln!("Not in matcher, or group and requires all...returning false");
                     return false;
                 }
             }

tests/require.rs

@@ -174,6 +174,23 @@ fn arg_require_group_3() {
 
 // REQUIRED_UNLESS
 
+#[test]
+fn issue_753() {
+    let m = App::new("test")
+        .arg(Arg::from_usage("-l, --list 'List available interfaces (and stop there)'"))
+        .arg(Arg::from_usage("-i, --iface=[INTERFACE] 'Ethernet interface for fetching NTP packets'")
+            .required_unless("list"))
+        .arg(Arg::from_usage("-f, --file=[TESTFILE] 'Fetch NTP packets from pcap file'")
+            .conflicts_with("iface")
+            .required_unless("list"))
+        .arg(Arg::from_usage("-s, --server=[SERVER_IP] 'NTP server IP address'")
+            .required_unless("list"))
+        .arg(Arg::from_usage("-p, --port=[SERVER_PORT] 'NTP server port'")
+            .default_value("123"))
+        .get_matches_from_safe(vec!["test", "--list"]);
+   assert!(m.is_ok());
+}
+
 #[test]
 fn required_unless() {
     let res = App::new("unlesstest")