diff --git a/src/build/arg.rs b/src/build/arg.rs index 7be3cc9f598..6f73b4c02c2 100644 --- a/src/build/arg.rs +++ b/src/build/arg.rs @@ -4339,6 +4339,8 @@ impl<'help> Arg<'help> { /// **NOTE:** When an argument is overridden it is essentially as if it never was used, any /// conflicts, requirements, etc. are evaluated **after** all "overrides" have been removed /// + /// **NOTE:** Overriding an argument implies they [conflict][Arg::conflicts_with`]. + /// /// **WARNING:** Positional arguments and options which accept /// [`Arg::multiple_occurrences`] cannot override themselves (or we /// would never be able to advance to the next positional). If a positional @@ -4454,6 +4456,8 @@ impl<'help> Arg<'help> { /// **NOTE:** When an argument is overridden it is essentially as if it never was used, any /// conflicts, requirements, etc. are evaluated **after** all "overrides" have been removed /// + /// **NOTE:** Overriding an argument implies they [conflict][Arg::conflicts_with_all`]. + /// /// # Examples /// /// ```rust diff --git a/src/parse/parser.rs b/src/parse/parser.rs index 95f027e79fc..4697ff01ab1 100644 --- a/src/parse/parser.rs +++ b/src/parse/parser.rs @@ -1,6 +1,6 @@ // Std use std::{ - cell::{Cell, RefCell}, + cell::Cell, ffi::{OsStr, OsString}, }; @@ -22,7 +22,6 @@ use crate::{INTERNAL_ERROR_MSG, INVALID_UTF8}; pub(crate) struct Parser<'help, 'cmd> { pub(crate) cmd: &'cmd mut Command<'help>, - pub(crate) overridden: RefCell>, seen: Vec, cur_idx: Cell, /// Index of the previous flag subcommand in a group of flags. @@ -37,7 +36,6 @@ impl<'help, 'cmd> Parser<'help, 'cmd> { pub(crate) fn new(cmd: &'cmd mut Command<'help>) -> Self { Parser { cmd, - overridden: Default::default(), seen: Vec::new(), cur_idx: Cell::new(0), flag_subcmd_at: None, @@ -1248,7 +1246,6 @@ impl<'help, 'cmd> Parser<'help, 'cmd> { for override_id in &arg.overrides { debug!("Parser::remove_overrides:iter:{:?}: removing", override_id); matcher.remove(override_id); - self.overridden.borrow_mut().push(override_id.clone()); } // Override anything that can override us @@ -1263,7 +1260,6 @@ impl<'help, 'cmd> Parser<'help, 'cmd> { for overrider_id in transitive { debug!("Parser::remove_overrides:iter:{:?}: removing", overrider_id); matcher.remove(overrider_id); - self.overridden.borrow_mut().push(overrider_id.clone()); } } diff --git a/src/parse/validator.rs b/src/parse/validator.rs index d2cd3538651..1a27fc5cbc6 100644 --- a/src/parse/validator.rs +++ b/src/parse/validator.rs @@ -532,16 +532,6 @@ impl<'help, 'cmd, 'parser> Validator<'help, 'cmd, 'parser> { conflicts: &mut Conflicts, ) -> bool { debug!("Validator::is_missing_required_ok: {}", a.name); - self.arg_has_conflicts(a, matcher, conflicts) || self.p.overridden.borrow().contains(&a.id) - } - - fn arg_has_conflicts( - &self, - a: &Arg<'help>, - matcher: &ArgMatcher, - conflicts: &mut Conflicts, - ) -> bool { - debug!("Validator::arg_has_conflicts: a={:?}", a.name); let conflicts = conflicts.gather_conflicts(self.p.cmd, matcher, &a.id); !conflicts.is_empty() } @@ -666,6 +656,10 @@ impl Conflicts { } } } + + // Overrides are implicitly conflicts + conf.extend(arg.overrides.iter().cloned()); + conf } else if let Some(group) = cmd.find_group(arg_id) { group.conflicts.clone() diff --git a/tests/builder/app_settings.rs b/tests/builder/app_settings.rs index 6c6453e469c..cdcdd804047 100644 --- a/tests/builder/app_settings.rs +++ b/tests/builder/app_settings.rs @@ -1028,6 +1028,44 @@ fn aaos_opts_w_other_overrides_rev_2() { assert_eq!(m.value_of("other"), Some("val")); } +#[test] +fn aaos_opts_w_override_as_conflict_1() { + // opts with other overrides, rev + let res = Command::new("posix") + .args_override_self(true) + .arg( + arg!(--opt "some option") + .required(true) + .overrides_with("other"), + ) + .arg(arg!(--other "some other option").required(true)) + .try_get_matches_from(vec!["", "--opt=some"]); + assert!(res.is_ok(), "{}", res.unwrap_err()); + let m = res.unwrap(); + assert!(m.is_present("opt")); + assert!(!m.is_present("other")); + assert_eq!(m.value_of("opt"), Some("some")); +} + +#[test] +fn aaos_opts_w_override_as_conflict_2() { + // opts with other overrides, rev + let res = Command::new("posix") + .args_override_self(true) + .arg( + arg!(--opt "some option") + .required(true) + .overrides_with("other"), + ) + .arg(arg!(--other "some other option").required(true)) + .try_get_matches_from(vec!["", "--other=some"]); + assert!(res.is_ok(), "{}", res.unwrap_err()); + let m = res.unwrap(); + assert!(!m.is_present("opt")); + assert!(m.is_present("other")); + assert_eq!(m.value_of("other"), Some("some")); +} + #[test] fn aaos_opts_mult() { // opts with multiple