Merge pull request #6237 from epage/conflict

 fix(parser): Don't report conflicting arguments in usage

Author: epage

clap_builder/src/parser/validator.rs

@@ -132,8 +132,7 @@ impl<'cmd> Validator<'cmd> {
         }
 
         debug!("Validator::build_conflict_err: name={name:?}");
-        let mut seen = FlatSet::new();
-        let conflicts = conflict_ids
+        let conflict_ids = conflict_ids
             .iter()
             .flat_map(|c_id| {
                 if self.cmd.find_group(c_id).is_some() {
@@ -142,16 +141,18 @@ impl<'cmd> Validator<'cmd> {
                     vec![c_id.clone()]
                 }
             })
-            .filter_map(|c_id| {
-                seen.insert(c_id.clone()).then(|| {
-                    let c_arg = self.cmd.find(&c_id).expect(INTERNAL_ERROR_MSG);
-                    c_arg.to_string()
-                })
+            .collect::<FlatSet<_>>()
+            .into_vec();
+        let conflicts = conflict_ids
+            .iter()
+            .map(|c_id| {
+                let c_arg = self.cmd.find(c_id).expect(INTERNAL_ERROR_MSG);
+                c_arg.to_string()
             })
             .collect();
 
         let former_arg = self.cmd.find(name).expect(INTERNAL_ERROR_MSG);
-        let usg = self.build_conflict_err_usage(matcher, conflict_ids);
+        let usg = self.build_conflict_err_usage(matcher, &conflict_ids);
         Err(Error::argument_conflict(
             self.cmd,
             former_arg.to_string(),

clap_builder/src/util/flat_set.rs

@@ -60,6 +60,10 @@ impl<T: PartialEq + Eq> FlatSet<T> {
     {
         self.inner.sort_by_key(f);
     }
+
+    pub(crate) fn into_vec(self) -> Vec<T> {
+        self.inner
+    }
 }
 
 impl<T: PartialEq + Eq> Default for FlatSet<T> {

tests/builder/groups.rs

@@ -1,4 +1,5 @@
 use clap::{arg, error::ErrorKind, Arg, ArgAction, ArgGroup, Command, Id};
+use snapbox::str;
 
 use super::utils;
 
@@ -131,14 +132,6 @@ fn empty_group() {
 #[test]
 #[cfg(feature = "error-context")]
 fn req_group_usage_string() {
-    static REQ_GROUP_USAGE: &str = "error: the following required arguments were not provided:
-  <base|--delete>
-
-Usage: clap-test <base|--delete>
-
-For more information, try '--help'.
-";
-
     let cmd = Command::new("req_group")
         .arg(arg!([base] "Base commit"))
         .arg(arg!(
@@ -150,20 +143,25 @@ For more information, try '--help'.
                 .required(true),
         );
 
-    utils::assert_output(cmd, "clap-test", REQ_GROUP_USAGE, true);
-}
-
-#[test]
-#[cfg(feature = "error-context")]
-fn req_group_with_conflict_usage_string() {
-    static REQ_GROUP_CONFLICT_USAGE: &str = "\
-error: the argument '--delete' cannot be used with '[base]'
+    utils::assert_output(
+        cmd,
+        "clap-test",
+        str![[r#"
+error: the following required arguments were not provided:
+  <base|--delete>
 
 Usage: clap-test <base|--delete>
 
 For more information, try '--help'.
-";
 
+"#]],
+        true,
+    );
+}
+
+#[test]
+#[cfg(feature = "error-context")]
+fn req_group_with_conflict_usage_string() {
     let cmd = Command::new("req_group")
         .arg(arg!([base] "Base commit").conflicts_with("delete"))
         .arg(arg!(
@@ -178,22 +176,21 @@ For more information, try '--help'.
     utils::assert_output(
         cmd,
         "clap-test --delete base",
-        REQ_GROUP_CONFLICT_USAGE,
+        str![[r#"
+error: the argument '--delete' cannot be used with '[base]'
+
+Usage: clap-test <base|--delete>
+
+For more information, try '--help'.
+
+"#]],
         true,
     );
 }
 
 #[test]
 #[cfg(feature = "error-context")]
 fn req_group_with_conflict_usage_string_only_options() {
-    static REQ_GROUP_CONFLICT_ONLY_OPTIONS: &str = "\
-error: the argument '--delete' cannot be used with '--all'
-
-Usage: clap-test <--all|--delete>
-
-For more information, try '--help'.
-";
-
     let cmd = Command::new("req_group")
         .arg(arg!(-a --all "All").conflicts_with("delete"))
         .arg(arg!(
@@ -207,7 +204,39 @@ For more information, try '--help'.
     utils::assert_output(
         cmd,
         "clap-test --delete --all",
-        REQ_GROUP_CONFLICT_ONLY_OPTIONS,
+        str![[r#"
+error: the argument '--delete' cannot be used with '--all'
+
+Usage: clap-test <--all|--delete>
+
+For more information, try '--help'.
+
+"#]],
+        true,
+    );
+}
+
+#[test]
+fn conflict_with_group() {
+    let cmd = Command::new("prog")
+        .group(ArgGroup::new("group").multiple(true))
+        .arg(arg!(--a).group("group"))
+        .arg(arg!(--b).group("group"))
+        .arg(arg!(--conflict).conflicts_with("group"));
+
+    utils::assert_output(
+        cmd,
+        "prog --a --conflict",
+        str![[r#"
+error: the argument '--conflict' cannot be used with:
+  --a
+  --b
+
+Usage: prog --conflict
+
+For more information, try '--help'.
+
+"#]],
         true,
     );
 }
@@ -264,19 +293,24 @@ fn group_overrides_required() {
 
 #[test]
 fn group_usage_use_val_name() {
-    static GROUP_USAGE_USE_VAL_NAME: &str = "\
+    let cmd = Command::new("prog")
+        .arg(Arg::new("a").value_name("A"))
+        .group(ArgGroup::new("group").arg("a").required(true));
+    utils::assert_output(
+        cmd,
+        "prog --help",
+        str![[r#"
 Usage: prog <A>
 
 Arguments:
   [A]  
 
 Options:
   -h, --help  Print help
-";
-    let cmd = Command::new("prog")
-        .arg(Arg::new("a").value_name("A"))
-        .group(ArgGroup::new("group").arg("a").required(true));
-    utils::assert_output(cmd, "prog --help", GROUP_USAGE_USE_VAL_NAME, false);
+
+"#]],
+        false,
+    );
 }
 
 #[test]
@@ -304,42 +338,50 @@ fn group_acts_like_arg() {
 
 #[test]
 fn conflict_with_overlapping_group_in_error() {
-    static ERR: &str = "\
-error: the argument '--major' cannot be used with '--minor'
-
-Usage: prog --major
-
-For more information, try '--help'.
-";
-
     let cmd = Command::new("prog")
         .group(ArgGroup::new("all").multiple(true))
         .arg(arg!(--major).group("vers").group("all"))
         .arg(arg!(--minor).group("vers").group("all"))
         .arg(arg!(--other).group("all"));
 
-    utils::assert_output(cmd, "prog --major --minor", ERR, true);
-}
-
-#[test]
-fn requires_group_with_overlapping_group_in_error() {
-    static ERR: &str = "\
-error: the following required arguments were not provided:
-  <--in|--spec>
+    utils::assert_output(
+        cmd,
+        "prog --major --minor",
+        str![[r#"
+error: the argument '--major' cannot be used with '--minor'
 
-Usage: prog --config <--in|--spec>
+Usage: prog --major
 
 For more information, try '--help'.
-";
 
+"#]],
+        true,
+    );
+}
+
+#[test]
+fn requires_group_with_overlapping_group_in_error() {
     let cmd = Command::new("prog")
         .group(ArgGroup::new("all").multiple(true))
         .group(ArgGroup::new("input").required(true))
         .arg(arg!(--in).group("input").group("all"))
         .arg(arg!(--spec).group("input").group("all"))
         .arg(arg!(--config).requires("input").group("all"));
 
-    utils::assert_output(cmd, "prog --config", ERR, true);
+    utils::assert_output(
+        cmd,
+        "prog --config",
+        str![[r#"
+error: the following required arguments were not provided:
+  <--in|--spec>
+
+Usage: prog --config <--in|--spec>
+
+For more information, try '--help'.
+
+"#]],
+        true,
+    );
 }
 
 /* This is used to be fixed in a hack, we need to find a better way to fix it.