Merge pull request #3820 from epage/panic

fix(assert): Tweak bad arg panics

CHANGELOG.md

@@ -19,6 +19,7 @@ _gated behind `unstable-v4`_
 - Verify `required` is not used with conditional required settings (#3660)
 - Disallow more `value_names` than `number_of_values` (#2695)
 - *(assert)* Always enforce that version is specified when the `ArgAction::Version` is used
+- *(assert)* Add missing `#[track_caller]`s to make it easier to debug asserts
 - *(help)* Use `Command::display_name` in the help title rather than `Command::bin_name`
 - *(parser)* Assert on unknown args when using external subcommands (#3703)
 - *(parser)* Always fill in `""` argument for external subcommands (#3263)

src/parser/matches/arg_matches.rs

@@ -459,6 +459,7 @@ impl ArgMatches {
 
     /// Deprecated, replaced with [`ArgMatches::get_one()`]
     #[deprecated(since = "3.2.0", note = "Replaced with `ArgMatches::get_one()`")]
+    #[cfg_attr(debug_assertions, track_caller)]
     pub fn value_of_t<R>(&self, name: &str) -> Result<R, Error>
     where
         R: FromStr,
@@ -480,6 +481,7 @@ impl ArgMatches {
 
     /// Deprecated, replaced with [`ArgMatches::get_one()`]
     #[deprecated(since = "3.2.0", note = "Replaced with `ArgMatches::get_one()`")]
+    #[cfg_attr(debug_assertions, track_caller)]
     pub fn value_of_t_or_exit<R>(&self, name: &str) -> R
     where
         R: FromStr,
@@ -491,6 +493,7 @@ impl ArgMatches {
 
     /// Deprecated, replaced with [`ArgMatches::get_many()`]
     #[deprecated(since = "3.2.0", note = "Replaced with `ArgMatches::get_many()`")]
+    #[cfg_attr(debug_assertions, track_caller)]
     pub fn values_of_t<R>(&self, name: &str) -> Result<Vec<R>, Error>
     where
         R: FromStr,
@@ -512,6 +515,7 @@ impl ArgMatches {
 
     /// Deprecated, replaced with [`ArgMatches::get_many()`]
     #[deprecated(since = "3.2.0", note = "Replaced with `ArgMatches::get_many()`")]
+    #[cfg_attr(debug_assertions, track_caller)]
     pub fn values_of_t_or_exit<R>(&self, name: &str) -> Vec<R>
     where
         R: FromStr,
@@ -527,6 +531,7 @@ impl ArgMatches {
         since = "3.2.0",
         note = "Replaced with either `ArgAction::SetTrue` or `ArgMatches::contains_id(...)`"
     )]
+    #[cfg_attr(debug_assertions, track_caller)]
     pub fn is_present<T: Key>(&self, id: T) -> bool {
         let id = Id::from(id);
 
@@ -557,6 +562,7 @@ impl ArgMatches {
     /// ```
     ///
     /// [`default_value`]: crate::Arg::default_value()
+    #[cfg_attr(debug_assertions, track_caller)]
     pub fn value_source<T: Key>(&self, id: T) -> Option<ValueSource> {
         let id = Id::from(id);
 
@@ -571,6 +577,7 @@ impl ArgMatches {
         since = "3.2.0",
         note = "Replaced with either `ArgAction::Count` or `ArgMatches::get_many(...).len()`"
     )]
+    #[cfg_attr(debug_assertions, track_caller)]
     pub fn occurrences_of<T: Key>(&self, id: T) -> u64 {
         #![allow(deprecated)]
         self.get_arg(&Id::from(id))
@@ -711,6 +718,7 @@ impl ArgMatches {
     /// assert_eq!(m.indices_of("option").unwrap().collect::<Vec<_>>(), &[2, 3, 4]);
     /// ```
     /// [delimiter]: crate::Arg::value_delimiter()
+    #[cfg_attr(debug_assertions, track_caller)]
     pub fn index_of<T: Key>(&self, id: T) -> Option<usize> {
         let arg = self.get_arg(&Id::from(id))?;
         let i = arg.get_index(0)?;
@@ -793,6 +801,7 @@ impl ArgMatches {
     /// ```
     /// [`ArgMatches::index_of`]: ArgMatches::index_of()
     /// [delimiter]: Arg::value_delimiter()
+    #[cfg_attr(debug_assertions, track_caller)]
     pub fn indices_of<T: Key>(&self, id: T) -> Option<Indices<'_>> {
         let arg = self.get_arg(&Id::from(id))?;
         let i = Indices {
@@ -1199,7 +1208,7 @@ impl ArgMatches {
                 return Err(MatchesError::UnknownArgument {});
             } else {
                 debug!(
-                    "`{:?}` is not a name of an argument or a group.\n\
+                    "`{:?}` is not an id of an argument or a group.\n\
                      Make sure you're using the name of the argument itself \
                      and not the name of short or long flags.",
                     _arg
@@ -1223,7 +1232,7 @@ impl ArgMatches {
                 );
             } else {
                 panic!(
-                    "`{:?}` is not a name of an argument or a group.\n\
+                    "`{:?}` is not an id of an argument or a group.\n\
                      Make sure you're using the name of the argument itself \
                      and not the name of short or long flags.",
                     arg

tests/builder/legacy/arg_matcher_assertions.rs

@@ -3,7 +3,7 @@ use clap::{Arg, Command};
 
 #[test]
 #[cfg(debug_assertions)]
-#[should_panic = "`f` is not a name of an argument or a group."]
+#[should_panic = "`f` is not an id of an argument or a group."]
 fn arg_matches_if_present_wrong_arg() {
     let m = Command::new("test")
         .arg(Arg::new("flag").short('f'))
@@ -16,7 +16,7 @@ fn arg_matches_if_present_wrong_arg() {
 
 #[test]
 #[cfg(debug_assertions)]
-#[should_panic = "`o` is not a name of an argument or a group."]
+#[should_panic = "`o` is not an id of an argument or a group."]
 fn arg_matches_value_of_wrong_arg() {
     let m = Command::new("test")
         .arg(Arg::new("opt").short('o').takes_value(true))