From be73195ecfe8a082783765f87036ce9ecf1ce7c8 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Mon, 25 Mar 2024 14:08:51 -0500 Subject: [PATCH 1/3] refactor(derive): Clarify tests --- tests/derive/non_literal_attributes.rs | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/tests/derive/non_literal_attributes.rs b/tests/derive/non_literal_attributes.rs index 63a1d0254c4..25ae36e034c 100644 --- a/tests/derive/non_literal_attributes.rs +++ b/tests/derive/non_literal_attributes.rs @@ -125,19 +125,19 @@ fn test_bool() { assert_eq!(result.unwrap_err().kind(), ErrorKind::NoEquals); } -fn parse_hex(input: &str) -> Result { - u64::from_str_radix(input, 16) -} - -#[derive(Parser, PartialEq, Debug)] -struct HexOpt { - #[arg(short, value_parser = parse_hex)] - number: u64, -} - #[test] #[cfg(feature = "error-context")] fn test_parse_hex_function_path() { + #[derive(Parser, PartialEq, Debug)] + struct HexOpt { + #[arg(short, value_parser = parse_hex)] + number: u64, + } + + fn parse_hex(input: &str) -> Result { + u64::from_str_radix(input, 16) + } + assert_eq!( HexOpt { number: 5 }, HexOpt::try_parse_from(["test", "-n", "5"]).unwrap() From 8eab48fa3c9d6a40128c601b2520d33c7b3e6c2a Mon Sep 17 00:00:00 2001 From: Ed Page Date: Mon, 25 Mar 2024 14:35:50 -0500 Subject: [PATCH 2/3] refactor(derive): Make it easier to work with 'Name' --- clap_derive/src/item.rs | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/clap_derive/src/item.rs b/clap_derive/src/item.rs index 114849f6950..e66af319a85 100644 --- a/clap_derive/src/item.rs +++ b/clap_derive/src/item.rs @@ -974,8 +974,8 @@ impl Item { quote!( #(#doc_comment)* #(#methods)* ) } - pub fn group_id(&self) -> TokenStream { - self.group_id.clone().raw() + pub fn group_id(&self) -> &Name { + &self.group_id } pub fn group_methods(&self) -> TokenStream { @@ -998,8 +998,8 @@ impl Item { quote!( #(#next_help_heading)* ) } - pub fn id(&self) -> TokenStream { - self.name.clone().raw() + pub fn id(&self) -> &Name { + &self.name } pub fn cased_name(&self) -> TokenStream { @@ -1410,16 +1410,6 @@ pub enum Name { } impl Name { - 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::*; @@ -1466,3 +1456,15 @@ impl Name { } } } + +impl ToTokens for Name { + fn to_tokens(&self, tokens: &mut TokenStream) { + match self { + Name::Assigned(t) => t.to_tokens(tokens), + Name::Derived(ident) => { + let s = ident.unraw().to_string(); + quote_spanned!(ident.span()=> #s).to_tokens(tokens) + } + } + } +} From df915fefefba42493290c4f7c4ac627807e9ebd4 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Mon, 25 Mar 2024 14:42:22 -0500 Subject: [PATCH 3/3] fix(derive): Re-allow expressions for id's Fixes #5407 --- clap_derive/src/derives/args.rs | 18 +++++++++++++++--- tests/derive/non_literal_attributes.rs | 17 +++++++++++++++++ 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/clap_derive/src/derives/args.rs b/clap_derive/src/derives/args.rs index 279f6ca0d7d..b1b16de5003 100644 --- a/clap_derive/src/derives/args.rs +++ b/clap_derive/src/derives/args.rs @@ -717,9 +717,21 @@ fn gen_parsers( }, Ty::Other => { - quote_spanned! { ty.span()=> - #arg_matches.#get_one(#id) - .ok_or_else(|| clap::Error::raw(clap::error::ErrorKind::MissingRequiredArgument, concat!("The following required argument was not provided: ", #id)))? + // Prefer `concat` where possible for reduced code size but fallback to `format!` to + // allow non-literal `id`s + match id { + Name::Assigned(_) => { + quote_spanned! { ty.span()=> + #arg_matches.#get_one(#id) + .ok_or_else(|| clap::Error::raw(clap::error::ErrorKind::MissingRequiredArgument, format!("The following required argument was not provided: {}", #id)))? + } + } + Name::Derived(_) => { + quote_spanned! { ty.span()=> + #arg_matches.#get_one(#id) + .ok_or_else(|| clap::Error::raw(clap::error::ErrorKind::MissingRequiredArgument, concat!("The following required argument was not provided: ", #id)))? + } + } } } }; diff --git a/tests/derive/non_literal_attributes.rs b/tests/derive/non_literal_attributes.rs index 25ae36e034c..eba4124f567 100644 --- a/tests/derive/non_literal_attributes.rs +++ b/tests/derive/non_literal_attributes.rs @@ -156,3 +156,20 @@ fn test_parse_hex_function_path() { err ); } + +#[test] +#[cfg(feature = "error-context")] +fn test_const_name() { + #[derive(Parser, PartialEq, Debug)] + struct Opt { + #[arg(id = NAME, short, long)] + number: u64, + } + + const NAME: &str = "fun"; + + assert_eq!( + Opt { number: 5 }, + Opt::try_parse_from(["test", "-f", "5"]).unwrap() + ); +}