diff --git a/src/build/app/debug_asserts.rs b/src/build/app/debug_asserts.rs index 7c635127045..f96c355490f 100644 --- a/src/build/app/debug_asserts.rs +++ b/src/build/app/debug_asserts.rs @@ -1,7 +1,8 @@ use crate::{ build::arg::{debug_asserts::assert_arg, ArgProvider}, + mkeymap::KeyType, util::Id, - App, AppSettings, ArgSettings, ValueHint, + App, AppSettings, Arg, ArgSettings, ValueHint, }; use std::cmp::Ordering; @@ -274,6 +275,8 @@ pub(crate) fn assert_app(app: &App) { detect_duplicate_flags(&long_flags, "long"); detect_duplicate_flags(&short_flags, "short"); + _verify_positionals(app); + if let Some(help_template) = app.template { assert!( !help_template.contains("{flags}"), @@ -410,3 +413,170 @@ fn assert_app_flags(app: &App) { #[cfg(feature = "unstable-multicall")] checker!(Multicall conflicts NoBinaryName); } + +#[cfg(debug_assertions)] +fn _verify_positionals(app: &App) -> bool { + debug!("App::_verify_positionals"); + // Because you must wait until all arguments have been supplied, this is the first chance + // to make assertions on positional argument indexes + // + // First we verify that the index highest supplied index, is equal to the number of + // positional arguments to verify there are no gaps (i.e. supplying an index of 1 and 3 + // but no 2) + + let highest_idx = app + .args + .keys() + .filter_map(|x| { + if let KeyType::Position(n) = x { + Some(*n) + } else { + None + } + }) + .max() + .unwrap_or(0); + + let num_p = app.args.keys().filter(|x| x.is_position()).count(); + + assert!( + highest_idx == num_p, + "Found positional argument whose index is {} but there \ + are only {} positional arguments defined", + highest_idx, + num_p + ); + + // Next we verify that only the highest index has takes multiple arguments (if any) + let only_highest = |a: &Arg| a.is_multiple() && (a.index.unwrap_or(0) != highest_idx); + if app.get_positionals().any(only_highest) { + // First we make sure if there is a positional that allows multiple values + // the one before it (second to last) has one of these: + // * a value terminator + // * ArgSettings::Last + // * The last arg is Required + + // We can't pass the closure (it.next()) to the macro directly because each call to + // find() (iterator, not macro) gets called repeatedly. + let last = &app.args[&KeyType::Position(highest_idx)]; + let second_to_last = &app.args[&KeyType::Position(highest_idx - 1)]; + + // Either the final positional is required + // Or the second to last has a terminator or .last(true) set + let ok = last.is_set(ArgSettings::Required) + || (second_to_last.terminator.is_some() || second_to_last.is_set(ArgSettings::Last)) + || last.is_set(ArgSettings::Last); + assert!( + ok, + "When using a positional argument with .multiple_values(true) that is *not the \ + last* positional argument, the last positional argument (i.e. the one \ + with the highest index) *must* have .required(true) or .last(true) set." + ); + + // We make sure if the second to last is Multiple the last is ArgSettings::Last + let ok = second_to_last.is_multiple() || last.is_set(ArgSettings::Last); + assert!( + ok, + "Only the last positional argument, or second to last positional \ + argument may be set to .multiple_values(true)" + ); + + // Next we check how many have both Multiple and not a specific number of values set + let count = app + .get_positionals() + .filter(|p| { + p.settings.is_set(ArgSettings::MultipleOccurrences) + || (p.settings.is_set(ArgSettings::MultipleValues) && p.num_vals.is_none()) + }) + .count(); + let ok = count <= 1 + || (last.is_set(ArgSettings::Last) + && last.is_multiple() + && second_to_last.is_multiple() + && count == 2); + assert!( + ok, + "Only one positional argument with .multiple_values(true) set is allowed per \ + command, unless the second one also has .last(true) set" + ); + } + + let mut found = false; + + if app.is_set(AppSettings::AllowMissingPositional) { + // Check that if a required positional argument is found, all positions with a lower + // index are also required. + let mut foundx2 = false; + + for p in app.get_positionals() { + if foundx2 && !p.is_set(ArgSettings::Required) { + assert!( + p.is_set(ArgSettings::Required), + "Found non-required positional argument with a lower \ + index than a required positional argument by two or more: {:?} \ + index {:?}", + p.name, + p.index + ); + } else if p.is_set(ArgSettings::Required) && !p.is_set(ArgSettings::Last) { + // Args that .last(true) don't count since they can be required and have + // positionals with a lower index that aren't required + // Imagine: prog [opt1] -- + // Both of these are valid invocations: + // $ prog r1 -- r2 + // $ prog r1 o1 -- r2 + if found { + foundx2 = true; + continue; + } + found = true; + continue; + } else { + found = false; + } + } + } else { + // Check that if a required positional argument is found, all positions with a lower + // index are also required + for p in (1..=num_p).rev().filter_map(|n| app.args.get(&n)) { + if found { + assert!( + p.is_set(ArgSettings::Required), + "Found non-required positional argument with a lower \ + index than a required positional argument: {:?} index {:?}", + p.name, + p.index + ); + } else if p.is_set(ArgSettings::Required) && !p.is_set(ArgSettings::Last) { + // Args that .last(true) don't count since they can be required and have + // positionals with a lower index that aren't required + // Imagine: prog [opt1] -- + // Both of these are valid invocations: + // $ prog r1 -- r2 + // $ prog r1 o1 -- r2 + found = true; + continue; + } + } + } + assert!( + app.get_positionals() + .filter(|p| p.is_set(ArgSettings::Last)) + .count() + < 2, + "Only one positional argument may have last(true) set. Found two." + ); + if app + .get_positionals() + .any(|p| p.is_set(ArgSettings::Last) && p.is_set(ArgSettings::Required)) + && app.has_subcommands() + && !app.is_set(AppSettings::SubcommandsNegateReqs) + { + panic!( + "Having a required positional argument with .last(true) set *and* child \ + subcommands without setting SubcommandsNegateReqs isn't compatible." + ); + } + + true +} diff --git a/src/parse/parser.rs b/src/parse/parser.rs index 10e9b624b05..b33002392ce 100644 --- a/src/parse/parser.rs +++ b/src/parse/parser.rs @@ -63,9 +63,6 @@ impl<'help, 'app> Parser<'help, 'app> { pub(crate) fn _build(&mut self) { debug!("Parser::_build"); - #[cfg(debug_assertions)] - self._verify_positionals(); - for group in &self.app.groups { if group.required { let idx = self.required.insert(group.id.clone()); @@ -76,180 +73,6 @@ impl<'help, 'app> Parser<'help, 'app> { } } - #[cfg(debug_assertions)] - fn _verify_positionals(&self) -> bool { - debug!("Parser::_verify_positionals"); - // Because you must wait until all arguments have been supplied, this is the first chance - // to make assertions on positional argument indexes - // - // First we verify that the index highest supplied index, is equal to the number of - // positional arguments to verify there are no gaps (i.e. supplying an index of 1 and 3 - // but no 2) - - let highest_idx = self - .app - .args - .keys() - .filter_map(|x| { - if let KeyType::Position(n) = x { - Some(*n) - } else { - None - } - }) - .max() - .unwrap_or(0); - - //_highest_idx(&self.positionals); - - let num_p = self.app.args.keys().filter(|x| x.is_position()).count(); - - assert!( - highest_idx == num_p, - "Found positional argument whose index is {} but there \ - are only {} positional arguments defined", - highest_idx, - num_p - ); - - // Next we verify that only the highest index has takes multiple arguments (if any) - let only_highest = |a: &Arg| a.is_multiple() && (a.index.unwrap_or(0) != highest_idx); - if self.app.get_positionals().any(only_highest) { - // First we make sure if there is a positional that allows multiple values - // the one before it (second to last) has one of these: - // * a value terminator - // * ArgSettings::Last - // * The last arg is Required - - // We can't pass the closure (it.next()) to the macro directly because each call to - // find() (iterator, not macro) gets called repeatedly. - let last = &self.app.args[&KeyType::Position(highest_idx)]; - let second_to_last = &self.app.args[&KeyType::Position(highest_idx - 1)]; - - // Either the final positional is required - // Or the second to last has a terminator or .last(true) set - let ok = last.is_set(ArgSettings::Required) - || (second_to_last.terminator.is_some() - || second_to_last.is_set(ArgSettings::Last)) - || last.is_set(ArgSettings::Last); - assert!( - ok, - "When using a positional argument with .multiple_values(true) that is *not the \ - last* positional argument, the last positional argument (i.e. the one \ - with the highest index) *must* have .required(true) or .last(true) set." - ); - - // We make sure if the second to last is Multiple the last is ArgSettings::Last - let ok = second_to_last.is_multiple() || last.is_set(ArgSettings::Last); - assert!( - ok, - "Only the last positional argument, or second to last positional \ - argument may be set to .multiple_values(true)" - ); - - // Next we check how many have both Multiple and not a specific number of values set - let count = self - .app - .get_positionals() - .filter(|p| { - p.settings.is_set(ArgSettings::MultipleOccurrences) - || (p.settings.is_set(ArgSettings::MultipleValues) && p.num_vals.is_none()) - }) - .count(); - let ok = count <= 1 - || (last.is_set(ArgSettings::Last) - && last.is_multiple() - && second_to_last.is_multiple() - && count == 2); - assert!( - ok, - "Only one positional argument with .multiple_values(true) set is allowed per \ - command, unless the second one also has .last(true) set" - ); - } - - let mut found = false; - - if self.is_set(AS::AllowMissingPositional) { - // Check that if a required positional argument is found, all positions with a lower - // index are also required. - let mut foundx2 = false; - - for p in self.app.get_positionals() { - if foundx2 && !p.is_set(ArgSettings::Required) { - assert!( - p.is_set(ArgSettings::Required), - "Found non-required positional argument with a lower \ - index than a required positional argument by two or more: {:?} \ - index {:?}", - p.name, - p.index - ); - } else if p.is_set(ArgSettings::Required) && !p.is_set(ArgSettings::Last) { - // Args that .last(true) don't count since they can be required and have - // positionals with a lower index that aren't required - // Imagine: prog [opt1] -- - // Both of these are valid invocations: - // $ prog r1 -- r2 - // $ prog r1 o1 -- r2 - if found { - foundx2 = true; - continue; - } - found = true; - continue; - } else { - found = false; - } - } - } else { - // Check that if a required positional argument is found, all positions with a lower - // index are also required - for p in (1..=num_p).rev().filter_map(|n| self.app.args.get(&n)) { - if found { - assert!( - p.is_set(ArgSettings::Required), - "Found non-required positional argument with a lower \ - index than a required positional argument: {:?} index {:?}", - p.name, - p.index - ); - } else if p.is_set(ArgSettings::Required) && !p.is_set(ArgSettings::Last) { - // Args that .last(true) don't count since they can be required and have - // positionals with a lower index that aren't required - // Imagine: prog [opt1] -- - // Both of these are valid invocations: - // $ prog r1 -- r2 - // $ prog r1 o1 -- r2 - found = true; - continue; - } - } - } - assert!( - self.app - .get_positionals() - .filter(|p| p.is_set(ArgSettings::Last)) - .count() - < 2, - "Only one positional argument may have last(true) set. Found two." - ); - if self - .app - .get_positionals() - .any(|p| p.is_set(ArgSettings::Last) && p.is_set(ArgSettings::Required)) - && self.app.has_subcommands() - && !self.is_set(AS::SubcommandsNegateReqs) - { - panic!( - "Having a required positional argument with .last(true) set *and* child \ - subcommands without setting SubcommandsNegateReqs isn't compatible." - ); - } - - true - } - // Should we color the help? pub(crate) fn color_help(&self) -> ColorChoice { #[cfg(feature = "color")]