Merge pull request #3390 from epage/unroll

fix(error): Be more accurate in smart usage

src/build/app/mod.rs

@@ -3238,16 +3238,24 @@ impl<'help> App<'help> {
         args
     }
 
-    pub(crate) fn unroll_requirements_for_arg(&self, arg: &Id, matcher: &ArgMatcher) -> Vec<Id> {
+    pub(crate) fn unroll_requirements_for_arg(
+        &self,
+        arg: &Id,
+        matcher: Option<&ArgMatcher>,
+    ) -> Vec<Id> {
         let requires_if_or_not = |(val, req_arg): &(ArgPredicate<'_>, Id)| -> Option<Id> {
             match val {
                 ArgPredicate::Equals(v) => {
-                    if matcher
-                        .get(arg)
-                        .map(|ma| ma.contains_val_os(v))
-                        .unwrap_or(false)
-                    {
-                        Some(req_arg.clone())
+                    if let Some(matcher) = matcher {
+                        if matcher
+                            .get(arg)
+                            .map(|ma| ma.contains_val_os(v))
+                            .unwrap_or(false)
+                        {
+                            Some(req_arg.clone())
+                        } else {
+                            None
+                        }
                     } else {
                         None
                     }

src/output/usage.rs

@@ -360,12 +360,10 @@ impl<'help, 'app> Usage<'help, 'app> {
         let mut unrolled_reqs = IndexSet::new();
 
         for a in self.required.iter() {
-            if let Some(m) = matcher {
-                for aa in self.app.unroll_requirements_for_arg(a, m) {
-                    // if we don't check for duplicates here this causes duplicate error messages
-                    // see https://github.com/clap-rs/clap/issues/2770
-                    unrolled_reqs.insert(aa);
-                }
+            for aa in self.app.unroll_requirements_for_arg(a, matcher) {
+                // if we don't check for duplicates here this causes duplicate error messages
+                // see https://github.com/clap-rs/clap/issues/2770
+                unrolled_reqs.insert(aa);
             }
             // always include the required arg itself. it will not be enumerated
             // by unroll_requirements_for_arg.

src/parse/validator.rs

@@ -272,7 +272,11 @@ impl<'help, 'app, 'parser> Validator<'help, 'app, 'parser> {
         for name in matcher.arg_names() {
             debug!("Validator::gather_requirements:iter:{:?}", name);
             if let Some(arg) = self.p.app.find(name) {
-                for req in self.p.app.unroll_requirements_for_arg(&arg.id, matcher) {
+                for req in self
+                    .p
+                    .app
+                    .unroll_requirements_for_arg(&arg.id, Some(matcher))
+                {
                     self.p.required.insert(req);
                 }
             } else if let Some(g) = self.p.app.find_group(name) {

tests/builder/double_require.rs

@@ -16,7 +16,7 @@ static ONLY_B_ERROR: &str = "error: The following required arguments were not pr
     -c
 
 USAGE:
-    prog [OPTIONS] -c -b
+    prog [OPTIONS] -b -c
 
 For more information try --help
 ";
@@ -25,7 +25,7 @@ static ONLY_C_ERROR: &str = "error: The following required arguments were not pr
     -b
 
 USAGE:
-    prog [OPTIONS] -b -c
+    prog [OPTIONS] -c -b
 
 For more information try --help
 ";

tests/builder/require.rs

@@ -115,6 +115,81 @@ fn positional_required_2() {
     assert_eq!(m.value_of("flag").unwrap(), "someval");
 }
 
+#[test]
+fn positional_required_with_requires() {
+    let app = App::new("positional_required")
+        .arg(Arg::new("flag").required(true).requires("opt"))
+        .arg(Arg::new("opt"))
+        .arg(Arg::new("bar"));
+
+    assert!(utils::compare_output(
+        app,
+        "clap-test",
+        POSITIONAL_REQ,
+        true
+    ));
+}
+
+static POSITIONAL_REQ: &str = "error: The following required arguments were not provided:
+    <flag>
+    <opt>
+
+USAGE:
+    clap-test <flag> <opt> [ARGS]
+
+For more information try --help
+";
+
+#[test]
+fn positional_required_with_requires_if_no_value() {
+    let app = App::new("positional_required")
+        .arg(Arg::new("flag").required(true).requires_if("val", "opt"))
+        .arg(Arg::new("opt"))
+        .arg(Arg::new("bar"));
+
+    assert!(utils::compare_output(
+        app,
+        "clap-test",
+        POSITIONAL_REQ_IF_NO_VAL,
+        true
+    ));
+}
+
+static POSITIONAL_REQ_IF_NO_VAL: &str = "error: The following required arguments were not provided:
+    <flag>
+
+USAGE:
+    clap-test <flag> [ARGS]
+
+For more information try --help
+";
+
+#[test]
+fn positional_required_with_requires_if_value() {
+    let app = App::new("positional_required")
+        .arg(Arg::new("flag").required(true).requires_if("val", "opt"))
+        .arg(Arg::new("foo").required(true))
+        .arg(Arg::new("opt"))
+        .arg(Arg::new("bar"));
+
+    assert!(utils::compare_output(
+        app,
+        "clap-test val",
+        POSITIONAL_REQ_IF_VAL,
+        true
+    ));
+}
+
+static POSITIONAL_REQ_IF_VAL: &str = "error: The following required arguments were not provided:
+    <foo>
+    <opt>
+
+USAGE:
+    clap-test <flag> <foo> <opt> [ARGS]
+
+For more information try --help
+";
+
 #[test]
 fn group_required() {
     let result = App::new("group_required")