From c567f0c0a0f8d526b1f125889b7e54e6f0de0e22 Mon Sep 17 00:00:00 2001 From: Roland Fredenhagen Date: Mon, 20 Jan 2025 02:20:32 +0100 Subject: [PATCH] feat: ValueEnum fallback --- Cargo.lock | 4 +- Cargo.toml | 1 + clap_derive/Cargo.toml | 2 +- clap_derive/src/attr.rs | 2 + clap_derive/src/derives/value_enum.rs | 64 ++++++++ clap_derive/src/item.rs | 13 ++ tests/derive/value_enum.rs | 149 ++++++++++++++++++ .../value_enum_fallback_two_fields.rs | 10 ++ .../value_enum_fallback_two_fields.stderr | 6 + tests/derive_ui/value_enum_two_fallback.rs | 11 ++ .../derive_ui/value_enum_two_fallback.stderr | 13 ++ .../derive_ui/value_parser_unsupported.stderr | 12 +- 12 files changed, 282 insertions(+), 5 deletions(-) create mode 100644 tests/derive_ui/value_enum_fallback_two_fields.rs create mode 100644 tests/derive_ui/value_enum_fallback_two_fields.stderr create mode 100644 tests/derive_ui/value_enum_two_fallback.rs create mode 100644 tests/derive_ui/value_enum_two_fallback.stderr diff --git a/Cargo.lock b/Cargo.lock index 21239f2de6c..4ff61f0d070 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3070,9 +3070,9 @@ dependencies = [ [[package]] name = "syn" -version = "2.0.70" +version = "2.0.87" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2f0209b68b3613b093e0ec905354eccaedcfe83b8cb37cbdeae64026c3064c16" +checksum = "25aa4ce346d03a6dcd68dd8b4010bcb74e54e62c90c573f394c46eae99aba32d" dependencies = [ "proc-macro2", "quote", diff --git a/Cargo.toml b/Cargo.toml index 59c8bed0f28..9d53d0b792b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -144,6 +144,7 @@ default = [ "usage", "error-context", "suggestions", +"unstable-derive-ui-tests" ] debug = ["clap_builder/debug", "clap_derive?/debug"] # Enables debug messages unstable-doc = ["clap_builder/unstable-doc", "derive"] # for docs.rs diff --git a/clap_derive/Cargo.toml b/clap_derive/Cargo.toml index 3de77477056..780f4b6986a 100644 --- a/clap_derive/Cargo.toml +++ b/clap_derive/Cargo.toml @@ -29,7 +29,7 @@ proc-macro = true bench = false [dependencies] -syn = { version = "2.0.8", features = ["full"] } +syn = { version = "2.0.73", features = ["full"] } quote = "1.0.9" proc-macro2 = "1.0.69" heck = "0.5.0" diff --git a/clap_derive/src/attr.rs b/clap_derive/src/attr.rs index 51736c4d1f6..0564925a961 100644 --- a/clap_derive/src/attr.rs +++ b/clap_derive/src/attr.rs @@ -102,6 +102,7 @@ impl Parse for ClapAttr { "long_help" => Some(MagicAttrName::LongHelp), "author" => Some(MagicAttrName::Author), "version" => Some(MagicAttrName::Version), + "fallback" => Some(MagicAttrName::Fallback), _ => None, }; @@ -168,6 +169,7 @@ pub(crate) enum MagicAttrName { DefaultValuesOsT, NextDisplayOrder, NextHelpHeading, + Fallback, } #[derive(Clone)] diff --git a/clap_derive/src/derives/value_enum.rs b/clap_derive/src/derives/value_enum.rs index a1ffa90ca77..9435363d8a6 100644 --- a/clap_derive/src/derives/value_enum.rs +++ b/clap_derive/src/derives/value_enum.rs @@ -49,6 +49,7 @@ pub(crate) fn gen_for_enum( let lits = lits(variants)?; let value_variants = gen_value_variants(&lits); let to_possible_value = gen_to_possible_value(item, &lits); + let from_str_for_fallback = gen_from_str_for_fallback(variants)?; Ok(quote! { #[allow( @@ -75,6 +76,7 @@ pub(crate) fn gen_for_enum( impl clap::ValueEnum for #item_name { #value_variants #to_possible_value + #from_str_for_fallback } }) } @@ -85,6 +87,9 @@ fn lits(variants: &[(&Variant, Item)]) -> Result, syn: if let Kind::Skip(_, _) = &*item.kind() { continue; } + if item.is_fallback() { + continue; + } if !matches!(variant.fields, Fields::Unit) { abort!(variant.span(), "`#[derive(ValueEnum)]` only supports unit variants. Non-unit variants must be skipped"); } @@ -128,3 +133,62 @@ fn gen_to_possible_value(item: &Item, lits: &[(TokenStream, Ident)]) -> TokenStr } } } + +fn gen_from_str_for_fallback(variants: &[(&Variant, Item)]) -> syn::Result { + let fallbacks: Vec<_> = variants + .iter() + .filter(|(_, item)| item.is_fallback()) + .collect(); + + match fallbacks.as_slice() { + [] => Ok(quote!()), + [(variant, _)] => { + let ident = &variant.ident; + let variant_initialization = match variant.fields.len() { + _ if matches!(variant.fields, Fields::Unit) => quote! {#ident}, + 0 => quote! {#ident{}}, + 1 => { + let member = variant + .fields + .members() + .next() + .expect("there should be exactly one field"); + quote! {#ident{ + #member: { + use std::convert::Into; + __input.into() + }, + }} + } + _ => abort!( + variant, + "`fallback` only supports Unit variants, or variants with a single field" + ), + }; + Ok(quote! { + fn from_str(__input: &::std::primitive::str, __ignore_case: ::std::primitive::bool) -> ::std::result::Result { + Ok(Self::value_variants() + .iter() + .find(|v| { + v.to_possible_value() + .expect("ValueEnum::value_variants contains only values with a corresponding ValueEnum::to_possible_value") + .matches(__input, __ignore_case) + }) + .cloned() + .unwrap_or_else(|| Self::#variant_initialization)) + } + }) + } + [first, second, ..] => { + let mut error = syn::Error::new_spanned( + first.0, + "`#[derive(ValueEnum)]` only supports one `fallback`.", + ); + error.combine(syn::Error::new_spanned( + second.0, + "second fallback defined here", + )); + Err(error) + } + } +} diff --git a/clap_derive/src/item.rs b/clap_derive/src/item.rs index ab32918e5f4..ddc7c981e34 100644 --- a/clap_derive/src/item.rs +++ b/clap_derive/src/item.rs @@ -49,6 +49,8 @@ pub(crate) struct Item { skip_group: bool, group_id: Name, group_methods: Vec, + /// Used as fallback value `ValueEnum`. + is_fallback: bool, kind: Sp, } @@ -279,6 +281,7 @@ impl Item { group_id, group_methods: vec![], kind, + is_fallback: false, } } @@ -835,6 +838,12 @@ impl Item { self.skip_group = true; } + Some(MagicAttrName::Fallback) => { + assert_attr_kind(attr, &[AttrKind::Value])?; + + self.is_fallback = true; + } + None // Magic only for the default, otherwise just forward to the builder | Some(MagicAttrName::Short) @@ -1077,6 +1086,10 @@ impl Item { pub(crate) fn skip_group(&self) -> bool { self.skip_group } + + pub(crate) fn is_fallback(&self) -> bool { + self.is_fallback + } } #[derive(Clone)] diff --git a/tests/derive/value_enum.rs b/tests/derive/value_enum.rs index e0ed09bc161..711ef5e400d 100644 --- a/tests/derive/value_enum.rs +++ b/tests/derive/value_enum.rs @@ -626,3 +626,152 @@ fn vec_type_default_value() { Opt::try_parse_from(["", "-a", "foo,baz"]).unwrap() ); } + +#[test] +fn unit_fallback() { + #[derive(clap::ValueEnum, PartialEq, Debug, Clone)] + enum ArgChoice { + Foo, + #[clap(fallback)] + Fallback, + } + + #[derive(Parser, PartialEq, Debug)] + struct Opt { + #[arg(value_enum)] + arg: ArgChoice, + } + + assert_eq!( + Opt { + arg: ArgChoice::Foo + }, + Opt::try_parse_from(["", "foo"]).unwrap() + ); + assert_eq!( + Opt { + arg: ArgChoice::Fallback + }, + Opt::try_parse_from(["", "not-foo"]).unwrap() + ); +} + +#[test] +fn empty_tuple_fallback() { + #[derive(clap::ValueEnum, PartialEq, Debug, Clone)] + enum ArgChoice { + Foo, + #[clap(fallback)] + Fallback(), + } + + #[derive(Parser, PartialEq, Debug)] + struct Opt { + #[arg(value_enum)] + arg: ArgChoice, + } + + assert_eq!( + Opt { + arg: ArgChoice::Foo + }, + Opt::try_parse_from(["", "foo"]).unwrap() + ); + assert_eq!( + Opt { + arg: ArgChoice::Fallback() + }, + Opt::try_parse_from(["", "not-foo"]).unwrap() + ); +} + +#[test] +fn empty_struct_fallback() { + #[derive(clap::ValueEnum, PartialEq, Debug, Clone)] + enum ArgChoice { + Foo, + #[clap(fallback)] + Fallback {}, + } + + #[derive(Parser, PartialEq, Debug)] + struct Opt { + #[arg(value_enum)] + arg: ArgChoice, + } + + assert_eq!( + Opt { + arg: ArgChoice::Foo + }, + Opt::try_parse_from(["", "foo"]).unwrap() + ); + assert_eq!( + Opt { + arg: ArgChoice::Fallback {} + }, + Opt::try_parse_from(["", "not-foo"]).unwrap() + ); +} + +#[test] +fn non_empty_struct_fallback() { + #[derive(clap::ValueEnum, PartialEq, Debug, Clone)] + enum ArgChoice { + Foo, + #[clap(fallback)] + Fallback { + value: String, + }, + } + + #[derive(Parser, PartialEq, Debug)] + struct Opt { + #[arg(value_enum)] + arg: ArgChoice, + } + + assert_eq!( + Opt { + arg: ArgChoice::Foo + }, + Opt::try_parse_from(["", "foo"]).unwrap() + ); + assert_eq!( + Opt { + arg: ArgChoice::Fallback { + value: String::from("not-foo") + } + }, + Opt::try_parse_from(["", "not-foo"]).unwrap() + ); +} + +#[test] +fn non_empty_tuple_fallback() { + #[derive(clap::ValueEnum, PartialEq, Debug, Clone)] + enum ArgChoice { + Foo, + #[clap(fallback)] + Fallback(String), + } + + #[derive(Parser, PartialEq, Debug)] + struct Opt { + #[arg(value_enum)] + arg: ArgChoice, + } + + assert_eq!( + Opt { + arg: ArgChoice::Foo + }, + Opt::try_parse_from(["", "foo"]).unwrap() + ); + assert_eq!( + Opt { + arg: ArgChoice::Fallback(String::from("not-foo")) + }, + Opt::try_parse_from(["", "not-foo"]).unwrap() + ); +} diff --git a/tests/derive_ui/value_enum_fallback_two_fields.rs b/tests/derive_ui/value_enum_fallback_two_fields.rs new file mode 100644 index 00000000000..81d2fca5728 --- /dev/null +++ b/tests/derive_ui/value_enum_fallback_two_fields.rs @@ -0,0 +1,10 @@ +use clap::ValueEnum; + +#[derive(ValueEnum, Clone, Debug)] +enum Opt { + #[clap(fallback)] + First(String, String), +} + +fn main() {} + diff --git a/tests/derive_ui/value_enum_fallback_two_fields.stderr b/tests/derive_ui/value_enum_fallback_two_fields.stderr new file mode 100644 index 00000000000..842da69be8c --- /dev/null +++ b/tests/derive_ui/value_enum_fallback_two_fields.stderr @@ -0,0 +1,6 @@ +error: `fallback` only supports Unit variants, or variants with a single field + --> tests/derive_ui/value_enum_fallback_two_fields.rs:5:5 + | +5 | / #[clap(fallback)] +6 | | First(String, String), + | |_________________________^ diff --git a/tests/derive_ui/value_enum_two_fallback.rs b/tests/derive_ui/value_enum_two_fallback.rs new file mode 100644 index 00000000000..23ff46b59d6 --- /dev/null +++ b/tests/derive_ui/value_enum_two_fallback.rs @@ -0,0 +1,11 @@ +use clap::ValueEnum; + +#[derive(ValueEnum, Clone, Debug)] +enum Opt { + #[clap(fallback)] + First(String), + #[clap(fallback)] + Second(String), +} + +fn main() {} diff --git a/tests/derive_ui/value_enum_two_fallback.stderr b/tests/derive_ui/value_enum_two_fallback.stderr new file mode 100644 index 00000000000..9295e101915 --- /dev/null +++ b/tests/derive_ui/value_enum_two_fallback.stderr @@ -0,0 +1,13 @@ +error: `#[derive(ValueEnum)]` only supports one `fallback`. + --> tests/derive_ui/value_enum_two_fallback.rs:5:5 + | +5 | / #[clap(fallback)] +6 | | First(String), + | |_________________^ + +error: second fallback defined here + --> tests/derive_ui/value_enum_two_fallback.rs:7:5 + | +7 | / #[clap(fallback)] +8 | | Second(String), + | |__________________^ diff --git a/tests/derive_ui/value_parser_unsupported.stderr b/tests/derive_ui/value_parser_unsupported.stderr index b414dec6aef..c65da0807ed 100644 --- a/tests/derive_ui/value_parser_unsupported.stderr +++ b/tests/derive_ui/value_parser_unsupported.stderr @@ -37,6 +37,14 @@ note: the traits `From`, `FromStr`, `ValueEnum`, and `ValueParserFactory` must | | pub trait ValueEnum: Sized + Clone { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - --> $RUST/core/src/convert/mod.rs - --> $RUST/core/src/str/traits.rs + | + ::: $RUST/core/src/convert/mod.rs + | + | pub trait From: Sized { + | ^^^^^^^^^^^^^^^^^^^^^^^^ + | + ::: $RUST/core/src/str/traits.rs + | + | pub trait FromStr: Sized { + | ^^^^^^^^^^^^^^^^^^^^^^^^ = note: this error originates in the macro `clap::value_parser` (in Nightly builds, run with -Z macro-backtrace for more info)