fix(derive): Improve flattening-skipped-group assert

epage · epage · commit 13dfdb38709d · 2024-07-31T15:19:29.000-05:00
- Improves the error message - Happens on initialization, rather than parse, making it so it will always show up and not just when certain parts of the CLI are exercised Fixes #5609
diff --git a/clap_derive/src/derives/args.rs b/clap_derive/src/derives/args.rs
@@ -228,15 +228,24 @@ pub(crate) fn gen_augment(
 
                 let next_help_heading = item.next_help_heading();
                 let next_display_order = item.next_display_order();
+                let flatten_group_assert = if matches!(**ty, Ty::Option) {
+                    quote_spanned! { kind.span()=>
+                        <#inner_type as clap::Args>::group_id().expect("cannot `#[flatten]` an `Option<Args>` with `#[group(skip)]");
+                    }
+                } else {
+                    quote! {}
+                };
                 if override_required {
                     Some(quote_spanned! { kind.span()=>
+                        #flatten_group_assert
                         let #app_var = #app_var
                             #next_help_heading
                             #next_display_order;
                         let #app_var = <#inner_type as clap::Args>::augment_args_for_update(#app_var);
                     })
                 } else {
                     Some(quote_spanned! { kind.span()=>
+                        #flatten_group_assert
                         let #app_var = #app_var
                             #next_help_heading
                             #next_display_order;
@@ -499,7 +508,7 @@ pub(crate) fn gen_constructor(fields: &[(&Field, Item)]) -> Result<TokenStream,
                         quote_spanned! { kind.span()=>
                             #field_name: {
                                 let group_id = <#inner_type as clap::Args>::group_id()
-                                    .expect("`#[arg(flatten)]`ed field type implements `Args::group_id`");
+                                    .expect("asserted during `Arg` creation");
                                 if #arg_matches.contains_id(group_id.as_str()) {
                                     Some(
                                         <#inner_type as clap::FromArgMatches>::from_arg_matches_mut(#arg_matches)?
diff --git a/tests/derive/flatten.rs b/tests/derive/flatten.rs
@@ -257,7 +257,7 @@ fn docstrings_ordering_with_multiple_clap_partial() {
 }
 
 #[test]
-#[should_panic = "#[arg(flatten)]`ed field type implements `Args::group_id"]
+#[should_panic = "cannot `#[flatten]` an `Option<Args>` with `#[group(skip)]"]
 fn flatten_skipped_group() {
     #[derive(clap::Parser, Debug)]
     struct Cli {

Original file line number	Diff line number	Diff line change
`@@ -257,7 +257,7 @@ fn docstrings_ordering_with_multiple_clap_partial() {`
`257`	`257`	`}`
`258`	`258`
`259`	`259`	`#[test]`
`260`		-#[should_panic = "#[arg(flatten)]`ed field type implements `Args::group_id"]
	`260`	+#[should_panic = "cannot `#[flatten]` an `Option<Args>` with `#[group(skip)]"]
`261`	`261`	`fn flatten_skipped_group() {`
`262`	`262`	`#[derive(clap::Parser, Debug)]`
`263`	`263`	`struct Cli {`