fix(derive)!: Error, don't panic!

The derive is generating `Error::raw` (from scratch or by converting
existing erors) and then the inherent `Parser` methods format them.

Fixes #2255
This commit is contained in:
Ed Page 2021-10-23 16:30:32 -05:00
parent f8bca3a84b
commit 53e10b41e3
4 changed files with 101 additions and 57 deletions

View file

@ -118,13 +118,14 @@ pub fn gen_from_arg_matches_for_struct(
)]
#[deny(clippy::correctness)]
impl clap::FromArgMatches for #struct_name {
fn from_arg_matches(__clap_arg_matches: &clap::ArgMatches) -> Option<Self> {
fn from_arg_matches(__clap_arg_matches: &clap::ArgMatches) -> Result<Self, clap::Error> {
let v = #struct_name #constructor;
Some(v)
::std::result::Result::Ok(v)
}
fn update_from_arg_matches(&mut self, __clap_arg_matches: &clap::ArgMatches) {
fn update_from_arg_matches(&mut self, __clap_arg_matches: &clap::ArgMatches) -> Result<(), clap::Error> {
#updater
::std::result::Result::Ok(())
}
}
}
@ -379,20 +380,30 @@ pub fn gen_constructor(fields: &Punctuated<Field, Comma>, parent_attribute: &Att
(Ty::Option, Some(sub_type)) => sub_type,
_ => &field.ty,
};
let unwrapper = match **ty {
Ty::Option => quote!(),
_ => quote_spanned!( ty.span()=> .expect("app should verify subcommand is required") ),
};
quote_spanned! { kind.span()=>
#field_name: {
<#subcmd_type as clap::FromArgMatches>::from_arg_matches(#arg_matches)
#unwrapper
}
match **ty {
Ty::Option => {
quote_spanned! { kind.span()=>
#field_name: {
if #arg_matches.subcommand_name().map(<#subcmd_type as clap::Subcommand>::has_subcommand).unwrap_or(false) {
Some(<#subcmd_type as clap::FromArgMatches>::from_arg_matches(#arg_matches)?)
} else {
None
}
}
}
},
_ => {
quote_spanned! { kind.span()=>
#field_name: {
<#subcmd_type as clap::FromArgMatches>::from_arg_matches(#arg_matches)?
}
}
},
}
}
Kind::Flatten => quote_spanned! { kind.span()=>
#field_name: clap::FromArgMatches::from_arg_matches(#arg_matches).unwrap()
#field_name: clap::FromArgMatches::from_arg_matches(#arg_matches)?
},
Kind::Skip(val) => match val {
@ -448,7 +459,7 @@ pub fn gen_updater(
};
let updater = quote_spanned! { ty.span()=>
<#subcmd_type as clap::FromArgMatches>::update_from_arg_matches(#field_name, #arg_matches);
<#subcmd_type as clap::FromArgMatches>::update_from_arg_matches(#field_name, #arg_matches)?;
};
let updater = match **ty {
@ -456,9 +467,9 @@ pub fn gen_updater(
if let Some(#field_name) = #field_name.as_mut() {
#updater
} else {
*#field_name = <#subcmd_type as clap::FromArgMatches>::from_arg_matches(
*#field_name = Some(<#subcmd_type as clap::FromArgMatches>::from_arg_matches(
#arg_matches
)
)?);
}
},
_ => quote_spanned! { kind.span()=>
@ -476,7 +487,7 @@ pub fn gen_updater(
Kind::Flatten => quote_spanned! { kind.span()=> {
#access
clap::FromArgMatches::update_from_arg_matches(#field_name, #arg_matches);
clap::FromArgMatches::update_from_arg_matches(#field_name, #arg_matches)?;
}
},
@ -504,26 +515,27 @@ fn gen_parsers(
let func = &parser.func;
let span = parser.kind.span();
let convert_type = inner_type(**ty, &field.ty);
let name = attrs.cased_name();
let (value_of, values_of, mut parse) = match *parser.kind {
FromStr => (
quote_spanned!(span=> value_of),
quote_spanned!(span=> values_of),
func.clone(),
quote_spanned!(func.span()=> |s| ::std::result::Result::Ok::<_, clap::Error>(#func(s))),
),
TryFromStr => (
quote_spanned!(span=> value_of),
quote_spanned!(span=> values_of),
quote_spanned!(func.span()=> |s| #func(s).unwrap()),
quote_spanned!(func.span()=> |s| #func(s).map_err(|err| clap::Error::raw(clap::ErrorKind::ValueValidation, format!("Invalid value for {}: {}", #name, err)))),
),
FromOsStr => (
quote_spanned!(span=> value_of_os),
quote_spanned!(span=> values_of_os),
func.clone(),
quote_spanned!(func.span()=> |s| ::std::result::Result::Ok::<_, clap::Error>(#func(s))),
),
TryFromOsStr => (
quote_spanned!(span=> value_of_os),
quote_spanned!(span=> values_of_os),
quote_spanned!(func.span()=> |s| #func(s).unwrap()),
quote_spanned!(func.span()=> |s| #func(s).map_err(|err| clap::Error::raw(clap::ErrorKind::ValueValidation, format!("Invalid value for {}: {}", #name, err)))),
),
FromOccurrences => (
quote_spanned!(span=> occurrences_of),
@ -536,13 +548,12 @@ fn gen_parsers(
let ci = attrs.case_insensitive();
parse = quote_spanned! { convert_type.span()=>
|s| <#convert_type as clap::ArgEnum>::from_str(s, #ci).expect("app should verify the choice was valid")
|s| <#convert_type as clap::ArgEnum>::from_str(s, #ci).map_err(|err| clap::Error::raw(clap::ErrorKind::ValueValidation, format!("Invalid value for {}: {}", #name, err)))
}
}
let flag = *attrs.parser().kind == ParserKind::FromFlag;
let occurrences = *attrs.parser().kind == ParserKind::FromOccurrences;
let name = attrs.cased_name();
// Give this identifier the same hygiene
// as the `arg_matches` parameter definition. This
// allows us to refer to `arg_matches` within a `quote_spanned` block
@ -565,12 +576,13 @@ fn gen_parsers(
quote_spanned! { ty.span()=>
#arg_matches.#value_of(#name)
.map(#parse)
.transpose()?
}
}
Ty::OptionOption => quote_spanned! { ty.span()=>
if #arg_matches.is_present(#name) {
Some(#arg_matches.#value_of(#name).map(#parse))
Some(#arg_matches.#value_of(#name).map(#parse).transpose()?)
} else {
None
}
@ -579,8 +591,9 @@ fn gen_parsers(
Ty::OptionVec => quote_spanned! { ty.span()=>
if #arg_matches.is_present(#name) {
Some(#arg_matches.#values_of(#name)
.map(|v| v.map::<#convert_type, _>(#parse).collect())
.unwrap_or_else(Vec::new))
.map(|v| v.map::<Result<#convert_type, clap::Error>, _>(#parse).collect::<Result<Vec<_>, clap::Error>>())
.transpose()?
.unwrap_or_else(Vec::new))
} else {
None
}
@ -589,7 +602,8 @@ fn gen_parsers(
Ty::Vec => {
quote_spanned! { ty.span()=>
#arg_matches.#values_of(#name)
.map(|v| v.map::<#convert_type, _>(#parse).collect())
.map(|v| v.map::<Result<#convert_type, clap::Error>, _>(#parse).collect::<Result<Vec<_>, clap::Error>>())
.transpose()?
.unwrap_or_else(Vec::new)
}
}
@ -605,8 +619,8 @@ fn gen_parsers(
Ty::Other => {
quote_spanned! { ty.span()=>
#arg_matches.#value_of(#name)
.map(#parse)
.expect("app should verify arg is required")
.ok_or_else(|| clap::Error::raw(clap::ErrorKind::MissingRequiredArgument, format!("The following required argument was not provided: {}", #name)))
.and_then(#parse)?
}
}
};

View file

@ -448,14 +448,14 @@ fn gen_from_arg_matches(
Unit => quote!(),
Unnamed(ref fields) if fields.unnamed.len() == 1 => {
let ty = &fields.unnamed[0];
quote!( ( <#ty as clap::FromArgMatches>::from_arg_matches(__clap_arg_matches).unwrap() ) )
quote!( ( <#ty as clap::FromArgMatches>::from_arg_matches(__clap_arg_matches)? ) )
}
Unnamed(..) => abort_call_site!("{}: tuple enums are not supported", variant.ident),
};
quote! {
if #sub_name == #subcommand_name_var {
return Some(#name :: #variant_name #constructor_block)
return ::std::result::Result::Ok(#name :: #variant_name #constructor_block)
}
}
});
@ -466,8 +466,8 @@ fn gen_from_arg_matches(
let ty = &fields.unnamed[0];
quote! {
if <#ty as clap::Subcommand>::has_subcommand(__clap_name) {
let __clap_res = <#ty as clap::FromArgMatches>::from_arg_matches(__clap_arg_matches).unwrap();
return Some(#name :: #variant_name (__clap_res));
let __clap_res = <#ty as clap::FromArgMatches>::from_arg_matches(__clap_arg_matches)?;
return ::std::result::Result::Ok(#name :: #variant_name (__clap_res));
}
}
}
@ -480,7 +480,7 @@ fn gen_from_arg_matches(
let wildcard = match ext_subcmd {
Some((span, var_name, str_ty, values_of)) => quote_spanned! { span=>
::std::option::Option::Some(#name::#var_name(
::std::result::Result::Ok(#name::#var_name(
::std::iter::once(#str_ty::from(#subcommand_name_var))
.chain(
#sub_arg_matches_var.#values_of("").into_iter().flatten().map(#str_ty::from)
@ -489,11 +489,13 @@ fn gen_from_arg_matches(
))
},
None => quote!(None),
None => quote! {
::std::result::Result::Err(clap::Error::raw(clap::ErrorKind::UnrecognizedSubcommand, format!("The subcommand '{}' wasn't recognized", #subcommand_name_var)))
},
};
quote! {
fn from_arg_matches(__clap_arg_matches: &clap::ArgMatches) -> Option<Self> {
fn from_arg_matches(__clap_arg_matches: &clap::ArgMatches) -> Result<Self, clap::Error> {
if let Some((#subcommand_name_var, #sub_arg_matches_var)) = __clap_arg_matches.subcommand() {
{
let __clap_arg_matches = #sub_arg_matches_var;
@ -504,7 +506,7 @@ fn gen_from_arg_matches(
#wildcard
} else {
None
::std::result::Result::Err(clap::Error::raw(clap::ErrorKind::MissingSubcommand, "A subcommand is required but one was not provided."))
}
}
}
@ -568,7 +570,7 @@ fn gen_update_from_arg_matches(
quote!(clap::FromArgMatches::update_from_arg_matches(
__clap_arg,
__clap_sub_arg_matches
)),
)?),
)
} else {
abort_call_site!("{}: tuple enums are not supported", variant.ident)
@ -592,8 +594,8 @@ fn gen_update_from_arg_matches(
quote! {
if <#ty as clap::Subcommand>::has_subcommand(__clap_name) {
if let #name :: #variant_name (child) = s {
<#ty as clap::FromArgMatches>::update_from_arg_matches(child, __clap_arg_matches);
return;
<#ty as clap::FromArgMatches>::update_from_arg_matches(child, __clap_arg_matches)?;
return ::std::result::Result::Ok(());
}
}
}
@ -609,16 +611,17 @@ fn gen_update_from_arg_matches(
fn update_from_arg_matches<'b>(
&mut self,
__clap_arg_matches: &clap::ArgMatches,
) {
) -> Result<(), clap::Error> {
if let Some((__clap_name, __clap_sub_arg_matches)) = __clap_arg_matches.subcommand() {
match self {
#( #subcommands ),*
s => {
#( #child_subcommands )*
*s = <Self as clap::FromArgMatches>::from_arg_matches(__clap_arg_matches).unwrap();
*s = <Self as clap::FromArgMatches>::from_arg_matches(__clap_arg_matches)?;
}
}
}
::std::result::Result::Ok(())
}
}
}

View file

@ -32,10 +32,10 @@ pub fn into_app(name: &Ident) {
pub fn from_arg_matches(name: &Ident) {
append_dummy(quote! {
impl clap::FromArgMatches for #name {
fn from_arg_matches(_m: &clap::ArgMatches) -> Option<Self> {
fn from_arg_matches(_m: &clap::ArgMatches) -> Result<Self, clap::Error> {
unimplemented!()
}
fn update_from_arg_matches(&mut self, matches: &clap::ArgMatches) {
fn update_from_arg_matches(&mut self, matches: &clap::ArgMatches) -> Result<(), clap::Error>{
unimplemented!()
}
}

View file

@ -74,14 +74,22 @@ pub trait Parser: FromArgMatches + IntoApp + Sized {
/// Parse from `std::env::args_os()`, exit on error
fn parse() -> Self {
let matches = <Self as IntoApp>::into_app().get_matches();
<Self as FromArgMatches>::from_arg_matches(&matches).expect("IntoApp validated everything")
let res =
<Self as FromArgMatches>::from_arg_matches(&matches).map_err(format_error::<Self>);
match res {
Ok(s) => s,
Err(e) => {
// Since this is more of a development-time error, we aren't doing as fancy of a quit
// as `get_matches`
e.exit()
}
}
}
/// Parse from `std::env::args_os()`, return Err on error.
fn try_parse() -> Result<Self, Error> {
let matches = <Self as IntoApp>::into_app().try_get_matches()?;
Ok(<Self as FromArgMatches>::from_arg_matches(&matches)
.expect("IntoApp validated everything"))
<Self as FromArgMatches>::from_arg_matches(&matches).map_err(format_error::<Self>)
}
/// Parse from iterator, exit on error
@ -92,7 +100,16 @@ pub trait Parser: FromArgMatches + IntoApp + Sized {
T: Into<OsString> + Clone,
{
let matches = <Self as IntoApp>::into_app().get_matches_from(itr);
<Self as FromArgMatches>::from_arg_matches(&matches).expect("IntoApp validated everything")
let res =
<Self as FromArgMatches>::from_arg_matches(&matches).map_err(format_error::<Self>);
match res {
Ok(s) => s,
Err(e) => {
// Since this is more of a development-time error, we aren't doing as fancy of a quit
// as `get_matches_from`
e.exit()
}
}
}
/// Parse from iterator, return Err on error.
@ -103,8 +120,7 @@ pub trait Parser: FromArgMatches + IntoApp + Sized {
T: Into<OsString> + Clone,
{
let matches = <Self as IntoApp>::into_app().try_get_matches_from(itr)?;
Ok(<Self as FromArgMatches>::from_arg_matches(&matches)
.expect("IntoApp validated everything"))
<Self as FromArgMatches>::from_arg_matches(&matches).map_err(format_error::<Self>)
}
/// Update from iterator, exit on error
@ -116,7 +132,13 @@ pub trait Parser: FromArgMatches + IntoApp + Sized {
{
// TODO find a way to get partial matches
let matches = <Self as IntoApp>::into_app_for_update().get_matches_from(itr);
<Self as FromArgMatches>::update_from_arg_matches(self, &matches);
let res = <Self as FromArgMatches>::update_from_arg_matches(self, &matches)
.map_err(format_error::<Self>);
if let Err(e) = res {
// Since this is more of a development-time error, we aren't doing as fancy of a quit
// as `get_matches_from`
e.exit()
}
}
/// Update from iterator, return Err on error.
@ -127,8 +149,8 @@ pub trait Parser: FromArgMatches + IntoApp + Sized {
T: Into<OsString> + Clone,
{
let matches = <Self as IntoApp>::into_app_for_update().try_get_matches_from(itr)?;
<Self as FromArgMatches>::update_from_arg_matches(self, &matches);
Ok(())
<Self as FromArgMatches>::update_from_arg_matches(self, &matches)
.map_err(format_error::<Self>)
}
}
@ -178,10 +200,10 @@ pub trait FromArgMatches: Sized {
/// }
/// }
/// ```
fn from_arg_matches(matches: &ArgMatches) -> Option<Self>;
fn from_arg_matches(matches: &ArgMatches) -> Result<Self, Error>;
/// Assign values from `ArgMatches` to `self`.
fn update_from_arg_matches(&mut self, matches: &ArgMatches);
fn update_from_arg_matches(&mut self, matches: &ArgMatches) -> Result<(), Error>;
}
/// Parse arguments into a user-defined container.
@ -347,10 +369,10 @@ impl<T: IntoApp> IntoApp for Box<T> {
}
impl<T: FromArgMatches> FromArgMatches for Box<T> {
fn from_arg_matches(matches: &ArgMatches) -> Option<Self> {
fn from_arg_matches(matches: &ArgMatches) -> Result<Self, Error> {
<T as FromArgMatches>::from_arg_matches(matches).map(Box::new)
}
fn update_from_arg_matches(&mut self, matches: &ArgMatches) {
fn update_from_arg_matches(&mut self, matches: &ArgMatches) -> Result<(), Error> {
<T as FromArgMatches>::update_from_arg_matches(self, matches)
}
}
@ -375,3 +397,8 @@ impl<T: Subcommand> Subcommand for Box<T> {
<T as Subcommand>::has_subcommand(name)
}
}
fn format_error<I: IntoApp>(err: crate::Error) -> crate::Error {
let mut app = I::into_app();
err.format(&mut app)
}