Skip to content

Commit

Permalink
Merge pull request #3782 from epage/parser
Browse files Browse the repository at this point in the history 
refactor(derive): Merge handling of bool/from_flag
  • Loading branch information
@epage
epage committed Jun 2, 2022
2 parents 58cf0ee + 773ba94 commit 77a0e66
Show file tree
Hide file tree
Showing 11 changed files with 125 additions and 169 deletions.
101 changes: 42 additions & 59 deletions clap_derive/src/attrs.rs
View file
Expand Up @@ -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;
Expand Down Expand Up @@ -43,13 +43,12 @@ pub struct Attrs {
doc_comment: Vec<Method>,
methods: Vec<Method>,
value_parser: Option<ValueParser>,
parser: Sp<Parser>,
parser: Option<Sp<Parser>>,
verbatim_doc_comment: Option<Ident>,
next_display_order: Option<Method>,
next_help_heading: Option<Method>,
help_heading: Option<Method>,
is_enum: bool,
has_custom_parser: bool,
kind: Sp<Kind>,
}

Expand All @@ -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"),
Expand Down Expand Up @@ -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"
);
}
Expand All @@ -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"
);
}
Expand Down Expand Up @@ -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"),
Expand Down Expand Up @@ -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"
);
}
Expand Down Expand Up @@ -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"
);
}
Expand Down Expand Up @@ -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"
);
}
Expand All @@ -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")
Expand Down Expand Up @@ -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),
}
}
Expand Down Expand Up @@ -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));
}
}
}
Expand Down Expand Up @@ -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<Parser> {
&self.parser
pub fn parser(&self, field_type: &Type) -> Sp<Parser> {
self.parser
.clone()
.unwrap_or_else(|| Parser::from_type(field_type, self.kind.span()))
}

pub fn kind(&self) -> Sp<Kind> {
Expand Down Expand Up @@ -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) => {
Expand All @@ -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(),
Expand Down Expand Up @@ -913,10 +890,16 @@ pub struct Parser {
}

impl Parser {
fn default_spanned(span: Span) -> Sp<Self> {
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<Self> {
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<Self> {
Expand Down
50 changes: 21 additions & 29 deletions clap_derive/src/derives/args.rs
View file
Expand Up @@ -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};
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -275,8 +275,6 @@ pub fn gen_augment(
};

let modifier = match **ty {
Ty::Bool => quote!(),

Ty::Option => {
quote_spanned! { ty.span()=>
.takes_value(true)
Expand Down Expand Up @@ -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 => (
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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()=>
Expand Down
10 changes: 4 additions & 6 deletions clap_derive/src/utils/ty.rs
View file
Expand Up @@ -9,7 +9,6 @@ use syn::{

#[derive(Copy, Clone, PartialEq, Debug)]
pub enum Ty {
Bool,
Vec,
Option,
OptionOption,
Expand All @@ -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") {
Expand All @@ -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)
Expand Down

0 comments on commit 77a0e66

Please sign in to comment.