diff --git a/clap_derive/src/attrs.rs b/clap_derive/src/attrs.rs index 7003681e..a3107001 100644 --- a/clap_derive/src/attrs.rs +++ b/clap_derive/src/attrs.rs @@ -14,7 +14,7 @@ use crate::{ parse::*, - utils::{process_doc_comment, Sp, Ty}, + utils::{inner_type, is_simple_ty, process_doc_comment, Sp, Ty}, }; use std::env; @@ -43,13 +43,12 @@ pub struct Attrs { doc_comment: Vec, methods: Vec, value_parser: Option, - parser: Sp, + parser: Option>, verbatim_doc_comment: Option, next_display_order: Option, next_help_heading: Option, help_heading: Option, is_enum: bool, - has_custom_parser: bool, kind: Sp, } @@ -71,11 +70,8 @@ impl Attrs { "`value_parse` attribute is only allowed on fields" ); } - if res.has_custom_parser { - abort!( - res.parser.span(), - "`parse` attribute is only allowed on fields" - ); + if let Some(parser) = res.parser.as_ref() { + abort!(parser.span(), "`parse` attribute is only allowed on fields"); } match &*res.kind { Kind::Subcommand(_) => abort!(res.kind.span(), "subcommand is only allowed on fields"), @@ -114,9 +110,9 @@ impl Attrs { "`value_parse` attribute is only allowed flattened entry" ); } - if res.has_custom_parser { + if let Some(parser) = res.parser.as_ref() { abort!( - res.parser.span(), + parser.span(), "parse attribute is not allowed for flattened entry" ); } @@ -140,9 +136,9 @@ impl Attrs { "`value_parse` attribute is only allowed for subcommand" ); } - if res.has_custom_parser { + if let Some(parser) = res.parser.as_ref() { abort!( - res.parser.span(), + parser.span(), "parse attribute is not allowed for subcommand" ); } @@ -214,11 +210,8 @@ impl Attrs { "`value_parse` attribute is only allowed on fields" ); } - if res.has_custom_parser { - abort!( - res.parser.span(), - "`parse` attribute is only allowed on fields" - ); + if let Some(parser) = res.parser.as_ref() { + abort!(parser.span(), "`parse` attribute is only allowed on fields"); } match &*res.kind { Kind::Subcommand(_) => abort!(res.kind.span(), "subcommand is only allowed on fields"), @@ -257,9 +250,9 @@ impl Attrs { "`value_parse` attribute is not allowed for flattened entry" ); } - if res.has_custom_parser { + if let Some(parser) = res.parser.as_ref() { abort!( - res.parser.span(), + parser.span(), "parse attribute is not allowed for flattened entry" ); } @@ -287,9 +280,9 @@ impl Attrs { "`value_parse` attribute is not allowed for subcommand" ); } - if res.has_custom_parser { + if let Some(parser) = res.parser.as_ref() { abort!( - res.parser.span(), + parser.span(), "parse attribute is not allowed for subcommand" ); } @@ -333,10 +326,10 @@ 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() { + if res.parser.is_some() { + if let Some(value_parser) = res.value_parser.as_ref() { abort!( - parser.span(), + value_parser.span(), "`value_parse` attribute conflicts with `parse` attribute" ); } @@ -347,26 +340,6 @@ impl Attrs { } match *ty { - Ty::Bool => { - if res.is_positional() && !res.has_custom_parser { - abort!(field.ty, - "`bool` cannot be used as positional parameter with default parser"; - help = "if you want to create a flag add `long` or `short`"; - help = "If you really want a boolean parameter \ - add an explicit parser, for example `parse(try_from_str)`"; - note = "see also https://github.com/clap-rs/clap/blob/master/examples/derive_ref/custom-bool.md"; - ) - } - if res.is_enum { - abort!(field.ty, "`arg_enum` is meaningless for bool") - } - if let Some(m) = res.find_default_method() { - abort!(m.name, "default_value is meaningless for bool") - } - if let Some(m) = res.find_method("required") { - abort!(m.name, "required is meaningless for bool") - } - } Ty::Option => { if let Some(m) = res.find_default_method() { abort!(m.name, "default_value is meaningless for Option") @@ -413,13 +386,12 @@ impl Attrs { doc_comment: vec![], methods: vec![], value_parser: None, - parser: Parser::default_spanned(default_span), + parser: None, verbatim_doc_comment: None, next_display_order: None, next_help_heading: None, help_heading: None, is_enum: false, - has_custom_parser: false, kind: Sp::new(Kind::Arg(Sp::new(Ty::Other, default_span)), default_span), } } @@ -623,8 +595,7 @@ impl Attrs { } Parse(ident, spec) => { - self.has_custom_parser = true; - self.parser = Parser::from_spec(ident, spec); + self.parser = Some(Parser::from_spec(ident, spec)); } } } @@ -740,18 +711,24 @@ impl Attrs { self.name.clone().translate(CasingStyle::ScreamingSnake) } - pub fn value_parser(&self) -> ValueParser { + pub fn value_parser(&self, field_type: &Type) -> Method { self.value_parser .clone() - .unwrap_or_else(|| ValueParser::Explicit(self.parser.value_parser())) + .map(|p| { + let inner_type = inner_type(field_type); + p.resolve(inner_type) + }) + .unwrap_or_else(|| self.parser(field_type).value_parser()) } pub fn custom_value_parser(&self) -> bool { self.value_parser.is_some() } - pub fn parser(&self) -> &Sp { - &self.parser + pub fn parser(&self, field_type: &Type) -> Sp { + self.parser + .clone() + .unwrap_or_else(|| Parser::from_type(field_type, self.kind.span())) } pub fn kind(&self) -> Sp { @@ -794,13 +771,13 @@ impl Attrs { } #[derive(Clone)] -pub enum ValueParser { +enum ValueParser { Explicit(Method), Implicit(Ident), } impl ValueParser { - pub fn resolve(self, inner_type: &Type) -> Method { + fn resolve(self, inner_type: &Type) -> Method { match self { Self::Explicit(method) => method, Self::Implicit(ident) => { @@ -815,7 +792,7 @@ impl ValueParser { } } - pub fn span(&self) -> Span { + fn span(&self) -> Span { match self { Self::Explicit(method) => method.name.span(), Self::Implicit(ident) => ident.span(), @@ -913,10 +890,16 @@ pub struct Parser { } impl Parser { - fn default_spanned(span: Span) -> Sp { - let kind = Sp::new(ParserKind::TryFromStr, span); - let func = quote_spanned!(span=> ::std::str::FromStr::from_str); - Sp::new(Parser { kind, func }, span) + fn from_type(field_type: &Type, span: Span) -> Sp { + if is_simple_ty(field_type, "bool") { + let kind = Sp::new(ParserKind::FromFlag, span); + let func = quote_spanned!(span=> ::std::convert::From::from); + Sp::new(Parser { kind, func }, span) + } else { + let kind = Sp::new(ParserKind::TryFromStr, span); + let func = quote_spanned!(span=> ::std::str::FromStr::from_str); + Sp::new(Parser { kind, func }, span) + } } fn from_spec(parse_ident: Ident, spec: ParserSpec) -> Sp { diff --git a/clap_derive/src/derives/args.rs b/clap_derive/src/derives/args.rs index c801eeb9..afd90c60 100644 --- a/clap_derive/src/derives/args.rs +++ b/clap_derive/src/derives/args.rs @@ -15,7 +15,7 @@ use crate::{ attrs::{Attrs, Kind, Name, ParserKind, DEFAULT_CASING, DEFAULT_ENV_CASING}, dummies, - utils::{inner_type, sub_type, Sp, Ty}, + utils::{inner_type, is_simple_ty, sub_type, Sp, Ty}, }; use proc_macro2::{Ident, Span, TokenStream}; @@ -241,13 +241,13 @@ pub fn gen_augment( } } Kind::Arg(ty) => { - let convert_type = inner_type(**ty, &field.ty); + let convert_type = inner_type(&field.ty); - let occurrences = *attrs.parser().kind == ParserKind::FromOccurrences; - let flag = *attrs.parser().kind == ParserKind::FromFlag; + let parser = attrs.parser(&field.ty); + let occurrences = *parser.kind == ParserKind::FromOccurrences; + let flag = *parser.kind == ParserKind::FromFlag; - let value_parser = attrs.value_parser().resolve(convert_type); - let parser = attrs.parser(); + let value_parser = attrs.value_parser(&field.ty); let func = &parser.func; let validator = match *parser.kind { @@ -275,8 +275,6 @@ pub fn gen_augment( }; let modifier = match **ty { - Ty::Bool => quote!(), - Ty::Option => { quote_spanned! { ty.span()=> .takes_value(true) @@ -521,10 +519,10 @@ fn gen_parsers( ) -> TokenStream { use self::ParserKind::*; - let parser = attrs.parser(); + let parser = attrs.parser(&field.ty); let func = &parser.func; let span = parser.kind.span(); - let convert_type = inner_type(**ty, &field.ty); + let convert_type = inner_type(&field.ty); let id = attrs.id(); let (get_one, get_many, deref, mut parse) = match *parser.kind { FromOccurrences => ( @@ -578,26 +576,14 @@ fn gen_parsers( } } - let flag = *attrs.parser().kind == ParserKind::FromFlag; - let occurrences = *attrs.parser().kind == ParserKind::FromOccurrences; + let flag = *parser.kind == ParserKind::FromFlag; + let occurrences = *parser.kind == ParserKind::FromOccurrences; // 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 let arg_matches = format_ident!("__clap_arg_matches"); let field_value = match **ty { - Ty::Bool => { - if update.is_some() { - quote_spanned! { ty.span()=> - *#field_name || #arg_matches.is_present(#id) - } - } else { - quote_spanned! { ty.span()=> - #arg_matches.is_present(#id) - } - } - } - Ty::Option => { quote_spanned! { ty.span()=> #arg_matches.#get_one(#id) @@ -645,11 +631,17 @@ fn gen_parsers( ) }, - Ty::Other if flag => quote_spanned! { ty.span()=> - #parse( - #arg_matches.is_present(#id) - ) - }, + Ty::Other if flag => { + if update.is_some() && is_simple_ty(&field.ty, "bool") { + quote_spanned! { ty.span()=> + *#field_name || #arg_matches.is_present(#id) + } + } else { + quote_spanned! { ty.span()=> + #parse(#arg_matches.is_present(#id)) + } + } + } Ty::Other => { quote_spanned! { ty.span()=> diff --git a/clap_derive/src/utils/ty.rs b/clap_derive/src/utils/ty.rs index 891aa37d..0bcb59f2 100644 --- a/clap_derive/src/utils/ty.rs +++ b/clap_derive/src/utils/ty.rs @@ -9,7 +9,6 @@ use syn::{ #[derive(Copy, Clone, PartialEq, Debug)] pub enum Ty { - Bool, Vec, Option, OptionOption, @@ -22,9 +21,7 @@ impl Ty { use self::Ty::*; let t = |kind| Sp::new(kind, ty.span()); - if is_simple_ty(ty, "bool") { - t(Bool) - } else if is_generic_ty(ty, "Vec") { + if is_generic_ty(ty, "Vec") { t(Vec) } else if let Some(subty) = subty_if_name(ty, "Option") { if is_generic_ty(subty, "Option") { @@ -40,8 +37,9 @@ impl Ty { } } -pub fn inner_type(ty: Ty, field_ty: &syn::Type) -> &syn::Type { - match ty { +pub fn inner_type(field_ty: &syn::Type) -> &syn::Type { + let ty = Ty::from_syn_ty(field_ty); + 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) diff --git a/tests/derive/flags.rs b/tests/derive/flags.rs index 9c5d9d9b..cbf2ef9f 100644 --- a/tests/derive/flags.rs +++ b/tests/derive/flags.rs @@ -189,3 +189,41 @@ fn ignore_qualified_bool_type() { Opt::try_parse_from(&["test", "success"]).unwrap() ); } + +#[test] +fn override_implicit_from_flag() { + #[derive(Parser, PartialEq, Debug)] + struct Opt { + #[clap(long, parse(try_from_str))] + arg: bool, + } + + assert_eq!( + Opt { arg: false }, + Opt::try_parse_from(&["test", "--arg", "false"]).unwrap() + ); + + assert_eq!( + Opt { arg: true }, + Opt::try_parse_from(&["test", "--arg", "true"]).unwrap() + ); +} + +#[test] +fn override_implicit_from_flag_positional() { + #[derive(Parser, PartialEq, Debug)] + struct Opt { + #[clap(parse(try_from_str))] + arg: bool, + } + + assert_eq!( + Opt { arg: false }, + Opt::try_parse_from(&["test", "false"]).unwrap() + ); + + assert_eq!( + Opt { arg: true }, + Opt::try_parse_from(&["test", "true"]).unwrap() + ); +} diff --git a/tests/derive_ui/bool_arg_enum.stderr b/tests/derive_ui/bool_arg_enum.stderr index feb3ed1c..9ac13d08 100644 --- a/tests/derive_ui/bool_arg_enum.stderr +++ b/tests/derive_ui/bool_arg_enum.stderr @@ -1,5 +1,22 @@ -error: `arg_enum` is meaningless for bool - --> $DIR/bool_arg_enum.rs:7:11 +error[E0277]: the trait bound `bool: ArgEnum` is not satisfied + --> tests/derive_ui/bool_arg_enum.rs:7:11 + | +7 | opts: bool, + | ^^^^ the trait `ArgEnum` is not implemented for `bool` + | +note: required by `clap::ArgEnum::from_str` + --> src/derive.rs + | + | fn from_str(input: &str, ignore_case: bool) -> Result { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error[E0618]: expected function, found enum variant `bool` + --> tests/derive_ui/bool_arg_enum.rs:7:11 | 7 | opts: bool, - | ^^^^ + | ^^^^ call expression requires function + | +help: `bool` is a unit variant, you need to write it without the parenthesis + | +7 | opts: bool, + | ~~~~ diff --git a/tests/derive_ui/bool_default_value.rs b/tests/derive_ui/bool_default_value.rs deleted file mode 100644 index a74073a6..00000000 --- a/tests/derive_ui/bool_default_value.rs +++ /dev/null @@ -1,21 +0,0 @@ -// Copyright 2018 Guillaume Pinot (@TeXitoi) -// -// Licensed under the Apache License, Version 2.0 or the MIT license -// , 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(short, default_value = true)] - b: bool, -} - -fn main() { - let opt = Opt::parse(); - println!("{:?}", opt); -} diff --git a/tests/derive_ui/bool_default_value.stderr b/tests/derive_ui/bool_default_value.stderr deleted file mode 100644 index d989d13b..00000000 --- a/tests/derive_ui/bool_default_value.stderr +++ /dev/null @@ -1,5 +0,0 @@ -error: default_value is meaningless for bool - --> $DIR/bool_default_value.rs:14:19 - | -14 | #[clap(short, default_value = true)] - | ^^^^^^^^^^^^^ diff --git a/tests/derive_ui/bool_required.rs b/tests/derive_ui/bool_required.rs deleted file mode 100644 index 09771035..00000000 --- a/tests/derive_ui/bool_required.rs +++ /dev/null @@ -1,21 +0,0 @@ -// Copyright 2018 Guillaume Pinot (@TeXitoi) -// -// Licensed under the Apache License, Version 2.0 or the MIT license -// , 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(short, required = true)] - b: bool, -} - -fn main() { - let opt = Opt::parse(); - println!("{:?}", opt); -} diff --git a/tests/derive_ui/bool_required.stderr b/tests/derive_ui/bool_required.stderr deleted file mode 100644 index adbbb3c2..00000000 --- a/tests/derive_ui/bool_required.stderr +++ /dev/null @@ -1,5 +0,0 @@ -error: required is meaningless for bool - --> $DIR/bool_required.rs:14:19 - | -14 | #[clap(short, required = true)] - | ^^^^^^^^ diff --git a/tests/derive_ui/positional_bool.rs b/tests/derive_ui/positional_bool.rs deleted file mode 100644 index 06bda9ae..00000000 --- a/tests/derive_ui/positional_bool.rs +++ /dev/null @@ -1,10 +0,0 @@ -use clap::Parser; - -#[derive(Parser, Debug)] -struct Opt { - verbose: bool, -} - -fn main() { - Opt::parse(); -} diff --git a/tests/derive_ui/positional_bool.stderr b/tests/derive_ui/positional_bool.stderr deleted file mode 100644 index 8d54aba1..00000000 --- a/tests/derive_ui/positional_bool.stderr +++ /dev/null @@ -1,10 +0,0 @@ -error: `bool` cannot be used as positional parameter with default parser - - = help: if you want to create a flag add `long` or `short` - = help: If you really want a boolean parameter add an explicit parser, for example `parse(try_from_str)` - = note: see also https://github.com/clap-rs/clap/blob/master/examples/derive_ref/custom-bool.md - - --> $DIR/positional_bool.rs:5:14 - | -5 | verbose: bool, - | ^^^^