Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: Clarify cause of debug asserts #3253

Merged
merged 1 commit into from Jan 4, 2022
Merged

fix: Clarify cause of debug asserts #3253

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
89 changes: 59 additions & 30 deletions src/build/app/debug_asserts.rs
View file
Open in desktop
Expand Up @@ -16,7 +16,8 @@ pub(crate) fn assert_app(app: &App) {
// PropagateVersion is meaningless if there is no version
assert!(
!app.settings.is_set(AppSettings::PropagateVersion),
"No version information via App::version or App::long_version to propagate"
"App {}: No version information via App::version or App::long_version to propagate",
app.get_name(),
);

// Used `App::mut_arg("version", ..) but did not provide any version information to display
Expand All @@ -27,7 +28,9 @@ pub(crate) fn assert_app(app: &App) {

if has_mutated_version {
assert!(app.settings.is_set(AppSettings::NoAutoVersion),
"Used App::mut_arg(\"version\", ..) without providing App::version, App::long_version or using AppSettings::NoAutoVersion");
"App {}: Used App::mut_arg(\"version\", ..) without providing App::version, App::long_version or using AppSettings::NoAutoVersion"
,app.get_name()
);
}
}

Expand Down Expand Up @@ -71,17 +74,21 @@ pub(crate) fn assert_app(app: &App) {
// 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",
"App {}: Argument names must be unique, but '{}' is in use by more than one argument or group",
app.get_name(),
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, \
"App {}: Long option names must be unique for each argument, \
but '--{}' is in use by both '{}' and '{}'",
l, first.name, second.name
app.get_name(),
l,
first.name,
second.name
)
}
}
Expand All @@ -90,9 +97,12 @@ pub(crate) fn assert_app(app: &App) {
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, \
"App {}: Short option names must be unique for each argument, \
but '-{}' is in use by both '{}' and '{}'",
s, first.name, second.name
app.get_name(),
s,
first.name,
second.name
)
}
}
Expand All @@ -103,11 +113,13 @@ pub(crate) fn assert_app(app: &App) {
app.two_args_of(|x| x.is_positional() && x.index == Some(idx))
{
panic!(
"Argument '{}' has the same index as '{}' \
"App {}: 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
app.get_name(),
first.name,
second.name
)
}
}
Expand All @@ -116,7 +128,8 @@ pub(crate) fn assert_app(app: &App) {
for req in &arg.requires {
assert!(
app.id_exists(&req.1),
"Argument or group '{:?}' specified in 'requires*' for '{}' does not exist",
"App {}: Argument or group '{:?}' specified in 'requires*' for '{}' does not exist",
app.get_name(),
req.1,
arg.name,
);
Expand All @@ -125,7 +138,8 @@ pub(crate) fn assert_app(app: &App) {
for req in &arg.r_ifs {
assert!(
app.id_exists(&req.0),
"Argument or group '{:?}' specified in 'required_if_eq*' for '{}' does not exist",
"App {}: Argument or group '{:?}' specified in 'required_if_eq*' for '{}' does not exist",
app.get_name(),
req.0,
arg.name
);
Expand All @@ -134,7 +148,8 @@ pub(crate) fn assert_app(app: &App) {
for req in &arg.r_ifs_all {
assert!(
app.id_exists(&req.0),
"Argument or group '{:?}' specified in 'required_if_eq_all' for '{}' does not exist",
"App {}: Argument or group '{:?}' specified in 'required_if_eq_all' for '{}' does not exist",
app.get_name(),
req.0,
arg.name
);
Expand All @@ -143,7 +158,8 @@ pub(crate) fn assert_app(app: &App) {
for req in &arg.r_unless {
assert!(
app.id_exists(req),
"Argument or group '{:?}' specified in 'required_unless*' for '{}' does not exist",
"App {}: Argument or group '{:?}' specified in 'required_unless*' for '{}' does not exist",
app.get_name(),
req,
arg.name,
);
Expand All @@ -153,7 +169,8 @@ pub(crate) fn assert_app(app: &App) {
for req in &arg.blacklist {
assert!(
app.id_exists(req),
"Argument or group '{:?}' specified in 'conflicts_with*' for '{}' does not exist",
"App {}: Argument or group '{:?}' specified in 'conflicts_with*' for '{}' does not exist",
app.get_name(),
req,
arg.name,
);
Expand All @@ -162,39 +179,45 @@ pub(crate) fn assert_app(app: &App) {
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.",
"App {}: Flags or Options cannot have last(true) set. '{}' has both a long and last(true) set.",
app.get_name(),
arg.name
);
assert!(
arg.short.is_none(),
"Flags or Options cannot have last(true) set. '{}' has both a short and last(true) set.",
"App {}: Flags or Options cannot have last(true) set. '{}' has both a short and last(true) set.",
app.get_name(),
arg.name
);
}

assert!(
!(arg.is_set(ArgSettings::Required) && arg.get_global()),
"Global arguments cannot be required.\n\n\t'{}' is marked as both global and required",
"App {}: Global arguments cannot be required.\n\n\t'{}' is marked as both global and required",
app.get_name(),
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",
"App {}: Argument '{}' has both `validator` and `validator_os` set which is not allowed",
app.get_name(),
arg.name
);

if arg.value_hint == ValueHint::CommandWithArguments {
assert!(
arg.is_positional(),
"Argument '{}' has hint CommandWithArguments and must be positional.",
"App {}: Argument '{}' has hint CommandWithArguments and must be positional.",
app.get_name(),
arg.name
);

assert!(
app.is_set(AppSettings::TrailingVarArg),
"Positional argument '{}' has hint CommandWithArguments, so App must have TrailingVarArg set.",
"App {}: Positional argument '{}' has hint CommandWithArguments, so App must have TrailingVarArg set.",
app.get_name(),
arg.name
);
}
Expand All @@ -204,14 +227,16 @@ pub(crate) fn assert_app(app: &App) {
// 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",
"App {}: Argument group name must be unique\n\n\t'{}' is already in use",
app.get_name(),
group.name,
);

// Groups should not have naming conflicts with Args
assert!(
!app.args.args().any(|x| x.id == group.id),
"Argument group name '{}' must not conflict with argument name",
"App {}: Argument group name '{}' must not conflict with argument name",
app.get_name(),
group.name,
);

Expand All @@ -223,7 +248,8 @@ pub(crate) fn assert_app(app: &App) {
.args()
.any(|x| x.id == *arg && x.default_vals.is_empty())
}),
"Argument group '{}' is required but all of it's arguments have a default value.",
"App {}: Argument group '{}' is required but all of it's arguments have a default value.",
app.get_name(),
group.name
)
}
Expand All @@ -232,7 +258,8 @@ pub(crate) fn assert_app(app: &App) {
// Args listed inside groups should exist
assert!(
app.args.args().any(|x| x.id == *arg),
"Argument group '{}' contains non-existent argument '{:?}'",
"App {}: Argument group '{}' contains non-existent argument '{:?}'",
app.get_name(),
group.name,
arg
);
Expand All @@ -250,12 +277,14 @@ pub(crate) fn assert_app(app: &App) {
if let Some(help_template) = app.template {
assert!(
!help_template.contains("{flags}"),
"{}",
"`{flags}` template variable was removed in clap3, they are now included in `{options}`"
"App {}: {}",
app.get_name(),
"`{flags}` template variable was removed in clap3, they are now included in `{options}`",
);
assert!(
!help_template.contains("{unified}"),
"{}",
"App {}: {}",
app.get_name(),
"`{unified}` template variable was removed in clap3, use `{options}` instead"
);
}
Expand Down Expand Up @@ -351,7 +380,7 @@ fn assert_app_flags(app: &App) {

$(
if !app.is_set($b) {
s.push_str(&format!("\nAppSettings::{} is required when AppSettings::{} is set.\n", std::stringify!($b), std::stringify!($a)));
s.push_str(&format!(" AppSettings::{} is required when AppSettings::{} is set.\n", std::stringify!($b), std::stringify!($a)));
}
)+

Expand All @@ -366,12 +395,12 @@ fn assert_app_flags(app: &App) {

$(
if app.is_set($b) {
s.push_str(&format!("\nAppSettings::{} conflicts with AppSettings::{}.\n", std::stringify!($b), std::stringify!($a)));
s.push_str(&format!(" AppSettings::{} conflicts with AppSettings::{}.\n", std::stringify!($b), std::stringify!($a)));
}
)+

if !s.is_empty() {
panic!("{}", s)
panic!("{}\n{}", app.get_name(), s)
}
}
};
Expand Down
4 changes: 2 additions & 2 deletions src/build/arg/debug_asserts.rs
View file
Open in desktop
Expand Up @@ -56,12 +56,12 @@ fn assert_arg_flags(arg: &Arg) {

$(
if !arg.is_set($b) {
s.push_str(&format!("\nArgSettings::{} is required when ArgSettings::{} is set.\n", std::stringify!($b), std::stringify!($a)));
s.push_str(&format!(" ArgSettings::{} is required when ArgSettings::{} is set.\n", std::stringify!($b), std::stringify!($a)));
}
)+

if !s.is_empty() {
panic!("{}", s)
panic!("Argument {:?}\n{}", arg.get_name(), s)
}
}
}
Expand Down