From eaa9273f9198c44bc35015562a86454a12576052 Mon Sep 17 00:00:00 2001 From: Thayne McCombs Date: Fri, 9 Dec 2022 01:35:37 -0700 Subject: [PATCH 1/3] feat: Add support for deriving grouped options for Vec> Relates-To: #2924 --- clap_derive/src/derives/args.rs | 28 +++++++++++++-- clap_derive/src/item.rs | 2 +- clap_derive/src/utils/ty.rs | 26 +++++++++++--- tests/derive/main.rs | 1 + tests/derive/occurrences.rs | 62 +++++++++++++++++++++++++++++++++ 5 files changed, 111 insertions(+), 8 deletions(-) create mode 100644 tests/derive/occurrences.rs diff --git a/clap_derive/src/derives/args.rs b/clap_derive/src/derives/args.rs index ad2a6463..1c3ce8b2 100644 --- a/clap_derive/src/derives/args.rs +++ b/clap_derive/src/derives/args.rs @@ -288,6 +288,14 @@ pub fn gen_augment( } } + Ty::VecVec | Ty::OptionVecVec => { + quote_spanned! { ty.span() => + .value_name(#value_name) + #value_parser + #action + } + } + Ty::Other => { let required = item.find_default_method().is_none() && !override_required; // `ArgAction::takes_values` is assuming `ArgAction::default_value` will be @@ -431,7 +439,9 @@ pub fn gen_constructor(fields: &[(&Field, Item)]) -> TokenStream { Ty::Unit | Ty::Vec | Ty::OptionOption | - Ty::OptionVec => { + Ty::OptionVec | + Ty::VecVec | + Ty::OptionVecVec => { abort!( ty.span(), "{} types are not supported for subcommand", @@ -470,7 +480,9 @@ pub fn gen_constructor(fields: &[(&Field, Item)]) -> TokenStream { Ty::Unit | Ty::Vec | Ty::OptionOption | - Ty::OptionVec => { + Ty::OptionVec | + Ty::VecVec | + Ty::OptionVecVec => { abort!( ty.span(), "{} types are not supported for flatten", @@ -609,6 +621,7 @@ fn gen_parsers( let id = item.id(); let get_one = quote_spanned!(span=> remove_one::<#convert_type>); let get_many = quote_spanned!(span=> remove_many::<#convert_type>); + let get_occurrences = quote_spanned!(span=> remove_occurrences::<#convert_type>); let deref = quote!(|s| s); let parse = quote_spanned!(span=> |s| ::std::result::Result::Ok::<_, clap::Error>(s)); @@ -665,6 +678,17 @@ fn gen_parsers( } } + Ty::VecVec => quote_spanned! { ty.span()=> + #arg_matches.#get_occurrences(#id) + .map(|g| g.map(::std::iter::Iterator::collect).collect::>>()) + .unwrap_or_else(Vec::new) + }, + + Ty::OptionVecVec => quote_spanned! { ty.span()=> + #arg_matches.#get_occurrences(#id) + .map(|g| g.map(::std::iter::Iterator::collect).collect::>>()) + }, + Ty::Other => { quote_spanned! { ty.span()=> #arg_matches.#get_one(#id) diff --git a/clap_derive/src/item.rs b/clap_derive/src/item.rs index a068e360..5e8272ac 100644 --- a/clap_derive/src/item.rs +++ b/clap_derive/src/item.rs @@ -1121,7 +1121,7 @@ impl Action { fn default_action(field_type: &Type, span: Span) -> Method { let ty = Ty::from_syn_ty(field_type); let args = match *ty { - Ty::Vec | Ty::OptionVec => { + Ty::Vec | Ty::OptionVec | Ty::VecVec | Ty::OptionVecVec => { quote_spanned! { span=> clap::ArgAction::Append } diff --git a/clap_derive/src/utils/ty.rs b/clap_derive/src/utils/ty.rs index 1cf05144..32bc2c4f 100644 --- a/clap_derive/src/utils/ty.rs +++ b/clap_derive/src/utils/ty.rs @@ -11,9 +11,11 @@ use syn::{ pub enum Ty { Unit, Vec, + VecVec, Option, OptionOption, OptionVec, + OptionVecVec, Other, } @@ -24,13 +26,21 @@ impl Ty { if is_unit_ty(ty) { t(Unit) - } else if is_generic_ty(ty, "Vec") { - t(Vec) + } else if let Some(subty) = subty_if_name(ty, "Vec") { + if is_generic_ty(subty, "Vec") { + t(VecVec) + } else { + t(Vec) + } } else if let Some(subty) = subty_if_name(ty, "Option") { if is_generic_ty(subty, "Option") { t(OptionOption) - } else if is_generic_ty(subty, "Vec") { - t(OptionVec) + } else if let Some(subty) = subty_if_name(subty, "Vec") { + if is_generic_ty(subty, "Vec") { + t(OptionVecVec) + } else { + t(OptionVec) + } } else { t(Option) } @@ -46,6 +56,8 @@ impl Ty { Self::Option => "Option", Self::OptionOption => "Option>", Self::OptionVec => "Option>", + Self::VecVec => "Vec>", + Self::OptionVecVec => "Option>>", Self::Other => "...other...", } } @@ -55,9 +67,13 @@ 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 => { + Ty::OptionOption | Ty::OptionVec | Ty::VecVec => { sub_type(field_ty).and_then(sub_type).unwrap_or(field_ty) } + Ty::OptionVecVec => sub_type(field_ty) + .and_then(sub_type) + .and_then(sub_type) + .unwrap_or(field_ty), _ => field_ty, } } diff --git a/tests/derive/main.rs b/tests/derive/main.rs index 0c6ea7bd..072aab96 100644 --- a/tests/derive/main.rs +++ b/tests/derive/main.rs @@ -22,6 +22,7 @@ mod macros; mod naming; mod nested_subcommands; mod non_literal_attributes; +mod occurrences; mod options; mod privacy; mod raw_bool_literal; diff --git a/tests/derive/occurrences.rs b/tests/derive/occurrences.rs new file mode 100644 index 00000000..8b2d8f2b --- /dev/null +++ b/tests/derive/occurrences.rs @@ -0,0 +1,62 @@ +#![cfg(feature = "unstable-grouped")] +use clap::Parser; + +#[test] +fn test_vec_of_vec() { + #[derive(Parser, Debug, PartialEq)] + struct Opt { + #[arg(short = 'p', num_args = 2)] + points: Vec>, + } + + assert_eq!( + Opt { + points: vec![vec![1, 2], vec![0, 0]] + }, + Opt::try_parse_from(&["test", "-p", "1", "2", "-p", "0", "0"]).unwrap() + ); +} + +#[test] +fn test_vec_vec_empty() { + #[derive(Parser, Debug, PartialEq)] + struct Opt { + #[arg(short = 'p', num_args = 2)] + points: Vec>, + } + + assert_eq!( + Opt { points: vec![] }, + Opt::try_parse_from(&["test"]).unwrap() + ); +} + +#[test] +fn test_option_vec_vec() { + #[derive(Parser, Debug, PartialEq)] + struct Opt { + #[arg(short = 'p', num_args = 2)] + points: Option>>, + } + + assert_eq!( + Opt { + points: Some(vec![vec![1, 2], vec![3, 4]]) + }, + Opt::try_parse_from(&["test", "-p", "1", "2", "-p", "3", "4"]).unwrap() + ); +} + +#[test] +fn test_option_vec_vec_empty() { + #[derive(Parser, Debug, PartialEq)] + struct Opt { + #[arg(short = 'p', num_args = 2)] + points: Option>>, + } + + assert_eq!( + Opt { points: None }, + Opt::try_parse_from(&["test"]).unwrap() + ); +} From ccce3c3db5ce9f59d52012c0a0e22988de2f9b9a Mon Sep 17 00:00:00 2001 From: Thayne McCombs Date: Wed, 4 Jan 2023 00:51:51 -0700 Subject: [PATCH 2/3] test: Add test for opting out of Vec> derive --- tests/derive/occurrences.rs | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/tests/derive/occurrences.rs b/tests/derive/occurrences.rs index 8b2d8f2b..c8cbb9e3 100644 --- a/tests/derive/occurrences.rs +++ b/tests/derive/occurrences.rs @@ -17,6 +17,28 @@ fn test_vec_of_vec() { ); } +#[test] +fn test_vec_of_vec_opt_out() { + fn parser(s: &str) -> Result>, std::convert::Infallible> { + Ok(s.split(':') + .map(|v| v.split(',').map(str::to_owned).collect()) + .collect()) + } + + #[derive(Parser, PartialEq, Debug)] + struct Opt { + #[arg(value_parser = parser, short = 'p')] + arg: ::std::vec::Vec>, + } + + assert_eq!( + Opt { + arg: vec![vec!["1".into(), "2".into()], vec!["a".into(), "b".into()]], + }, + Opt::try_parse_from(["test", "-p", "1,2:a,b"]).unwrap(), + ); +} + #[test] fn test_vec_vec_empty() { #[derive(Parser, Debug, PartialEq)] From 73df3d1a1eaacc0f79261d932c4239b7c4f80ede Mon Sep 17 00:00:00 2001 From: Thayne McCombs Date: Sun, 8 Jan 2023 00:27:28 -0700 Subject: [PATCH 3/3] chore: Put Vec> behave for derive behind unstable-v5 flag --- clap_derive/src/utils/ty.rs | 32 ++++++++++++++++++++------------ tests/derive/occurrences.rs | 12 +++++------- 2 files changed, 25 insertions(+), 19 deletions(-) diff --git a/clap_derive/src/utils/ty.rs b/clap_derive/src/utils/ty.rs index 32bc2c4f..a87ab169 100644 --- a/clap_derive/src/utils/ty.rs +++ b/clap_derive/src/utils/ty.rs @@ -26,21 +26,13 @@ impl Ty { if is_unit_ty(ty) { t(Unit) - } else if let Some(subty) = subty_if_name(ty, "Vec") { - if is_generic_ty(subty, "Vec") { - t(VecVec) - } else { - t(Vec) - } + } else if let Some(vt) = get_vec_ty(ty, Vec, VecVec) { + t(vt) } else if let Some(subty) = subty_if_name(ty, "Option") { if is_generic_ty(subty, "Option") { t(OptionOption) - } else if let Some(subty) = subty_if_name(subty, "Vec") { - if is_generic_ty(subty, "Vec") { - t(OptionVecVec) - } else { - t(OptionVec) - } + } else if let Some(vt) = get_vec_ty(subty, OptionVec, OptionVecVec) { + t(vt) } else { t(Option) } @@ -155,3 +147,19 @@ where { iter.next().filter(|_| iter.next().is_none()) } + +#[cfg(feature = "unstable-v5")] +fn get_vec_ty(ty: &Type, vec_ty: Ty, vecvec_ty: Ty) -> Option { + subty_if_name(ty, "Vec").map(|subty| { + if is_generic_ty(subty, "Vec") { + vecvec_ty + } else { + vec_ty + } + }) +} + +#[cfg(not(feature = "unstable-v5"))] +fn get_vec_ty(ty: &Type, vec_ty: Ty, _vecvec_ty: Ty) -> Option { + is_generic_ty(ty, "Vec").then(|| vec_ty) +} diff --git a/tests/derive/occurrences.rs b/tests/derive/occurrences.rs index c8cbb9e3..db4ae37e 100644 --- a/tests/derive/occurrences.rs +++ b/tests/derive/occurrences.rs @@ -1,4 +1,4 @@ -#![cfg(feature = "unstable-grouped")] +#![cfg(all(feature = "unstable-grouped", feature = "unstable-v5"))] use clap::Parser; #[test] @@ -19,23 +19,21 @@ fn test_vec_of_vec() { #[test] fn test_vec_of_vec_opt_out() { - fn parser(s: &str) -> Result>, std::convert::Infallible> { - Ok(s.split(':') - .map(|v| v.split(',').map(str::to_owned).collect()) - .collect()) + fn parser(s: &str) -> Result, std::convert::Infallible> { + Ok(s.split(',').map(str::to_owned).collect()) } #[derive(Parser, PartialEq, Debug)] struct Opt { #[arg(value_parser = parser, short = 'p')] - arg: ::std::vec::Vec>, + arg: Vec<::std::vec::Vec>, } assert_eq!( Opt { arg: vec![vec!["1".into(), "2".into()], vec!["a".into(), "b".into()]], }, - Opt::try_parse_from(["test", "-p", "1,2:a,b"]).unwrap(), + Opt::try_parse_from(["test", "-p", "1,2", "-p", "a,b"]).unwrap(), ); }