From 024cdc01b34767e8f9b44ecd7181f31c659cc068 Mon Sep 17 00:00:00 2001
From: Ed Page <eopage@gmail.com>
Date: Wed, 4 May 2022 11:05:34 -0500
Subject: [PATCH] fix(error): Don't duplicate args in usage

Gave up trying to decipher the existing logic for safe ways to
de-duplicate manually and switched to an `IndexSet` to enforce only one
of each argument exists.

Fixes #3556
---
 src/output/usage.rs      | 12 ++++++------
 src/parse/validator.rs   |  5 ++++-
 tests/builder/require.rs | 27 +++++++++++++++++++++++++++
 3 files changed, 37 insertions(+), 7 deletions(-)

diff --git a/src/output/usage.rs b/src/output/usage.rs
index be2cf23c1543..f223d9c127a8 100644
--- a/src/output/usage.rs
+++ b/src/output/usage.rs
@@ -333,14 +333,14 @@ impl<'help, 'cmd> Usage<'help, 'cmd> {
         incls: &[Id],
         matcher: Option<&ArgMatcher>,
         incl_last: bool,
-    ) -> Vec<String> {
+    ) -> IndexSet<String> {
         debug!(
             "Usage::get_required_usage_from: incls={:?}, matcher={:?}, incl_last={:?}",
             incls,
             matcher.is_some(),
             incl_last
         );
-        let mut ret_val = Vec::new();
+        let mut ret_val = IndexSet::new();
 
         let mut unrolled_reqs = IndexSet::new();
 
@@ -399,7 +399,7 @@ impl<'help, 'cmd> Usage<'help, 'cmd> {
         {
             debug!("Usage::get_required_usage_from:iter:{:?}", a);
             let arg = self.cmd.find(a).expect(INTERNAL_ERROR_MSG).to_string();
-            ret_val.push(arg);
+            ret_val.insert(arg);
         }
         let mut g_vec: Vec<String> = vec![];
         for g in unrolled_reqs
@@ -423,7 +423,7 @@ impl<'help, 'cmd> Usage<'help, 'cmd> {
                 g_vec.push(elem);
             }
         }
-        ret_val.extend_from_slice(&g_vec);
+        ret_val.extend(g_vec);
 
         let mut pvec = unrolled_reqs
             .iter()
@@ -438,9 +438,9 @@ impl<'help, 'cmd> Usage<'help, 'cmd> {
         pvec.sort_by_key(|(ind, _)| *ind); // sort by index
 
         for (_, p) in pvec {
-            debug!("Usage::get_required_usage_from:iter:{:?}", p.id);
+            debug!("Usage::get_required_usage_from:push:{:?}", p.id);
             if !args_in_groups.contains(&p.id) {
-                ret_val.push(p.to_string());
+                ret_val.insert(p.to_string());
             }
         }
 
diff --git a/src/parse/validator.rs b/src/parse/validator.rs
index 4a709efc5919..c1e0e0e6ebab 100644
--- a/src/parse/validator.rs
+++ b/src/parse/validator.rs
@@ -566,7 +566,10 @@ impl<'help, 'cmd> Validator<'help, 'cmd> {
 
         let usg = Usage::new(self.cmd).required(&self.required);
 
-        let req_args = usg.get_required_usage_from(&incl, Some(matcher), true);
+        let req_args = usg
+            .get_required_usage_from(&incl, Some(matcher), true)
+            .into_iter()
+            .collect::<Vec<_>>();
 
         debug!(
             "Validator::missing_required_error: req_args={:#?}",
diff --git a/tests/builder/require.rs b/tests/builder/require.rs
index 7caa4795135c..8136a7dd4f47 100644
--- a/tests/builder/require.rs
+++ b/tests/builder/require.rs
@@ -1345,3 +1345,30 @@ fn required_unless_all_on_default_value() {
 
     assert!(result.is_err(), "{:?}", result.unwrap());
 }
+
+#[test]
+fn required_error_doesnt_duplicate() {
+    let cmd = Command::new("Clap-created-USAGE-string-bug")
+        .arg(Arg::new("a").required(true))
+        .arg(
+            Arg::new("b")
+                .short('b')
+                .takes_value(true)
+                .conflicts_with("c"),
+        )
+        .arg(
+            Arg::new("c")
+                .short('c')
+                .takes_value(true)
+                .conflicts_with("b"),
+        );
+    const EXPECTED: &str = "\
+error: The argument '-b <b>' cannot be used with '-c <c>'
+
+USAGE:
+    clap-test -b <b> <a>
+
+For more information try --help
+";
+    utils::assert_output(cmd, "clap-test aaa -b bbb -c ccc", EXPECTED, true);
+}