Skip to content

Commit

Permalink
fix(derive): Don't change case of Arg id's (unstable)
Browse files Browse the repository at this point in the history 
This will make it easier to reference arguments with different
attributes.

Fixes #3282
  • Loading branch information
@epage
epage committed May 9, 2022
1 parent f7e4dd2 commit 2e35403
Show file tree
Hide file tree
Showing 4 changed files with 165 additions and 69 deletions.
21 changes: 21 additions & 0 deletions clap_derive/src/attrs.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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::*;

Expand Down
38 changes: 19 additions & 19 deletions clap_derive/src/derives/args.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
);
Expand Down Expand Up @@ -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),
Expand All @@ -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),
Expand All @@ -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),
Expand All @@ -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)))
}
}

Expand All @@ -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))
Expand All @@ -614,33 +614,33 @@ 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)
}
}

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)?
}
}
};

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
}
Expand Down
165 changes: 119 additions & 46 deletions tests/derive/help.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand All @@ -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")
Expand Down Expand Up @@ -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"));
}

Expand All @@ -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"));
}

Expand Down Expand Up @@ -218,15 +279,27 @@ 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>
clap <SUBCOMMAND>
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]
Expand Down
Loading

0 comments on commit 2e35403

Please sign in to comment.