fix!: Change arg! to use ArgAction

Fixes clap-rs#3795

CHANGELOG.md

@@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 - `ErrorKind::EmptyValue` replaced with `ErrorKind::InvalidValue`
 - `ErrorKind::UnrecognizedSubcommand` replaced with `ErrorKind::InvalidSubcommand`
+- `arg!` now sets `ArgAction::SetTrue`, `ArgAction::Count`, `ArgAction::Set`, or `ArgAction::Append` as appropriate
 - *(env)* Parse `--help` and `--version` like any `ArgAction::SetTrue` flag
 - *(derive)* `subcommand_required(true).arg_required_else_help(true)` is set instead of `SubcommandRequiredElseHelp`
 

clap_bench/benches/03_complex.rs

@@ -27,9 +27,9 @@ macro_rules! create_app {
                     .value_parser(OPT3_VALS),
                 arg!([positional3] ... "tests positionals with specific values")
                     .value_parser(POS3_VALS),
-                arg!(--multvals "Tests multiple values not mult occs").required(false).value_names(&["one", "two"]),
+                arg!(--multvals <s> "Tests multiple values not mult occs").required(false).value_names(&["one", "two"]),
                 arg!(
-                    --multvalsmo "Tests multiple values, not mult occs"
+                    --multvalsmo <s> "Tests multiple values, not mult occs"
                 ).multiple_values(true).required(false).value_names(&["one", "two"]),
                 arg!(--minvals2 <minvals> ... "Tests 2 min vals").min_values(2).multiple_values(true).required(false),
                 arg!(--maxvals3 <maxvals> ... "Tests 3 max vals").max_values(3).multiple_values(true).required(false),

src/builder/arg.rs

@@ -3879,10 +3879,10 @@ impl<'help> Arg<'help> {
     ///         "prog", "-f", "-d", "-c"]);
     ///             //    ^~~~~~~~~~~~^~~~~ flag is overridden by color
     ///
-    /// assert!(m.is_present("color"));
-    /// assert!(m.is_present("debug")); // even though flag conflicts with debug, it's as if flag
+    /// assert!(*m.get_one::<bool>("color").unwrap());
+    /// assert!(*m.get_one::<bool>("debug").unwrap()); // even though flag conflicts with debug, it's as if flag
     ///                                 // was never used because it was overridden with color
-    /// assert!(!m.is_present("flag"));
+    /// assert!(!*m.get_one::<bool>("flag").unwrap());
     /// ```
     /// Care must be taken when using this setting, and having an arg override with itself. This
     /// is common practice when supporting things like shell aliases, config files, etc.
@@ -3897,19 +3897,18 @@ impl<'help> Arg<'help> {
     /// let m = Command::new("posix")
     ///             .arg(arg!(--flag  "some flag").overrides_with("flag"))
     ///             .get_matches_from(vec!["posix", "--flag", "--flag"]);
-    /// assert!(m.is_present("flag"));
+    /// assert!(*m.get_one::<bool>("flag").unwrap());
     /// ```
     ///
     /// Making an arg [`Arg::multiple_occurrences`] and override itself
-    /// is essentially meaningless. Therefore clap ignores an override of self
-    /// if it's a flag and it already accepts multiple occurrences.
+    /// is essentially meaningless. Therefore clap ignores an override of self.
     ///
     /// ```
     /// # use clap::{Command, arg};
     /// let m = Command::new("posix")
     ///             .arg(arg!(--flag ...  "some flag").overrides_with("flag"))
     ///             .get_matches_from(vec!["", "--flag", "--flag", "--flag", "--flag"]);
-    /// assert!(m.is_present("flag"));
+    /// assert_eq!(*m.get_one::<u8>("flag").unwrap(), 4);
     /// ```
     ///
     /// Now notice with options (which *do not* set
@@ -3921,7 +3920,6 @@ impl<'help> Arg<'help> {
     /// let m = Command::new("posix")
     ///             .arg(arg!(--opt <val> "some option").overrides_with("opt"))
     ///             .get_matches_from(vec!["", "--opt=some", "--opt=other"]);
-    /// assert!(m.is_present("opt"));
     /// assert_eq!(m.get_one::<String>("opt").unwrap(), "other");
     /// ```
     ///
@@ -3938,7 +3936,6 @@ impl<'help> Arg<'help> {
     ///                     .overrides_with("opt")
     ///             )
     ///             .get_matches_from(vec!["", "--opt", "1", "2", "--opt", "3", "4", "5"]);
-    /// assert!(m.is_present("opt"));
     /// assert_eq!(m.get_many::<String>("opt").unwrap().collect::<Vec<_>>(), &["3", "4", "5"]);
     /// ```
     ///
@@ -3952,7 +3949,6 @@ impl<'help> Arg<'help> {
     ///                 .multiple_values(true)
     ///                 .overrides_with("opt"))
     ///             .get_matches_from(vec!["", "--opt", "first", "over", "--opt", "other", "val"]);
-    /// assert!(m.is_present("opt"));
     /// assert_eq!(m.get_many::<String>("opt").unwrap().collect::<Vec<_>>(), &["first", "over", "other", "val"]);
     /// ```
     #[must_use]
@@ -3985,11 +3981,11 @@ impl<'help> Arg<'help> {
     ///         "prog", "-f", "-d", "-c"]);
     ///             //    ^~~~~~^~~~~~~~~ flag and debug are overridden by color
     ///
-    /// assert!(m.is_present("color")); // even though flag conflicts with color, it's as if flag
+    /// assert!(*m.get_one::<bool>("color").unwrap()); // even though flag conflicts with color, it's as if flag
     ///                                 // and debug were never used because they were overridden
     ///                                 // with color
-    /// assert!(!m.is_present("debug"));
-    /// assert!(!m.is_present("flag"));
+    /// assert!(!*m.get_one::<bool>("debug").unwrap());
+    /// assert!(!*m.get_one::<bool>("flag").unwrap());
     /// ```
     #[must_use]
     pub fn overrides_with_all<T: Key>(mut self, names: &[T]) -> Self {

src/macros.rs

@@ -200,17 +200,14 @@ macro_rules! arg_impl {
             @arg
             ({
                 debug_assert_eq!($arg.get_value_names(), None, "Flags should precede values");
-                #[allow(deprecated)]
-                {
-                    debug_assert!(!$arg.is_multiple_occurrences_set(), "Flags should precede `...`");
-                }
+                debug_assert!(!matches!($arg.get_action(), $crate::ArgAction::Append), "Flags should precede `...`");
 
                 let mut arg = $arg;
                 let long = $crate::arg_impl! { @string $long };
                 if arg.get_id().is_empty() {
                     arg = arg.id(long);
                 }
-                arg.long(long)
+                arg.long(long).action($crate::ArgAction::SetTrue)
             })
             $($tail)*
         }
@@ -225,17 +222,14 @@ macro_rules! arg_impl {
             @arg
             ({
                 debug_assert_eq!($arg.get_value_names(), None, "Flags should precede values");
-                #[allow(deprecated)]
-                {
-                    debug_assert!(!$arg.is_multiple_occurrences_set(), "Flags should precede `...`");
-                }
+                debug_assert!(!matches!($arg.get_action(), $crate::ArgAction::Append), "Flags should precede `...`");
 
                 let mut arg = $arg;
                 let long = $crate::arg_impl! { @string $long };
                 if arg.get_id().is_empty() {
                     arg = arg.id(long);
                 }
-                arg.long(long)
+                arg.long(long).action($crate::ArgAction::SetTrue)
             })
             $($tail)*
         }
@@ -251,12 +245,9 @@ macro_rules! arg_impl {
             ({
                 debug_assert_eq!($arg.get_long(), None, "Short flags should precede long flags");
                 debug_assert_eq!($arg.get_value_names(), None, "Flags should precede values");
-                #[allow(deprecated)]
-                {
-                    debug_assert!(!$arg.is_multiple_occurrences_set(), "Flags should precede `...`");
-                }
+                debug_assert!(!matches!($arg.get_action(), $crate::ArgAction::Append), "Flags should precede `...`");
 
-                $arg.short($crate::arg_impl! { @char $short })
+                $arg.short($crate::arg_impl! { @char $short }).action($crate::ArgAction::SetTrue)
             })
             $($tail)*
         }
@@ -272,12 +263,9 @@ macro_rules! arg_impl {
             ({
                 debug_assert_eq!($arg.get_long(), None, "Short flags should precede long flags");
                 debug_assert_eq!($arg.get_value_names(), None, "Flags should precede values");
-                #[allow(deprecated)]
-                {
-                    debug_assert!(!$arg.is_multiple_occurrences_set(), "Flags should precede `...`");
-                }
+                debug_assert!(!matches!($arg.get_action(), $crate::ArgAction::Append), "Flags should precede `...`");
 
-                $arg.short($crate::arg_impl! { @char $short })
+                $arg.short($crate::arg_impl! { @char $short }).action($crate::ArgAction::SetTrue)
             })
             $($tail)*
         }
@@ -291,10 +279,7 @@ macro_rules! arg_impl {
         $crate::arg_impl! {
             @arg
             ({
-                #[allow(deprecated)]
-                {
-                    debug_assert!(!$arg.is_multiple_occurrences_set(), "Values should precede `...`");
-                }
+                debug_assert!(!matches!($arg.get_action(), $crate::ArgAction::Append), "Flags should precede `...`");
                 debug_assert_eq!($arg.get_value_names(), None, "Multiple values not yet supported");
 
                 let mut arg = $arg;
@@ -306,7 +291,7 @@ macro_rules! arg_impl {
                 if arg.get_id().is_empty() {
                     arg = arg.id(value_name);
                 }
-                arg.value_name(value_name)
+                arg.value_name(value_name).action($crate::ArgAction::Set)
             })
             $($tail)*
         }
@@ -320,10 +305,7 @@ macro_rules! arg_impl {
         $crate::arg_impl! {
             @arg
             ({
-                #[allow(deprecated)]
-                {
-                    debug_assert!(!$arg.is_multiple_occurrences_set(), "Values should precede `...`");
-                }
+                debug_assert!(!matches!($arg.get_action(), $crate::ArgAction::Append), "Flags should precede `...`");
                 debug_assert_eq!($arg.get_value_names(), None, "Multiple values not yet supported");
 
                 let mut arg = $arg;
@@ -335,7 +317,7 @@ macro_rules! arg_impl {
                 if arg.get_id().is_empty() {
                     arg = arg.id(value_name);
                 }
-                arg.value_name(value_name)
+                arg.value_name(value_name).action($crate::ArgAction::Set)
             })
             $($tail)*
         }
@@ -349,10 +331,7 @@ macro_rules! arg_impl {
         $crate::arg_impl! {
             @arg
             ({
-                #[allow(deprecated)]
-                {
-                    debug_assert!(!$arg.is_multiple_occurrences_set(), "Values should precede `...`");
-                }
+                debug_assert!(!matches!($arg.get_action(), $crate::ArgAction::Append), "Flags should precede `...`");
                 debug_assert_eq!($arg.get_value_names(), None, "Multiple values not yet supported");
 
                 let mut arg = $arg;
@@ -368,7 +347,7 @@ macro_rules! arg_impl {
                 if arg.get_id().is_empty() {
                     arg = arg.id(value_name);
                 }
-                arg.value_name(value_name)
+                arg.value_name(value_name).action($crate::ArgAction::Set)
             })
             $($tail)*
         }
@@ -382,10 +361,7 @@ macro_rules! arg_impl {
         $crate::arg_impl! {
             @arg
             ({
-                #[allow(deprecated)]
-                {
-                    debug_assert!(!$arg.is_multiple_occurrences_set(), "Values should precede `...`");
-                }
+                debug_assert!(!matches!($arg.get_action(), $crate::ArgAction::Append), "Flags should precede `...`");
                 debug_assert_eq!($arg.get_value_names(), None, "Multiple values not yet supported");
 
                 let mut arg = $arg;
@@ -401,7 +377,7 @@ macro_rules! arg_impl {
                 if arg.get_id().is_empty() {
                     arg = arg.id(value_name);
                 }
-                arg.value_name(value_name)
+                arg.value_name(value_name).action($crate::ArgAction::Set)
             })
             $($tail)*
         }
@@ -414,9 +390,15 @@ macro_rules! arg_impl {
     ) => {
         $crate::arg_impl! {
             @arg
-            ({#[allow(deprecated)]{
-                $arg.multiple_occurrences(true)
-            }})
+            ({
+                if $arg.get_long().is_none() && $arg.get_short().is_none() {
+                    $arg.multiple_values(true)
+                } else if $arg.is_takes_value_set() {
+                    $arg.action($crate::ArgAction::Append)
+                } else {
+                    $arg.action($crate::ArgAction::Count)
+                }
+            })
             $($tail)*
         }
     };
@@ -493,7 +475,7 @@ macro_rules! arg_impl {
 /// `...` (three consecutive dots/periods) specifies that this argument may occur multiple
 /// times (not to be confused with multiple values per occurrence).
 ///
-/// See [`Arg::multiple_occurrences`][crate::Arg::multiple_occurrences].
+/// See [`ArgAction::Count`][crate::ArgAction::Count] and [`ArgAction::Append`][crate::ArgAction::Append].
 ///
 /// ### Help String
 ///
@@ -504,13 +486,17 @@ macro_rules! arg_impl {
 ///
 /// ```rust
 /// # use clap::{Command, Arg, arg};
-/// Command::new("prog")
+/// let cmd = Command::new("prog")
 ///     .args(&[
 ///         arg!(--config <FILE> "a required file for the configuration and no short"),
 ///         arg!(-d --debug ... "turns on debugging information and allows multiples"),
 ///         arg!([input] "an optional input file to use")
-/// ])
-/// # ;
+///     ]);
+///
+/// let m = cmd.try_get_matches_from(["prog", "--config", "file.toml"]).unwrap();
+/// assert_eq!(m.get_one::<String>("config").unwrap(), "file.toml");
+/// assert_eq!(*m.get_one::<u8>("debug").unwrap(), 0);
+/// assert_eq!(m.get_one::<String>("input"), None);
 /// ```
 /// [`Arg`]: ./struct.Arg.html
 #[macro_export]

tests/builder/groups.rs

@@ -86,9 +86,9 @@ fn arg_group_new_of_arg_name() {
 #[test]
 fn group_single_value() {
     let res = Command::new("group")
-        .arg(arg!(-f --flag "some flag"))
         .arg(arg!(-c --color [color] "some option"))
-        .group(ArgGroup::new("grp").args(&["flag", "color"]))
+        .arg(arg!(-h --hostname <name> "another option").required(false))
+        .group(ArgGroup::new("grp").args(&["hostname", "color"]))
         .try_get_matches_from(vec!["", "-c", "blue"]);
     assert!(res.is_ok(), "{}", res.unwrap_err());
 
@@ -100,26 +100,12 @@ fn group_single_value() {
     );
 }
 
-#[test]
-fn group_single_flag() {
-    let res = Command::new("group")
-        .arg(arg!(-f --flag "some flag"))
-        .arg(arg!(-c --color [color] "some option"))
-        .group(ArgGroup::new("grp").args(&["flag", "color"]))
-        .try_get_matches_from(vec!["", "-f"]);
-    assert!(res.is_ok(), "{}", res.unwrap_err());
-
-    let m = res.unwrap();
-    assert!(m.contains_id("grp"));
-    assert!(m.get_one::<String>("grp").map(|v| v.as_str()).is_none());
-}
-
 #[test]
 fn group_empty() {
     let res = Command::new("group")
-        .arg(arg!(-f --flag "some flag"))
         .arg(arg!(-c --color [color] "some option"))
-        .group(ArgGroup::new("grp").args(&["flag", "color"]))
+        .arg(arg!(-h --hostname <name> "another option").required(false))
+        .group(ArgGroup::new("grp").args(&["hostname", "color"]))
         .try_get_matches_from(vec![""]);
     assert!(res.is_ok(), "{}", res.unwrap_err());
 
@@ -129,11 +115,15 @@ fn group_empty() {
 }
 
 #[test]
-fn group_reqired_flags_empty() {
+fn group_required_flags_empty() {
     let result = Command::new("group")
-        .arg(arg!(-f --flag "some flag"))
         .arg(arg!(-c --color "some option"))
-        .group(ArgGroup::new("grp").required(true).args(&["flag", "color"]))
+        .arg(arg!(-h --hostname <name> "another option").required(false))
+        .group(
+            ArgGroup::new("grp")
+                .required(true)
+                .args(&["hostname", "color"]),
+        )
         .try_get_matches_from(vec![""]);
     assert!(result.is_err());
     let err = result.err().unwrap();
@@ -143,9 +133,9 @@ fn group_reqired_flags_empty() {
 #[test]
 fn group_multi_value_single_arg() {
     let res = Command::new("group")
-        .arg(arg!(-f --flag "some flag"))
         .arg(arg!(-c --color <color> "some option").multiple_values(true))
-        .group(ArgGroup::new("grp").args(&["flag", "color"]))
+        .arg(arg!(-h --hostname <name> "another option").required(false))
+        .group(ArgGroup::new("grp").args(&["hostname", "color"]))
         .try_get_matches_from(vec!["", "-c", "blue", "red", "green"]);
     assert!(res.is_ok(), "{:?}", res.unwrap_err().kind());
 

tests/builder/help.rs

@@ -1731,7 +1731,7 @@ fn issue_1052_require_delim_help() {
         .about("tests stuff")
         .version("1.3")
         .arg(
-            arg!(-f --fake "some help")
+            arg!(-f --fake <s> "some help")
                 .required(true)
                 .value_names(&["some", "val"])
                 .takes_value(true)
@@ -1750,7 +1750,7 @@ fn custom_headers_headers() {
         .about("does stuff")
         .version("1.4")
         .arg(
-            arg!(-f --fake "some help")
+            arg!(-f --fake <s> "some help")
                 .required(true)
                 .value_names(&["some", "val"])
                 .takes_value(true)
@@ -1802,7 +1802,7 @@ fn multiple_custom_help_headers() {
         .about("does stuff")
         .version("1.4")
         .arg(
-            arg!(-f --fake "some help")
+            arg!(-f --fake <s> "some help")
                 .required(true)
                 .value_names(&["some", "val"])
                 .takes_value(true)