From 1b0088ecb7b770c431eedce754f7f491f1a477d5 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Mon, 9 Nov 2020 06:43:16 -0800 Subject: [PATCH] imp: Make `clap_derive` call `FromStr::from_str` only once per argument. Currently the way `clap_derive` uses a `from_str` function is to call it once as a validator, discarding the parsed value, and then to call it again to recompute the parsed value. This is unfortunate in cases where `from_str` is expensive or has side effects. This PR changes `clap_derive` to not register `from_str` as a validator so that it doesn't do the first of these two calls. Then, instead of doing `unwrap()` on the other call, it handles the error. This eliminates the redundancy, and also avoids the small performance hit mentioned in [the documentation about validators]. [the documentation about validators]: https://docs.rs/clap-v3/3.0.0-beta.1/clap_v3/struct.Arg.html#method.validator This PR doesn't yet use colorized messages for errors generated during parsing because the `ColorWhen` setting isn't currently available. That's fixable with some refactoring, but I'm interested in getting feedback on the overall approach here first. --- clap_derive/src/derives/from_arg_matches.rs | 198 +++++----- clap_derive/src/derives/into_app.rs | 33 +- clap_derive/src/derives/subcommand.rs | 36 +- clap_derive/src/dummies.rs | 14 +- clap_derive/tests/custom-string-parsers.rs | 4 +- clap_derive/tests/effectful.rs | 28 ++ clap_derive/tests/validator.rs | 31 ++ src/derive.rs | 47 ++- src/parse/matches/arg_matches.rs | 392 ++++++++++++++++++-- 9 files changed, 610 insertions(+), 173 deletions(-) create mode 100644 clap_derive/tests/effectful.rs create mode 100644 clap_derive/tests/validator.rs diff --git a/clap_derive/src/derives/from_arg_matches.rs b/clap_derive/src/derives/from_arg_matches.rs index 33e2c7bf04e5..608daccce758 100644 --- a/clap_derive/src/derives/from_arg_matches.rs +++ b/clap_derive/src/derives/from_arg_matches.rs @@ -43,12 +43,16 @@ pub fn gen_for_struct( )] #[deny(clippy::correctness)] impl ::clap::FromArgMatches for #struct_name { - fn from_arg_matches(arg_matches: &::clap::ArgMatches) -> Self { - #struct_name #constructor + fn try_from_arg_matches(arg_matches: &::clap::ArgMatches) -> ::clap::Result { + Ok(#struct_name #constructor) } - fn update_from_arg_matches(&mut self, arg_matches: &::clap::ArgMatches) { + fn try_update_from_arg_matches( + &mut self, + arg_matches: &::clap::ArgMatches + ) -> ::clap::Result<()> { #updater + Ok(()) } } } @@ -69,11 +73,14 @@ pub fn gen_for_enum(name: &Ident) -> TokenStream { )] #[deny(clippy::correctness)] impl ::clap::FromArgMatches for #name { - fn from_arg_matches(arg_matches: &::clap::ArgMatches) -> Self { - <#name as ::clap::Subcommand>::from_subcommand(arg_matches.subcommand()).unwrap() + fn try_from_arg_matches(arg_matches: &::clap::ArgMatches) -> ::clap::Result { + <#name as ::clap::Subcommand>::from_subcommand(arg_matches.subcommand()) } - fn update_from_arg_matches(&mut self, arg_matches: &::clap::ArgMatches) { - <#name as ::clap::Subcommand>::update_from_subcommand(self, arg_matches.subcommand()); + fn try_update_from_arg_matches( + &mut self, + arg_matches: &::clap::ArgMatches + ) -> ::clap::Result<()> { + <#name as ::clap::Subcommand>::update_from_subcommand(self, arg_matches.subcommand()) } } } @@ -83,7 +90,7 @@ fn gen_arg_enum_parse(ty: &Type, attrs: &Attrs) -> TokenStream { let ci = attrs.case_insensitive(); quote_spanned! { ty.span()=> - |s| <#ty as ::clap::ArgEnum>::from_str(s, #ci).unwrap() + |s| <#ty as ::clap::ArgEnum>::from_str(s, #ci) } } @@ -99,35 +106,18 @@ fn gen_parsers( let parser = attrs.parser(); let func = &parser.func; let span = parser.kind.span(); - let (value_of, values_of, mut parse) = match *parser.kind { - FromStr => ( - quote_spanned!(span=> value_of), - quote_spanned!(span=> values_of), - func.clone(), - ), - TryFromStr => ( - quote_spanned!(span=> value_of), - quote_spanned!(span=> values_of), - quote_spanned!(func.span()=> |s| #func(s).unwrap()), - ), - FromOsStr => ( - quote_spanned!(span=> value_of_os), - quote_spanned!(span=> values_of_os), - func.clone(), - ), - TryFromOsStr => ( - quote_spanned!(span=> value_of_os), - quote_spanned!(span=> values_of_os), - quote_spanned!(func.span()=> |s| #func(s).unwrap()), - ), - FromOccurrences => ( - quote_spanned!(span=> occurrences_of), - quote!(), - func.clone(), - ), - FromFlag => (quote!(), quote!(), func.clone()), + + // The operand type of the `parse` function. + let parse_operand_type = match *parser.kind { + FromStr | TryFromStr => quote_spanned!(ty.span()=> &str), + FromOsStr | TryFromOsStr => quote_spanned!(ty.span()=> &::std::ffi::OsStr), + FromOccurrences => quote_spanned!(ty.span()=> u64), + FromFlag => quote_spanned!(ty.span()=> bool), }; + // Wrap `parse` in a closure so that we can give the operand a concrete type. + let mut parse = quote_spanned!(span=> |s: #parse_operand_type| #func(s)); + let flag = *attrs.parser().kind == ParserKind::FromFlag; let occurrences = *attrs.parser().kind == ParserKind::FromOccurrences; let name = attrs.cased_name(); @@ -136,35 +126,68 @@ fn gen_parsers( // allows us to refer to `arg_matches` within a `quote_spanned` block let arg_matches = quote! { arg_matches }; - let field_value = match **ty { - Ty::Bool => { - if update.is_some() { - quote_spanned! { ty.span()=> - *#field_name || #arg_matches.is_present(#name) - } - } else { - quote_spanned! { ty.span()=> - #arg_matches.is_present(#name) + if attrs.is_enum() { + match **ty { + Ty::Option => { + if let Some(subty) = subty_if_name(&field.ty, "Option") { + parse = gen_arg_enum_parse(subty, &attrs); } } - } - - Ty::Option => { - if attrs.is_enum() { - if let Some(subty) = subty_if_name(&field.ty, "Option") { + Ty::Vec => { + if let Some(subty) = subty_if_name(&field.ty, "Vec") { parse = gen_arg_enum_parse(subty, &attrs); } } - - quote_spanned! { ty.span()=> - #arg_matches.#value_of(#name) - .map(#parse) + Ty::Other => { + parse = gen_arg_enum_parse(&field.ty, &attrs); } + _ => {} } + } + + let (value_of, values_of) = match *parser.kind { + FromStr => ( + quote_spanned!(span=> #arg_matches.value_of(#name).map(#parse)), + quote_spanned!(span=> + #arg_matches.values_of(#name).map(|values| values.map(#parse)).collect() + ), + ), + TryFromStr => ( + quote_spanned!(span=> #arg_matches.parse_optional_t(#name, #parse)?), + quote_spanned!(span=> #arg_matches.parse_optional_vec_t(#name, #parse)?), + ), + FromOsStr => ( + quote_spanned!(span=> #arg_matches.value_of_os(#name).map(#parse)), + quote_spanned!(span=> + #arg_matches.values_of_os(#name).map(|values| values.map(#parse).collect()) + ), + ), + TryFromOsStr => ( + quote_spanned!(span=> #arg_matches.parse_optional_t_os(#name, #parse)?), + quote_spanned!(span=> #arg_matches.parse_optional_vec_t_os(#name, #parse)?), + ), + FromOccurrences => ( + quote_spanned!(span=> (#parse)(#arg_matches.occurrences_of(#name))), + quote!(), + ), + FromFlag => ( + quote_spanned!(span => (#parse)(#arg_matches.is_present(#name))), + quote!(), + ), + }; + + let field_value = match **ty { + Ty::Bool => quote_spanned! { ty.span()=> + #arg_matches.is_present(#name) + }, + + Ty::Option => quote_spanned! { ty.span()=> + #value_of + }, Ty::OptionOption => quote_spanned! { ty.span()=> if #arg_matches.is_present(#name) { - Some(#arg_matches.#value_of(#name).map(#parse)) + Some(#value_of) } else { None } @@ -172,47 +195,27 @@ 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(#parse).collect()) - .unwrap_or_else(Vec::new)) + Some(#values_of.unwrap_or_else(Vec::new)) } else { None } }, - Ty::Vec => { - if attrs.is_enum() { - if let Some(subty) = subty_if_name(&field.ty, "Vec") { - parse = gen_arg_enum_parse(subty, &attrs); - } - } - - quote_spanned! { ty.span()=> - #arg_matches.#values_of(#name) - .map(|v| v.map(#parse).collect()) - .unwrap_or_else(Vec::new) - } - } + Ty::Vec => quote_spanned! { ty.span()=> + #values_of.unwrap_or_else(Vec::new) + }, Ty::Other if occurrences => quote_spanned! { ty.span()=> - #parse(#arg_matches.#value_of(#name)) + #value_of }, Ty::Other if flag => quote_spanned! { ty.span()=> - #parse(#arg_matches.is_present(#name)) + #value_of }, - Ty::Other => { - if attrs.is_enum() { - parse = gen_arg_enum_parse(&field.ty, &attrs); - } - - quote_spanned! { ty.span()=> - #arg_matches.#value_of(#name) - .map(#parse) - .unwrap() - } - } + Ty::Other => quote_spanned! { ty.span()=> + #value_of.unwrap() + }, }; if let Some(access) = update { @@ -248,20 +251,25 @@ pub fn gen_constructor(fields: &Punctuated, parent_attribute: &Att (Ty::Option, Some(sub_type)) => sub_type, _ => &field.ty, }; - let unwrapper = match **ty { - Ty::Option => quote!(), - _ => quote_spanned!( ty.span()=> .unwrap() ), + let from_subcommand = quote_spanned! { kind.span() => + <#subcmd_type as ::clap::Subcommand>::from_subcommand(#arg_matches.subcommand()) }; - quote_spanned! { kind.span()=> - #field_name: { - <#subcmd_type as ::clap::Subcommand>::from_subcommand(#arg_matches.subcommand()) - #unwrapper - } + match **ty { + Ty::Option => quote_spanned! { kind.span()=> + #field_name: match #from_subcommand { + Ok(cmd) => Some(cmd), + Err(::clap::Error { kind: ::clap::ErrorKind::UnrecognizedSubcommand, .. }) => None, + Err(e) => return Err(e), + } + }, + _ => quote_spanned! { kind.span()=> + #field_name: #from_subcommand? + }, } } Kind::Flatten => quote_spanned! { kind.span()=> - #field_name: ::clap::FromArgMatches::from_arg_matches(#arg_matches) + #field_name: ::clap::FromArgMatches::try_from_arg_matches(#arg_matches)? }, Kind::Skip(val) => match val { @@ -315,7 +323,7 @@ pub fn gen_updater( }; let updater = quote_spanned!{ ty.span()=> - <#subcmd_type as ::clap::Subcommand>::update_from_subcommand(#field_name, subcmd); + <#subcmd_type as ::clap::Subcommand>::update_from_subcommand(#field_name, subcmd)?; }; let updater = match **ty { @@ -323,9 +331,9 @@ pub fn gen_updater( if let Some(#field_name) = #field_name.as_mut() { #updater } else { - *#field_name = <#subcmd_type as ::clap::Subcommand>::from_subcommand( + *#field_name = Some(<#subcmd_type as ::clap::Subcommand>::from_subcommand( subcmd - ) + )?) } }, _ => quote_spanned!{ kind.span()=> diff --git a/clap_derive/src/derives/into_app.rs b/clap_derive/src/derives/into_app.rs index b9fb2247aec8..343f6ee8dac7 100644 --- a/clap_derive/src/derives/into_app.rs +++ b/clap_derive/src/derives/into_app.rs @@ -230,33 +230,9 @@ pub fn gen_app_augmentation( }) } Kind::Arg(ty) => { - let convert_type = match **ty { - Ty::Vec | Ty::Option => sub_type(&field.ty).unwrap_or(&field.ty), - Ty::OptionOption | Ty::OptionVec => { - sub_type(&field.ty).and_then(sub_type).unwrap_or(&field.ty) - } - _ => &field.ty, - }; - - let occurrences = *attrs.parser().kind == ParserKind::FromOccurrences; - let flag = *attrs.parser().kind == ParserKind::FromFlag; - let parser = attrs.parser(); - let func = &parser.func; - - let validator = match *parser.kind { - _ if attrs.is_enum() => quote!(), - ParserKind::TryFromStr => quote_spanned! { func.span()=> - .validator(|s| { - #func(s) - .map(|_: #convert_type| ()) - }) - }, - ParserKind::TryFromOsStr => quote_spanned! { func.span()=> - .validator_os(|s| #func(s).map(|_: #convert_type| ())) - }, - _ => quote!(), - }; + let occurrences = parser.kind == ParserKind::FromOccurrences; + let flag = parser.kind == ParserKind::FromFlag; let modifier = match **ty { Ty::Bool => quote!(), @@ -273,7 +249,6 @@ pub fn gen_app_augmentation( quote_spanned! { ty.span()=> .takes_value(true) #possible_values - #validator } } @@ -282,14 +257,12 @@ pub fn gen_app_augmentation( .multiple(false) .min_values(0) .max_values(1) - #validator }, Ty::OptionVec => quote_spanned! { ty.span()=> .takes_value(true) .multiple(true) .min_values(0) - #validator }, Ty::Vec => { @@ -305,7 +278,6 @@ pub fn gen_app_augmentation( .takes_value(true) .multiple(true) #possible_values - #validator } } @@ -330,7 +302,6 @@ pub fn gen_app_augmentation( .takes_value(true) .required(#required) #possible_values - #validator } } }; diff --git a/clap_derive/src/derives/subcommand.rs b/clap_derive/src/derives/subcommand.rs index a7b365b09784..b4d889d1b9c6 100644 --- a/clap_derive/src/derives/subcommand.rs +++ b/clap_derive/src/derives/subcommand.rs @@ -247,7 +247,7 @@ fn gen_from_subcommand( quote! { Some((#sub_name, arg_matches)) => { - Some(#name :: #variant_name #constructor_block) + Ok(#name :: #variant_name #constructor_block) } } }); @@ -257,8 +257,13 @@ fn gen_from_subcommand( Unnamed(ref fields) if fields.unnamed.len() == 1 => { let ty = &fields.unnamed[0]; quote! { - if let Some(res) = <#ty as ::clap::Subcommand>::from_subcommand(other) { - return Some(#name :: #variant_name (res)); + match <#ty as ::clap::Subcommand>::from_subcommand(other) { + Ok(res) => return Ok(#name :: #variant_name (res)), + Err(::clap::Error { + kind: ::clap::ErrorKind::UnrecognizedSubcommand, + .. + }) => {} + Err(e) => return Err(e), } } } @@ -271,10 +276,13 @@ fn gen_from_subcommand( let wildcard = match ext_subcmd { Some((span, var_name, str_ty, values_of)) => quote_spanned! { span=> - None => ::std::option::Option::None, + None => Err(::clap::Error::with_description( + format!("The subcommand wasn't recognized"), + ::clap::ErrorKind::UnrecognizedSubcommand + )), Some((external, arg_matches)) => { - ::std::option::Option::Some(#name::#var_name( + ::std::result::Result::Ok(#name::#var_name( ::std::iter::once(#str_ty::from(external)) .chain( arg_matches.#values_of("").into_iter().flatten().map(#str_ty::from) @@ -284,11 +292,14 @@ fn gen_from_subcommand( } }, - None => quote!(_ => None), + None => quote!(_ => Err(::clap::Error::with_description( + format!("The subcommand wasn't recognized"), + ::clap::ErrorKind::UnrecognizedSubcommand + ))), }; quote! { - fn from_subcommand(subcommand: Option<(&str, &::clap::ArgMatches)>) -> Option { + fn from_subcommand(subcommand: Option<(&str, &::clap::ArgMatches)>) -> ::clap::Result { match subcommand { #( #match_arms, )* other => { @@ -385,7 +396,7 @@ fn gen_update_from_subcommand( ( quote!((ref mut arg)), quote! { - <#ty as ::clap::Subcommand>::update_from_subcommand(arg, Some((name, arg_matches))); + <#ty as ::clap::Subcommand>::update_from_subcommand(arg, Some((name, arg_matches)))?; }, ) } @@ -403,16 +414,19 @@ fn gen_update_from_subcommand( fn update_from_subcommand<'b>( &mut self, subcommand: Option<(&str, &::clap::ArgMatches)> - ) { + ) -> ::clap::Result<()> { if let Some((name, arg_matches)) = subcommand { match (name, self) { #( #subcommands ),* #( #child_subcommands ),* - (_, s) => if let Some(sub) = ::from_subcommand(Some((name, arg_matches))) { - *s = sub; + (_, s) => match ::from_subcommand(Some((name, arg_matches))) { + Ok(sub) => *s = sub, + Err(::clap::Error { kind: ::clap::ErrorKind::UnrecognizedSubcommand, .. }) => (), + Err(e) => return Err(e), } } } + Ok(()) } } } diff --git a/clap_derive/src/dummies.rs b/clap_derive/src/dummies.rs index 596d822abd69..f12c32dcc166 100644 --- a/clap_derive/src/dummies.rs +++ b/clap_derive/src/dummies.rs @@ -40,10 +40,13 @@ 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) -> Self { + fn try_from_arg_matches(_m: &::clap::ArgMatches) -> ::clap::Result { unimplemented!() } - fn update_from_arg_matches(&mut self, matches: &::clap::ArgMatches) { + fn try_update_from_arg_matches( + &mut self, + matches: &::clap::ArgMatches + ) -> ::clap::Result<()> { unimplemented!() } } @@ -53,10 +56,13 @@ pub fn from_arg_matches(name: &Ident) { pub fn subcommand(name: &Ident) { append_dummy(quote! { impl ::clap::Subcommand for #name { - fn from_subcommand(_sub: Option<(&str, &::clap::ArgMatches)>) -> Option { + fn from_subcommand(_sub: Option<(&str, &::clap::ArgMatches)>) -> ::clap::Result { unimplemented!() } - fn update_from_subcommand(&mut self, _sub: Option<(&str, &::clap::ArgMatches)>) { + fn update_from_subcommand( + &mut self, + _sub: Option<(&str, &::clap::ArgMatches)> + ) -> ::clap::Result<()> { unimplemented!() } fn augment_subcommands(_app: ::clap::App<'_>) -> ::clap::App<'_> { diff --git a/clap_derive/tests/custom-string-parsers.rs b/clap_derive/tests/custom-string-parsers.rs index 32ebf70a36a0..07b5fc4caa71 100644 --- a/clap_derive/tests/custom-string-parsers.rs +++ b/clap_derive/tests/custom-string-parsers.rs @@ -14,7 +14,7 @@ use clap::Clap; -use std::ffi::{CString, OsStr}; +use std::ffi::{CString, OsStr, OsString}; use std::num::ParseIntError; use std::path::PathBuf; @@ -104,7 +104,7 @@ fn custom_parser_2(_: &str) -> Result<&'static str, ErrCode> { fn custom_parser_3(_: &OsStr) -> &'static str { "C" } -fn custom_parser_4(_: &OsStr) -> Result<&'static str, String> { +fn custom_parser_4(_: &OsStr) -> Result<&'static str, OsString> { Ok("D") } diff --git a/clap_derive/tests/effectful.rs b/clap_derive/tests/effectful.rs new file mode 100644 index 000000000000..90262f2a27f2 --- /dev/null +++ b/clap_derive/tests/effectful.rs @@ -0,0 +1,28 @@ +use clap::Clap; +use std::str::FromStr; +use std::sync::atomic::{AtomicU32, Ordering::SeqCst}; + +static NUM_CALLS: AtomicU32 = AtomicU32::new(0); + +#[derive(Debug)] +struct Effectful {} + +impl FromStr for Effectful { + type Err = String; + + fn from_str(_s: &str) -> Result { + NUM_CALLS.fetch_add(1, SeqCst); + Ok(Self {}) + } +} + +#[derive(Clap, Debug)] +struct Opt { + effectful: Effectful, +} + +#[test] +fn effectful() { + let _opt = Opt::parse_from(&["test", "arg"]); + assert_eq!(NUM_CALLS.load(SeqCst), 1); +} diff --git a/clap_derive/tests/validator.rs b/clap_derive/tests/validator.rs new file mode 100644 index 000000000000..f5756f9efcfd --- /dev/null +++ b/clap_derive/tests/validator.rs @@ -0,0 +1,31 @@ +use clap::Clap; + +fn non_negative(x: &str) -> Result<(), String> { + if x.trim_start().starts_with('-') { + Err(String::from("the value must be non-negative")) + } else { + Ok(()) + } +} + +#[derive(Clap, Debug)] +#[clap(name = "basic")] +struct Opt { + /// Set speed to a non-negative value. + #[clap(short, long, validator = non_negative)] + speed: f64, +} + +#[test] +fn use_validator() { + let opt = Opt::parse_from(&["test", "--speed=2.0"]); + assert_eq!(opt.speed, 2.0); + + let err = Opt::try_parse_from(&["test", "--speed=-2.0"]).expect_err("validator should fail"); + assert!(err + .to_string() + .contains("error: Invalid value for \'--speed \': the value must be non-negative")); + + let err = Opt::try_parse_from(&["test", "--speed=bogus"]).expect_err("parsing should fail"); + assert!(err.to_string().contains("invalid float literal")); +} diff --git a/src/derive.rs b/src/derive.rs index d456a892f5de..d50840592989 100644 --- a/src/derive.rs +++ b/src/derive.rs @@ -80,7 +80,7 @@ pub trait Clap: FromArgMatches + IntoApp + Sized { /// Parse from `std::env::args_os()`, return Err on error. fn try_parse() -> Result { let matches = ::into_app().try_get_matches()?; - Ok(::from_arg_matches(&matches)) + ::try_from_arg_matches(&matches) } /// Parse from iterator, exit on error @@ -102,7 +102,7 @@ pub trait Clap: FromArgMatches + IntoApp + Sized { T: Into + Clone, { let matches = ::into_app().try_get_matches_from(itr)?; - Ok(::from_arg_matches(&matches)) + ::try_from_arg_matches(&matches) } /// Update from iterator, exit on error @@ -181,18 +181,33 @@ pub trait FromArgMatches: Sized { /// } /// } /// ``` - fn from_arg_matches(matches: &ArgMatches) -> Self; + fn from_arg_matches(matches: &ArgMatches) -> Self { + Self::try_from_arg_matches(matches).unwrap_or_else(|e| e.exit()) + } /// @TODO@ @release @docs - fn update_from_arg_matches(&mut self, matches: &ArgMatches); + fn update_from_arg_matches(&mut self, matches: &ArgMatches) { + Self::try_update_from_arg_matches(self, matches).unwrap_or_else(|e| e.exit()) + } + + /// Similar to `from_arg_matches`, but instead of exiting returns errors + /// via a `Result` value. + fn try_from_arg_matches(matches: &ArgMatches) -> Result; + + /// Similar to `update_from_arg_matches`, but instead of exiting returns errors + /// via a `Result` value. + fn try_update_from_arg_matches(&mut self, matches: &ArgMatches) -> Result<(), Error>; } /// @TODO @release @docs pub trait Subcommand: Sized { /// @TODO @release @docs - fn from_subcommand(subcommand: Option<(&str, &ArgMatches)>) -> Option; + fn from_subcommand(subcommand: Option<(&str, &ArgMatches)>) -> Result; /// @TODO @release @docs - fn update_from_subcommand(&mut self, subcommand: Option<(&str, &ArgMatches)>); + fn update_from_subcommand( + &mut self, + subcommand: Option<(&str, &ArgMatches)>, + ) -> Result<(), Error>; /// @TODO @release @docs fn augment_subcommands(app: App<'_>) -> App<'_>; /// @TODO @release @docs @@ -255,17 +270,31 @@ impl FromArgMatches for Box { fn from_arg_matches(matches: &ArgMatches) -> Self { Box::new(::from_arg_matches(matches)) } + fn update_from_arg_matches(&mut self, matches: &ArgMatches) { ::update_from_arg_matches(self, matches); } + + fn try_from_arg_matches(matches: &ArgMatches) -> Result { + Ok(Box::new(::try_from_arg_matches( + matches, + )?)) + } + + fn try_update_from_arg_matches(&mut self, matches: &ArgMatches) -> Result<(), Error> { + ::try_update_from_arg_matches(self, matches) + } } impl Subcommand for Box { - fn from_subcommand(subcommand: Option<(&str, &ArgMatches)>) -> Option { + fn from_subcommand(subcommand: Option<(&str, &ArgMatches)>) -> Result { ::from_subcommand(subcommand).map(Box::new) } - fn update_from_subcommand(&mut self, subcommand: Option<(&str, &ArgMatches)>) { - ::update_from_subcommand(self, subcommand); + fn update_from_subcommand( + &mut self, + subcommand: Option<(&str, &ArgMatches)>, + ) -> Result<(), Error> { + ::update_from_subcommand(self, subcommand) } fn augment_subcommands(app: App<'_>) -> App<'_> { ::augment_subcommands(app) diff --git a/src/parse/matches/arg_matches.rs b/src/parse/matches/arg_matches.rs index c7d06e69b5b4..6b2c472878a7 100644 --- a/src/parse/matches/arg_matches.rs +++ b/src/parse/matches/arg_matches.rs @@ -344,8 +344,112 @@ impl ArgMatches { R: FromStr, ::Err: Display, { - if let Some(v) = self.value_of(name) { - v.parse::().map_err(|e| { + self.parse_t(name, R::from_str) + } + + /// Gets the value of a specific argument (i.e. an argument that takes an additional + /// value at runtime) and then converts it into the result type using `parse`. + /// + /// There are two types of errors, parse failures and those where the argument wasn't present + /// (such as a non-required argument). Check [`ErrorKind`] to distinguish them. + /// + /// *NOTE:* If getting a value for an option or positional argument that allows multiples, + /// prefer [`ArgMatches::parse_vec_t`] as this method will only return the *first* + /// value. + /// + /// # Panics + /// + /// This method will [`panic!`] if the value contains invalid UTF-8 code points. + /// + /// # Examples + /// + /// ``` + /// # extern crate clap; + /// # use clap::App; + /// # use std::str::FromStr; + /// let matches = App::new("myapp") + /// .arg("[length] 'Set the length to use as a pos whole num, i.e. 20'") + /// .get_matches_from(&["test", "12"]); + /// + /// // Specify the type explicitly (or use turbofish) + /// let len: u32 = matches.parse_t("length", FromStr::from_str).unwrap_or_else(|e| e.exit()); + /// assert_eq!(len, 12); + /// + /// // You can often leave the type for rustc to figure out + /// let also_len = matches + /// .parse_t("length", FromStr::from_str) + /// .unwrap_or_else(|e| e.exit()); + /// // Something that expects u32 + /// let _: u32 = also_len; + /// ``` + /// + /// [`ErrorKind`]: enum.ErrorKind.html + /// [`ArgMatches::parse_vec_t`]: ./struct.ArgMatches.html#method.parse_vec_t + /// [`panic!`]: https://doc.rust-lang.org/std/macro.panic!.html + pub fn parse_t( + &self, + name: &str, + parse: impl Fn(&str) -> Result, + ) -> Result + where + E: Display, + { + self.parse_optional_t(name, parse)? + .ok_or_else(|| Error::argument_not_found_auto(name.to_string())) + } + + /// Gets the optional value of a specific argument (i.e. an argument that takes an additional + /// value at runtime) and then converts it into the result type using `parse`. + /// + /// In the case where the argument wasn't present, `Ok(None)` is returned. In the + /// case where it was present and parsing succeeded, `Ok(Some(value))` is returned. + /// If parsing (of any value) has failed, returns Err. + /// + /// *NOTE:* If getting a value for an option or positional argument that allows multiples, + /// prefer [`ArgMatches::parse_optional_vec_t`] as this method will only return the *first* + /// value. + /// + /// # Panics + /// + /// This method will [`panic!`] if the value contains invalid UTF-8 code points. + /// + /// # Examples + /// + /// ``` + /// # extern crate clap; + /// # use clap::App; + /// # use std::str::FromStr; + /// let matches = App::new("myapp") + /// .arg("[length] @20 'Set the length to use as a pos whole num, i.e. 20'") + /// .get_matches_from(&["test", "12"]); + /// + /// // Specify the type explicitly (or use turbofish) + /// let len: Option = matches + /// .parse_optional_t("length", FromStr::from_str) + /// .unwrap_or_else(|e| e.exit()); + /// assert_eq!(len, Some(12)); + /// + /// // You can often leave the type for rustc to figure out + /// let also_len = matches + /// .parse_optional_t("length", FromStr::from_str) + /// .unwrap_or_else(|e| e.exit()); + /// // Something that expects Option + /// let _: Option = also_len; + /// ``` + /// + /// [`ErrorKind`]: enum.ErrorKind.html + /// [`ArgMatches::parse_optional_vec_t`]: ./struct.ArgMatches.html#method.parse_optional_vec_t + /// [`panic!`]: https://doc.rust-lang.org/std/macro.panic!.html + pub fn parse_optional_t( + &self, + name: &str, + parse: impl Fn(&str) -> Result, + ) -> Result, Error> + where + E: Display, + { + Ok(match self.value_of(name) { + Some(v) => Some(parse(v).map_err(|e| { let message = format!( "The argument '{}' isn't a valid value for '{}': {}", v, name, e @@ -357,10 +461,100 @@ impl ArgMatches { message.into(), ColorChoice::Auto, ) - }) - } else { - Err(Error::argument_not_found_auto(name.to_string())) - } + })?), + None => None, + }) + } + + /// Gets the value of a specific argument (i.e. an argument that takes an additional + /// value at runtime) and then converts it into the result type using `parse`, which takes + /// the OS version of the string value of the argument. + /// + /// There are two types of errors, parse failures and those where the argument wasn't present + /// (such as a non-required argument). Check [`ErrorKind`] to distinguish them. + /// + /// *NOTE:* If getting a value for an option or positional argument that allows multiples, + /// prefer [`ArgMatches::parse_vec_t_os`] as `Arg::parse_t_os` will only return the *first* + /// value. + /// + /// # Examples + /// + #[cfg_attr(not(unix), doc = " ```ignore")] + #[cfg_attr(unix, doc = " ```")] + /// # use clap::{App, Arg}; + /// use std::ffi::OsString; + /// use std::os::unix::ffi::{OsStrExt,OsStringExt}; + /// + /// let m = App::new("utf8") + /// .arg(Arg::from(" 'some arg'")) + /// .get_matches_from(vec![OsString::from("myprog"), + /// // "Hi {0xe9}!" + /// OsString::from_vec(vec![b'H', b'i', b' ', 0xe9, b'!'])]); + /// assert_eq!(&*m.value_of_os("arg").unwrap().as_bytes(), [b'H', b'i', b' ', 0xe9, b'!']); + /// ``` + /// [`String`]: https://doc.rust-lang.org/std/string/struct.String.html + /// [`ArgMatches::parse_vec_t_os`]: ./struct.ArgMatches.html#method.parse_vec_t_os + pub fn parse_t_os( + &self, + name: &str, + parse: impl Fn(&OsStr) -> Result, + ) -> Result { + self.parse_optional_t_os(name, parse)? + .ok_or_else(|| Error::argument_not_found_auto(name.to_string())) + } + + /// Gets optional the value of a specific argument (i.e. an argument that takes an additional + /// value at runtime) and then converts it into the result type using `parse`, which takes + /// the OS version of the string value of the argument. + /// + /// In the case where the argument wasn't present, `Ok(None)` is returned. In the + /// case where it was present and parsing succeeded, `Ok(Some(value))` is returned. + /// If parsing (of any value) has failed, returns Err. + /// + /// *NOTE:* If getting a value for an option or positional argument that allows multiples, + /// prefer [`ArgMatches::parse_vec_t_os`] as `Arg::parse_t_os` will only return the *first* + /// value. + /// + /// # Examples + /// + #[cfg_attr(not(unix), doc = " ```ignore")] + #[cfg_attr(unix, doc = " ```")] + /// # use clap::{App, Arg}; + /// use std::ffi::OsString; + /// use std::os::unix::ffi::{OsStrExt,OsStringExt}; + /// + /// let m = App::new("utf8") + /// .arg(Arg::from(" 'some arg'")) + /// .get_matches_from(vec![OsString::from("myprog"), + /// // "Hi {0xe9}!" + /// OsString::from_vec(vec![b'H', b'i', b' ', 0xe9, b'!'])]); + /// assert_eq!(&*m.value_of_os("arg").unwrap().as_bytes(), [b'H', b'i', b' ', 0xe9, b'!']); + /// ``` + /// [`String`]: https://doc.rust-lang.org/std/string/struct.String.html + /// [`ArgMatches::parse_vec_t_os`]: ./struct.ArgMatches.html#method.parse_vec_t_os + pub fn parse_optional_t_os( + &self, + name: &str, + parse: impl Fn(&OsStr) -> Result, + ) -> Result, Error> { + Ok(match self.value_of_os(name) { + Some(v) => Some(parse(v).map_err(|e| { + let message = format!( + "The argument '{}' isn't a valid value for '{}': {}", + v.to_string_lossy(), + name, + e.to_string_lossy() + ); + + Error::value_validation( + name.to_string(), + v.to_string_lossy().to_string(), + message.into(), + ColorChoice::Auto, + ) + })?), + None => None, + }) } /// Gets the value of a specific argument (i.e. an argument that takes an additional @@ -436,23 +630,179 @@ impl ArgMatches { R: FromStr, ::Err: Display, { - if let Some(vals) = self.values_of(name) { - vals.map(|v| { - v.parse::().map_err(|e| { - let message = format!("The argument '{}' isn't a valid value: {}", v, e); + self.parse_vec_t(name, R::from_str) + } + + /// Gets the typed values of a specific argument (i.e. an argument that takes multiple + /// values at runtime) and then converts them into the result type using `parse`. + /// + /// If parsing (of any value) has failed, returns Err. + /// + /// # Panics + /// + /// This method will [`panic!`] if any of the values contains invalid UTF-8 code points. + /// + /// # Examples + /// + /// ``` + /// # extern crate clap; + /// # use clap::App; + /// # use std::str::FromStr; + /// let matches = App::new("myapp") + /// .arg("[length]... 'A sequence of integers because integers are neat!'") + /// .get_matches_from(&["test", "12", "77", "40"]); + /// + /// // Specify the type explicitly (or use turbofish) + /// let len: Vec = matches + /// .parse_vec_t("length", FromStr::from_str) + /// .unwrap_or_else(|e| e.exit()); + /// assert_eq!(len, vec![12, 77, 40]); + /// + /// // You can often leave the type for rustc to figure out + /// let also_len = matches + /// .parse_vec_t("length", FromStr::from_str) + /// .unwrap_or_else(|e| e.exit()); + /// // Something that expects Vec + /// let _: Vec = also_len; + /// ``` + /// + /// [`panic!`]: https://doc.rust-lang.org/std/macro.panic!.html + pub fn parse_vec_t( + &self, + name: &str, + parse: impl Fn(&str) -> Result, + ) -> Result, Error> + where + E: Display, + { + self.parse_optional_vec_t(name, parse)? + .ok_or_else(|| Error::argument_not_found_auto(name.to_string())) + } + + /// Gets the optional typed values of a specific argument (i.e. an argument that takes multiple + /// values at runtime) and then converts them into the result type using `parse`. + /// + /// In the case where the argument wasn't present, `Ok(None)` is returned. In the + /// case where it was present and parsing succeeded, `Ok(Some(vec))` is returned. + /// If parsing (of any value) has failed, returns Err. + /// + /// # Panics + /// + /// This method will [`panic!`] if any of the values contains invalid UTF-8 code points. + /// + /// # Examples + /// + /// ``` + /// # extern crate clap; + /// # use clap::App; + /// # use std::str::FromStr; + /// let matches = App::new("myapp") + /// .arg("[length]... 'A sequence of integers because integers are neat!'") + /// .get_matches_from(&["test", "12", "77", "40"]); + /// + /// // Specify the type explicitly (or use turbofish) + /// let len: Option> = matches + /// .parse_optional_vec_t("length", FromStr::from_str) + /// .unwrap_or_else(|e| e.exit()); + /// assert_eq!(len, Some(vec![12, 77, 40])); + /// + /// // You can often leave the type for rustc to figure out + /// let also_len = matches + /// .parse_optional_vec_t("length", FromStr::from_str) + /// .unwrap_or_else(|e| e.exit()); + /// // Something that expects Option> + /// let _: Option> = also_len; + /// ``` + /// + /// [`panic!`]: https://doc.rust-lang.org/std/macro.panic!.html + pub fn parse_optional_vec_t( + &self, + name: &str, + parse: impl Fn(&str) -> Result, + ) -> Result>, Error> + where + E: Display, + { + Ok(match self.values_of(name) { + Some(vals) => Some( + vals.map(|v| { + parse(v).map_err(|e| { + let message = format!("The argument '{}' isn't a valid value: {}", v, e); - Error::value_validation( - name.to_string(), - v.to_string(), - message.into(), - ColorChoice::Auto, - ) + Error::value_validation( + name.to_string(), + v.to_string(), + message.into(), + ColorChoice::Auto, + ) + }) }) - }) - .collect() - } else { - Err(Error::argument_not_found_auto(name.to_string())) - } + .collect::>()?, + ), + None => None, + }) + } + + /// Gets the typed values of a specific argument (i.e. an argument that takes multiple + /// values at runtime) and then converts them into the result type using `parse`, which takes + /// the OS version of the string value of the argument. + /// + /// If parsing (of any value) has failed, returns Err. + /// + /// # Panics + /// + /// This method will [`panic!`] if any of the values contains invalid UTF-8 code points. + /// + /// [`panic!`]: https://doc.rust-lang.org/std/macro.panic!.html + pub fn parse_vec_t_os( + &self, + name: &str, + parse: impl Fn(&OsStr) -> Result, + ) -> Result, Error> { + self.parse_optional_vec_t_os(name, parse)? + .ok_or_else(|| Error::argument_not_found_auto(name.to_string())) + } + + /// Gets the typed values of a specific argument (i.e. an argument that takes multiple + /// values at runtime) and then converts them into the result type using `parse`, which takes + /// the OS version of the string value of the argument. + /// + /// In the case where the argument wasn't present, `Ok(None)` is returned. In the + /// case where it was present and parsing succeeded, `Ok(Some(vec))` is returned. + /// If parsing (of any value) has failed, returns Err. + /// + /// # Panics + /// + /// This method will [`panic!`] if any of the values contains invalid UTF-8 code points. + /// + /// [`panic!`]: https://doc.rust-lang.org/std/macro.panic!.html + pub fn parse_optional_vec_t_os( + &self, + name: &str, + parse: impl Fn(&OsStr) -> Result, + ) -> Result>, Error> { + Ok(match self.values_of_os(name) { + Some(vals) => Some( + vals.map(|v| { + parse(v).map_err(|e| { + let message = format!( + "The argument '{}' isn't a valid value: {}", + v.to_string_lossy(), + e.to_string_lossy() + ); + + Error::value_validation( + name.to_string(), + v.to_string_lossy().to_string(), + message.into(), + ColorChoice::Auto, + ) + }) + }) + .collect::>()?, + ), + None => None, + }) } /// Gets the typed values of a specific argument (i.e. an argument that takes multiple