From 50ad905b6a777c1d6e34cd45972d0afcf236ed49 Mon Sep 17 00:00:00 2001
From: Ed Page <eopage@gmail.com>
Date: Thu, 29 Sep 2022 15:56:53 -0500
Subject: [PATCH] feat(derive): Implicitly populate groups from some structs

This implements the basics for #3165, just missing
- `flatten` support (waiting on improved group support)
- `group` attributes
---
 clap_derive/src/derives/args.rs | 38 ++++++++++++++++++++++++++++++++-
 tests/derive/groups.rs          | 33 ++++++++++++++++++++++++++++
 tests/derive/utils.rs           | 15 +++++++++++++
 3 files changed, 85 insertions(+), 1 deletion(-)

diff --git a/clap_derive/src/derives/args.rs b/clap_derive/src/derives/args.rs
index 9c99c3fb516..a92d02ee8dc 100644
--- a/clap_derive/src/derives/args.rs
+++ b/clap_derive/src/derives/args.rs
@@ -318,8 +318,44 @@ pub fn gen_augment(
     let group_app_methods = if parent_item.skip_group() {
         quote!()
     } else {
+        let literal_group_members = fields
+            .iter()
+            .filter_map(|(_field, item)| {
+                let kind = item.kind();
+                if matches!(*kind, Kind::Arg(_)) {
+                    Some(item.id())
+                } else {
+                    None
+                }
+            })
+            .collect::<Vec<_>>();
+        let literal_group_members_len = literal_group_members.len();
+        let mut literal_group_members = quote! {{
+            let members: [clap::Id; #literal_group_members_len] = [#( clap::Id::from(#literal_group_members) ),* ];
+            members
+        }};
+        // HACK: Validation isn't ready yet for nested arg groups, so just don't populate the group in
+        // that situation
+        let possible_group_members_len = fields
+            .iter()
+            .filter(|(_field, item)| {
+                let kind = item.kind();
+                matches!(*kind, Kind::Flatten)
+            })
+            .count();
+        if 0 < possible_group_members_len {
+            literal_group_members = quote! {{
+                let members: [clap::Id; 0] = [];
+                members
+            }};
+        }
+
         quote!(
-            .group(clap::ArgGroup::new(#group_id).multiple(true))
+            .group(
+                clap::ArgGroup::new(#group_id)
+                    .multiple(true)
+                    .args(#literal_group_members)
+            )
         )
     };
     quote! {{
diff --git a/tests/derive/groups.rs b/tests/derive/groups.rs
index 86548d3d3c6..99cba73ef98 100644
--- a/tests/derive/groups.rs
+++ b/tests/derive/groups.rs
@@ -1,5 +1,7 @@
 use clap::Parser;
 
+use crate::utils::assert_output;
+
 #[test]
 fn test_safely_nest_parser() {
     #[derive(Parser, Debug, PartialEq)]
@@ -22,6 +24,37 @@ fn test_safely_nest_parser() {
     );
 }
 
+#[test]
+fn implicit_struct_group() {
+    #[derive(Parser, Debug)]
+    struct Opt {
+        #[arg(short, long, requires = "Source")]
+        add: bool,
+
+        #[command(flatten)]
+        source: Source,
+    }
+
+    #[derive(clap::Args, Debug)]
+    struct Source {
+        crates: Vec<String>,
+        #[arg(long)]
+        path: Option<std::path::PathBuf>,
+        #[arg(long)]
+        git: Option<String>,
+    }
+
+    const OUTPUT: &str = "\
+error: The following required arguments were not provided:
+  <CRATES|--path <PATH>|--git <GIT>>
+
+Usage: prog --add <CRATES|--path <PATH>|--git <GIT>>
+
+For more information try '--help'
+";
+    assert_output::<Opt>("prog --add", OUTPUT, true);
+}
+
 #[test]
 fn skip_group_avoids_duplicate_ids() {
     #[derive(Parser, Debug)]
diff --git a/tests/derive/utils.rs b/tests/derive/utils.rs
index 10ee19467b4..a6a0e36d917 100644
--- a/tests/derive/utils.rs
+++ b/tests/derive/utils.rs
@@ -53,3 +53,18 @@ pub fn get_subcommand_long_help<T: CommandFactory>(subcmd: &str) -> String {
 
     output
 }
+
+#[track_caller]
+pub fn assert_output<P: clap::Parser + std::fmt::Debug>(args: &str, expected: &str, stderr: bool) {
+    let res = P::try_parse_from(args.split(' ').collect::<Vec<_>>());
+    let err = res.unwrap_err();
+    let actual = err.render().to_string();
+    assert_eq!(
+        stderr,
+        err.use_stderr(),
+        "Should Use STDERR failed. Should be {} but is {}",
+        stderr,
+        err.use_stderr()
+    );
+    snapbox::assert_eq(expected, actual)
+}