Repository: clap-rs/clap
Pull request #4281 (2022-09-28 21:10:30)

fix(parser): Allow one-off self-overrides
bat needed this.

See also #4261

<!--
Thanks for helping out!

Please link the appropriate issue from your PR.

If you don't have an issue, we'd recommend starting with one first so the PR can focus on the
implementation (unless its an obvious bug or documentation fix that will have
little conversation).
-->



Patch:
diff --git a/src/builder/action.rs b/src/builder/action.rs
index dc7b79a7..01d1f07b 100644
--- a/src/builder/action.rs
+++ b/src/builder/action.rs
@@ -169,7 +169,7 @@ pub enum ArgAction {
     ///             .action(clap::ArgAction::SetFalse)
     ///     );
     ///
-    /// let matches = cmd.clone().try_get_matches_from(["mycmd", "--flag", "--flag"]).unwrap();
+    /// let matches = cmd.clone().try_get_matches_from(["mycmd", "--flag"]).unwrap();
     /// assert!(matches.contains_id("flag"));
     /// assert_eq!(
     ///     matches.get_one::<bool>("flag").copied(),
diff --git a/src/builder/debug_asserts.rs b/src/builder/debug_asserts.rs
index e40d9369..684e5a31 100644
--- a/src/builder/debug_asserts.rs
+++ b/src/builder/debug_asserts.rs
@@ -686,11 +686,6 @@ fn assert_arg(arg: &Arg) {
         "Argument '{}' cannot conflict with itself",
         arg.get_id(),
     );
-    assert!(
-        !arg.overrides.iter().any(|x| *x == arg.id),
-        "Argument '{}' cannot override itself, its the default",
-        arg.get_id(),
-    );
 
     assert_eq!(
         arg.get_action().takes_values(),
diff --git a/src/parser/parser.rs b/src/parser/parser.rs
index fb43923f..319d25c1 100644
--- a/src/parser/parser.rs
+++ b/src/parser/parser.rs
@@ -1180,7 +1180,9 @@ impl<'cmd> Parser<'cmd> {
                     self.cur_idx.set(self.cur_idx.get() + 1);
                     debug!("Parser::react: cur_idx:={}", self.cur_idx.get());
                 }
-                if matcher.remove(&arg.id) && !self.cmd.is_args_override_self() {
+                if matcher.remove(&arg.id)
+                    && !(self.cmd.is_args_override_self() || arg.overrides.contains(arg.get_id()))
+                {
                     return Err(ClapError::argument_conflict(
                         self.cmd,
                         arg.to_string(),
@@ -1221,7 +1223,9 @@ impl<'cmd> Parser<'cmd> {
                     raw_vals
                 };
 
-                if matcher.remove(&arg.id) && !self.cmd.is_args_override_self() {
+                if matcher.remove(&arg.id)
+                    && !(self.cmd.is_args_override_self() || arg.overrides.contains(arg.get_id()))
+                {
                     return Err(ClapError::argument_conflict(
                         self.cmd,
                         arg.to_string(),
@@ -1240,7 +1244,9 @@ impl<'cmd> Parser<'cmd> {
                     raw_vals
                 };
 
-                if matcher.remove(&arg.id) && self.cmd.is_args_override_self() {
+                if matcher.remove(&arg.id)
+                    && !(self.cmd.is_args_override_self() || arg.overrides.contains(arg.get_id()))
+                {
                     return Err(ClapError::argument_conflict(
                         self.cmd,
                         arg.to_string(),


Test patch:
diff --git a/tests/builder/action.rs b/tests/builder/action.rs
index d9b24873..4c5bf810 100644
--- a/tests/builder/action.rs
+++ b/tests/builder/action.rs
@@ -240,8 +240,15 @@ fn set_false() {
     assert_eq!(matches.contains_id("mammal"), true);
     assert_eq!(matches.index_of("mammal"), Some(1));
 
+    let result = cmd
+        .clone()
+        .try_get_matches_from(["test", "--mammal", "--mammal"]);
+    let err = result.err().unwrap();
+    assert_eq!(err.kind(), ErrorKind::ArgumentConflict);
+
     let matches = cmd
         .clone()
+        .args_override_self(true)
         .try_get_matches_from(["test", "--mammal", "--mammal"])
         .unwrap();
     assert_eq!(*matches.get_one::<bool>("mammal").unwrap(), false);
diff --git a/tests/builder/posix_compatible.rs b/tests/builder/posix_compatible.rs
index c6e202aa..f5e7739d 100644
--- a/tests/builder/posix_compatible.rs
+++ b/tests/builder/posix_compatible.rs
@@ -1,16 +1,36 @@
 use clap::{arg, error::ErrorKind, Arg, ArgAction, Command};
 
 #[test]
-#[should_panic = "Argument 'flag' cannot override itself"]
 fn flag_overrides_itself() {
-    Command::new("posix")
+    let res = Command::new("posix")
         .arg(
             arg!(--flag  "some flag"
             )
             .action(ArgAction::SetTrue)
             .overrides_with("flag"),
         )
-        .build();
+        .try_get_matches_from(vec!["", "--flag", "--flag"]);
+    assert!(res.is_ok(), "{}", res.unwrap_err());
+    let m = res.unwrap();
+    assert!(*m.get_one::<bool>("flag").expect("defaulted by clap"));
+}
+
+#[test]
+fn option_overrides_itself() {
+    let res = Command::new("posix")
+        .arg(
+            arg!(--opt <val> "some option")
+                .required(false)
+                .overrides_with("opt"),
+        )
+        .try_get_matches_from(vec!["", "--opt=some", "--opt=other"]);
+    assert!(res.is_ok(), "{}", res.unwrap_err());
+    let m = res.unwrap();
+    assert!(m.contains_id("opt"));
+    assert_eq!(
+        m.get_one::<String>("opt").map(|v| v.as_str()),
+        Some("other")
+    );
 }
 
 #[test]