From ec6b6e1e36124467dd1b50cb5b2eb195869caed4 Mon Sep 17 00:00:00 2001 From: CreepySkeleton Date: Thu, 30 Apr 2020 20:52:45 +0300 Subject: [PATCH] Make validator take &str instead of String --- benches/05_ripgrep.rs | 2 +- clap_derive/src/derives/into_app.rs | 4 ++-- examples/15_custom_validator.rs | 2 +- examples/18_builder_macro.rs | 2 +- src/build/app/mod.rs | 7 +++++++ src/build/arg/mod.rs | 6 +++--- src/parse/errors.rs | 2 +- src/parse/validator.rs | 2 +- tests/validators.rs | 19 +++++++++++++++++++ 9 files changed, 36 insertions(+), 10 deletions(-) create mode 100644 tests/validators.rs diff --git a/benches/05_ripgrep.rs b/benches/05_ripgrep.rs index 6fa12e770fb..7a5f2858573 100644 --- a/benches/05_ripgrep.rs +++ b/benches/05_ripgrep.rs @@ -934,7 +934,7 @@ lazy_static! { }; } -fn validate_number(s: String) -> Result<(), String> { +fn validate_number(s: &str) -> Result<(), String> { s.parse::() .map(|_| ()) .map_err(|err| err.to_string()) diff --git a/clap_derive/src/derives/into_app.rs b/clap_derive/src/derives/into_app.rs index b42d55b331c..38a642049c0 100644 --- a/clap_derive/src/derives/into_app.rs +++ b/clap_derive/src/derives/into_app.rs @@ -205,13 +205,13 @@ pub fn gen_app_augmentation( _ if attrs.is_enum() => quote!(), ParserKind::TryFromStr => quote_spanned! { func.span()=> .validator(|s| { - #func(s.as_str()) + #func(s) .map(|_: #convert_type| ()) .map_err(|e| e.to_string()) }) }, ParserKind::TryFromOsStr => quote_spanned! { func.span()=> - .validator_os(|s| #func(&s).map(|_: #convert_type| ())) + .validator_os(|s| #func(s).map(|_: #convert_type| ())) }, _ => quote!(), }; diff --git a/examples/15_custom_validator.rs b/examples/15_custom_validator.rs index 976816ebafb..e8255950e86 100644 --- a/examples/15_custom_validator.rs +++ b/examples/15_custom_validator.rs @@ -21,7 +21,7 @@ fn main() { println!("The .PNG file is: {}", matches.value_of("input").unwrap()); } -fn is_png(val: String) -> Result<(), String> { +fn is_png(val: &str) -> Result<(), String> { // val is the argument value passed in by the user // val has type of String. if val.ends_with(".png") { diff --git a/examples/18_builder_macro.rs b/examples/18_builder_macro.rs index ddc355cc8b3..38bc2973dd3 100644 --- a/examples/18_builder_macro.rs +++ b/examples/18_builder_macro.rs @@ -5,7 +5,7 @@ use clap::clap_app; fn main() { // Validation example testing that a file exists - let file_exists = |path| { + let file_exists = |path: &str| { if std::fs::metadata(path).is_ok() { Ok(()) } else { diff --git a/src/build/app/mod.rs b/src/build/app/mod.rs index 2cebbd4c62c..3f4136edbd8 100644 --- a/src/build/app/mod.rs +++ b/src/build/app/mod.rs @@ -1711,6 +1711,13 @@ impl<'b> App<'b> { "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 + ); } for group in &self.groups { diff --git a/src/build/arg/mod.rs b/src/build/arg/mod.rs index 8024595fed7..7a92a03723f 100644 --- a/src/build/arg/mod.rs +++ b/src/build/arg/mod.rs @@ -25,7 +25,7 @@ use crate::{ INTERNAL_ERROR_MSG, }; -type Validator = Rc Result<(), String>>; +type Validator = Rc Result<(), String>>; type ValidatorOs = Rc Result<(), String>>; /// The abstract representation of a command line argument. Used to set all the options and @@ -1927,7 +1927,7 @@ impl<'help> Arg<'help> { /// /// ```rust /// # use clap::{App, Arg}; - /// fn has_at(v: String) -> Result<(), String> { + /// fn has_at(v: &str) -> Result<(), String> { /// if v.contains("@") { return Ok(()); } /// Err(String::from("The value did not contain the required @ sigil")) /// } @@ -1947,7 +1947,7 @@ impl<'help> Arg<'help> { /// [`Rc`]: https://doc.rust-lang.org/std/rc/struct.Rc.html pub fn validator(mut self, f: F) -> Self where - F: Fn(String) -> Result + 'static, + F: Fn(&str) -> Result + 'static, E: ToString, { self.validator = Some(Rc::new(move |s| { diff --git a/src/parse/errors.rs b/src/parse/errors.rs index 2fde154f2e7..c5215d19701 100644 --- a/src/parse/errors.rs +++ b/src/parse/errors.rs @@ -130,7 +130,7 @@ pub enum ErrorKind { /// /// ```rust /// # use clap::{App, Arg, ErrorKind}; - /// fn is_numeric(val: String) -> Result<(), String> { + /// fn is_numeric(val: &str) -> Result<(), String> { /// match val.parse::() { /// Ok(..) => Ok(()), /// Err(..) => Err(String::from("Value wasn't a number!")), diff --git a/src/parse/validator.rs b/src/parse/validator.rs index c03ccad86cd..845712873aa 100644 --- a/src/parse/validator.rs +++ b/src/parse/validator.rs @@ -137,7 +137,7 @@ impl<'b, 'c, 'z> Validator<'b, 'c, 'z> { } if let Some(ref vtor) = arg.validator { debug!("Validator::validate_arg_values: checking validator..."); - if let Err(e) = vtor(val.to_string_lossy().into_owned()) { + if let Err(e) = vtor(&*val.to_string_lossy()) { debug!("error"); return Err(Error::value_validation(Some(arg), &e, self.p.app.color())?); } else { diff --git a/tests/validators.rs b/tests/validators.rs new file mode 100644 index 00000000000..5abedf6cd40 --- /dev/null +++ b/tests/validators.rs @@ -0,0 +1,19 @@ +use clap::{App, Arg}; + +#[cfg(debug_assertions)] +#[test] +#[should_panic = "Argument 'test' has both `validator` and `validator_os` set which is not allowed"] +fn both_validator_and_validator_os() { + let _ = App::new("test") + .arg( + Arg::with_name("test") + .validator(|val| val.parse::().map_err(|e| e.to_string())) + .validator_os(|val| { + val.to_str() + .unwrap() + .parse::() + .map_err(|e| e.to_string()) + }), + ) + .try_get_matches_from(&["app", "1"]); +}