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

Issue:
`rename_all` on a `Subcommand` applies to variant-struct fields
### Please complete the following tasks

- [X] I have searched the [discussions](https://github.com/clap-rs/clap/discussions)
- [X] I have searched the existing issues

### Rust Version

rustc 1.57.0 (f1edd0429 2021-11-29)

### Clap Version

v3.0.6

### Minimal reproducible code

```rust
use clap::{Parser, Subcommand};

#[derive(Parser, Debug)]
#[clap(author, version, about, long_about = None)]
struct Cli {
    #[clap(long)]
    out_dir: Option<String>,

    #[clap(subcommand)]
    command: Commands,
}

#[derive(Subcommand, Debug)]
#[clap(rename_all = "snake_case")]
enum Commands {
    /// Adds files to myapp
    Add {
        #[clap(long)]
        out_dir: Option<String>,
    },
}

fn main() {
    let cli = Cli::parse();
    dbg!(cli);
}
```


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

```console
$ cargo run -- add --help
```

### Actual Behaviour

```
test-clap-add 
Adds files to myapp

USAGE:
    test-clap add [OPTIONS]

OPTIONS:
    -h, --help                 Print help information
        --out_dir <OUT_DIR>    
```

### Expected Behaviour

```
test-clap-add 
Adds files to myapp

USAGE:
    test-clap add [OPTIONS]

OPTIONS:
    -h, --help                 Print help information
        --out-dir <OUT_DIR>    
```

### Additional Context

Found via https://github.com/rust-lang/rustc-perf/pull/1142

### Debug Output

_No response_


Discussion / hints:

Fix patch:
diff --git a/clap_derive/src/attrs.rs b/clap_derive/src/attrs.rs
index 9a7bb7e7b29..aaee167e31c 100644
--- a/clap_derive/src/attrs.rs
+++ b/clap_derive/src/attrs.rs
@@ -671,6 +671,16 @@ impl Attrs {
         quote!( #(#next_help_heading)*  #(#help_heading)* )
     }
 
+    #[cfg(feature = "unstable-v4")]
+    pub fn id(&self) -> TokenStream {
+        self.name.clone().raw()
+    }
+
+    #[cfg(not(feature = "unstable-v4"))]
+    pub fn id(&self) -> TokenStream {
+        self.cased_name()
+    }
+
     pub fn cased_name(&self) -> TokenStream {
         self.name.clone().translate(*self.casing)
     }
@@ -916,6 +926,17 @@ pub enum Name {
 }
 
 impl Name {
+    #[cfg(feature = "unstable-v4")]
+    pub fn raw(self) -> TokenStream {
+        match self {
+            Name::Assigned(tokens) => tokens,
+            Name::Derived(ident) => {
+                let s = ident.unraw().to_string();
+                quote_spanned!(ident.span()=> #s)
+            }
+        }
+    }
+
     pub fn translate(self, style: CasingStyle) -> TokenStream {
         use CasingStyle::*;
 
diff --git a/clap_derive/src/derives/args.rs b/clap_derive/src/derives/args.rs
index 077fe4a64dd..e1486ea2bd3 100644
--- a/clap_derive/src/derives/args.rs
+++ b/clap_derive/src/derives/args.rs
@@ -342,12 +342,12 @@ pub fn gen_augment(
                     }
                 };
 
-                let name = attrs.cased_name();
+                let id = attrs.id();
                 let methods = attrs.field_methods(true);
 
                 Some(quote_spanned! { field.span()=>
                     let #app_var = #app_var.arg(
-                        clap::Arg::new(#name)
+                        clap::Arg::new(#id)
                             #modifier
                             #methods
                     );
@@ -528,7 +528,7 @@ fn gen_parsers(
     let func = &parser.func;
     let span = parser.kind.span();
     let convert_type = inner_type(**ty, &field.ty);
-    let name = attrs.cased_name();
+    let id = attrs.id();
     let (value_of, values_of, mut parse) = match *parser.kind {
         FromStr => (
             quote_spanned!(span=> value_of),
@@ -538,7 +538,7 @@ fn gen_parsers(
         TryFromStr => (
             quote_spanned!(span=> value_of),
             quote_spanned!(span=> values_of),
-            quote_spanned!(func.span()=> |s| #func(s).map_err(|err| clap::Error::raw(clap::ErrorKind::ValueValidation, format!("Invalid value for {}: {}", #name, err)))),
+            quote_spanned!(func.span()=> |s| #func(s).map_err(|err| clap::Error::raw(clap::ErrorKind::ValueValidation, format!("Invalid value for {}: {}", #id, err)))),
         ),
         FromOsStr => (
             quote_spanned!(span=> value_of_os),
@@ -548,7 +548,7 @@ fn gen_parsers(
         TryFromOsStr => (
             quote_spanned!(span=> value_of_os),
             quote_spanned!(span=> values_of_os),
-            quote_spanned!(func.span()=> |s| #func(s).map_err(|err| clap::Error::raw(clap::ErrorKind::ValueValidation, format!("Invalid value for {}: {}", #name, err)))),
+            quote_spanned!(func.span()=> |s| #func(s).map_err(|err| clap::Error::raw(clap::ErrorKind::ValueValidation, format!("Invalid value for {}: {}", #id, err)))),
         ),
         FromOccurrences => (
             quote_spanned!(span=> occurrences_of),
@@ -561,7 +561,7 @@ fn gen_parsers(
         let ci = attrs.ignore_case();
 
         parse = quote_spanned! { convert_type.span()=>
-            |s| <#convert_type as clap::ArgEnum>::from_str(s, #ci).map_err(|err| clap::Error::raw(clap::ErrorKind::ValueValidation, format!("Invalid value for {}: {}", #name, err)))
+            |s| <#convert_type as clap::ArgEnum>::from_str(s, #ci).map_err(|err| clap::Error::raw(clap::ErrorKind::ValueValidation, format!("Invalid value for {}: {}", #id, err)))
         }
     }
 
@@ -576,34 +576,34 @@ fn gen_parsers(
         Ty::Bool => {
             if update.is_some() {
                 quote_spanned! { ty.span()=>
-                    *#field_name || #arg_matches.is_present(#name)
+                    *#field_name || #arg_matches.is_present(#id)
                 }
             } else {
                 quote_spanned! { ty.span()=>
-                    #arg_matches.is_present(#name)
+                    #arg_matches.is_present(#id)
                 }
             }
         }
 
         Ty::Option => {
             quote_spanned! { ty.span()=>
-                #arg_matches.#value_of(#name)
+                #arg_matches.#value_of(#id)
                     .map(#parse)
                     .transpose()?
             }
         }
 
         Ty::OptionOption => quote_spanned! { ty.span()=>
-            if #arg_matches.is_present(#name) {
-                Some(#arg_matches.#value_of(#name).map(#parse).transpose()?)
+            if #arg_matches.is_present(#id) {
+                Some(#arg_matches.#value_of(#id).map(#parse).transpose()?)
             } else {
                 None
             }
         },
 
         Ty::OptionVec => quote_spanned! { ty.span()=>
-            if #arg_matches.is_present(#name) {
-                Some(#arg_matches.#values_of(#name)
+            if #arg_matches.is_present(#id) {
+                Some(#arg_matches.#values_of(#id)
                     .map(|v| v.map::<::std::result::Result<#convert_type, clap::Error>, _>(#parse).collect::<::std::result::Result<Vec<_>, clap::Error>>())
                     .transpose()?
                     .unwrap_or_else(Vec::new))
@@ -614,7 +614,7 @@ fn gen_parsers(
 
         Ty::Vec => {
             quote_spanned! { ty.span()=>
-                #arg_matches.#values_of(#name)
+                #arg_matches.#values_of(#id)
                     .map(|v| v.map::<::std::result::Result<#convert_type, clap::Error>, _>(#parse).collect::<::std::result::Result<Vec<_>, clap::Error>>())
                     .transpose()?
                     .unwrap_or_else(Vec::new)
@@ -622,17 +622,17 @@ fn gen_parsers(
         }
 
         Ty::Other if occurrences => quote_spanned! { ty.span()=>
-            #parse(#arg_matches.#value_of(#name))
+            #parse(#arg_matches.#value_of(#id))
         },
 
         Ty::Other if flag => quote_spanned! { ty.span()=>
-            #parse(#arg_matches.is_present(#name))
+            #parse(#arg_matches.is_present(#id))
         },
 
         Ty::Other => {
             quote_spanned! { ty.span()=>
-                #arg_matches.#value_of(#name)
-                    .ok_or_else(|| clap::Error::raw(clap::ErrorKind::MissingRequiredArgument, format!("The following required argument was not provided: {}", #name)))
+                #arg_matches.#value_of(#id)
+                    .ok_or_else(|| clap::Error::raw(clap::ErrorKind::MissingRequiredArgument, format!("The following required argument was not provided: {}", #id)))
                     .and_then(#parse)?
             }
         }
@@ -640,7 +640,7 @@ fn gen_parsers(
 
     if let Some(access) = update {
         quote_spanned! { field.span()=>
-            if #arg_matches.is_present(#name) {
+            if #arg_matches.is_present(#id) {
                 #access
                 *#field_name = #field_value
             }


Test patch:
diff --git a/tests/derive/help.rs b/tests/derive/help.rs
index 4cd1d943c9e..1362ac866c8 100644
--- a/tests/derive/help.rs
+++ b/tests/derive/help.rs
@@ -14,16 +14,26 @@ fn arg_help_heading_applied() {
 
     let cmd = CliOptions::command();
 
-    let should_be_in_section_a = cmd
-        .get_arguments()
-        .find(|a| a.get_id() == "should-be-in-section-a")
-        .unwrap();
+    let should_be_in_section_a = if cfg!(feature = "unstable-v4") {
+        cmd.get_arguments()
+            .find(|a| a.get_id() == "should_be_in_section_a")
+            .unwrap()
+    } else {
+        cmd.get_arguments()
+            .find(|a| a.get_id() == "should-be-in-section-a")
+            .unwrap()
+    };
     assert_eq!(should_be_in_section_a.get_help_heading(), Some("HEADING A"));
 
-    let should_be_in_section_b = cmd
-        .get_arguments()
-        .find(|a| a.get_id() == "no-section")
-        .unwrap();
+    let should_be_in_section_b = if cfg!(feature = "unstable-v4") {
+        cmd.get_arguments()
+            .find(|a| a.get_id() == "no_section")
+            .unwrap()
+    } else {
+        cmd.get_arguments()
+            .find(|a| a.get_id() == "no-section")
+            .unwrap()
+    };
     assert_eq!(should_be_in_section_b.get_help_heading(), None);
 }
 
@@ -42,16 +52,26 @@ fn app_help_heading_applied() {
 
     let cmd = CliOptions::command();
 
-    let should_be_in_section_a = cmd
-        .get_arguments()
-        .find(|a| a.get_id() == "should-be-in-section-a")
-        .unwrap();
+    let should_be_in_section_a = if cfg!(feature = "unstable-v4") {
+        cmd.get_arguments()
+            .find(|a| a.get_id() == "should_be_in_section_a")
+            .unwrap()
+    } else {
+        cmd.get_arguments()
+            .find(|a| a.get_id() == "should-be-in-section-a")
+            .unwrap()
+    };
     assert_eq!(should_be_in_section_a.get_help_heading(), Some("HEADING A"));
 
-    let should_be_in_default_section = cmd
-        .get_arguments()
-        .find(|a| a.get_id() == "should-be-in-default-section")
-        .unwrap();
+    let should_be_in_default_section = if cfg!(feature = "unstable-v4") {
+        cmd.get_arguments()
+            .find(|a| a.get_id() == "should_be_in_default_section")
+            .unwrap()
+    } else {
+        cmd.get_arguments()
+            .find(|a| a.get_id() == "should-be-in-default-section")
+            .unwrap()
+    };
     assert_eq!(
         should_be_in_default_section.get_help_heading(),
         Some("DEFAULT")
@@ -119,47 +139,83 @@ fn app_help_heading_flattened() {
 
     let cmd = CliOptions::command();
 
-    let should_be_in_section_a = cmd
-        .get_arguments()
-        .find(|a| a.get_id() == "should-be-in-section-a")
-        .unwrap();
+    let should_be_in_section_a = if cfg!(feature = "unstable-v4") {
+        cmd.get_arguments()
+            .find(|a| a.get_id() == "should_be_in_section_a")
+            .unwrap()
+    } else {
+        cmd.get_arguments()
+            .find(|a| a.get_id() == "should-be-in-section-a")
+            .unwrap()
+    };
     assert_eq!(should_be_in_section_a.get_help_heading(), Some("HEADING A"));
 
-    let should_be_in_section_b = cmd
-        .get_arguments()
-        .find(|a| a.get_id() == "should-be-in-section-b")
-        .unwrap();
+    let should_be_in_section_b = if cfg!(feature = "unstable-v4") {
+        cmd.get_arguments()
+            .find(|a| a.get_id() == "should_be_in_section_b")
+            .unwrap()
+    } else {
+        cmd.get_arguments()
+            .find(|a| a.get_id() == "should-be-in-section-b")
+            .unwrap()
+    };
     assert_eq!(should_be_in_section_b.get_help_heading(), Some("HEADING B"));
 
-    let should_be_in_default_section = cmd
-        .get_arguments()
-        .find(|a| a.get_id() == "should-be-in-default-section")
-        .unwrap();
+    let should_be_in_default_section = if cfg!(feature = "unstable-v4") {
+        cmd.get_arguments()
+            .find(|a| a.get_id() == "should_be_in_default_section")
+            .unwrap()
+    } else {
+        cmd.get_arguments()
+            .find(|a| a.get_id() == "should-be-in-default-section")
+            .unwrap()
+    };
     assert_eq!(should_be_in_default_section.get_help_heading(), None);
 
     let sub_a_two = cmd.find_subcommand("sub-a-two").unwrap();
 
-    let should_be_in_sub_a = sub_a_two
-        .get_arguments()
-        .find(|a| a.get_id() == "should-be-in-sub-a")
-        .unwrap();
+    let should_be_in_sub_a = if cfg!(feature = "unstable-v4") {
+        sub_a_two
+            .get_arguments()
+            .find(|a| a.get_id() == "should_be_in_sub_a")
+            .unwrap()
+    } else {
+        sub_a_two
+            .get_arguments()
+            .find(|a| a.get_id() == "should-be-in-sub-a")
+            .unwrap()
+    };
     assert_eq!(should_be_in_sub_a.get_help_heading(), Some("SUB A"));
 
     let sub_b_one = cmd.find_subcommand("sub-b-one").unwrap();
 
-    let should_be_in_sub_b = sub_b_one
-        .get_arguments()
-        .find(|a| a.get_id() == "should-be-in-sub-b")
-        .unwrap();
+    let should_be_in_sub_b = if cfg!(feature = "unstable-v4") {
+        sub_b_one
+            .get_arguments()
+            .find(|a| a.get_id() == "should_be_in_sub_b")
+            .unwrap()
+    } else {
+        sub_b_one
+            .get_arguments()
+            .find(|a| a.get_id() == "should-be-in-sub-b")
+            .unwrap()
+    };
     assert_eq!(should_be_in_sub_b.get_help_heading(), Some("SUB B"));
 
     let sub_c = cmd.find_subcommand("sub-c").unwrap();
     let sub_c_one = sub_c.find_subcommand("sub-c-one").unwrap();
 
-    let should_be_in_sub_c = sub_c_one
-        .get_arguments()
-        .find(|a| a.get_id() == "should-be-in-sub-c")
-        .unwrap();
+    let should_be_in_sub_c = if cfg!(feature = "unstable-v4") {
+        sub_c_one
+            .get_arguments()
+            .find(|a| a.get_id() == "should_be_in_sub_c")
+            .unwrap()
+    } else {
+        sub_c_one
+            .get_arguments()
+            .find(|a| a.get_id() == "should-be-in-sub-c")
+            .unwrap()
+    };
     assert_eq!(should_be_in_sub_c.get_help_heading(), Some("SUB C"));
 }
 
@@ -180,10 +236,15 @@ fn flatten_field_with_help_heading() {
 
     let cmd = CliOptions::command();
 
-    let should_be_in_section_a = cmd
-        .get_arguments()
-        .find(|a| a.get_id() == "should-be-in-section-a")
-        .unwrap();
+    let should_be_in_section_a = if cfg!(feature = "unstable-v4") {
+        cmd.get_arguments()
+            .find(|a| a.get_id() == "should_be_in_section_a")
+            .unwrap()
+    } else {
+        cmd.get_arguments()
+            .find(|a| a.get_id() == "should-be-in-section-a")
+            .unwrap()
+    };
     assert_eq!(should_be_in_section_a.get_help_heading(), Some("HEADING A"));
 }
 
@@ -218,7 +279,8 @@ fn derive_generated_error_has_full_context() {
         result.unwrap()
     );
 
-    let expected = r#"error: The following required argument was not provided: req-str
+    if cfg!(feature = "unstable-v4") {
+        let expected = r#"error: The following required argument was not provided: req_str
 
 USAGE:
     clap --req-str <REQ_STR>
@@ -226,7 +288,18 @@ USAGE:
 
 For more information try --help
 "#;
-    assert_eq!(result.unwrap_err().to_string(), expected);
+        assert_eq!(result.unwrap_err().to_string(), expected);
+    } else {
+        let expected = r#"error: The following required argument was not provided: req-str
+
+USAGE:
+    clap --req-str <REQ_STR>
+    clap <SUBCOMMAND>
+
+For more information try --help
+"#;
+        assert_eq!(result.unwrap_err().to_string(), expected);
+    }
 }
 
 #[test]
diff --git a/tests/derive/main.rs b/tests/derive/main.rs
index 33ce6715d81..3e2c10e338c 100644
--- a/tests/derive/main.rs
+++ b/tests/derive/main.rs
@@ -2,7 +2,6 @@
 
 mod app_name;
 mod arg_enum;
-mod argument_naming;
 mod arguments;
 mod author_version_about;
 mod basic;
@@ -18,6 +17,7 @@ mod generic;
 mod help;
 mod issues;
 mod macros;
+mod naming;
 mod nested_subcommands;
 mod non_literal_attributes;
 mod options;
diff --git a/tests/derive/argument_naming.rs b/tests/derive/naming.rs
similarity index 93%
rename from tests/derive/argument_naming.rs
rename to tests/derive/naming.rs
index 61bf439c82b..e3018a02f7c 100644
--- a/tests/derive/argument_naming.rs
+++ b/tests/derive/naming.rs
@@ -1,31 +1,5 @@
 use clap::Parser;
 
-#[test]
-fn test_single_word_enum_variant_is_default_renamed_into_kebab_case() {
-    #[derive(Parser, Debug, PartialEq)]
-    enum Opt {
-        Command { foo: u32 },
-    }
-
-    assert_eq!(
-        Opt::Command { foo: 0 },
-        Opt::try_parse_from(&["test", "command", "0"]).unwrap()
-    );
-}
-
-#[test]
-fn test_multi_word_enum_variant_is_renamed() {
-    #[derive(Parser, Debug, PartialEq)]
-    enum Opt {
-        FirstCommand { foo: u32 },
-    }
-
-    assert_eq!(
-        Opt::FirstCommand { foo: 0 },
-        Opt::try_parse_from(&["test", "first-command", "0"]).unwrap()
-    );
-}
-
 #[test]
 fn test_standalone_long_generates_kebab_case() {
     #[derive(Parser, Debug, PartialEq)]
@@ -235,6 +209,60 @@ fn test_rename_all_is_not_propagated_from_struct_into_flattened() {
     );
 }
 
+#[test]
+fn test_lower_is_renamed() {
+    #[derive(Parser, Debug, PartialEq)]
+    struct Opt {
+        #[clap(rename_all = "lower", long)]
+        foo_option: bool,
+    }
+
+    assert_eq!(
+        Opt { foo_option: true },
+        Opt::try_parse_from(&["test", "--foooption"]).unwrap()
+    );
+}
+
+#[test]
+fn test_upper_is_renamed() {
+    #[derive(Parser, Debug, PartialEq)]
+    struct Opt {
+        #[clap(rename_all = "upper", long)]
+        foo_option: bool,
+    }
+
+    assert_eq!(
+        Opt { foo_option: true },
+        Opt::try_parse_from(&["test", "--FOOOPTION"]).unwrap()
+    );
+}
+
+#[test]
+fn test_single_word_enum_variant_is_default_renamed_into_kebab_case() {
+    #[derive(Parser, Debug, PartialEq)]
+    enum Opt {
+        Command { foo: u32 },
+    }
+
+    assert_eq!(
+        Opt::Command { foo: 0 },
+        Opt::try_parse_from(&["test", "command", "0"]).unwrap()
+    );
+}
+
+#[test]
+fn test_multi_word_enum_variant_is_renamed() {
+    #[derive(Parser, Debug, PartialEq)]
+    enum Opt {
+        FirstCommand { foo: u32 },
+    }
+
+    assert_eq!(
+        Opt::FirstCommand { foo: 0 },
+        Opt::try_parse_from(&["test", "first-command", "0"]).unwrap()
+    );
+}
+
 #[test]
 fn test_rename_all_is_not_propagated_from_struct_into_subcommand() {
     #[derive(Parser, Debug, PartialEq)]
@@ -261,14 +289,14 @@ fn test_rename_all_is_not_propagated_from_struct_into_subcommand() {
 }
 
 #[test]
-fn test_rename_all_is_propagated_from_enum_to_variants_and_their_fields() {
+fn test_rename_all_is_propagated_from_enum_to_variants() {
     #[derive(Parser, Debug, PartialEq)]
     #[clap(rename_all = "screaming_snake")]
     enum Opt {
         FirstVariant,
         SecondVariant {
             #[clap(long)]
-            foo: bool,
+            foo: String,
         },
     }
 
@@ -276,10 +304,25 @@ fn test_rename_all_is_propagated_from_enum_to_variants_and_their_fields() {
         Opt::FirstVariant,
         Opt::try_parse_from(&["test", "FIRST_VARIANT"]).unwrap()
     );
+}
+
+#[test]
+fn test_rename_all_is_propagated_from_enum_to_variant_fields() {
+    #[derive(Parser, Debug, PartialEq)]
+    #[clap(rename_all = "screaming_snake")]
+    enum Opt {
+        FirstVariant,
+        SecondVariant {
+            #[clap(long)]
+            foo: String,
+        },
+    }
 
     assert_eq!(
-        Opt::SecondVariant { foo: true },
-        Opt::try_parse_from(&["test", "SECOND_VARIANT", "--FOO"]).unwrap()
+        Opt::SecondVariant {
+            foo: "value".into()
+        },
+        Opt::try_parse_from(&["test", "SECOND_VARIANT", "--FOO", "value"]).unwrap()
     );
 }
 
@@ -309,31 +352,3 @@ fn test_rename_all_is_propagation_can_be_overridden() {
         Opt::try_parse_from(&["test", "SECOND_VARIANT", "--foo-option"]).unwrap()
     );
 }
-
-#[test]
-fn test_lower_is_renamed() {
-    #[derive(Parser, Debug, PartialEq)]
-    struct Opt {
-        #[clap(rename_all = "lower", long)]
-        foo_option: bool,
-    }
-
-    assert_eq!(
-        Opt { foo_option: true },
-        Opt::try_parse_from(&["test", "--foooption"]).unwrap()
-    );
-}
-
-#[test]
-fn test_upper_is_renamed() {
-    #[derive(Parser, Debug, PartialEq)]
-    struct Opt {
-        #[clap(rename_all = "upper", long)]
-        foo_option: bool,
-    }
-
-    assert_eq!(
-        Opt { foo_option: true },
-        Opt::try_parse_from(&["test", "--FOOOPTION"]).unwrap()
-    );
-}