From bff380f4f8d3c493fcd9d5afda9b077dc824ee30 Mon Sep 17 00:00:00 2001 From: Nikhil Gupta <17176722+gupnik@users.noreply.github.com> Date: Wed, 4 Sep 2024 15:08:06 +0530 Subject: [PATCH 1/3] Adds generics support in derive-impl --- .../support/procedural/src/derive_impl.rs | 74 +++++++++++++++++-- substrate/frame/support/procedural/src/lib.rs | 1 + 2 files changed, 68 insertions(+), 7 deletions(-) diff --git a/substrate/frame/support/procedural/src/derive_impl.rs b/substrate/frame/support/procedural/src/derive_impl.rs index 54755f1163a1..6d268425c3d3 100644 --- a/substrate/frame/support/procedural/src/derive_impl.rs +++ b/substrate/frame/support/procedural/src/derive_impl.rs @@ -17,13 +17,13 @@ //! Implementation of the `derive_impl` attribute macro. -use derive_syn_parse::Parse; use macro_magic::mm_core::ForeignPath; use proc_macro2::TokenStream as TokenStream2; use quote::{quote, ToTokens}; use std::collections::HashSet; use syn::{ - parse2, parse_quote, spanned::Spanned, token, Ident, ImplItem, ItemImpl, Path, Result, Token, + parse2, parse_quote, spanned::Spanned, token, AngleBracketedGenericArguments, Ident, ImplItem, + ItemImpl, Path, PathArguments, PathSegment, Result, Token, }; mod keyword { @@ -56,18 +56,61 @@ fn is_runtime_type(item: &syn::ImplItemType) -> bool { false }) } - -#[derive(Parse, Debug)] pub struct DeriveImplAttrArgs { pub default_impl_path: Path, + pub generics: Option, _as: Option, - #[parse_if(_as.is_some())] pub disambiguation_path: Option, _comma: Option, - #[parse_if(_comma.is_some())] pub no_aggregated_types: Option, } +impl syn::parse::Parse for DeriveImplAttrArgs { + fn parse(input: syn::parse::ParseStream) -> Result { + let mut default_impl_path: Path = input.parse()?; + // Extract the generics if any + let (default_impl_path, generics) = match default_impl_path.clone().segments.last() { + Some(PathSegment { ident, arguments: PathArguments::AngleBracketed(args) }) => { + default_impl_path.segments.pop(); + default_impl_path + .segments + .push(PathSegment { ident: ident.clone(), arguments: PathArguments::None }); + // let mut args = args.clone(); + // args.colon2_token = None; + (default_impl_path, Some(args.clone())) + }, + _ => (default_impl_path, None), + }; + + let lookahead = input.lookahead1(); + let (_as, disambiguation_path) = if lookahead.peek(Token![as]) { + let _as: Token![as] = input.parse()?; + let disambiguation_path: Path = input.parse()?; + (Some(_as), Some(disambiguation_path)) + } else { + (None, None) + }; + + let lookahead = input.lookahead1(); + let (_comma, no_aggregated_types) = if lookahead.peek(Token![,]) { + let _comma: Token![,] = input.parse()?; + let no_aggregated_types: keyword::no_aggregated_types = input.parse()?; + (Some(_comma), Some(no_aggregated_types)) + } else { + (None, None) + }; + + Ok(DeriveImplAttrArgs { + default_impl_path, + generics, + _as, + disambiguation_path, + _comma, + no_aggregated_types, + }) + } +} + impl ForeignPath for DeriveImplAttrArgs { fn foreign_path(&self) -> &Path { &self.default_impl_path @@ -77,6 +120,7 @@ impl ForeignPath for DeriveImplAttrArgs { impl ToTokens for DeriveImplAttrArgs { fn to_tokens(&self, tokens: &mut TokenStream2) { tokens.extend(self.default_impl_path.to_token_stream()); + tokens.extend(self.generics.to_token_stream()); tokens.extend(self._as.to_token_stream()); tokens.extend(self.disambiguation_path.to_token_stream()); tokens.extend(self._comma.to_token_stream()); @@ -117,6 +161,7 @@ fn combine_impls( default_impl_path: Path, disambiguation_path: Path, inject_runtime_types: bool, + generics: Option, ) -> ItemImpl { let (existing_local_keys, existing_unsupported_items): (HashSet, HashSet) = local_impl @@ -155,7 +200,7 @@ fn combine_impls( // modify and insert uncolliding type items let modified_item: ImplItem = parse_quote! { #( #cfg_attrs )* - type #ident = <#default_impl_path as #disambiguation_path>::#ident; + type #ident = <#default_impl_path #generics as #disambiguation_path>::#ident; }; return Some(modified_item) } @@ -216,6 +261,7 @@ pub fn derive_impl( local_tokens: TokenStream2, disambiguation_path: Option, no_aggregated_types: Option, + generics: Option, ) -> Result { let local_impl = parse2::(local_tokens)?; let foreign_impl = parse2::(foreign_tokens)?; @@ -234,6 +280,7 @@ pub fn derive_impl( default_impl_path, disambiguation_path, no_aggregated_types.is_none(), + generics, ); Ok(quote!(#combined_impl)) @@ -301,3 +348,16 @@ fn test_disambiguation_path() { compute_disambiguation_path(None, foreign_impl.clone(), parse_quote!(SomeType)); assert_eq!(disambiguation_path.unwrap(), parse_quote!(SomeTrait)); } + +#[test] +fn test_derive_impl_attr_args_parsing_with_generic() { + let args = parse2::(quote!( + some::path::TestDefaultConfig as some::path::DefaultConfig + )) + .unwrap(); + assert_eq!(args.default_impl_path, parse_quote!(some::path::TestDefaultConfig)); + assert_eq!(args.generics.unwrap().args[0], parse_quote!(Config)); + let args = parse2::(quote!(TestDefaultConfig)).unwrap(); + assert_eq!(args.default_impl_path, parse_quote!(TestDefaultConfig)); + assert_eq!(args.generics.unwrap().args[0], parse_quote!(Config2)); +} diff --git a/substrate/frame/support/procedural/src/lib.rs b/substrate/frame/support/procedural/src/lib.rs index ef99faee86ae..9924312068da 100644 --- a/substrate/frame/support/procedural/src/lib.rs +++ b/substrate/frame/support/procedural/src/lib.rs @@ -682,6 +682,7 @@ pub fn derive_impl(attrs: TokenStream, input: TokenStream) -> TokenStream { input.into(), custom_attrs.disambiguation_path, custom_attrs.no_aggregated_types, + custom_attrs.generics, ) .unwrap_or_else(|r| r.into_compile_error()) .into() From 47b834dbda2ce8880af5763f2bad8998772e8b38 Mon Sep 17 00:00:00 2001 From: Nikhil Gupta <17176722+gupnik@users.noreply.github.com> Date: Wed, 4 Sep 2024 15:16:25 +0530 Subject: [PATCH 2/3] Minor --- substrate/frame/support/procedural/src/derive_impl.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/substrate/frame/support/procedural/src/derive_impl.rs b/substrate/frame/support/procedural/src/derive_impl.rs index 6d268425c3d3..2950ae8e4c0a 100644 --- a/substrate/frame/support/procedural/src/derive_impl.rs +++ b/substrate/frame/support/procedural/src/derive_impl.rs @@ -75,8 +75,6 @@ impl syn::parse::Parse for DeriveImplAttrArgs { default_impl_path .segments .push(PathSegment { ident: ident.clone(), arguments: PathArguments::None }); - // let mut args = args.clone(); - // args.colon2_token = None; (default_impl_path, Some(args.clone())) }, _ => (default_impl_path, None), From d8ef5337a5a377af76c35eb09aaea9f58f3a6cd8 Mon Sep 17 00:00:00 2001 From: Nikhil Gupta <17176722+gupnik@users.noreply.github.com> Date: Wed, 18 Sep 2024 11:15:40 +0530 Subject: [PATCH 3/3] Addresses review comment --- substrate/frame/support/procedural/src/derive_impl.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/substrate/frame/support/procedural/src/derive_impl.rs b/substrate/frame/support/procedural/src/derive_impl.rs index 2950ae8e4c0a..69117c026816 100644 --- a/substrate/frame/support/procedural/src/derive_impl.rs +++ b/substrate/frame/support/procedural/src/derive_impl.rs @@ -77,7 +77,8 @@ impl syn::parse::Parse for DeriveImplAttrArgs { .push(PathSegment { ident: ident.clone(), arguments: PathArguments::None }); (default_impl_path, Some(args.clone())) }, - _ => (default_impl_path, None), + Some(PathSegment { arguments: PathArguments::None, .. }) => (default_impl_path, None), + _ => return Err(syn::Error::new(default_impl_path.span(), "Invalid default impl path")), }; let lookahead = input.lookahead1();