Repository: clap-rs/clap (language: rust)

Issue:
Unclear panic error: "`#[arg(flatten)]`ed field type implements `Args::group_id`"
### Please complete the following tasks

- [X] I have searched the [discussions](https://github.com/clap-rs/clap/discussions)
- [X] I have searched the [open](https://github.com/clap-rs/clap/issues) and [rejected](https://github.com/clap-rs/clap/issues?q=is%3Aissue+label%3AS-wont-fix+is%3Aclosed) issues

### Rust Version

rustc 1.80.0 (051478957 2024-07-21)

### Clap Version

4.5.11

### Minimal reproducible code

```rust
#[derive(clap::Parser)]
struct Args {
    #[clap(flatten)]
    args: Option<subcmd::Args>,
}

mod subcmd {
    #[derive(clap::Args)]
    #[group(skip)]
    pub struct Args {
        #[clap(short)]
        param: bool,
    }
}

fn main() {
    use clap::Parser;
    Args::parse();
}
```


### Steps to reproduce the bug with the above code

`cargo run`

### Actual Behaviour

```
thread 'main' panicked at src/main.rs:3:12:
`#[arg(flatten)]`ed field type implements `Args::group_id`
note: run with `RUST_BACKTRACE=1` environment variable to display a backtra
```

### Expected Behaviour

A better error message, something to the effect of
```
Optional field which is `#[arg(flatten)]`ed must implement `Args::group_id`.
Replace `#[group(skip)]` with `#[group(id = "...")]`.
```

Actually since this is a proc-macro, perhaps a more elaborate error would be possible. But an improvement for the `expect` message should suffice I believe.

### Additional Context

The relevant line in clap_derive is [here](https://github.com/clap-rs/clap/blob/16fba4b9f91cf772ab1f6e7b1016a257e5f6cf62/clap_derive/src/derives/args.rs#L502).

Of note, `Command::debug_assert` fails to catch this issue. Not sure if that's a bug or not but seems relevant.

### Debug Output

_No response_


Discussion / hints:

Fix patch:
diff --git a/clap_derive/src/derives/args.rs b/clap_derive/src/derives/args.rs
index 4105608829d..d6fecc01970 100644
--- a/clap_derive/src/derives/args.rs
+++ b/clap_derive/src/derives/args.rs
@@ -228,8 +228,16 @@ 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;
@@ -237,6 +245,7 @@ pub(crate) fn gen_augment(
                     })
                 } 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)?


Test patch:
diff --git a/tests/derive/flatten.rs b/tests/derive/flatten.rs
index bdd0387338c..95ad43e2bee 100644
--- a/tests/derive/flatten.rs
+++ b/tests/derive/flatten.rs
@@ -255,3 +255,22 @@ fn docstrings_ordering_with_multiple_clap_partial() {
 
     assert!(short_help.contains("This is the docstring for Flattened"));
 }
+
+#[test]
+#[should_panic = "cannot `#[flatten]` an `Option<Args>` with `#[group(skip)]"]
+fn flatten_skipped_group() {
+    #[derive(clap::Parser, Debug)]
+    struct Cli {
+        #[clap(flatten)]
+        args: Option<Args>,
+    }
+
+    #[derive(clap::Args, Debug)]
+    #[group(skip)]
+    struct Args {
+        #[clap(short)]
+        param: bool,
+    }
+
+    Cli::try_parse_from(["test"]).unwrap();
+}