Skip to content

Commit

Permalink
refactor(derive): Merge handling of bool/from_flag
Browse files Browse the repository at this point in the history
This will make it easier to divide off parser logic for adding in
actions.

This does mean we can't provide error reporting on bad values with
`bool` but
- We should have also been doing that for `from_flag`
- We'll be dropping this soon in clap4 anyways
  • Loading branch information
epage committed Jun 2, 2022
1 parent dfc55cd commit 773ba94
Show file tree
Hide file tree
Showing 11 changed files with 71 additions and 119 deletions.
20 changes: 0 additions & 20 deletions clap_derive/src/attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -340,26 +340,6 @@ impl Attrs {
}

match *ty {
Ty::Bool => {
if res.is_positional() && res.parser.is_none() {
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
32 changes: 12 additions & 20 deletions clap_derive/src/derives/args.rs
Original file line number Diff line number Diff line change
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 @@ -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 @@ -586,18 +584,6 @@ fn gen_parsers(
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
5 changes: 1 addition & 4 deletions clap_derive/src/utils/ty.rs
Original file line number Diff line number Diff line change
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 Down
38 changes: 38 additions & 0 deletions tests/derive/flags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
);
}
23 changes: 20 additions & 3 deletions tests/derive_ui/bool_arg_enum.stderr
Original file line number Diff line number Diff line change
@@ -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,
| ~~~~
21 changes: 0 additions & 21 deletions tests/derive_ui/bool_default_value.rs

This file was deleted.

5 changes: 0 additions & 5 deletions tests/derive_ui/bool_default_value.stderr

This file was deleted.

21 changes: 0 additions & 21 deletions tests/derive_ui/bool_required.rs

This file was deleted.

5 changes: 0 additions & 5 deletions tests/derive_ui/bool_required.stderr

This file was deleted.

10 changes: 0 additions & 10 deletions tests/derive_ui/positional_bool.rs

This file was deleted.

10 changes: 0 additions & 10 deletions tests/derive_ui/positional_bool.stderr

This file was deleted.

0 comments on commit 773ba94

Please sign in to comment.