Mirrors/clap
2
0
Fork 0
mirror of https://github.com/clap-rs/clap synced 2024-11-10 14:54:15 +00:00
Code Issues Projects Releases Packages Wiki Activity

Fix and rearrange debug asserts

Browse source
This commit is contained in:
CreepySkeleton 2020-08-02 20:53:07 +03:00
parent 619658e17a
commit 32414fbdff
3 changed files with 314 additions and 301 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

282
src/build/app/debug_asserts.rs Normal file
View file

@ -0,0 +1,282 @@
use crate::{App, AppSettings, ArgSettings};
use std::cmp::Ordering;
#[derive(Eq)]
enum Flag<'a> {
App(String, &'a str),
Arg(String, &'a str),
}
impl PartialEq for Flag<'_> {
fn eq(&self, other: &Flag) -> bool {
self.cmp(other) == Ordering::Equal
}
}
impl PartialOrd for Flag<'_> {
fn partial_cmp(&self, other: &Flag) -> Option<Ordering> {
use Flag::*;
match (self, other) {
(App(s1, _), App(s2, _))
| (Arg(s1, _), Arg(s2, _))
| (App(s1, _), Arg(s2, _))
| (Arg(s1, _), App(s2, _)) => {
if s1 == s2 {
Some(Ordering::Equal)
} else {
s1.partial_cmp(s2)
}
}
}
}
}
impl Ord for Flag<'_> {
fn cmp(&self, other: &Self) -> Ordering {
self.partial_cmp(other).unwrap()
}
}
pub(crate) fn assert_app(app: &App) {
debug!("App::_debug_asserts");
let mut short_flags = vec![];
let mut long_flags = vec![];
for sc in &app.subcommands {
if let Some(s) = sc.short_flag.as_ref() {
short_flags.push(Flag::App(format!("-{}", s), &sc.name));
}
for (short_alias, _) in &sc.short_flag_aliases {
short_flags.push(Flag::App(format!("-{}", short_alias), &sc.name));
}
if let Some(l) = sc.long_flag.as_ref() {
long_flags.push(Flag::App(format!("--{}", l), &sc.name));
}
for (long_alias, _) in &sc.long_flag_aliases {
long_flags.push(Flag::App(format!("--{}", long_alias), &sc.name));
}
}
for arg in &app.args.args {
arg._debug_asserts();
if let Some(s) = arg.short.as_ref() {
short_flags.push(Flag::Arg(format!("-{}", s), &*arg.name));
}
for (short_alias, _) in &arg.short_aliases {
short_flags.push(Flag::Arg(format!("-{}", short_alias), &arg.name));
}
if let Some(l) = arg.long.as_ref() {
long_flags.push(Flag::Arg(format!("--{}", l), &*arg.name));
}
for (long_alias, _) in &arg.aliases {
long_flags.push(Flag::Arg(format!("--{}", long_alias), &arg.name));
}
// Name conflicts
assert!(
app.two_args_of(|x| x.id == arg.id).is_none(),
"Argument names must be unique, but '{}' is in use by more than one argument or group",
arg.name,
);
// Long conflicts
if let Some(l) = arg.long {
if let Some((first, second)) = app.two_args_of(|x| x.long == Some(l)) {
panic!(
"Long option names must be unique for each argument, \
but '--{}' is in use by both '{}' and '{}'",
l, first.name, second.name
)
}
}
// Short conflicts
if let Some(s) = arg.short {
if let Some((first, second)) = app.two_args_of(|x| x.short == Some(s)) {
panic!(
"Short option names must be unique for each argument, \
but '-{}' is in use by both '{}' and '{}'",
s, first.name, second.name
)
}
}
// Index conflicts
if let Some(idx) = arg.index {
if let Some((first, second)) =
app.two_args_of(|x| x.is_positional() && x.index == Some(idx))
{
panic!(
"Argument '{}' has the same index as '{}' \
and they are both positional arguments\n\n\t \
Use Arg::multiple_values(true) to allow one \
positional argument to take multiple values",
first.name, second.name
)
}
}
// requires, r_if, r_unless
for req in &arg.requires {
assert!(
app.id_exists(&req.1),
"Argument or group '{:?}' specified in 'requires*' for '{}' does not exist",
req.1,
arg.name,
);
}
for req in &arg.r_ifs {
assert!(
app.id_exists(&req.0),
"Argument or group '{:?}' specified in 'required_if*' for '{}' does not exist",
req.0,
arg.name
);
}
for req in &arg.r_unless {
assert!(
app.id_exists(req),
"Argument or group '{:?}' specified in 'required_unless*' for '{}' does not exist",
req,
arg.name,
);
}
// blacklist
for req in &arg.blacklist {
assert!(
app.id_exists(req),
"Argument or group '{:?}' specified in 'conflicts_with*' for '{}' does not exist",
req,
arg.name,
);
}
if arg.is_set(ArgSettings::Last) {
assert!(
arg.long.is_none(),
"Flags or Options cannot have last(true) set. '{}' has both a long and last(true) set.",
arg.name
);
assert!(
arg.short.is_none(),
"Flags or Options cannot have last(true) set. '{}' has both a short and last(true) set.",
arg.name
);
}
assert!(
!(arg.is_set(ArgSettings::Required) && arg.global),
"Global arguments cannot be required.\n\n\t'{}' is marked as both global and required",
arg.name
);
// validators
assert!(
arg.validator.is_none() || arg.validator_os.is_none(),
"Argument '{}' has both `validator` and `validator_os` set which is not allowed",
arg.name
);
if arg.value_hint == ValueHint::CommandWithArguments {
assert!(
arg.short.is_none() && arg.long.is_none(),
"Argument '{}' has hint CommandWithArguments and must be positional.",
arg.name
);
assert!(
self.is_set(AppSettings::TrailingVarArg),
"Positional argument '{}' has hint CommandWithArguments, so App must have TrailingVarArg set.",
arg.name
);
}
}
for group in &app.groups {
// Name conflicts
assert!(
app.groups.iter().filter(|x| x.id == group.id).count() < 2,
"Argument group name must be unique\n\n\t'{}' is already in use",
group.name,
);
// Groups should not have naming conflicts with Args
assert!(
!app.args.args.iter().any(|x| x.id == group.id),
"Argument group name '{}' must not conflict with argument name",
group.name,
);
// Args listed inside groups should exist
for arg in &group.args {
assert!(
app.args.args.iter().any(|x| x.id == *arg),
"Argument group '{}' contains non-existent argument '{:?}'",
group.name,
arg
)
}
}
// Conflicts between flags and subcommands
long_flags.sort_unstable();
short_flags.sort_unstable();
detect_duplicate_flags(&long_flags, "long");
detect_duplicate_flags(&short_flags, "short");
app._panic_on_missing_help(app.g_settings.is_set(AppSettings::HelpRequired));
}
fn detect_duplicate_flags(flags: &[Flag], short_or_long: &str) {
use Flag::*;
for (one, two) in find_duplicates(flags) {
match (one, two) {
(App(flag, one), App(_, another)) if one != another => panic!(
"the '{}' {} flag is specified for both '{}' and '{}' subcommands",
flag, short_or_long, one, another
),
(Arg(flag, one), Arg(_, another)) if one != another => panic!(
"{} option names must be unique, but '{}' is in use by both '{}' and '{}'",
short_or_long, flag, one, another
),
(Arg(flag, arg), App(_, sub)) | (App(flag, sub), Arg(_, arg)) => panic!(
"the '{}' {} flag for the '{}' argument conflicts with the short flag \
for '{}' subcommand",
flag, short_or_long, arg, sub
),
_ => {}
}
}
}
/// Find duplicates in a sorted array.
///
/// The algorithm is simple: the array is sorted, duplicates
/// must be placed next to each other, we can check only adjacent elements.
fn find_duplicates<T: PartialEq>(slice: &[T]) -> impl Iterator<Item = (&T, &T)> {
slice.windows(2).filter_map(|w| {
if w[0] == w[1] {
Some((&w[0], &w[1]))
} else {
None
}
})
}

321
src/build/app/mod.rs
View file

@ -1,3 +1,5 @@
#[cfg(debug_assertions)]
mod debug_asserts;
mod settings;
#[cfg(test)]
mod tests;
@ -1888,41 +1890,6 @@ impl<'help> App<'help> {
}
}
// Allows checking for conflicts between `Args` and `Apps` with subcommand flags
#[cfg(debug_assertions)]
#[derive(Debug)]
enum Flag<'r, 'help> {
App(&'r App<'help>, String),
Arg(&'r Arg<'help>, String),
}
#[cfg(debug_assertions)]
impl Flag<'_, '_> {
pub fn value(&self) -> &str {
match self {
Self::App(_, value) => value,
Self::Arg(_, value) => value,
}
}
pub fn id(&self) -> &Id {
match self {
Self::App(app, _) => &app.id,
Self::Arg(arg, _) => &arg.id,
}
}
}
#[cfg(debug_assertions)]
impl fmt::Display for Flag<'_, '_> {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match self {
Self::App(app, _) => write!(f, "App named '{}'", app.name),
Self::Arg(arg, _) => write!(f, "Arg named '{}'", arg.name),
}
}
}
// Internally used only
impl<'help> App<'help> {
fn _do_parse(&mut self, it: &mut Input) -> ClapResult<ArgMatches> {
@ -1997,7 +1964,7 @@ impl<'help> App<'help> {
self.settings.set(AppSettings::Built);
#[cfg(debug_assertions)]
self._debug_asserts();
self::debug_asserts::assert_app(self);
}
fn _panic_on_missing_help(&self, help_required_globally: bool) {
@ -2025,69 +1992,7 @@ impl<'help> App<'help> {
}
#[cfg(debug_assertions)]
fn two_long_flags_of<F>(&self, condition: F) -> Option<(Flag, Flag)>
where
F: Fn(&Flag) -> bool,
{
let mut flags: Vec<Flag> = Vec::new();
for sc in &self.subcommands {
if let Some(long) = sc.long_flag {
flags.push(Flag::App(&sc, long.to_string()));
}
flags.extend(
sc.get_all_long_flag_aliases()
.map(|alias| Flag::App(&sc, alias.to_string())),
);
self.args.args.iter().for_each(|arg| {
flags.extend(
arg.aliases
.iter()
.map(|(alias, _)| Flag::Arg(arg, alias.to_string())),
)
});
flags.extend(
self.args
.args
.iter()
.filter_map(|arg| arg.long.map(|l| Flag::Arg(arg, l.to_string()))),
);
}
two_elements_of(flags.into_iter().filter(|f| condition(f)))
}
#[cfg(debug_assertions)]
fn two_short_flags_of<F>(&self, condition: F) -> Option<(Flag, Flag)>
where
F: Fn(&Flag) -> bool,
{
let mut flags: Vec<Flag> = Vec::new();
for sc in &self.subcommands {
if let Some(short) = sc.short_flag {
flags.push(Flag::App(&sc, short.to_string()));
}
flags.extend(
sc.get_all_short_flag_aliases()
.map(|alias| Flag::App(&sc, alias.to_string())),
);
self.args.args.iter().for_each(|arg| {
flags.extend(
arg.short_aliases
.iter()
.map(|(alias, _)| Flag::Arg(arg, alias.to_string())),
)
});
flags.extend(
self.args
.args
.iter()
.filter_map(|arg| arg.short.map(|l| Flag::Arg(arg, l.to_string()))),
);
}
two_elements_of(flags.into_iter().filter(|f| condition(f)))
}
#[cfg(debug_assertions)]
fn two_args_of<F>(&self, condition: F) -> Option<(&Arg, &Arg)>
fn two_args_of<F>(&self, condition: F) -> Option<(&Arg<'help>, &Arg<'help>)>
where
F: Fn(&Arg) -> bool,
{
@ -2103,197 +2008,6 @@ impl<'help> App<'help> {
two_elements_of(self.groups.iter().filter(|a| condition(a)))
}
// Perform some expensive assertions on the Parser itself
#[allow(clippy::cognitive_complexity)]
#[cfg(debug_assertions)]
fn _debug_asserts(&self) {
debug!("App::_debug_asserts");
for sc in &self.subcommands {
// Conflicts between flag subcommands and long args
if let Some(l) = sc.long_flag {
if let Some((first, second)) = self.two_long_flags_of(|f| f.value() == l) {
// Prevent conflicts with itself
if first.id() != second.id() {
panic!(
"Long option names must be unique for each argument, \
but '--{}' is used by both an {} and an {}",
l, first, second
);
}
}
}
// Conflicts between flag subcommands and long args
if let Some(s) = sc.short_flag {
if let Some((first, second)) =
self.two_short_flags_of(|f| f.value() == s.to_string())
{
// Prevent conflicts with itself
if first.id() != second.id() {
panic!(
"Short option names must be unique for each argument, \
but '-{}' is used by both an {} and an {}",
s, first, second
);
}
}
}
}
for arg in &self.args.args {
arg._debug_asserts();
// Name conflicts
assert!(
self.two_args_of(|x| x.id == arg.id).is_none(),
"Argument names must be unique, but '{}' is in use by more than one argument or group",
arg.name,
);
// Long conflicts
if let Some(l) = arg.long {
if let Some((first, second)) = self.two_args_of(|x| x.long == Some(l)) {
panic!(
"Long option names must be unique for each argument, \
but '--{}' is in use by both '{}' and '{}'",
l, first.name, second.name
)
}
}
// Short conflicts
if let Some(s) = arg.short {
if let Some((first, second)) = self.two_args_of(|x| x.short == Some(s)) {
panic!(
"Short option names must be unique for each argument, \
but '-{}' is in use by both '{}' and '{}'",
s, first.name, second.name
)
}
}
// Index conflicts
if let Some(idx) = arg.index {
if let Some((first, second)) =
self.two_args_of(|x| x.is_positional() && x.index == Some(idx))
{
panic!(
"Argument '{}' has the same index as '{}' \
and they are both positional arguments\n\n\t \
Use Arg::multiple_values(true) to allow one \
positional argument to take multiple values",
first.name, second.name
)
}
}
// requires, r_if, r_unless
for req in &arg.requires {
assert!(
self.id_exists(&req.1),
"Argument or group '{:?}' specified in 'requires*' for '{}' does not exist",
req.1,
arg.name,
);
}
for req in &arg.r_ifs {
assert!(
self.id_exists(&req.0),
"Argument or group '{:?}' specified in 'required_if*' for '{}' does not exist",
req.0,
arg.name
);
}
for req in &arg.r_unless {
assert!(
self.id_exists(req),
"Argument or group '{:?}' specified in 'required_unless*' for '{}' does not exist",
req, arg.name,
);
}
// blacklist
for req in &arg.blacklist {
assert!(
self.id_exists(req),
"Argument or group '{:?}' specified in 'conflicts_with*' for '{}' does not exist",
req, arg.name,
);
}
if arg.is_set(ArgSettings::Last) {
assert!(
arg.long.is_none(),
"Flags or Options cannot have last(true) set. '{}' has both a long and last(true) set.",
arg.name
);
assert!(
arg.short.is_none(),
"Flags or Options cannot have last(true) set. '{}' has both a short and last(true) set.",
arg.name
);
}
assert!(
!(arg.is_set(ArgSettings::Required) && arg.global),
"Global arguments cannot be required.\n\n\t'{}' is marked as both global and required",
arg.name
);
// validators
assert!(
arg.validator.is_none() || arg.validator_os.is_none(),
"Argument '{}' has both `validator` and `validator_os` set which is not allowed",
arg.name
);
if arg.value_hint == ValueHint::CommandWithArguments {
assert!(
arg.short.is_none() && arg.long.is_none(),
"Argument '{}' has hint CommandWithArguments and must be positional.",
arg.name
);
assert!(
self.is_set(AppSettings::TrailingVarArg),
"Positional argument '{}' has hint CommandWithArguments, so App must have TrailingVarArg set.",
arg.name
);
}
}
for group in &self.groups {
// Name conflicts
assert!(
self.groups.iter().filter(|x| x.id == group.id).count() < 2,
"Argument group name must be unique\n\n\t'{}' is already in use",
group.name,
);
// Groups should not have naming conflicts with Args
assert!(
!self.args.args.iter().any(|x| x.id == group.id),
"Argument group name '{}' must not conflict with argument name",
group.name,
);
// Args listed inside groups should exist
for arg in &group.args {
assert!(
self.args.args.iter().any(|x| x.id == *arg),
"Argument group '{}' contains non-existent argument '{:?}'",
group.name,
arg
)
}
}
self._panic_on_missing_help(self.g_settings.is_set(AppSettings::HelpRequired));
}
pub(crate) fn _propagate(&mut self, prop: Propagation) {
macro_rules! propagate_subcmd {
($_self:expr, $sc:expr) => {{
@ -2616,11 +2330,28 @@ impl<'help> App<'help> {
/// Iterate through all the names of all subcommands (not recursively), including aliases.
/// Used for suggestions.
pub(crate) fn all_subcommand_names(&self) -> impl Iterator<Item = &str> {
self.get_subcommands().map(|s| s.get_name()).chain(
self.get_subcommands()
.flat_map(|s| s.aliases.iter().map(|&(n, _)| n)),
)
pub(crate) fn all_subcommand_names<'a>(&'a self) -> impl Iterator<Item = &'a str>
where
'help: 'a,
{
let a: Vec<_> = self
.get_subcommands()
.flat_map(|sc| {
let name = sc.get_name();
let aliases = sc.get_all_aliases();
std::iter::once(name).chain(aliases)
})
.collect();
// Strictly speaking, we don't need this trip through the Vec.
// We should have been able to return FlatMap iter above directly.
//
// Unfortunately, that would trigger
// https://github.com/rust-lang/rust/issues/34511#issuecomment-373423999
//
// I think this "collect to vec" solution is better then the linked one
// because it's simpler and it doesn't really matter performance-wise.
a.into_iter()
}
pub(crate) fn unroll_args_in_group(&self, group: &Id) -> Vec<Id> {

12
tests/flag_subcommands.rs
View file

@ -281,7 +281,7 @@ fn flag_subcommand_multiple() {
#[cfg(debug_assertions)]
#[test]
#[should_panic = "Short option names must be unique for each argument, but \'-f\' is used by both an App named \'some\' and an Arg named \'test\'"]
#[should_panic = "the \'-f\' short flag for the \'test\' argument conflicts with the short flag for \'some\' subcommand"]
fn flag_subcommand_short_conflict_with_arg() {
let _ = App::new("test")
.subcommand(App::new("some").short_flag('f').long_flag("some"))
@ -291,7 +291,7 @@ fn flag_subcommand_short_conflict_with_arg() {
#[cfg(debug_assertions)]
#[test]
#[should_panic = "Short option names must be unique for each argument, but \'-f\' is used by both an App named \'some\' and an App named \'result\'"]
#[should_panic = "the \'-f\' short flag is specified for both \'some\' and \'result\' subcommands"]
fn flag_subcommand_short_conflict_with_alias() {
let _ = App::new("test")
.subcommand(App::new("some").short_flag('f').long_flag("some"))
@ -301,7 +301,7 @@ fn flag_subcommand_short_conflict_with_alias() {
#[cfg(debug_assertions)]
#[test]
#[should_panic = "Long option names must be unique for each argument, but \'--flag\' is used by both an App named \'some\' and an App named \'result\'"]
#[should_panic = "the \'--flag\' long flag is specified for both \'some\' and \'result\' subcommands"]
fn flag_subcommand_long_conflict_with_alias() {
let _ = App::new("test")
.subcommand(App::new("some").long_flag("flag"))
@ -311,7 +311,7 @@ fn flag_subcommand_long_conflict_with_alias() {
#[cfg(debug_assertions)]
#[test]
#[should_panic = "Short option names must be unique for each argument, but \'-f\' is used by both an App named \'some\' and an Arg named \'test\'"]
#[should_panic = "the \'-f\' short flag for the \'test\' argument conflicts with the short flag for \'some\' subcommand"]
fn flag_subcommand_short_conflict_with_arg_alias() {
let _ = App::new("test")
.subcommand(App::new("some").short_flag('f').long_flag("some"))
@ -321,7 +321,7 @@ fn flag_subcommand_short_conflict_with_arg_alias() {
#[cfg(debug_assertions)]
#[test]
#[should_panic = "Long option names must be unique for each argument, but \'--some\' is used by both an App named \'some\' and an Arg named \'test\'"]
#[should_panic = "the \'--some\' long flag for the \'test\' argument conflicts with the short flag for \'some\' subcommand"]
fn flag_subcommand_long_conflict_with_arg_alias() {
let _ = App::new("test")
.subcommand(App::new("some").short_flag('f').long_flag("some"))
@ -331,7 +331,7 @@ fn flag_subcommand_long_conflict_with_arg_alias() {
#[cfg(debug_assertions)]
#[test]
#[should_panic = "Long option names must be unique for each argument, but \'--flag\' is used by both an App named \'some\' and an Arg named \'flag\'"]
#[should_panic = "the \'--flag\' long flag for the \'flag\' argument conflicts with the short flag for \'some\' subcommand"]
fn flag_subcommand_long_conflict_with_arg() {
let _ = App::new("test")
.subcommand(App::new("some").short_flag('a').long_flag("flag"))