Skip to content

Commit

Permalink
feat(derive): Reserve the T group name
Browse files Browse the repository at this point in the history
Since the `name` is changed to be the package name, we can't use it as
(1) its not as predictable and (2) it can lead to conflicts if a
`Parser` is flattened into a `Parser`
  • Loading branch information
epage committed Sep 13, 2022
1 parent 01c0975 commit d3bf545
Show file tree
Hide file tree
Showing 6 changed files with 58 additions and 6 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -198,6 +198,7 @@ Subtle changes (i.e. compiler won't catch):
- *(env)* Parse `--help` and `--version` like any `ArgAction::SetTrue` flag (#3776)
- *(derive)* Leave `Arg::id` as `verbatim` casing, requiring updating of string references to other args like in `conflicts_with` or `requires` (#3282)
- *(derive)* Doc comments for `ValueEnum` variants will now show up in `--help` (#3312)
- *(derive)* When deriving `Args`, and `ArgGroup` is created using the type's name, reserving it for future use (#2621, #4209)

Easier to catch changes:

Expand Down
6 changes: 5 additions & 1 deletion clap_derive/src/derives/args.rs
Expand Up @@ -15,6 +15,7 @@
use proc_macro2::{Ident, Span, TokenStream};
use proc_macro_error::{abort, abort_call_site};
use quote::{format_ident, quote, quote_spanned};
use syn::ext::IdentExt;
use syn::{
punctuated::Punctuated, spanned::Spanned, token::Comma, Data, DataStruct, DeriveInput, Field,
Fields, Generics,
Expand Down Expand Up @@ -317,10 +318,13 @@ pub fn gen_augment(
quote!()
};
let initial_app_methods = parent_item.initial_top_level_methods();
let group_id = parent_item.ident().unraw().to_string();
let final_app_methods = parent_item.final_top_level_methods();
quote! {{
#deprecations
let #app_var = #app_var #initial_app_methods;
let #app_var = #app_var
#initial_app_methods
.group(clap::ArgGroup::new(#group_id).multiple(true));
#( #args )*
#app_var #final_app_methods
}}
Expand Down
30 changes: 26 additions & 4 deletions clap_derive/src/item.rs
Expand Up @@ -36,6 +36,7 @@ pub const DEFAULT_ENV_CASING: CasingStyle = CasingStyle::ScreamingSnake;
#[derive(Clone)]
pub struct Item {
name: Name,
ident: Ident,
casing: Sp<CasingStyle>,
env_casing: Sp<CasingStyle>,
ty: Option<Type>,
Expand All @@ -54,13 +55,14 @@ pub struct Item {

impl Item {
pub fn from_args_struct(input: &DeriveInput, name: Name) -> Self {
let ident = input.ident.clone();
let span = input.ident.span();
let attrs = &input.attrs;
let argument_casing = Sp::new(DEFAULT_CASING, span);
let env_casing = Sp::new(DEFAULT_ENV_CASING, span);
let kind = Sp::new(Kind::Command(Sp::new(Ty::Other, span)), span);

let mut res = Self::new(name, None, argument_casing, env_casing, kind);
let mut res = Self::new(name, ident, None, argument_casing, env_casing, kind);
let parsed_attrs = ClapAttr::parse_all(attrs);
res.infer_kind(&parsed_attrs);
res.push_attrs(&parsed_attrs);
Expand All @@ -70,13 +72,14 @@ impl Item {
}

pub fn from_subcommand_enum(input: &DeriveInput, name: Name) -> Self {
let ident = input.ident.clone();
let span = input.ident.span();
let attrs = &input.attrs;
let argument_casing = Sp::new(DEFAULT_CASING, span);
let env_casing = Sp::new(DEFAULT_ENV_CASING, span);
let kind = Sp::new(Kind::Command(Sp::new(Ty::Other, span)), span);

let mut res = Self::new(name, None, argument_casing, env_casing, kind);
let mut res = Self::new(name, ident, None, argument_casing, env_casing, kind);
let parsed_attrs = ClapAttr::parse_all(attrs);
res.infer_kind(&parsed_attrs);
res.push_attrs(&parsed_attrs);
Expand All @@ -86,13 +89,14 @@ impl Item {
}

pub fn from_value_enum(input: &DeriveInput, name: Name) -> Self {
let ident = input.ident.clone();
let span = input.ident.span();
let attrs = &input.attrs;
let argument_casing = Sp::new(DEFAULT_CASING, span);
let env_casing = Sp::new(DEFAULT_ENV_CASING, span);
let kind = Sp::new(Kind::Value, span);

let mut res = Self::new(name, None, argument_casing, env_casing, kind);
let mut res = Self::new(name, ident, None, argument_casing, env_casing, kind);
let parsed_attrs = ClapAttr::parse_all(attrs);
res.infer_kind(&parsed_attrs);
res.push_attrs(&parsed_attrs);
Expand All @@ -116,6 +120,7 @@ impl Item {
env_casing: Sp<CasingStyle>,
) -> Self {
let name = variant.ident.clone();
let ident = variant.ident.clone();
let span = variant.span();
let ty = match variant.fields {
syn::Fields::Unnamed(syn::FieldsUnnamed { ref unnamed, .. }) if unnamed.len() == 1 => {
Expand All @@ -126,7 +131,14 @@ impl Item {
}
};
let kind = Sp::new(Kind::Command(ty), span);
let mut res = Self::new(Name::Derived(name), None, struct_casing, env_casing, kind);
let mut res = Self::new(
Name::Derived(name),
ident,
None,
struct_casing,
env_casing,
kind,
);
let parsed_attrs = ClapAttr::parse_all(&variant.attrs);
res.infer_kind(&parsed_attrs);
res.push_attrs(&parsed_attrs);
Expand Down Expand Up @@ -161,10 +173,12 @@ impl Item {
argument_casing: Sp<CasingStyle>,
env_casing: Sp<CasingStyle>,
) -> Self {
let ident = variant.ident.clone();
let span = variant.span();
let kind = Sp::new(Kind::Value, span);
let mut res = Self::new(
Name::Derived(variant.ident.clone()),
ident,
None,
argument_casing,
env_casing,
Expand All @@ -186,11 +200,13 @@ impl Item {
env_casing: Sp<CasingStyle>,
) -> Self {
let name = field.ident.clone().unwrap();
let ident = field.ident.clone().unwrap();
let span = field.span();
let ty = Ty::from_syn_ty(&field.ty);
let kind = Sp::new(Kind::Arg(ty), span);
let mut res = Self::new(
Name::Derived(name),
ident,
Some(field.ty.clone()),
struct_casing,
env_casing,
Expand Down Expand Up @@ -234,13 +250,15 @@ impl Item {

fn new(
name: Name,
ident: Ident,
ty: Option<Type>,
casing: Sp<CasingStyle>,
env_casing: Sp<CasingStyle>,
kind: Sp<Kind>,
) -> Self {
Self {
name,
ident,
ty,
casing,
env_casing,
Expand Down Expand Up @@ -894,6 +912,10 @@ impl Item {
quote!( #(#next_help_heading)* )
}

pub fn ident(&self) -> &Ident {
&self.ident
}

pub fn id(&self) -> TokenStream {
self.name.clone().raw()
}
Expand Down
23 changes: 23 additions & 0 deletions tests/derive/groups.rs
@@ -0,0 +1,23 @@
use clap::Parser;

#[test]
fn test_safely_nest_parser() {
#[derive(Parser, Debug, PartialEq)]
struct Opt {
#[command(flatten)]
foo: Foo,
}

#[derive(Parser, Debug, PartialEq)]
struct Foo {
#[arg(long)]
foo: bool,
}

assert_eq!(
Opt {
foo: Foo { foo: true }
},
Opt::try_parse_from(&["test", "--foo"]).unwrap()
);
}
1 change: 1 addition & 0 deletions tests/derive/main.rs
Expand Up @@ -13,6 +13,7 @@ mod explicit_name_no_renaming;
mod flags;
mod flatten;
mod generic;
mod groups;
mod help;
mod issues;
mod macros;
Expand Down
3 changes: 2 additions & 1 deletion tests/derive/naming.rs
@@ -1,3 +1,4 @@
use clap::Args;
use clap::Parser;

#[test]
Expand Down Expand Up @@ -251,7 +252,7 @@ fn test_rename_all_is_not_propagated_from_struct_into_flattened() {
foo: Foo,
}

#[derive(Parser, Debug, PartialEq)]
#[derive(Args, Debug, PartialEq)]
struct Foo {
#[arg(long)]
foo: bool,
Expand Down

0 comments on commit d3bf545

Please sign in to comment.