Skip to content

Commit

Permalink
feat(derive): Allow users to opt-in to ValueParser
Browse files Browse the repository at this point in the history 
For clap 3, its opt-in as a precaution against breaking
compatibility in some weird cases.

This does require the types to implement `Clone`.

Fixes clap-rs#3734
Fixes clap-rs#3496
Fixes clap-rs#3589
  • Loading branch information
@epage
epage committed May 20, 2022
1 parent 0628d04 commit b52c7f1
Show file tree
Hide file tree
Showing 5 changed files with 147 additions and 33 deletions.
72 changes: 71 additions & 1 deletion clap_derive/src/attrs.rs
View file
Expand Up @@ -42,6 +42,7 @@ pub struct Attrs {
ty: Option<Type>,
doc_comment: Vec<Method>,
methods: Vec<Method>,
value_parser: Option<Method>,
parser: Sp<Parser>,
verbatim_doc_comment: Option<Ident>,
next_display_order: Option<Method>,
Expand All @@ -64,6 +65,12 @@ impl Attrs {
res.push_attrs(attrs);
res.push_doc_comment(attrs, "about");

if let Some(parser) = res.value_parser.as_ref() {
abort!(
parser.span(),
"`value_parse` attribute is only allowed on fields"
);
}
if res.has_custom_parser {
abort!(
res.parser.span(),
Expand Down Expand Up @@ -101,6 +108,12 @@ impl Attrs {

match &*res.kind {
Kind::Flatten => {
if let Some(parser) = res.value_parser.as_ref() {
abort!(
parser.span(),
"`value_parse` attribute is only allowed flattened entry"
);
}
if res.has_custom_parser {
abort!(
res.parser.span(),
Expand All @@ -121,6 +134,12 @@ impl Attrs {
Kind::ExternalSubcommand => (),

Kind::Subcommand(_) => {
if let Some(parser) = res.value_parser.as_ref() {
abort!(
parser.span(),
"`value_parse` attribute is only allowed for subcommand"
);
}
if res.has_custom_parser {
abort!(
res.parser.span(),
Expand Down Expand Up @@ -189,6 +208,12 @@ impl Attrs {
res.push_attrs(&variant.attrs);
res.push_doc_comment(&variant.attrs, "help");

if let Some(parser) = res.value_parser.as_ref() {
abort!(
parser.span(),
"`value_parse` attribute is only allowed on fields"
);
}
if res.has_custom_parser {
abort!(
res.parser.span(),
Expand Down Expand Up @@ -226,6 +251,12 @@ impl Attrs {

match &*res.kind {
Kind::Flatten => {
if let Some(parser) = res.value_parser.as_ref() {
abort!(
parser.span(),
"`value_parse` attribute is not allowed for flattened entry"
);
}
if res.has_custom_parser {
abort!(
res.parser.span(),
Expand All @@ -250,6 +281,12 @@ impl Attrs {
}

Kind::Subcommand(_) => {
if let Some(parser) = res.value_parser.as_ref() {
abort!(
parser.span(),
"`value_parse` attribute is not allowed for subcommand"
);
}
if res.has_custom_parser {
abort!(
res.parser.span(),
Expand Down Expand Up @@ -297,6 +334,12 @@ impl Attrs {
Kind::Arg(orig_ty) => {
let mut ty = Ty::from_syn_ty(&field.ty);
if res.has_custom_parser {
if let Some(parser) = res.value_parser.as_ref() {
abort!(
parser.name.span(),
"`value_parse` attribute conflicts with `parse` attribute"
);
}
match *ty {
Ty::Option | Ty::Vec | Ty::OptionVec => (),
_ => ty = Sp::new(Ty::Other, ty.span()),
Expand Down Expand Up @@ -369,6 +412,7 @@ impl Attrs {
env_casing,
doc_comment: vec![],
methods: vec![],
value_parser: None,
parser: Parser::default_spanned(default_span),
verbatim_doc_comment: None,
next_display_order: None,
Expand All @@ -383,6 +427,8 @@ impl Attrs {
fn push_method(&mut self, name: Ident, arg: impl ToTokens) {
if name == "name" {
self.name = Name::Assigned(quote!(#arg));
} else if name == "value_parser" {
self.value_parser = Some(Method::new(name, quote!(#arg)));
} else {
self.methods.push(Method::new(name, quote!(#arg)));
}
Expand Down Expand Up @@ -689,6 +735,16 @@ impl Attrs {
self.name.clone().translate(CasingStyle::ScreamingSnake)
}

pub fn value_parser(&self) -> Method {
self.value_parser
.clone()
.unwrap_or_else(|| self.parser.value_parser())
}

pub fn custom_value_parser(&self) -> bool {
self.value_parser.is_some()
}

pub fn parser(&self) -> &Sp<Parser> {
&self.parser
}
Expand Down Expand Up @@ -865,9 +921,23 @@ impl Parser {
let parser = Parser { kind, func };
Sp::new(parser, parse_ident.span())
}

fn value_parser(&self) -> Method {
let func = Ident::new("value_parser", self.kind.span());
match *self.kind {
ParserKind::FromStr | ParserKind::TryFromStr => {
Method::new(func, quote!(clap::builder::ValueParser::string()))
}
ParserKind::FromOsStr | ParserKind::TryFromOsStr => {
Method::new(func, quote!(clap::builder::ValueParser::os_string()))
}
ParserKind::FromOccurrences => Method::new(func, quote!(clap::value_parser!(usize))),
ParserKind::FromFlag => Method::new(func, quote!(clap::ValueParser::bool())),
}
}
}

#[derive(Debug, PartialEq, Clone)]
#[derive(Debug, PartialEq, Clone, Copy)]
pub enum ParserKind {
FromStr,
TryFromStr,
Expand Down
69 changes: 37 additions & 32 deletions clap_derive/src/derives/args.rs
View file
Expand Up @@ -246,11 +246,12 @@ pub fn gen_augment(
let occurrences = *attrs.parser().kind == ParserKind::FromOccurrences;
let flag = *attrs.parser().kind == ParserKind::FromFlag;

let value_parser = attrs.value_parser();
let parser = attrs.parser();
let func = &parser.func;

let validator = match *parser.kind {
_ if attrs.is_enum() => quote!(),
_ if attrs.custom_value_parser() || attrs.is_enum() => quote!(),
ParserKind::TryFromStr => quote_spanned! { func.span()=>
.validator(|s| {
#func(s)
Expand All @@ -265,21 +266,9 @@ pub fn gen_augment(
| ParserKind::FromFlag
| ParserKind::FromOccurrences => quote!(),
};
let allow_invalid_utf8 = match *parser.kind {
_ if attrs.is_enum() => quote!(),
ParserKind::FromOsStr | ParserKind::TryFromOsStr => {
quote_spanned! { func.span()=>
.allow_invalid_utf8(true)
}
}
ParserKind::FromStr
| ParserKind::TryFromStr
| ParserKind::FromFlag
| ParserKind::FromOccurrences => quote!(),
};

let value_name = attrs.value_name();
let possible_values = if attrs.is_enum() {
let possible_values = if attrs.is_enum() && !attrs.custom_value_parser() {
gen_arg_enum_possible_values(convert_type)
} else {
quote!()
Expand All @@ -294,7 +283,7 @@ pub fn gen_augment(
.value_name(#value_name)
#possible_values
#validator
#allow_invalid_utf8
#value_parser
}
}

Expand All @@ -306,7 +295,7 @@ pub fn gen_augment(
.multiple_values(false)
#possible_values
#validator
#allow_invalid_utf8
#value_parser
},

Ty::OptionVec => quote_spanned! { ty.span()=>
Expand All @@ -315,7 +304,7 @@ pub fn gen_augment(
.multiple_occurrences(true)
#possible_values
#validator
#allow_invalid_utf8
#value_parser
},

Ty::Vec => {
Expand All @@ -325,7 +314,7 @@ pub fn gen_augment(
.multiple_occurrences(true)
#possible_values
#validator
#allow_invalid_utf8
#value_parser
}
}

Expand All @@ -345,7 +334,7 @@ pub fn gen_augment(
.required(#required)
#possible_values
#validator
#allow_invalid_utf8
#value_parser
}
}
};
Expand Down Expand Up @@ -537,35 +526,51 @@ fn gen_parsers(
let span = parser.kind.span();
let convert_type = inner_type(**ty, &field.ty);
let id = attrs.id();
let (get_one, get_many, mut parse) = match *parser.kind {
let (get_one, get_many, deref, mut parse) = match *parser.kind {
FromOccurrences => (
quote_spanned!(span=> occurrences_of),
quote!(),
quote!(|s| ::std::ops::Deref::deref(s)),
func.clone(),
),
FromFlag => (
quote!(),
quote!(),
quote!(|s| ::std::ops::Deref::deref(s)),
func.clone(),
),
_ if attrs.custom_value_parser() => (
quote_spanned!(span=> get_one::<#convert_type>),
quote_spanned!(span=> get_many::<#convert_type>),
quote!(|s| s),
quote_spanned!(func.span()=> |s| ::std::result::Result::Ok::<_, clap::Error>(s.clone())),
),
FromStr => (
quote_spanned!(span=> get_one::<String>),
quote_spanned!(span=> get_many::<String>),
quote!(|s| ::std::ops::Deref::deref(s)),
quote_spanned!(func.span()=> |s| ::std::result::Result::Ok::<_, clap::Error>(#func(s))),
),
TryFromStr => (
quote_spanned!(span=> get_one::<String>),
quote_spanned!(span=> get_many::<String>),
quote!(|s| ::std::ops::Deref::deref(s)),
quote_spanned!(func.span()=> |s| #func(s).map_err(|err| clap::Error::raw(clap::ErrorKind::ValueValidation, format!("Invalid value for {}: {}", #id, err)))),
),
FromOsStr => (
quote_spanned!(span=> get_one::<::std::ffi::OsString>),
quote_spanned!(span=> get_many::<::std::ffi::OsString>),
quote!(|s| ::std::ops::Deref::deref(s)),
quote_spanned!(func.span()=> |s| ::std::result::Result::Ok::<_, clap::Error>(#func(s))),
),
TryFromOsStr => (
quote_spanned!(span=> get_one::<::std::ffi::OsString>),
quote_spanned!(span=> get_many::<::std::ffi::OsString>),
quote!(|s| ::std::ops::Deref::deref(s)),
quote_spanned!(func.span()=> |s| #func(s).map_err(|err| clap::Error::raw(clap::ErrorKind::ValueValidation, format!("Invalid value for {}: {}", #id, err)))),
),
FromOccurrences => (
quote_spanned!(span=> occurrences_of),
quote!(),
func.clone(),
),
FromFlag => (quote!(), quote!(), func.clone()),
};
if attrs.is_enum() {
if attrs.is_enum() && !attrs.custom_value_parser() {
let ci = attrs.ignore_case();

parse = quote_spanned! { convert_type.span()=>
Expand Down Expand Up @@ -597,7 +602,7 @@ fn gen_parsers(
quote_spanned! { ty.span()=>
#arg_matches.#get_one(#id)
.expect("unexpected type")
.map(|s| ::std::ops::Deref::deref(s))
.map(#deref)
.map(#parse)
.transpose()?
}
Expand All @@ -608,7 +613,7 @@ fn gen_parsers(
Some(
#arg_matches.#get_one(#id)
.expect("unexpected type")
.map(|s| ::std::ops::Deref::deref(s))
.map(#deref)
.map(#parse).transpose()?
)
} else {
Expand All @@ -620,7 +625,7 @@ fn gen_parsers(
if #arg_matches.is_present(#id) {
Some(#arg_matches.#get_many(#id)
.expect("unexpected type")
.map(|v| v.map(|s| ::std::ops::Deref::deref(s)).map::<::std::result::Result<#convert_type, clap::Error>, _>(#parse).collect::<::std::result::Result<Vec<_>, clap::Error>>())
.map(|v| v.map(#deref).map::<::std::result::Result<#convert_type, clap::Error>, _>(#parse).collect::<::std::result::Result<Vec<_>, clap::Error>>())
.transpose()?
.unwrap_or_else(Vec::new))
} else {
Expand All @@ -632,7 +637,7 @@ fn gen_parsers(
quote_spanned! { ty.span()=>
#arg_matches.#get_many(#id)
.expect("unexpected type")
.map(|v| v.map(|s| ::std::ops::Deref::deref(s)).map::<::std::result::Result<#convert_type, clap::Error>, _>(#parse).collect::<::std::result::Result<Vec<_>, clap::Error>>())
.map(|v| v.map(#deref).map::<::std::result::Result<#convert_type, clap::Error>, _>(#parse).collect::<::std::result::Result<Vec<_>, clap::Error>>())
.transpose()?
.unwrap_or_else(Vec::new)
}
Expand All @@ -654,7 +659,7 @@ fn gen_parsers(
quote_spanned! { ty.span()=>
#arg_matches.#get_one(#id)
.expect("unexpected type")
.map(|s| ::std::ops::Deref::deref(s))
.map(#deref)
.ok_or_else(|| clap::Error::raw(clap::ErrorKind::MissingRequiredArgument, format!("The following required argument was not provided: {}", #id)))
.and_then(#parse)?
}
Expand Down
13 changes: 13 additions & 0 deletions tests/derive/options.rs
View file
Expand Up @@ -471,3 +471,16 @@ fn two_option_vec_types() {
Opt::try_parse_from(&["test"]).unwrap()
);
}

#[test]
fn explicit_value_parser() {
#[derive(Parser, PartialEq, Debug)]
struct Opt {
#[clap(long, value_parser = clap::value_parser!(i32))]
arg: i32,
}
assert_eq!(
Opt { arg: 42 },
Opt::try_parse_from(&["test", "--arg", "42"]).unwrap()
);
}
21 changes: 21 additions & 0 deletions tests/derive_ui/parse_with_value_parser.rs
View file
@@ -0,0 +1,21 @@
// Copyright 2018 Guillaume Pinot (@TeXitoi) <texitoi@texitoi.eu>
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

use clap::Parser;

#[derive(Parser, Debug)]
#[clap(name = "basic")]
struct Opt {
#[clap(parse(from_str), value_parser = clap::value_parser!(String))]
s: String,
}

fn main() {
let opt = Opt::parse();
println!("{:?}", opt);
}
5 changes: 5 additions & 0 deletions tests/derive_ui/parse_with_value_parser.stderr
View file
@@ -0,0 +1,5 @@
error: `value_parse` attribute conflicts with `parse` attribute
--> tests/derive_ui/parse_with_value_parser.rs:14:29
|
14 | #[clap(parse(from_str), value_parser = clap::value_parser!(String))]
| ^^^^^^^^^^^^

0 comments on commit b52c7f1

Please sign in to comment.