Merge pull request clap-rs#3420 from epage/default

 Requires and Conflicts should ignore default_value

src/build/app/mod.rs

@@ -3286,32 +3286,10 @@ impl<'help> App<'help> {
         args
     }
 
-    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 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
-                    }
-                }
-                ArgPredicate::IsPresent => Some(req_arg.clone()),
-            }
-        };
-
+    pub(crate) fn unroll_arg_requires<F>(&self, func: F, arg: &Id) -> Vec<Id>
+    where
+        F: Fn(&(ArgPredicate<'_>, Id)) -> Option<Id>,
+    {
         let mut processed = vec![];
         let mut r_vec = vec![arg];
         let mut args = vec![];
@@ -3324,7 +3302,7 @@ impl<'help> App<'help> {
             processed.push(a);
 
             if let Some(arg) = self.find(a) {
-                for r in arg.requires.iter().filter_map(requires_if_or_not) {
+                for r in arg.requires.iter().filter_map(&func) {
                     if let Some(req) = self.find(&r) {
                         if !req.requires.is_empty() {
                             r_vec.push(&req.id)

src/build/arg/mod.rs

@@ -627,8 +627,6 @@ impl<'help> Arg<'help> {
     ///
     /// **NOTE:** [Conflicting] rules and [override] rules take precedence over being required
     ///
-    /// **NOTE:** An argument is considered present when there is a [`Arg::default_value`]
-    ///
     /// # Examples
     ///
     /// ```rust
@@ -3810,8 +3808,6 @@ impl<'help> Arg<'help> {
     /// This argument is [required] only if the specified `arg` is present at runtime and its value
     /// equals `val`.
     ///
-    /// **NOTE:** An argument is considered present when there is a [`Arg::default_value`]
-    ///
     /// # Examples
     ///
     /// ```rust
@@ -4058,8 +4054,6 @@ impl<'help> Arg<'help> {
     /// if this arg (`self`) is present and its value equals to `val`.
     /// If it does, `another_arg` will be marked as required.
     ///
-    /// **NOTE:** An argument is considered present when there is a [`Arg::default_value`]
-    ///
     /// # Examples
     ///
     /// ```rust
@@ -4120,8 +4114,6 @@ impl<'help> Arg<'help> {
     ///
     /// The requirement will only become valid if this arg's value equals `val`.
     ///
-    /// **NOTE:** An argument is considered present when there is a [`Arg::default_value`]
-    ///
     /// # Examples
     ///
     /// ```rust
@@ -4178,8 +4170,6 @@ impl<'help> Arg<'help> {
     /// **NOTE:** [Conflicting] rules and [override] rules take precedence over being required
     /// by default.
     ///
-    /// **NOTE:** An argument is considered present when there is a [`Arg::default_value`]
-    ///
     /// # Examples
     ///
     /// ```rust
@@ -4255,8 +4245,6 @@ impl<'help> Arg<'help> {
     ///
     /// **NOTE** [`Arg::exclusive(true)`] allows specifying an argument which conflicts with every other argument.
     ///
-    /// **NOTE:** An argument is considered present when there is a [`Arg::default_value`]
-    ///
     /// # Examples
     ///
     /// ```rust
@@ -4306,8 +4294,6 @@ impl<'help> Arg<'help> {
     ///
     /// **NOTE:** [`Arg::exclusive(true)`] allows specifying an argument which conflicts with every other argument.
     ///
-    /// **NOTE:** An argument is considered present when there is a [`Arg::default_value`]
-    ///
     /// # Examples
     ///
     /// ```rust

src/build/arg_group.rs

@@ -244,9 +244,6 @@ impl<'help> ArgGroup<'help> {
     /// Use of more than one arg is an error." Vice setting `ArgGroup::multiple(true)` which
     /// states, '*At least* one arg from this group must be used. Using multiple is OK."
     ///
-    /// **NOTE:** An argument is considered present when there is a
-    /// [`Arg::default_value`](crate::Arg::default_value)
-    ///
     /// # Examples
     ///
     /// ```rust
@@ -282,9 +279,6 @@ impl<'help> ArgGroup<'help> {
     /// [argument requirement rules], you can name other arguments or groups that must be present
     /// when any one of the arguments from this group is used.
     ///
-    /// **NOTE:** An argument is considered present when there is a
-    /// [`Arg::default_value`](crate::Arg::default_value)
-    ///
     /// **NOTE:** The name provided may be an argument or group name
     ///
     /// # Examples
@@ -324,9 +318,6 @@ impl<'help> ArgGroup<'help> {
     ///
     /// **NOTE:** The names provided may be an argument or group name
     ///
-    /// **NOTE:** An argument is considered present when there is a
-    /// [`Arg::default_value`](crate::Arg::default_value)
-    ///
     /// # Examples
     ///
     /// ```rust
@@ -368,9 +359,6 @@ impl<'help> ArgGroup<'help> {
     ///
     /// **NOTE:** The name provided may be an argument, or group name
     ///
-    /// **NOTE:** An argument is considered present when there is a
-    /// [`Arg::default_value`](crate::Arg::default_value)
-    ///
     /// # Examples
     ///
     /// ```rust
@@ -405,9 +393,6 @@ impl<'help> ArgGroup<'help> {
     ///
     /// **NOTE:** The names provided may be an argument, or group name
     ///
-    /// **NOTE:** An argument is considered present when there is a
-    /// [`Arg::default_value`](crate::Arg::default_value)
-    ///
     /// # Examples
     ///
     /// ```rust

src/output/usage.rs

@@ -3,7 +3,7 @@ use indexmap::IndexSet;
 // Internal
 use crate::{
     build::AppSettings as AS,
-    build::{App, Arg, ArgSettings},
+    build::{App, Arg, ArgPredicate, ArgSettings},
     parse::ArgMatcher,
     util::{ChildGraph, Id},
     INTERNAL_ERROR_MSG,
@@ -360,7 +360,21 @@ impl<'help, 'app> Usage<'help, 'app> {
         let mut unrolled_reqs = IndexSet::new();
 
         for a in self.required.iter() {
-            for aa in self.app.unroll_requirements_for_arg(a, matcher) {
+            let is_relevant = |(val, req_arg): &(ArgPredicate<'_>, Id)| -> Option<Id> {
+                let required = match val {
+                    ArgPredicate::Equals(_) => {
+                        if let Some(matcher) = matcher {
+                            matcher.check_explicit(a, *val)
+                        } else {
+                            false
+                        }
+                    }
+                    ArgPredicate::IsPresent => true,
+                };
+                required.then(|| req_arg.clone())
+            };
+
+            for aa in self.app.unroll_arg_requires(is_relevant, a) {
                 // 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);

src/parse/arg_matcher.rs

@@ -3,7 +3,7 @@ use std::{collections::HashMap, ffi::OsString, mem, ops::Deref};
 
 // Internal
 use crate::{
-    build::{App, Arg, ArgSettings},
+    build::{App, Arg, ArgPredicate, ArgSettings},
     parse::{ArgMatches, MatchedArg, SubCommand, ValueType},
     util::Id,
 };
@@ -101,13 +101,6 @@ impl ArgMatcher {
         self.0.args.contains_key(arg)
     }
 
-    pub(crate) fn contains_explicit(&self, arg: &Id) -> bool {
-        self.0
-            .args
-            .get(arg)
-            .map_or(false, |a| a.source() != ValueType::DefaultValue)
-    }
-
     pub(crate) fn is_empty(&self) -> bool {
         self.0.args.is_empty()
     }
@@ -132,6 +125,10 @@ impl ArgMatcher {
         self.0.args.iter()
     }
 
+    pub(crate) fn check_explicit<'a>(&self, arg: &Id, predicate: ArgPredicate<'a>) -> bool {
+        self.get(arg).map_or(false, |a| a.check_explicit(predicate))
+    }
+
     pub(crate) fn inc_occurrence_of_arg(&mut self, arg: &Arg) {
         let id = &arg.id;
         debug!("ArgMatcher::inc_occurrence_of_arg: id={:?}", id);

src/parse/matches/matched_arg.rs

@@ -5,6 +5,7 @@ use std::{
     slice::Iter,
 };
 
+use crate::build::ArgPredicate;
 use crate::util::eq_ignore_case;
 use crate::INTERNAL_ERROR_MSG;
 
@@ -116,29 +117,22 @@ impl MatchedArg {
         }
     }
 
-    pub(crate) fn contains_val(&self, val: &str) -> bool {
-        self.vals_flatten().any(|v| {
-            if self.ignore_case {
-                // If `v` isn't utf8, it can't match `val`, so `OsStr::to_str` should be fine
-                v.to_str().map_or(false, |v| eq_ignore_case(v, val))
-            } else {
-                OsString::as_os_str(v) == OsStr::new(val)
-            }
-        })
-    }
-
-    pub(crate) fn contains_val_os(&self, val: &OsStr) -> bool {
-        self.vals_flatten().any(|v| {
-            if self.ignore_case {
-                eq_ignore_case(&v.to_string_lossy(), &val.to_string_lossy())
-            } else {
-                v.as_os_str() == val
-            }
-        })
-    }
+    pub(crate) fn check_explicit(&self, predicate: ArgPredicate) -> bool {
+        if self.ty == ValueType::DefaultValue {
+            return false;
+        }
 
-    pub(crate) fn source(&self) -> ValueType {
-        self.ty
+        match predicate {
+            ArgPredicate::Equals(val) => self.vals_flatten().any(|v| {
+                if self.ignore_case {
+                    // If `v` isn't utf8, it can't match `val`, so `OsStr::to_str` should be fine
+                    eq_ignore_case(&v.to_string_lossy(), &val.to_string_lossy())
+                } else {
+                    OsString::as_os_str(v) == OsStr::new(val)
+                }
+            }),
+            ArgPredicate::IsPresent => true,
+        }
     }
 
     pub(crate) fn update_ty(&mut self, ty: ValueType) {