From d8e25498b2e555c6fe5e98b842206723dbf4289f Mon Sep 17 00:00:00 2001
From: Daniel Parks <dp+git@oxidized.org>
Date: Sat, 30 Jul 2022 17:07:15 -0700
Subject: [PATCH] fix(derive): Add "id" attribute

Backported from future version 4 (original commit
1a2ae767388a452d99173c4d3043eb2d0e8a5b41)

> Previously the Arg id was set with the "name" attribute. This allows use
> of an "id" attribute to match the underlying struct.
>
> A side effect of this is that the "id" attribute may also be used on
> Commands. This isn't desired, but given the current architecture of the
> attribute parser, it's hard to avoid.
>
> Fixes: #3785
---
 clap_derive/src/attrs.rs |  2 +-
 src/_derive/mod.rs       |  4 ++-
 tests/derive/naming.rs   | 56 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 60 insertions(+), 2 deletions(-)

diff --git a/clap_derive/src/attrs.rs b/clap_derive/src/attrs.rs
index 228c11eba68..2c5b47d9525 100644
--- a/clap_derive/src/attrs.rs
+++ b/clap_derive/src/attrs.rs
@@ -450,7 +450,7 @@ impl Attrs {
     }
 
     fn push_method(&mut self, name: Ident, arg: impl ToTokens) {
-        if name == "name" {
+        if name == "name" || name == "id" {
             self.name = Name::Assigned(quote!(#arg));
         } else if name == "value_parser" {
             self.value_parser = Some(ValueParser::Explicit(Method::new(name, quote!(#arg))));
diff --git a/src/_derive/mod.rs b/src/_derive/mod.rs
index b209ff69467..fc9f35db5b2 100644
--- a/src/_derive/mod.rs
+++ b/src/_derive/mod.rs
@@ -181,8 +181,10 @@
 //! - e.g. `#[clap(max_values(3))]` would translate to `arg.max_values(3)`
 //!
 //! **Magic attributes**:
-//! - `name = <expr>`: [`Arg::id`][crate::Arg::id]
+//! - `id = <expr>`: [`Arg::id`][crate::Arg::id]
 //!   - When not present: case-converted field name is used
+//! - `name = <expr>`: [`Arg::id`][crate::Arg::id]
+//!   - **Deprecated:** use `id`
 //! - `value_parser [= <expr>]`: [`Arg::value_parser`][crate::Arg::value_parser]
 //!   - When not present: will auto-select an implementation based on the field type using
 //!     [`value_parser!][crate::value_parser!]
diff --git a/tests/derive/naming.rs b/tests/derive/naming.rs
index d0b897f7e89..dfd2d344608 100644
--- a/tests/derive/naming.rs
+++ b/tests/derive/naming.rs
@@ -57,6 +57,34 @@ fn test_standalone_long_ignores_afterwards_defined_custom_name() {
     );
 }
 
+#[test]
+fn test_standalone_long_uses_previous_defined_custom_id() {
+    #[derive(Parser, Debug, PartialEq)]
+    struct Opt {
+        #[clap(id = "foo", long)]
+        foo_option: bool,
+    }
+
+    assert_eq!(
+        Opt { foo_option: true },
+        Opt::try_parse_from(&["test", "--foo"]).unwrap()
+    );
+}
+
+#[test]
+fn test_standalone_long_ignores_afterwards_defined_custom_id() {
+    #[derive(Parser, Debug, PartialEq)]
+    struct Opt {
+        #[clap(long, id = "foo")]
+        foo_option: bool,
+    }
+
+    assert_eq!(
+        Opt { foo_option: true },
+        Opt::try_parse_from(&["test", "--foo-option"]).unwrap()
+    );
+}
+
 #[test]
 fn test_standalone_short_generates_kebab_case() {
     #[derive(Parser, Debug, PartialEq)]
@@ -114,6 +142,34 @@ fn test_standalone_short_ignores_afterwards_defined_custom_name() {
     );
 }
 
+#[test]
+fn test_standalone_short_uses_previous_defined_custom_id() {
+    #[derive(Parser, Debug, PartialEq)]
+    struct Opt {
+        #[clap(id = "option", short)]
+        foo_option: bool,
+    }
+
+    assert_eq!(
+        Opt { foo_option: true },
+        Opt::try_parse_from(&["test", "-o"]).unwrap()
+    );
+}
+
+#[test]
+fn test_standalone_short_ignores_afterwards_defined_custom_id() {
+    #[derive(Parser, Debug, PartialEq)]
+    struct Opt {
+        #[clap(short, id = "option")]
+        foo_option: bool,
+    }
+
+    assert_eq!(
+        Opt { foo_option: true },
+        Opt::try_parse_from(&["test", "-f"]).unwrap()
+    );
+}
+
 #[test]
 fn test_standalone_long_uses_previous_defined_casing() {
     #[derive(Parser, Debug, PartialEq)]