From a628b44f51338349d29e9ed2be360a35530290a8 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 6 Oct 2021 13:11:25 -0500 Subject: [PATCH] fix(derive): Subcommands not working within macro_rules This carries over a test case from https://github.com/TeXitoi/structopt/pull/448, and re-fixes it according to the changes we've made since we forked. I also tried to identify other cases and quote them to avoid playing whack-a-mole with this. This is a part of #2809 --- clap_derive/src/derives/args.rs | 10 ++--- clap_derive/src/derives/into_app.rs | 9 ++-- clap_derive/src/derives/subcommand.rs | 64 ++++++++++++++------------- clap_derive/tests/nested.rs | 20 +++++++++ 4 files changed, 64 insertions(+), 39 deletions(-) diff --git a/clap_derive/src/derives/args.rs b/clap_derive/src/derives/args.rs index 9e1d1ebba63f..e44c853135ce 100644 --- a/clap_derive/src/derives/args.rs +++ b/clap_derive/src/derives/args.rs @@ -20,7 +20,7 @@ use crate::{ use proc_macro2::{Ident, Span, TokenStream}; use proc_macro_error::{abort, abort_call_site}; -use quote::{quote, quote_spanned}; +use quote::{format_ident, quote, quote_spanned}; use syn::{ punctuated::Punctuated, spanned::Spanned, token::Comma, Attribute, Data, DataStruct, DeriveInput, Field, Fields, Type, @@ -374,7 +374,7 @@ pub fn gen_constructor(fields: &Punctuated, parent_attribute: &Att ); let field_name = field.ident.as_ref().unwrap(); let kind = attrs.kind(); - let arg_matches = quote! { arg_matches }; + let arg_matches = format_ident!("arg_matches"); match &*kind { Kind::ExternalSubcommand => { abort! { kind.span(), @@ -440,7 +440,7 @@ pub fn gen_updater( } else { quote!() }; - let arg_matches = quote! { arg_matches }; + let arg_matches = format_ident!("arg_matches"); match &*kind { Kind::ExternalSubcommand => { @@ -542,10 +542,10 @@ fn gen_parsers( let flag = *attrs.parser().kind == ParserKind::FromFlag; let occurrences = *attrs.parser().kind == ParserKind::FromOccurrences; let name = attrs.cased_name(); - // Use `quote!` to give this identifier the same hygiene + // Give this identifier the same hygiene // as the `arg_matches` parameter definition. This // allows us to refer to `arg_matches` within a `quote_spanned` block - let arg_matches = quote! { arg_matches }; + let arg_matches = format_ident!("arg_matches"); let field_value = match **ty { Ty::Bool => { diff --git a/clap_derive/src/derives/into_app.rs b/clap_derive/src/derives/into_app.rs index 1a7b144b6125..276b072213f7 100644 --- a/clap_derive/src/derives/into_app.rs +++ b/clap_derive/src/derives/into_app.rs @@ -96,6 +96,7 @@ pub fn gen_for_enum(enum_name: &Ident, attrs: &[Attribute]) -> TokenStream { Sp::call_site(DEFAULT_ENV_CASING), ); let name = attrs.cased_name(); + let app_var = Ident::new("app", Span::call_site()); quote! { #[allow(dead_code, unreachable_code, unused_variables)] @@ -112,14 +113,14 @@ pub fn gen_for_enum(enum_name: &Ident, attrs: &[Attribute]) -> TokenStream { #[deny(clippy::correctness)] impl clap::IntoApp for #enum_name { fn into_app<'b>() -> clap::App<'b> { - let app = clap::App::new(#name) + let #app_var = clap::App::new(#name) .setting(clap::AppSettings::SubcommandRequiredElseHelp); - <#enum_name as clap::Subcommand>::augment_subcommands(app) + <#enum_name as clap::Subcommand>::augment_subcommands(#app_var) } fn into_app_for_update<'b>() -> clap::App<'b> { - let app = clap::App::new(#name); - <#enum_name as clap::Subcommand>::augment_subcommands_for_update(app) + let #app_var = clap::App::new(#name); + <#enum_name as clap::Subcommand>::augment_subcommands_for_update(#app_var) } } } diff --git a/clap_derive/src/derives/subcommand.rs b/clap_derive/src/derives/subcommand.rs index 0b0139fbdc52..5b067e7cf5f1 100644 --- a/clap_derive/src/derives/subcommand.rs +++ b/clap_derive/src/derives/subcommand.rs @@ -20,7 +20,7 @@ use crate::{ use proc_macro2::{Ident, Span, TokenStream}; use proc_macro_error::{abort, abort_call_site}; -use quote::{quote, quote_spanned}; +use quote::{format_ident, quote, quote_spanned}; use syn::{ punctuated::Punctuated, spanned::Spanned, Attribute, Data, DataEnum, DeriveInput, FieldsUnnamed, Token, Variant, @@ -119,6 +119,8 @@ fn gen_augment( ) -> TokenStream { use syn::Fields::*; + let app_var = Ident::new("app", Span::call_site()); + let subcommands: Vec<_> = variants .iter() .map(|variant| { @@ -145,11 +147,11 @@ fn gen_augment( Some(subty) => { if is_simple_ty(subty, "OsString") { quote_spanned! { kind.span()=> - let app = app.setting(clap::AppSettings::AllowExternalSubcommands).setting(clap::AppSettings::AllowInvalidUtf8ForExternalSubcommands); + let #app_var = #app_var.setting(clap::AppSettings::AllowExternalSubcommands).setting(clap::AppSettings::AllowInvalidUtf8ForExternalSubcommands); } } else { quote_spanned! { kind.span()=> - let app = app.setting(clap::AppSettings::AllowExternalSubcommands); + let #app_var = #app_var.setting(clap::AppSettings::AllowExternalSubcommands); } } } @@ -167,11 +169,11 @@ fn gen_augment( let ty = &unnamed[0]; if override_required { quote! { - let app = <#ty as clap::Subcommand>::augment_subcommands_for_update(app); + let #app_var = <#ty as clap::Subcommand>::augment_subcommands_for_update(#app_var); } } else { quote! { - let app = <#ty as clap::Subcommand>::augment_subcommands(app); + let #app_var = <#ty as clap::Subcommand>::augment_subcommands(#app_var); } } } @@ -182,24 +184,24 @@ fn gen_augment( }, Kind::Subcommand(_) => { - let app_var = Ident::new("subcommand", Span::call_site()); + let subcommand_var = Ident::new("subcommand", Span::call_site()); let arg_block = match variant.fields { Named(_) => { abort!(variant, "non single-typed tuple enums are not supported") } - Unit => quote!( #app_var ), + Unit => quote!( #subcommand_var ), Unnamed(FieldsUnnamed { ref unnamed, .. }) if unnamed.len() == 1 => { let ty = &unnamed[0]; if override_required { quote_spanned! { ty.span()=> { - <#ty as clap::Subcommand>::augment_subcommands_for_update(#app_var) + <#ty as clap::Subcommand>::augment_subcommands_for_update(#subcommand_var) } } } else { quote_spanned! { ty.span()=> { - <#ty as clap::Subcommand>::augment_subcommands(#app_var) + <#ty as clap::Subcommand>::augment_subcommands(#subcommand_var) } } } @@ -213,34 +215,34 @@ fn gen_augment( let from_attrs = attrs.top_level_methods(); let version = attrs.version(); quote! { - let app = app.subcommand({ - let #app_var = clap::App::new(#name); - let #app_var = #arg_block; - let #app_var = #app_var.setting(::clap::AppSettings::SubcommandRequiredElseHelp); - #app_var#from_attrs#version + let #app_var = #app_var.subcommand({ + let #subcommand_var = clap::App::new(#name); + let #subcommand_var = #arg_block; + let #subcommand_var = #subcommand_var.setting(::clap::AppSettings::SubcommandRequiredElseHelp); + #subcommand_var#from_attrs#version }); } } _ => { - let app_var = Ident::new("subcommand", Span::call_site()); + let subcommand_var = Ident::new("subcommand", Span::call_site()); let arg_block = match variant.fields { Named(ref fields) => { - args::gen_augment(&fields.named, &app_var, &attrs, override_required) + args::gen_augment(&fields.named, &subcommand_var, &attrs, override_required) } - Unit => quote!( #app_var ), + Unit => quote!( #subcommand_var ), Unnamed(FieldsUnnamed { ref unnamed, .. }) if unnamed.len() == 1 => { let ty = &unnamed[0]; if override_required { quote_spanned! { ty.span()=> { - <#ty as clap::Args>::augment_args_for_update(#app_var) + <#ty as clap::Args>::augment_args_for_update(#subcommand_var) } } } else { quote_spanned! { ty.span()=> { - <#ty as clap::Args>::augment_args(#app_var) + <#ty as clap::Args>::augment_args(#subcommand_var) } } } @@ -254,10 +256,10 @@ fn gen_augment( let from_attrs = attrs.top_level_methods(); let version = attrs.version(); quote! { - let app = app.subcommand({ - let #app_var = clap::App::new(#name); - let #app_var = #arg_block; - #app_var#from_attrs#version + let #app_var = #app_var.subcommand({ + let #subcommand_var = clap::App::new(#name); + let #subcommand_var = #arg_block; + #subcommand_var#from_attrs#version }); } } @@ -268,9 +270,9 @@ fn gen_augment( let app_methods = parent_attribute.top_level_methods(); let version = parent_attribute.version(); quote! { - let app = app #app_methods; + let #app_var = #app_var #app_methods; #( #subcommands )*; - app #version + #app_var #version } } @@ -350,6 +352,8 @@ fn gen_from_arg_matches( let mut ext_subcmd = None; + let subcommand_name_var = format_ident!("name"); + let sub_arg_matches_var = format_ident!("sub_arg_matches"); let (flatten_variants, variants): (Vec<_>, Vec<_>) = variants .iter() .filter_map(|variant| { @@ -434,7 +438,7 @@ fn gen_from_arg_matches( }; quote! { - if #sub_name == name { + if #sub_name == #subcommand_name_var { return Some(#name :: #variant_name #constructor_block) } } @@ -460,9 +464,9 @@ fn gen_from_arg_matches( let wildcard = match ext_subcmd { Some((span, var_name, str_ty, values_of)) => quote_spanned! { span=> ::std::option::Option::Some(#name::#var_name( - ::std::iter::once(#str_ty::from(name)) + ::std::iter::once(#str_ty::from(#subcommand_name_var)) .chain( - sub_arg_matches.#values_of("").into_iter().flatten().map(#str_ty::from) + #sub_arg_matches_var.#values_of("").into_iter().flatten().map(#str_ty::from) ) .collect::<::std::vec::Vec<_>>() )) @@ -473,9 +477,9 @@ fn gen_from_arg_matches( quote! { fn from_arg_matches(arg_matches: &clap::ArgMatches) -> Option { - if let Some((name, sub_arg_matches)) = arg_matches.subcommand() { + if let Some((#subcommand_name_var, #sub_arg_matches_var)) = arg_matches.subcommand() { { - let arg_matches = sub_arg_matches; + let arg_matches = #sub_arg_matches_var; #( #subcommands )* } diff --git a/clap_derive/tests/nested.rs b/clap_derive/tests/nested.rs index 1642aba04fa3..c185912e3b59 100644 --- a/clap_derive/tests/nested.rs +++ b/clap_derive/tests/nested.rs @@ -31,3 +31,23 @@ fn use_option() { expand_ty!(my_field: Option); } + +#[test] +fn issue_447() { + macro_rules! Command { + ( $name:ident, [ + #[$meta:meta] $var:ident($inner:ty) + ] ) => { + #[derive(Debug, PartialEq, clap::Clap)] + enum $name { + #[$meta] + $var($inner), + } + }; + } + + Command! {GitCmd, [ + #[clap(external_subcommand)] + Ext(Vec) + ]} +}