fix!: Require/default conditional APIs are more explicit

This helps with
- API cleanup by not having ambigious `None`, see clap-rs#950
- Removes ambiguity with `None` when using owned/borrowed types for
  clap-rs#1041

CHANGELOG.md

@@ -43,6 +43,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - Looking up a group in `ArgMatches` now returns the arg `Id`s, rather than the values.
 - Various `Arg`, `Command`, and `ArgGroup` calls were switched from accepting `&[]` to `[]` via `IntoIterator`
 - `Arg::short_aliases` and other builder functions that took `&[]` need the `&` dropped
+- Changed `Arg::requires_ifs` and `Arg::default_value*_ifs*` to taking an `ArgPredicate`, removing ambiguity with `None` when accepting owned and borrowed types
+- Replaced `Arg::requires_all` with `Arg::requires_ifs` now that it takes an `ArgPredicate`
 - *(help)* Make `DeriveDisplayOrder` the default and removed the setting.  To sort help, set `next_display_order(None)` (#2808)
 - *(help)* Subcommand display order respects `Command::next_display_order` instead of `DeriveDisplayOrder` and using its own initial display order value (#2808)
 - *(env)* Parse `--help` and `--version` like any `ArgAction::SetTrue` flag (#3776)

src/builder/arg.rs

@@ -2502,9 +2502,6 @@ impl<'help> Arg<'help> {
 
     /// Specifies the value of the argument if `arg` has been used at runtime.
     ///
-    /// If `val` is set to `None`, `arg` only needs to be present. If `val` is set to `"some-val"`
-    /// then `arg` must be present at runtime **and** have the value `val`.
-    ///
     /// If `default` is set to `None`, `default_value` will be removed.
     ///
     /// **NOTE:** This setting is perfectly compatible with [`Arg::default_value`] but slightly
@@ -2522,13 +2519,14 @@ impl<'help> Arg<'help> {
     ///
     /// ```rust
     /// # use clap::{Command, Arg, ArgAction};
+    /// # use clap::builder::{ArgPredicate};
     /// let m = Command::new("prog")
     ///     .arg(Arg::new("flag")
     ///         .long("flag")
     ///         .action(ArgAction::SetTrue))
     ///     .arg(Arg::new("other")
     ///         .long("other")
-    ///         .default_value_if("flag", None, Some("default")))
+    ///         .default_value_if("flag", ArgPredicate::IsPresent, Some("default")))
     ///     .get_matches_from(vec![
     ///         "prog", "--flag"
     ///     ]);
@@ -2546,7 +2544,7 @@ impl<'help> Arg<'help> {
     ///         .action(ArgAction::SetTrue))
     ///     .arg(Arg::new("other")
     ///         .long("other")
-    ///         .default_value_if("flag", Some("true"), Some("default")))
+    ///         .default_value_if("flag", "true", Some("default")))
     ///     .get_matches_from(vec![
     ///         "prog"
     ///     ]);
@@ -2564,7 +2562,7 @@ impl<'help> Arg<'help> {
     ///         .long("opt"))
     ///     .arg(Arg::new("other")
     ///         .long("other")
-    ///         .default_value_if("opt", Some("special"), Some("default")))
+    ///         .default_value_if("opt", "special", Some("default")))
     ///     .get_matches_from(vec![
     ///         "prog", "--opt", "special"
     ///     ]);
@@ -2583,7 +2581,7 @@ impl<'help> Arg<'help> {
     ///         .long("opt"))
     ///     .arg(Arg::new("other")
     ///         .long("other")
-    ///         .default_value_if("opt", Some("special"), Some("default")))
+    ///         .default_value_if("opt", "special", Some("default")))
     ///     .get_matches_from(vec![
     ///         "prog", "--opt", "hahaha"
     ///     ]);
@@ -2603,7 +2601,7 @@ impl<'help> Arg<'help> {
     ///     .arg(Arg::new("other")
     ///         .long("other")
     ///         .default_value("default")
-    ///         .default_value_if("flag", Some("true"), None))
+    ///         .default_value_if("flag", "true", None))
     ///     .get_matches_from(vec![
     ///         "prog", "--flag"
     ///     ]);
@@ -2616,10 +2614,10 @@ impl<'help> Arg<'help> {
     pub fn default_value_if(
         self,
         arg_id: impl Into<Id>,
-        val: Option<&'help str>,
+        val: impl Into<ArgPredicate>,
         default: Option<&'help str>,
     ) -> Self {
-        self.default_value_if_os(arg_id, val.map(OsStr::new), default.map(OsStr::new))
+        self.default_value_if_os(arg_id, val.into(), default.map(OsStr::new))
     }
 
     /// Provides a conditional default value in the exact same manner as [`Arg::default_value_if`]
@@ -2631,7 +2629,7 @@ impl<'help> Arg<'help> {
     pub fn default_value_if_os(
         mut self,
         arg_id: impl Into<Id>,
-        val: Option<&'help OsStr>,
+        val: impl Into<ArgPredicate>,
         default: Option<&'help OsStr>,
     ) -> Self {
         self.default_vals_ifs
@@ -2662,8 +2660,8 @@ impl<'help> Arg<'help> {
     ///     .arg(Arg::new("other")
     ///         .long("other")
     ///         .default_value_ifs([
-    ///             ("flag", Some("true"), Some("default")),
-    ///             ("opt", Some("channal"), Some("chan")),
+    ///             ("flag", "true", Some("default")),
+    ///             ("opt", "channal", Some("chan")),
     ///         ]))
     ///     .get_matches_from(vec![
     ///         "prog", "--opt", "channal"
@@ -2683,8 +2681,8 @@ impl<'help> Arg<'help> {
     ///     .arg(Arg::new("other")
     ///         .long("other")
     ///         .default_value_ifs([
-    ///             ("flag", Some("true"), Some("default")),
-    ///             ("opt", Some("channal"), Some("chan")),
+    ///             ("flag", "true", Some("default")),
+    ///             ("opt", "channal", Some("chan")),
     ///         ]))
     ///     .get_matches_from(vec![
     ///         "prog"
@@ -2698,6 +2696,7 @@ impl<'help> Arg<'help> {
     ///
     /// ```rust
     /// # use clap::{Command, Arg, ArgAction};
+    /// # use clap::builder::ArgPredicate;
     /// let m = Command::new("prog")
     ///     .arg(Arg::new("flag")
     ///         .long("flag")
@@ -2708,8 +2707,8 @@ impl<'help> Arg<'help> {
     ///     .arg(Arg::new("other")
     ///         .long("other")
     ///         .default_value_ifs([
-    ///             ("flag", None, Some("default")),
-    ///             ("opt", Some("channal"), Some("chan")),
+    ///             ("flag", ArgPredicate::IsPresent, Some("default")),
+    ///             ("opt", ArgPredicate::Equals("channal".into()), Some("chan")),
     ///         ]))
     ///     .get_matches_from(vec![
     ///         "prog", "--opt", "channal", "--flag"
@@ -2722,10 +2721,10 @@ impl<'help> Arg<'help> {
     #[must_use]
     pub fn default_value_ifs(
         mut self,
-        ifs: impl IntoIterator<Item = (impl Into<Id>, Option<&'help str>, Option<&'help str>)>,
+        ifs: impl IntoIterator<Item = (impl Into<Id>, impl Into<ArgPredicate>, Option<&'help str>)>,
     ) -> Self {
         for (arg, val, default) in ifs {
-            self = self.default_value_if_os(arg, val.map(OsStr::new), default.map(OsStr::new));
+            self = self.default_value_if_os(arg, val, default.map(OsStr::new));
         }
         self
     }
@@ -2738,7 +2737,7 @@ impl<'help> Arg<'help> {
     #[must_use]
     pub fn default_value_ifs_os(
         mut self,
-        ifs: impl IntoIterator<Item = (impl Into<Id>, Option<&'help OsStr>, Option<&'help OsStr>)>,
+        ifs: impl IntoIterator<Item = (impl Into<Id>, impl Into<ArgPredicate>, Option<&'help OsStr>)>,
     ) -> Self {
         for (arg, val, default) in ifs {
             self = self.default_value_if_os(arg.into(), val, default);
@@ -3208,10 +3207,10 @@ impl<'help> Arg<'help> {
         self
     }
 
-    /// Require another argument if this arg was present at runtime and its value equals to `val`.
+    /// Require another argument if this arg matches the [`ArgPredicate`]
     ///
     /// This method takes `value, another_arg` pair. At runtime, clap will check
-    /// if this arg (`self`) is present and its value equals to `val`.
+    /// if this arg (`self`) matches the [`ArgPredicate`].
     /// If it does, `another_arg` will be marked as required.
     ///
     /// # Examples
@@ -3264,15 +3263,15 @@ impl<'help> Arg<'help> {
     /// [Conflicting]: Arg::conflicts_with()
     /// [override]: Arg::overrides_with()
     #[must_use]
-    pub fn requires_if(mut self, val: &'help str, arg_id: impl Into<Id>) -> Self {
-        self.requires
-            .push((ArgPredicate::Equals(val.to_owned().into()), arg_id.into()));
+    pub fn requires_if(mut self, val: impl Into<ArgPredicate>, arg_id: impl Into<Id>) -> Self {
+        self.requires.push((val.into(), arg_id.into()));
         self
     }
 
     /// Allows multiple conditional requirements.
     ///
-    /// The requirement will only become valid if this arg's value equals `val`.
+    /// The requirement will only become valid if this arg's value matches the
+    /// [`ArgPredicate`].
     ///
     /// # Examples
     ///
@@ -3311,66 +3310,19 @@ impl<'help> Arg<'help> {
     /// assert!(res.is_err()); // We  used --config=special.conf so --option <val> is required
     /// assert_eq!(res.unwrap_err().kind(), ErrorKind::MissingRequiredArgument);
     /// ```
-    /// [`Arg::requires(name)`]: Arg::requires()
-    /// [Conflicting]: Arg::conflicts_with()
-    /// [override]: Arg::overrides_with()
-    #[must_use]
-    pub fn requires_ifs(
-        mut self,
-        ifs: impl IntoIterator<Item = (&'help str, impl Into<Id>)>,
-    ) -> Self {
-        self.requires.extend(
-            ifs.into_iter()
-                .map(|(val, arg)| (ArgPredicate::Equals(val.to_owned().into()), arg.into())),
-        );
-        self
-    }
-
-    /// Require these arguments names when this one is present
     ///
-    /// i.e. when using this argument, the following arguments *must* be present.
-    ///
-    /// **NOTE:** [Conflicting] rules and [override] rules take precedence over being required
-    /// by default.
-    ///
-    /// # Examples
-    ///
-    /// ```rust
-    /// # use clap::Arg;
-    /// Arg::new("config")
-    ///     .requires_all(["input", "output"])
-    /// # ;
-    /// ```
-    ///
-    /// Setting `Arg::requires_all([arg, arg2])` requires that all the arguments be used at
-    /// runtime if the defining argument is used. If the defining argument isn't used, the other
-    /// argument isn't required
+    /// Setting `Arg::requires_ifs` with [`ArgPredicate::IsPresent`] and *not* supplying all the
+    /// arguments is an error.
     ///
     /// ```rust
-    /// # use clap::{Command, Arg, ArgAction};
+    /// # use clap::{Command, Arg, error::ErrorKind, ArgAction, builder::ArgPredicate};
     /// let res = Command::new("prog")
     ///     .arg(Arg::new("cfg")
     ///         .action(ArgAction::Set)
-    ///         .requires("input")
-    ///         .long("config"))
-    ///     .arg(Arg::new("input"))
-    ///     .arg(Arg::new("output"))
-    ///     .try_get_matches_from(vec![
-    ///         "prog"
-    ///     ]);
-    ///
-    /// assert!(res.is_ok()); // We didn't use cfg, so input and output weren't required
-    /// ```
-    ///
-    /// Setting `Arg::requires_all([arg, arg2])` and *not* supplying all the arguments is an
-    /// error.
-    ///
-    /// ```rust
-    /// # use clap::{Command, Arg, error::ErrorKind, ArgAction};
-    /// let res = Command::new("prog")
-    ///     .arg(Arg::new("cfg")
-    ///         .action(ArgAction::Set)
-    ///         .requires_all(["input", "output"])
+    ///         .requires_ifs([
+    ///             (ArgPredicate::IsPresent, "input"),
+    ///             (ArgPredicate::IsPresent, "output"),
+    ///         ])
     ///         .long("config"))
     ///     .arg(Arg::new("input"))
     ///     .arg(Arg::new("output"))
@@ -3382,15 +3334,17 @@ impl<'help> Arg<'help> {
     /// // We didn't use output
     /// assert_eq!(res.unwrap_err().kind(), ErrorKind::MissingRequiredArgument);
     /// ```
+    ///
+    /// [`Arg::requires(name)`]: Arg::requires()
     /// [Conflicting]: Arg::conflicts_with()
     /// [override]: Arg::overrides_with()
     #[must_use]
-    pub fn requires_all(mut self, names: impl IntoIterator<Item = impl Into<Id>>) -> Self {
-        self.requires.extend(
-            names
-                .into_iter()
-                .map(|s| (ArgPredicate::IsPresent, s.into())),
-        );
+    pub fn requires_ifs(
+        mut self,
+        ifs: impl IntoIterator<Item = (impl Into<ArgPredicate>, impl Into<Id>)>,
+    ) -> Self {
+        self.requires
+            .extend(ifs.into_iter().map(|(val, arg)| (val.into(), arg.into())));
         self
     }
 

src/builder/arg_group.rs

@@ -355,7 +355,7 @@ impl ArgGroup {
     /// assert_eq!(err.kind(), ErrorKind::MissingRequiredArgument);
     /// ```
     /// [required group]: ArgGroup::required()
-    /// [argument requirement rules]: crate::Arg::requires_all()
+    /// [argument requirement rules]: crate::Arg::requires_ifs()
     #[must_use]
     pub fn requires_all(mut self, ns: impl IntoIterator<Item = impl Into<Id>>) -> Self {
         for n in ns {

src/builder/arg_predicate.rs

@@ -1,16 +1,18 @@
 use crate::OsStr;
 
+/// Operations to perform on argument values
+///
+/// These do not apply to [`ValueSource::DefaultValue`][crate::parser::ValueSource::DefaultValue]
 #[derive(Clone, Debug, PartialEq, Eq)]
-pub(crate) enum ArgPredicate {
+pub enum ArgPredicate {
+    /// Is the argument present?
     IsPresent,
+    /// Does the argument match the specified value?
     Equals(OsStr),
 }
 
-impl<'help> From<Option<&'help std::ffi::OsStr>> for ArgPredicate {
-    fn from(other: Option<&'help std::ffi::OsStr>) -> Self {
-        match other {
-            Some(other) => Self::Equals(other.to_owned().into()),
-            None => Self::IsPresent,
-        }
+impl<S: Into<OsStr>> From<S> for ArgPredicate {
+    fn from(other: S) -> Self {
+        Self::Equals(other.into())
     }
 }

src/builder/mod.rs

@@ -21,6 +21,7 @@ mod tests;
 pub use action::ArgAction;
 pub use arg::Arg;
 pub use arg_group::ArgGroup;
+pub use arg_predicate::ArgPredicate;
 pub use command::Command;
 pub use possible_value::PossibleValue;
 pub use range::ValueRange;
@@ -45,5 +46,4 @@ pub use value_parser::_AnonymousValueParser;
 
 pub(crate) use action::CountType;
 pub(crate) use arg::render_arg_val;
-pub(crate) use arg_predicate::ArgPredicate;
 pub(crate) use arg_settings::{ArgFlags, ArgSettings};

tests/builder/action.rs

@@ -1,5 +1,6 @@
 #![allow(clippy::bool_assert_comparison)]
 
+use clap::builder::ArgPredicate;
 use clap::Arg;
 use clap::ArgAction;
 use clap::Command;
@@ -126,7 +127,7 @@ fn set_true_with_default_value_if_present() {
             Arg::new("mammal")
                 .long("mammal")
                 .action(ArgAction::SetTrue)
-                .default_value_if("dog", None, Some("true")),
+                .default_value_if("dog", ArgPredicate::IsPresent, Some("true")),
         )
         .arg(Arg::new("dog").long("dog").action(ArgAction::SetTrue));
 
@@ -153,7 +154,7 @@ fn set_true_with_default_value_if_value() {
             Arg::new("mammal")
                 .long("mammal")
                 .action(ArgAction::SetTrue)
-                .default_value_if("dog", Some("true"), Some("true")),
+                .default_value_if("dog", "true", Some("true")),
         )
         .arg(Arg::new("dog").long("dog").action(ArgAction::SetTrue));
 
@@ -263,7 +264,7 @@ fn set_false_with_default_value_if_present() {
             Arg::new("mammal")
                 .long("mammal")
                 .action(ArgAction::SetFalse)
-                .default_value_if("dog", None, Some("false")),
+                .default_value_if("dog", ArgPredicate::IsPresent, Some("false")),
         )
         .arg(Arg::new("dog").long("dog").action(ArgAction::SetFalse));
 
@@ -290,7 +291,7 @@ fn set_false_with_default_value_if_value() {
             Arg::new("mammal")
                 .long("mammal")
                 .action(ArgAction::SetFalse)
-                .default_value_if("dog", Some("false"), Some("false")),
+                .default_value_if("dog", "false", Some("false")),
         )
         .arg(Arg::new("dog").long("dog").action(ArgAction::SetFalse));
 
@@ -366,7 +367,7 @@ fn count_with_default_value_if_present() {
             Arg::new("mammal")
                 .long("mammal")
                 .action(ArgAction::Count)
-                .default_value_if("dog", None, Some("10")),
+                .default_value_if("dog", ArgPredicate::IsPresent, Some("10")),
         )
         .arg(Arg::new("dog").long("dog").action(ArgAction::Count));
 
@@ -393,7 +394,7 @@ fn count_with_default_value_if_value() {
             Arg::new("mammal")
                 .long("mammal")
                 .action(ArgAction::Count)
-                .default_value_if("dog", Some("2"), Some("10")),
+                .default_value_if("dog", "2", Some("10")),
         )
         .arg(Arg::new("dog").long("dog").action(ArgAction::Count));