From 19f3b3715e8ffd3ea48b1c29dfce1c03fbeeb37d Mon Sep 17 00:00:00 2001
From: Kurtis Nusbaum <kcommiter@gmail.com>
Date: Fri, 3 Feb 2023 22:05:46 +0000
Subject: [PATCH] feat(derive): Add additional group methods

This adds the ability derive additional options for the group creation.
Needed for clap-rs#4574.
---
 clap_derive/src/attr.rs         |  2 --
 clap_derive/src/derives/args.rs |  5 +++--
 clap_derive/src/item.rs         | 23 +++++++++++------------
 tests/derive/groups.rs          | 19 +------------------
 4 files changed, 15 insertions(+), 34 deletions(-)

diff --git a/clap_derive/src/attr.rs b/clap_derive/src/attr.rs
index 21fe9c680e7e..e282a8f94edd 100644
--- a/clap_derive/src/attr.rs
+++ b/clap_derive/src/attr.rs
@@ -86,7 +86,6 @@ impl Parse for ClapAttr {
         let magic = match name_str.as_str() {
             "rename_all" => Some(MagicAttrName::RenameAll),
             "rename_all_env" => Some(MagicAttrName::RenameAllEnv),
-            "required" => Some(MagicAttrName::Required),
             "skip" => Some(MagicAttrName::Skip),
             "next_display_order" => Some(MagicAttrName::NextDisplayOrder),
             "next_help_heading" => Some(MagicAttrName::NextHelpHeading),
@@ -169,7 +168,6 @@ pub enum MagicAttrName {
     Version,
     RenameAllEnv,
     RenameAll,
-    Required,
     Skip,
     DefaultValueT,
     DefaultValuesT,
diff --git a/clap_derive/src/derives/args.rs b/clap_derive/src/derives/args.rs
index 3b48a73e7910..87a6390d86f7 100644
--- a/clap_derive/src/derives/args.rs
+++ b/clap_derive/src/derives/args.rs
@@ -368,7 +368,6 @@ pub fn gen_augment(
         quote!()
     } else {
         let group_id = parent_item.ident().unraw().to_string();
-        let required = parent_item.required_group();
         let literal_group_members = fields
             .iter()
             .filter_map(|(_field, item)| {
@@ -401,11 +400,13 @@ pub fn gen_augment(
             }};
         }
 
+        let group_methods = parent_item.group_methods();
+
         quote!(
             .group(
                 clap::ArgGroup::new(#group_id)
                     .multiple(true)
-                    .required(#required)
+                    #group_methods
                     .args(#literal_group_members)
             )
         )
diff --git a/clap_derive/src/item.rs b/clap_derive/src/item.rs
index 40eec4333241..c10e939ec0a6 100644
--- a/clap_derive/src/item.rs
+++ b/clap_derive/src/item.rs
@@ -48,8 +48,8 @@ pub struct Item {
     next_help_heading: Option<Method>,
     is_enum: bool,
     is_positional: bool,
-    required_group: bool,
     skip_group: bool,
+    group_methods: Vec<Method>,
     kind: Sp<Kind>,
 }
 
@@ -273,8 +273,8 @@ impl Item {
             next_help_heading: None,
             is_enum: false,
             is_positional: true,
-            required_group: false,
             skip_group: false,
+            group_methods: vec![],
             kind,
         }
     }
@@ -327,7 +327,10 @@ impl Item {
             if name == "short" || name == "long" {
                 self.is_positional = false;
             }
-            self.methods.push(Method::new(name, quote!(#arg)));
+            match kind {
+                AttrKind::Group => self.group_methods.push(Method::new(name, quote!(#arg))),
+                _ => self.methods.push(Method::new(name, quote!(#arg))),
+            };
         }
     }
 
@@ -826,10 +829,6 @@ impl Item {
                     self.env_casing = CasingStyle::from_lit(lit);
                 }
 
-                Some(MagicAttrName::Required) if actual_attr_kind == AttrKind::Group => {
-                    self.required_group = true;
-                }
-
                 Some(MagicAttrName::Skip) if actual_attr_kind == AttrKind::Group => {
                     self.skip_group = true;
                 }
@@ -844,7 +843,6 @@ impl Item {
                 | Some(MagicAttrName::LongHelp)
                 | Some(MagicAttrName::Author)
                 | Some(MagicAttrName::Version)
-                | Some(MagicAttrName::Required)
                  => {
                     let expr = attr.value_or_abort();
                     self.push_method(*attr.kind.get(), attr.name.clone(), expr);
@@ -971,6 +969,11 @@ impl Item {
         quote!( #(#doc_comment)* #(#methods)* )
     }
 
+    pub fn group_methods(&self) -> TokenStream {
+        let group_methods = &self.group_methods;
+        quote!( #(#group_methods)* )
+    }
+
     pub fn deprecations(&self) -> proc_macro2::TokenStream {
         let deprecations = &self.deprecations;
         quote!( #(#deprecations)* )
@@ -1066,10 +1069,6 @@ impl Item {
             .any(|m| m.name != "help" && m.name != "long_help")
     }
 
-    pub fn required_group(&self) -> bool {
-        self.required_group
-    }
-
     pub fn skip_group(&self) -> bool {
         self.skip_group
     }
diff --git a/tests/derive/groups.rs b/tests/derive/groups.rs
index 400bdb72d944..8458f131be39 100644
--- a/tests/derive/groups.rs
+++ b/tests/derive/groups.rs
@@ -127,12 +127,10 @@ fn required_group() {
     struct Opt {
         #[command(flatten)]
         source: Source,
-        #[command(flatten)]
-        dest: Dest,
     }
 
     #[derive(clap::Args, Debug)]
-    #[group(required)]
+    #[group(required = true)]
     struct Source {
         #[arg(long)]
         from_path: Option<std::path::PathBuf>,
@@ -140,27 +138,12 @@ fn required_group() {
         from_git: Option<String>,
     }
 
-    #[derive(clap::Args, Debug)]
-    #[group(required = true)]
-    struct Dest {
-        #[arg(long)]
-        to_path: Option<std::path::PathBuf>,
-        #[arg(long)]
-        to_git: Option<String>,
-    }
-
     use clap::CommandFactory;
     let source_id = clap::Id::from("Source");
-    let dest_id = clap::Id::from("Dest");
     let opt_command = Opt::command();
     let source_group = opt_command
         .get_groups()
         .find(|g| g.get_id() == &source_id)
         .unwrap();
-    let dest_group = opt_command
-        .get_groups()
-        .find(|g| g.get_id() == &dest_id)
-        .unwrap();
     assert!(source_group.is_required_set());
-    assert!(dest_group.is_required_set());
 }