fix(derive): Don't change case of Arg id's (unstable)

This will make it easier to reference arguments with different
attributes.

Fixes #3282
This commit is contained in:
Ed Page 2022-05-09 10:35:52 -05:00
parent f7e4dd23d6
commit 2e3540355a
4 changed files with 165 additions and 69 deletions

View file

@ -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::*;

View file

@ -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
}

View file

@ -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]

View file

@ -296,7 +296,7 @@ fn test_rename_all_is_propagated_from_enum_to_variants() {
FirstVariant,
SecondVariant {
#[clap(long)]
foo: bool,
foo: String,
},
}
@ -314,13 +314,15 @@ fn test_rename_all_is_propagated_from_enum_to_variant_fields() {
FirstVariant,
SecondVariant {
#[clap(long)]
foo: bool,
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()
);
}