diff --git a/src/build/app/mod.rs b/src/build/app/mod.rs index 0093e623..74060def 100644 --- a/src/build/app/mod.rs +++ b/src/build/app/mod.rs @@ -2382,12 +2382,13 @@ impl<'help> App<'help> { fn _do_parse(&mut self, it: &mut Input) -> ClapResult { debug!("App::_do_parse"); - let mut matcher = ArgMatcher::default(); // If there are global arguments, or settings we need to propagate them down to subcommands // before parsing in case we run into a subcommand self._build(); + let mut matcher = ArgMatcher::new(self); + // do the real parsing let mut parser = Parser::new(self); if let Err(error) = parser.get_matches_with(&mut matcher, it) { diff --git a/src/build/arg/mod.rs b/src/build/arg/mod.rs index 6495c619..9da23599 100644 --- a/src/build/arg/mod.rs +++ b/src/build/arg/mod.rs @@ -4267,7 +4267,7 @@ impl<'help> Arg<'help> { /// ]); /// /// assert!(res.is_ok()); - /// assert_eq!(res.unwrap().value_of("config"), None); + /// assert_eq!(res.unwrap().value_of("cfg"), Some("")); /// ``` /// /// By adding this setting, we can forbid empty values. diff --git a/src/parse/arg_matcher.rs b/src/parse/arg_matcher.rs index 5a9ea35f..3c7298e1 100644 --- a/src/parse/arg_matcher.rs +++ b/src/parse/arg_matcher.rs @@ -3,7 +3,7 @@ use std::{collections::HashMap, ffi::OsString, mem, ops::Deref}; // Internal use crate::{ - build::{Arg, ArgSettings}, + build::{App, Arg, ArgSettings}, parse::{ArgMatches, MatchedArg, SubCommand, ValueType}, util::Id, }; @@ -16,12 +16,27 @@ pub(crate) struct ArgMatcher(pub(crate) ArgMatches); impl Deref for ArgMatcher { type Target = ArgMatches; + fn deref(&self) -> &Self::Target { &self.0 } } impl ArgMatcher { + pub(crate) fn new(app: &App) -> Self { + ArgMatcher(ArgMatches { + #[cfg(debug_assertions)] + valid_args: { + let args = app.args.args().map(|a| a.id.clone()); + let groups = app.groups.iter().map(|g| g.id.clone()); + args.chain(groups).collect() + }, + #[cfg(debug_assertions)] + valid_subcommands: app.subcommands.iter().map(|sc| sc.id.clone()).collect(), + ..Default::default() + }) + } + pub(crate) fn into_inner(self) -> ArgMatches { self.0 } diff --git a/src/parse/matches/arg_matches.rs b/src/parse/matches/arg_matches.rs index eb694bad..5c5b0d80 100644 --- a/src/parse/matches/arg_matches.rs +++ b/src/parse/matches/arg_matches.rs @@ -72,18 +72,51 @@ pub(crate) struct SubCommand { /// } /// ``` /// [`App::get_matches`]: crate::App::get_matches() -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug, Clone, Default, PartialEq, Eq)] pub struct ArgMatches { + #[cfg(debug_assertions)] + pub(crate) valid_args: Vec, + #[cfg(debug_assertions)] + pub(crate) valid_subcommands: Vec, pub(crate) args: IndexMap, pub(crate) subcommand: Option>, } -impl Default for ArgMatches { - fn default() -> Self { - ArgMatches { - args: IndexMap::new(), - subcommand: None, +// Private methods +impl ArgMatches { + #[inline] + fn get_arg(&self, arg: &Id) -> Option<&MatchedArg> { + #[cfg(debug_assertions)] + { + if *arg != Id::empty_hash() && !self.valid_args.contains(arg) { + panic!( + "`'{:?}' is not a name 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 + ); + } } + + self.args.get(arg) + } + + #[inline] + fn get_subcommand(&self, id: &Id) -> Option<&SubCommand> { + #[cfg(debug_assertions)] + { + if *id != Id::empty_hash() && !self.valid_subcommands.contains(id) { + panic!("'{:?}' is not a name of a subcommand.", id); + } + } + + if let Some(ref sc) = self.subcommand { + if sc.id == *id { + return Some(sc); + } + } + + None } } @@ -121,7 +154,7 @@ impl ArgMatches { /// [`default_value`]: crate::Arg::default_value() /// [`occurrences_of`]: crate::ArgMatches::occurrences_of() pub fn value_of(&self, id: T) -> Option<&str> { - if let Some(arg) = self.args.get(&Id::from(id)) { + if let Some(arg) = self.get_arg(&Id::from(id)) { if let Some(v) = arg.first() { return Some(v.to_str().expect(INVALID_UTF8)); } @@ -161,7 +194,7 @@ impl ArgMatches { /// [`occurrences_of`]: ArgMatches::occurrences_of() /// [`Arg::values_of_lossy`]: ArgMatches::values_of_lossy() pub fn value_of_lossy(&self, id: T) -> Option> { - if let Some(arg) = self.args.get(&Id::from(id)) { + if let Some(arg) = self.get_arg(&Id::from(id)) { if let Some(v) = arg.first() { return Some(v.to_string_lossy()); } @@ -204,8 +237,7 @@ impl ArgMatches { /// [`occurrences_of`]: ArgMatches::occurrences_of() /// [`ArgMatches::values_of_os`]: ArgMatches::values_of_os() pub fn value_of_os(&self, id: T) -> Option<&OsStr> { - self.args - .get(&Id::from(id)) + self.get_arg(&Id::from(id)) .and_then(|arg| arg.first().map(OsString::as_os_str)) } @@ -235,7 +267,7 @@ impl ArgMatches { /// ``` /// [`Iterator`]: std::iter::Iterator pub fn values_of(&self, id: T) -> Option { - self.args.get(&Id::from(id)).map(|arg| { + self.get_arg(&Id::from(id)).map(|arg| { fn to_str_slice(o: &OsString) -> &str { o.to_str().expect(INVALID_UTF8) } @@ -259,8 +291,7 @@ impl ArgMatches { arg.vals() .map(|g| g.iter().map(|x| x.to_str().expect(INVALID_UTF8)).collect()) }; - self.args - .get(&Id::from(id)) + self.get_arg(&Id::from(id)) .map(arg_values) .map(|iter| GroupedValues { iter }) } @@ -293,7 +324,7 @@ impl ArgMatches { /// assert_eq!(itr.next(), None); /// ``` pub fn values_of_lossy(&self, id: T) -> Option> { - self.args.get(&Id::from(id)).map(|arg| { + self.get_arg(&Id::from(id)).map(|arg| { arg.vals_flatten() .map(|v| v.to_string_lossy().into_owned()) .collect() @@ -338,7 +369,7 @@ impl ArgMatches { o } - self.args.get(&Id::from(id)).map(|arg| OsValues { + self.get_arg(&Id::from(id)).map(|arg| OsValues { iter: arg.vals_flatten().map(to_str_slice), }) } @@ -551,6 +582,10 @@ impl ArgMatches { /// [`occurrences_of`]: ArgMatches::occurrences_of() pub fn is_present(&self, id: T) -> bool { let id = Id::from(id); + + #[cfg(debug_assertions)] + self.get_arg(&id); + self.args.contains_key(&id) } @@ -594,7 +629,7 @@ impl ArgMatches { /// assert_eq!(m.occurrences_of("flag"), 1); /// ``` pub fn occurrences_of(&self, id: T) -> u64 { - self.args.get(&Id::from(id)).map_or(0, |a| a.occurs) + self.get_arg(&Id::from(id)).map_or(0, |a| a.occurs) } /// Gets the starting index of the argument in respect to all other arguments. Indices are @@ -727,7 +762,7 @@ impl ArgMatches { /// ``` /// [delimiter]: crate::Arg::value_delimiter() pub fn index_of(&self, name: T) -> Option { - if let Some(arg) = self.args.get(&Id::from(name)) { + if let Some(arg) = self.get_arg(&Id::from(name)) { if let Some(i) = arg.get_index(0) { return Some(i); } @@ -807,7 +842,7 @@ impl ArgMatches { /// [`ArgMatches::index_of`]: ArgMatches::index_of() /// [delimiter]: Arg::value_delimiter() pub fn indices_of(&self, id: T) -> Option> { - self.args.get(&Id::from(id)).map(|arg| Indices { + self.get_arg(&Id::from(id)).map(|arg| Indices { iter: arg.indices(), }) } @@ -844,12 +879,7 @@ impl ArgMatches { /// [`Subcommand`]: crate::Subcommand /// [`App`]: crate::App pub fn subcommand_matches(&self, id: T) -> Option<&ArgMatches> { - if let Some(ref s) = self.subcommand { - if s.id == id.into() { - return Some(&s.matches); - } - } - None + self.get_subcommand(&id.into()).map(|sc| &sc.matches) } /// Because [`Subcommand`]s are essentially "sub-[`App`]s" they have their own [`ArgMatches`] diff --git a/src/parse/parser.rs b/src/parse/parser.rs index 29d16272..41042e5a 100644 --- a/src/parse/parser.rs +++ b/src/parse/parser.rs @@ -702,7 +702,7 @@ impl<'help, 'app> Parser<'help, 'app> { }; // Collect the external subcommand args - let mut sc_m = ArgMatcher::default(); + let mut sc_m = ArgMatcher::new(self.app); while let Some((v, _)) = it.next() { if !self.is_set(AS::AllowInvalidUtf8ForExternalSubcommands) @@ -993,7 +993,6 @@ impl<'help, 'app> Parser<'help, 'app> { let partial_parsing_enabled = self.is_set(AS::IgnoreErrors); if let Some(sc) = self.app.subcommands.iter_mut().find(|s| s.name == sc_name) { - let mut sc_matcher = ArgMatcher::default(); // Display subcommand name, short and long in usage let mut sc_names = sc.name.clone(); let mut flag_subcmd = false; @@ -1030,6 +1029,8 @@ impl<'help, 'app> Parser<'help, 'app> { // Ensure all args are built and ready to parse sc._build(); + let mut sc_matcher = ArgMatcher::new(sc); + debug!("Parser::parse_subcommand: About to parse sc={}", sc.name); { diff --git a/tests/arg_matcher_assertions.rs b/tests/arg_matcher_assertions.rs new file mode 100644 index 00000000..805734a4 --- /dev/null +++ b/tests/arg_matcher_assertions.rs @@ -0,0 +1,37 @@ +use clap::{App, Arg}; + +#[test] +#[cfg(debug_assertions)] +#[should_panic = "'f' is not a name of an argument or a group."] +fn arg_matches_if_present_wrong_arg() { + let m = App::new("test") + .arg(Arg::new("flag").short('f')) + .get_matches_from(&["test", "-f"]); + + assert!(m.is_present("flag")); + m.is_present("f"); +} + +#[test] +#[cfg(debug_assertions)] +#[should_panic = "'o' is not a name of an argument or a group."] +fn arg_matches_value_of_wrong_arg() { + let m = App::new("test") + .arg(Arg::new("opt").short('o').takes_value(true)) + .get_matches_from(&["test", "-o", "val"]); + + assert_eq!(m.value_of("opt"), Some("val")); + m.value_of("o"); +} + +#[test] +#[cfg(debug_assertions)] +#[should_panic = "'seed' is not a name of a subcommand."] +fn arg_matches_subcommand_matches_wrong_sub() { + let m = App::new("test") + .subcommand(App::new("speed")) + .get_matches_from(&["test", "speed"]); + + assert!(m.subcommand_matches("speed").is_some()); + m.subcommand_matches("seed"); +} diff --git a/tests/fixtures/app.yaml b/tests/fixtures/app.yaml index e91eb958..57fb17ce 100644 --- a/tests/fixtures/app.yaml +++ b/tests/fixtures/app.yaml @@ -47,7 +47,7 @@ args: - positional2 - option3: short: O - long: Option + long: option3 about: tests options with specific value sets takes_value: true possible_values: diff --git a/tests/global_args.rs b/tests/global_args.rs index 4630d8bc..f155fb0b 100644 --- a/tests/global_args.rs +++ b/tests/global_args.rs @@ -94,5 +94,4 @@ fn global_arg_available_in_subcommand() { assert!(m.is_present("global")); assert!(m.subcommand_matches("ping").unwrap().is_present("global")); - assert!(!m.subcommand_matches("ping").unwrap().is_present("not")); } diff --git a/tests/help.rs b/tests/help.rs index 9233514b..16e36d45 100644 --- a/tests/help.rs +++ b/tests/help.rs @@ -41,7 +41,7 @@ OPTIONS: --multvals Tests multiple values, not mult occs --multvalsmo Tests multiple values, and mult occs -o, --option ... tests options - -O, --Option specific vals [possible values: fast, slow] + -O, --option3 specific vals [possible values: fast, slow] -V, --version Print version information SUBCOMMANDS: diff --git a/tests/tests.rs b/tests/tests.rs index 1942e68e..ec6f5e5b 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -150,7 +150,7 @@ pub fn check_complex_output(args: &str, out: &str) { .unwrap(); } - let _ = match matches.value_of("Option3").unwrap_or("") { + let _ = match matches.value_of("option3").unwrap_or("") { "fast" => writeln!(w, "option3 present quickly"), "slow" => writeln!(w, "option3 present slowly"), _ => writeln!(w, "option3 NOT present"), diff --git a/tests/utils.rs b/tests/utils.rs index af2e1d46..5cb1db87 100644 --- a/tests/utils.rs +++ b/tests/utils.rs @@ -70,7 +70,7 @@ pub fn complex_app() -> App<'static> { .conflicts_with("option") .requires("positional2"), Arg::from("[positional2] 'tests positionals with exclusions'"), - Arg::from("-O --Option [option3] 'specific vals'").possible_values(opt3_vals), + Arg::from("-O --option3 [option3] 'specific vals'").possible_values(opt3_vals), Arg::from("[positional3]... 'tests specific values'").possible_values(pos3_vals), Arg::from("--multvals [one] [two] 'Tests multiple values, not mult occs'"), Arg::from("--multvalsmo... [one] [two] 'Tests multiple values, and mult occs'"),