Merge pull request #3782 from epage/parser

refactor(derive): Merge handling of bool/from_flag
This commit is contained in:
Ed Page 2022-06-02 14:47:53 -05:00 committed by GitHub
commit 77a0e66f6e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
11 changed files with 125 additions and 169 deletions

View file

@ -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<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>,
}
@ -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<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> {
@ -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<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> {

View file

@ -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()=>

View file

@ -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)

View file

@ -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()
);
}

View file

@ -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<Self, String> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
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,
| ~~~~

View file

@ -1,21 +0,0 @@
// 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(short, default_value = true)]
b: bool,
}
fn main() {
let opt = Opt::parse();
println!("{:?}", opt);
}

View file

@ -1,5 +0,0 @@
error: default_value is meaningless for bool
--> $DIR/bool_default_value.rs:14:19
|
14 | #[clap(short, default_value = true)]
| ^^^^^^^^^^^^^

View file

@ -1,21 +0,0 @@
// 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(short, required = true)]
b: bool,
}
fn main() {
let opt = Opt::parse();
println!("{:?}", opt);
}

View file

@ -1,5 +0,0 @@
error: required is meaningless for bool
--> $DIR/bool_required.rs:14:19
|
14 | #[clap(short, required = true)]
| ^^^^^^^^

View file

@ -1,10 +0,0 @@
use clap::Parser;
#[derive(Parser, Debug)]
struct Opt {
verbose: bool,
}
fn main() {
Opt::parse();
}

View file

@ -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,
| ^^^^