fix(parser): Provide default value for Actions

Actions were inspired by Python and Python does not implicitly default
any field when an action is given.  From a Builder API perspective, this
seemed fine because we tend to focus the Builder API on giving the user
all information so they can make their own decisions.  When working on
the Derive API, this became a problem because users were going to have
to migrate from an implied default to an explicit default when a common
default is good enough most of the time.  This shouldn't interfere with
Builder users getting more details when needed.

This also highlighted two problems
- We set the index for defaults
- We don't debug_assert when applying conditional requirements with a
  default present

src/builder/action.rs

@@ -119,6 +119,8 @@ pub enum ArgAction {
     IncOccurrence,
     /// When encountered, act as if `"true"` was encountered on the command-line
     ///
+    /// If no [`default_value`][super::Arg::default_value] is set, it will be `false`.
+    ///
     /// No value is allowed
     ///
     /// # Examples
@@ -133,17 +135,27 @@ pub enum ArgAction {
     ///             .action(clap::builder::ArgAction::SetTrue)
     ///     );
     ///
-    /// let matches = cmd.try_get_matches_from(["mycmd", "--flag", "--flag"]).unwrap();
+    /// let matches = cmd.clone().try_get_matches_from(["mycmd", "--flag", "--flag"]).unwrap();
     /// assert!(matches.is_present("flag"));
     /// assert_eq!(matches.occurrences_of("flag"), 0);
     /// assert_eq!(
     ///     matches.get_one::<bool>("flag").copied(),
     ///     Some(true)
     /// );
+    ///
+    /// let matches = cmd.try_get_matches_from(["mycmd"]).unwrap();
+    /// assert!(matches.is_present("flag"));
+    /// assert_eq!(matches.occurrences_of("flag"), 0);
+    /// assert_eq!(
+    ///     matches.get_one::<bool>("flag").copied(),
+    ///     Some(false)
+    /// );
     /// ```
     SetTrue,
     /// When encountered, act as if `"false"` was encountered on the command-line
     ///
+    /// If no [`default_value`][super::Arg::default_value] is set, it will be `true`.
+    ///
     /// No value is allowed
     ///
     /// # Examples
@@ -158,17 +170,27 @@ pub enum ArgAction {
     ///             .action(clap::builder::ArgAction::SetFalse)
     ///     );
     ///
-    /// let matches = cmd.try_get_matches_from(["mycmd", "--flag", "--flag"]).unwrap();
+    /// let matches = cmd.clone().try_get_matches_from(["mycmd", "--flag", "--flag"]).unwrap();
     /// assert!(matches.is_present("flag"));
     /// assert_eq!(matches.occurrences_of("flag"), 0);
     /// assert_eq!(
     ///     matches.get_one::<bool>("flag").copied(),
     ///     Some(false)
     /// );
+    ///
+    /// let matches = cmd.try_get_matches_from(["mycmd"]).unwrap();
+    /// assert!(matches.is_present("flag"));
+    /// assert_eq!(matches.occurrences_of("flag"), 0);
+    /// assert_eq!(
+    ///     matches.get_one::<bool>("flag").copied(),
+    ///     Some(true)
+    /// );
     /// ```
     SetFalse,
     /// When encountered, increment a `u64` counter
     ///
+    /// If no [`default_value`][super::Arg::default_value] is set, it will be `0`.
+    ///
     /// No value is allowed
     ///
     /// # Examples
@@ -183,13 +205,21 @@ pub enum ArgAction {
     ///             .action(clap::builder::ArgAction::Count)
     ///     );
     ///
-    /// let matches = cmd.try_get_matches_from(["mycmd", "--flag", "--flag"]).unwrap();
+    /// let matches = cmd.clone().try_get_matches_from(["mycmd", "--flag", "--flag"]).unwrap();
     /// assert!(matches.is_present("flag"));
     /// assert_eq!(matches.occurrences_of("flag"), 0);
     /// assert_eq!(
     ///     matches.get_one::<u64>("flag").copied(),
     ///     Some(2)
     /// );
+    ///
+    /// let matches = cmd.try_get_matches_from(["mycmd"]).unwrap();
+    /// assert!(matches.is_present("flag"));
+    /// assert_eq!(matches.occurrences_of("flag"), 0);
+    /// assert_eq!(
+    ///     matches.get_one::<u64>("flag").copied(),
+    ///     Some(0)
+    /// );
     /// ```
     Count,
     /// When encountered, display [`Command::print_help`][super::App::print_help]
@@ -246,6 +276,20 @@ pub enum ArgAction {
 }
 
 impl ArgAction {
+    pub(crate) fn default_value(&self) -> Option<&'static std::ffi::OsStr> {
+        match self {
+            Self::Set => None,
+            Self::Append => None,
+            Self::StoreValue => None,
+            Self::IncOccurrence => None,
+            Self::SetTrue => Some(std::ffi::OsStr::new("false")),
+            Self::SetFalse => Some(std::ffi::OsStr::new("true")),
+            Self::Count => Some(std::ffi::OsStr::new("0")),
+            Self::Help => None,
+            Self::Version => None,
+        }
+    }
+
     pub(crate) fn takes_value(&self) -> bool {
         match self {
             Self::Set => true,

src/builder/arg.rs

@@ -4892,6 +4892,11 @@ impl<'help> Arg<'help> {
             self.settings.set(ArgSettings::TakesValue);
         }
         if let Some(action) = self.action.as_ref() {
+            if let Some(default_value) = action.default_value() {
+                if self.default_vals.is_empty() {
+                    self.default_vals = vec![default_value];
+                }
+            }
             if action.takes_value() {
                 self.settings.set(ArgSettings::TakesValue);
             } else {

tests/builder/action.rs

@@ -81,10 +81,10 @@ fn set_true() {
         Command::new("test").arg(Arg::new("mammal").long("mammal").action(ArgAction::SetTrue));
 
     let matches = cmd.clone().try_get_matches_from(["test"]).unwrap();
-    assert_eq!(matches.get_one::<bool>("mammal"), None);
-    assert_eq!(matches.is_present("mammal"), false);
+    assert_eq!(*matches.get_one::<bool>("mammal").unwrap(), false);
+    assert_eq!(matches.is_present("mammal"), true);
     assert_eq!(matches.occurrences_of("mammal"), 0);
-    assert_eq!(matches.index_of("mammal"), None);
+    assert_eq!(matches.index_of("mammal"), Some(1));
 
     let matches = cmd
         .clone()
@@ -106,7 +106,7 @@ fn set_true() {
 }
 
 #[test]
-fn set_true_with_default_value() {
+fn set_true_with_explicit_default_value() {
     let cmd = Command::new("test").arg(
         Arg::new("mammal")
             .long("mammal")
@@ -141,6 +141,10 @@ fn set_true_with_default_value_if_present() {
         )
         .arg(Arg::new("dog").long("dog").action(ArgAction::SetTrue));
 
+    let matches = cmd.clone().try_get_matches_from(["test"]).unwrap();
+    assert_eq!(*matches.get_one::<bool>("dog").unwrap(), false);
+    assert_eq!(*matches.get_one::<bool>("mammal").unwrap(), false);
+
     let matches = cmd.clone().try_get_matches_from(["test", "--dog"]).unwrap();
     assert_eq!(*matches.get_one::<bool>("dog").unwrap(), true);
     assert_eq!(*matches.get_one::<bool>("mammal").unwrap(), true);
@@ -149,7 +153,7 @@ fn set_true_with_default_value_if_present() {
         .clone()
         .try_get_matches_from(["test", "--mammal"])
         .unwrap();
-    assert_eq!(matches.get_one::<bool>("dog"), None);
+    assert_eq!(*matches.get_one::<bool>("dog").unwrap(), false);
     assert_eq!(*matches.get_one::<bool>("mammal").unwrap(), true);
 }
 
@@ -164,6 +168,10 @@ fn set_true_with_default_value_if_value() {
         )
         .arg(Arg::new("dog").long("dog").action(ArgAction::SetTrue));
 
+    let matches = cmd.clone().try_get_matches_from(["test"]).unwrap();
+    assert_eq!(*matches.get_one::<bool>("dog").unwrap(), false);
+    assert_eq!(*matches.get_one::<bool>("mammal").unwrap(), false);
+
     let matches = cmd.clone().try_get_matches_from(["test", "--dog"]).unwrap();
     assert_eq!(*matches.get_one::<bool>("dog").unwrap(), true);
     assert_eq!(*matches.get_one::<bool>("mammal").unwrap(), true);
@@ -172,7 +180,7 @@ fn set_true_with_default_value_if_value() {
         .clone()
         .try_get_matches_from(["test", "--mammal"])
         .unwrap();
-    assert_eq!(matches.get_one::<bool>("dog"), None);
+    assert_eq!(*matches.get_one::<bool>("dog").unwrap(), false);
     assert_eq!(*matches.get_one::<bool>("mammal").unwrap(), true);
 }
 
@@ -191,12 +199,12 @@ fn set_true_with_required_if_eq() {
         .clone()
         .try_get_matches_from(["test", "--mammal"])
         .unwrap();
-    assert_eq!(matches.get_one::<u64>("dog"), None);
+    assert_eq!(*matches.get_one::<bool>("dog").unwrap(), false);
     assert_eq!(*matches.get_one::<bool>("mammal").unwrap(), true);
 
-    cmd.clone()
-        .try_get_matches_from(["test", "--dog"])
-        .unwrap_err();
+    let matches = cmd.clone().try_get_matches_from(["test", "--dog"]).unwrap();
+    assert_eq!(*matches.get_one::<bool>("dog").unwrap(), true);
+    assert_eq!(*matches.get_one::<bool>("mammal").unwrap(), false);
 
     let matches = cmd
         .clone()
@@ -215,10 +223,10 @@ fn set_false() {
     );
 
     let matches = cmd.clone().try_get_matches_from(["test"]).unwrap();
-    assert_eq!(matches.get_one::<bool>("mammal"), None);
-    assert_eq!(matches.is_present("mammal"), false);
+    assert_eq!(*matches.get_one::<bool>("mammal").unwrap(), true);
+    assert_eq!(matches.is_present("mammal"), true);
     assert_eq!(matches.occurrences_of("mammal"), 0);
-    assert_eq!(matches.index_of("mammal"), None);
+    assert_eq!(matches.index_of("mammal"), Some(1));
 
     let matches = cmd
         .clone()
@@ -240,7 +248,7 @@ fn set_false() {
 }
 
 #[test]
-fn set_false_with_default_value() {
+fn set_false_with_explicit_default_value() {
     let cmd = Command::new("test").arg(
         Arg::new("mammal")
             .long("mammal")
@@ -275,6 +283,10 @@ fn set_false_with_default_value_if_present() {
         )
         .arg(Arg::new("dog").long("dog").action(ArgAction::SetFalse));
 
+    let matches = cmd.clone().try_get_matches_from(["test"]).unwrap();
+    assert_eq!(*matches.get_one::<bool>("dog").unwrap(), true);
+    assert_eq!(*matches.get_one::<bool>("mammal").unwrap(), true);
+
     let matches = cmd.clone().try_get_matches_from(["test", "--dog"]).unwrap();
     assert_eq!(*matches.get_one::<bool>("dog").unwrap(), false);
     assert_eq!(*matches.get_one::<bool>("mammal").unwrap(), false);
@@ -283,7 +295,7 @@ fn set_false_with_default_value_if_present() {
         .clone()
         .try_get_matches_from(["test", "--mammal"])
         .unwrap();
-    assert_eq!(matches.get_one::<bool>("dog"), None);
+    assert_eq!(*matches.get_one::<bool>("dog").unwrap(), true);
     assert_eq!(*matches.get_one::<bool>("mammal").unwrap(), false);
 }
 
@@ -298,6 +310,10 @@ fn set_false_with_default_value_if_value() {
         )
         .arg(Arg::new("dog").long("dog").action(ArgAction::SetFalse));
 
+    let matches = cmd.clone().try_get_matches_from(["test"]).unwrap();
+    assert_eq!(*matches.get_one::<bool>("dog").unwrap(), true);
+    assert_eq!(*matches.get_one::<bool>("mammal").unwrap(), true);
+
     let matches = cmd.clone().try_get_matches_from(["test", "--dog"]).unwrap();
     assert_eq!(*matches.get_one::<bool>("dog").unwrap(), false);
     assert_eq!(*matches.get_one::<bool>("mammal").unwrap(), false);
@@ -306,7 +322,7 @@ fn set_false_with_default_value_if_value() {
         .clone()
         .try_get_matches_from(["test", "--mammal"])
         .unwrap();
-    assert_eq!(matches.get_one::<bool>("dog"), None);
+    assert_eq!(*matches.get_one::<bool>("dog").unwrap(), true);
     assert_eq!(*matches.get_one::<bool>("mammal").unwrap(), false);
 }
 
@@ -315,10 +331,10 @@ fn count() {
     let cmd = Command::new("test").arg(Arg::new("mammal").long("mammal").action(ArgAction::Count));
 
     let matches = cmd.clone().try_get_matches_from(["test"]).unwrap();
-    assert_eq!(matches.get_one::<u64>("mammal"), None);
-    assert_eq!(matches.is_present("mammal"), false);
+    assert_eq!(*matches.get_one::<u64>("mammal").unwrap(), 0);
+    assert_eq!(matches.is_present("mammal"), true);
     assert_eq!(matches.occurrences_of("mammal"), 0);
-    assert_eq!(matches.index_of("mammal"), None);
+    assert_eq!(matches.index_of("mammal"), Some(1));
 
     let matches = cmd
         .clone()
@@ -340,7 +356,7 @@ fn count() {
 }
 
 #[test]
-fn count_with_default_value() {
+fn count_with_explicit_default_value() {
     let cmd = Command::new("test").arg(
         Arg::new("mammal")
             .long("mammal")
@@ -375,6 +391,10 @@ fn count_with_default_value_if_present() {
         )
         .arg(Arg::new("dog").long("dog").action(ArgAction::Count));
 
+    let matches = cmd.clone().try_get_matches_from(["test"]).unwrap();
+    assert_eq!(*matches.get_one::<u64>("dog").unwrap(), 0);
+    assert_eq!(*matches.get_one::<u64>("mammal").unwrap(), 0);
+
     let matches = cmd.clone().try_get_matches_from(["test", "--dog"]).unwrap();
     assert_eq!(*matches.get_one::<u64>("dog").unwrap(), 1);
     assert_eq!(*matches.get_one::<u64>("mammal").unwrap(), 10);
@@ -383,7 +403,7 @@ fn count_with_default_value_if_present() {
         .clone()
         .try_get_matches_from(["test", "--mammal"])
         .unwrap();
-    assert_eq!(matches.get_one::<u64>("dog"), None);
+    assert_eq!(*matches.get_one::<u64>("dog").unwrap(), 0);
     assert_eq!(*matches.get_one::<u64>("mammal").unwrap(), 1);
 }
 
@@ -398,9 +418,13 @@ fn count_with_default_value_if_value() {
         )
         .arg(Arg::new("dog").long("dog").action(ArgAction::Count));
 
+    let matches = cmd.clone().try_get_matches_from(["test"]).unwrap();
+    assert_eq!(*matches.get_one::<u64>("dog").unwrap(), 0);
+    assert_eq!(*matches.get_one::<u64>("mammal").unwrap(), 0);
+
     let matches = cmd.clone().try_get_matches_from(["test", "--dog"]).unwrap();
     assert_eq!(*matches.get_one::<u64>("dog").unwrap(), 1);
-    assert_eq!(matches.get_one::<u64>("mammal"), None);
+    assert_eq!(*matches.get_one::<u64>("mammal").unwrap(), 0);
 
     let matches = cmd
         .clone()
@@ -413,6 +437,6 @@ fn count_with_default_value_if_value() {
         .clone()
         .try_get_matches_from(["test", "--mammal"])
         .unwrap();
-    assert_eq!(matches.get_one::<u64>("dog"), None);
+    assert_eq!(*matches.get_one::<u64>("dog").unwrap(), 0);
     assert_eq!(*matches.get_one::<u64>("mammal").unwrap(), 1);
 }