From ee3d12ec56656a764d00d8126326aca637036c7f Mon Sep 17 00:00:00 2001 From: shir0kamii Date: Wed, 30 Mar 2022 02:48:34 +0200 Subject: [PATCH 1/2] fix(derive): Abort on non-unit variant --- clap_derive/src/derives/arg_enum.rs | 2 +- examples/derive_ref/README.md | 1 + tests/derive_ui/arg_enum_non_unit.rs | 10 ++++++++++ tests/derive_ui/arg_enum_non_unit.stderr | 7 +++++++ 4 files changed, 19 insertions(+), 1 deletion(-) create mode 100644 tests/derive_ui/arg_enum_non_unit.rs create mode 100644 tests/derive_ui/arg_enum_non_unit.stderr diff --git a/clap_derive/src/derives/arg_enum.rs b/clap_derive/src/derives/arg_enum.rs index 8b020cb4..48af76c9 100644 --- a/clap_derive/src/derives/arg_enum.rs +++ b/clap_derive/src/derives/arg_enum.rs @@ -35,7 +35,7 @@ pub fn derive_arg_enum(input: &DeriveInput) -> TokenStream { pub fn gen_for_enum(name: &Ident, attrs: &[Attribute], e: &DataEnum) -> TokenStream { if !e.variants.iter().all(|v| matches!(v.fields, Fields::Unit)) { - return quote!(); + abort_call_site!("`#[derive(ArgEnum)]` only supports non-unit variants"); }; let attrs = Attrs::from_struct( diff --git a/examples/derive_ref/README.md b/examples/derive_ref/README.md index 2b0955fe..c95c714d 100644 --- a/examples/derive_ref/README.md +++ b/examples/derive_ref/README.md @@ -82,6 +82,7 @@ fn main() { - `Subcommand` defines available subcommands. - Subcommand arguments can be defined in a struct-variant or automatically flattened with a tuple-variant. - `ArgEnum` allows parsing a value directly into an `enum`, erroring on unsupported values. + - The derive doesn't work on enums that contain non-unit variants See also the [tutorial](../tutorial_derive/README.md) and [examples](../README.md). diff --git a/tests/derive_ui/arg_enum_non_unit.rs b/tests/derive_ui/arg_enum_non_unit.rs new file mode 100644 index 00000000..be031005 --- /dev/null +++ b/tests/derive_ui/arg_enum_non_unit.rs @@ -0,0 +1,10 @@ +use clap::ArgEnum; + +#[derive(ArgEnum, Clone, Debug)] +enum Opt { + Foo(usize), +} + +fn main() { + println!("{:?}", Opt::Foo(42)); +} diff --git a/tests/derive_ui/arg_enum_non_unit.stderr b/tests/derive_ui/arg_enum_non_unit.stderr new file mode 100644 index 00000000..7c8a6cbf --- /dev/null +++ b/tests/derive_ui/arg_enum_non_unit.stderr @@ -0,0 +1,7 @@ +error: `#[derive(ArgEnum)]` only supports non-unit variants + --> tests/derive_ui/arg_enum_non_unit.rs:3:10 + | +3 | #[derive(ArgEnum, Clone, Debug)] + | ^^^^^^^ + | + = note: this error originates in the derive macro `ArgEnum` (in Nightly builds, run with -Z macro-backtrace for more info) From fb4755d1c3ef8819fa1927b5f418357bb6a9e266 Mon Sep 17 00:00:00 2001 From: shir0kamii Date: Wed, 30 Mar 2022 03:16:11 +0200 Subject: [PATCH 2/2] feat(derive): Don't abort when non-unit variant is skipped --- clap_derive/src/derives/arg_enum.rs | 13 +++++----- examples/derive_ref/README.md | 2 +- tests/derive/arg_enum.rs | 31 ++++++++++++++++++++++++ tests/derive_ui/arg_enum_non_unit.stderr | 10 +++----- 4 files changed, 42 insertions(+), 14 deletions(-) diff --git a/clap_derive/src/derives/arg_enum.rs b/clap_derive/src/derives/arg_enum.rs index 48af76c9..77067e90 100644 --- a/clap_derive/src/derives/arg_enum.rs +++ b/clap_derive/src/derives/arg_enum.rs @@ -15,11 +15,11 @@ use crate::{ }; use proc_macro2::{Span, TokenStream}; -use proc_macro_error::abort_call_site; +use proc_macro_error::{abort, abort_call_site}; use quote::quote; use syn::{ - punctuated::Punctuated, token::Comma, Attribute, Data, DataEnum, DeriveInput, Fields, Ident, - Variant, + punctuated::Punctuated, spanned::Spanned, token::Comma, Attribute, Data, DataEnum, DeriveInput, + Fields, Ident, Variant, }; pub fn derive_arg_enum(input: &DeriveInput) -> TokenStream { @@ -34,10 +34,6 @@ pub fn derive_arg_enum(input: &DeriveInput) -> TokenStream { } pub fn gen_for_enum(name: &Ident, attrs: &[Attribute], e: &DataEnum) -> TokenStream { - if !e.variants.iter().all(|v| matches!(v.fields, Fields::Unit)) { - abort_call_site!("`#[derive(ArgEnum)]` only supports non-unit variants"); - }; - let attrs = Attrs::from_struct( Span::call_site(), attrs, @@ -86,6 +82,9 @@ fn lits( if let Kind::Skip(_) = &*attrs.kind() { None } else { + if !matches!(variant.fields, Fields::Unit) { + abort!(variant.span(), "`#[derive(ArgEnum)]` only supports non-unit variants, unless they are skipped"); + } let fields = attrs.field_methods(false); let name = attrs.cased_name(); Some(( diff --git a/examples/derive_ref/README.md b/examples/derive_ref/README.md index c95c714d..e9d4ac19 100644 --- a/examples/derive_ref/README.md +++ b/examples/derive_ref/README.md @@ -82,7 +82,7 @@ fn main() { - `Subcommand` defines available subcommands. - Subcommand arguments can be defined in a struct-variant or automatically flattened with a tuple-variant. - `ArgEnum` allows parsing a value directly into an `enum`, erroring on unsupported values. - - The derive doesn't work on enums that contain non-unit variants + - The derive doesn't work on enums that contain non-unit variants, unless they are skipped See also the [tutorial](../tutorial_derive/README.md) and [examples](../README.md). diff --git a/tests/derive/arg_enum.rs b/tests/derive/arg_enum.rs index 957e870d..1cd8ee0d 100644 --- a/tests/derive/arg_enum.rs +++ b/tests/derive/arg_enum.rs @@ -321,6 +321,37 @@ fn skip_variant() { } } +#[test] +fn skip_non_unit_variant() { + #[derive(clap::ArgEnum, PartialEq, Debug, Clone)] + #[allow(dead_code)] // silence warning about `Baz` being unused + enum ArgChoice { + Foo, + Bar, + #[clap(skip)] + Baz(usize), + } + + assert_eq!( + ::value_variants() + .iter() + .map(clap::ArgEnum::to_possible_value) + .map(Option::unwrap) + .collect::>(), + vec![ + clap::PossibleValue::new("foo"), + clap::PossibleValue::new("bar") + ] + ); + + { + use clap::ArgEnum; + assert!(ArgChoice::from_str("foo", true).is_ok()); + assert!(ArgChoice::from_str("bar", true).is_ok()); + assert!(ArgChoice::from_str("baz", true).is_err()); + } +} + #[test] fn from_str_invalid() { #[derive(clap::ArgEnum, PartialEq, Debug, Clone)] diff --git a/tests/derive_ui/arg_enum_non_unit.stderr b/tests/derive_ui/arg_enum_non_unit.stderr index 7c8a6cbf..117cfcec 100644 --- a/tests/derive_ui/arg_enum_non_unit.stderr +++ b/tests/derive_ui/arg_enum_non_unit.stderr @@ -1,7 +1,5 @@ -error: `#[derive(ArgEnum)]` only supports non-unit variants - --> tests/derive_ui/arg_enum_non_unit.rs:3:10 +error: `#[derive(ArgEnum)]` only supports non-unit variants, unless they are skipped + --> tests/derive_ui/arg_enum_non_unit.rs:5:5 | -3 | #[derive(ArgEnum, Clone, Debug)] - | ^^^^^^^ - | - = note: this error originates in the derive macro `ArgEnum` (in Nightly builds, run with -Z macro-backtrace for more info) +5 | Foo(usize), + | ^^^