Fixes parsing for config attrs in pallet macros (#2677)

substrate/frame/support/procedural/src/derive_impl.rs

@@ -136,9 +136,15 @@ fn combine_impls(
 				return None
 			}
 			if let ImplItem::Type(typ) = item.clone() {
+				let cfg_attrs = typ
+					.attrs
+					.iter()
+					.filter(|attr| attr.path().get_ident().map_or(false, |ident| ident == "cfg"))
+					.map(|attr| attr.to_token_stream());
 				if is_runtime_type(&typ) {
 					let item: ImplItem = if inject_runtime_types {
 						parse_quote! {
+							#( #cfg_attrs )*
 							type #ident = #ident;
 						}
 					} else {
@@ -148,6 +154,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;
 				};
 				return Some(modified_item)

substrate/frame/support/procedural/src/pallet/expand/call.rs

@@ -241,6 +241,15 @@ pub fn expand_call(def: &mut Def) -> proc_macro2::TokenStream {
 		})
 		.collect::<Vec<_>>();
 
+	let cfg_attrs = methods
+		.iter()
+		.map(|method| {
+			let attrs =
+				method.cfg_attrs.iter().map(|attr| attr.to_token_stream()).collect::<Vec<_>>();
+			quote::quote!( #( #attrs )* )
+		})
+		.collect::<Vec<_>>();
+
 	let feeless_check = methods.iter().map(|method| &method.feeless_check).collect::<Vec<_>>();
 	let feeless_check_result =
 		feeless_check.iter().zip(args_name.iter()).map(|(feeless_check, arg_name)| {
@@ -297,6 +306,7 @@ pub fn expand_call(def: &mut Def) -> proc_macro2::TokenStream {
 				#frame_support::Never,
 			),
 			#(
+				#cfg_attrs
 				#[doc = #fn_doc]
 				#[codec(index = #call_index)]
 				#fn_name {
@@ -310,6 +320,7 @@ pub fn expand_call(def: &mut Def) -> proc_macro2::TokenStream {
 
 		impl<#type_impl_gen> #call_ident<#type_use_gen> #where_clause {
 			#(
+				#cfg_attrs
 				#[doc = #new_call_variant_doc]
 				pub fn #new_call_variant_fn_name(
 					#( #args_name_stripped: #args_type ),*
@@ -328,6 +339,7 @@ pub fn expand_call(def: &mut Def) -> proc_macro2::TokenStream {
 			fn get_dispatch_info(&self) -> #frame_support::dispatch::DispatchInfo {
 				match *self {
 					#(
+						#cfg_attrs
 						Self::#fn_name { #( #args_name_pattern_ref, )* } => {
 							let __pallet_base_weight = #fn_weight;
 
@@ -365,6 +377,7 @@ pub fn expand_call(def: &mut Def) -> proc_macro2::TokenStream {
 			fn is_feeless(&self, origin: &Self::Origin) -> bool {
 				match *self {
 					#(
+						#cfg_attrs
 						Self::#fn_name { #( #args_name_pattern_ref, )* } => {
 							#feeless_check_result
 						},
@@ -379,13 +392,13 @@ pub fn expand_call(def: &mut Def) -> proc_macro2::TokenStream {
 		{
 			fn get_call_name(&self) -> &'static str {
 				match *self {
-					#( Self::#fn_name { .. } => stringify!(#fn_name), )*
+					#( #cfg_attrs Self::#fn_name { .. } => stringify!(#fn_name), )*
 					Self::__Ignore(_, _) => unreachable!("__PhantomItem cannot be used."),
 				}
 			}
 
 			fn get_call_names() -> &'static [&'static str] {
-				&[ #( stringify!(#fn_name), )* ]
+				&[ #( #cfg_attrs stringify!(#fn_name), )* ]
 			}
 		}
 
@@ -394,13 +407,13 @@ pub fn expand_call(def: &mut Def) -> proc_macro2::TokenStream {
 		{
 			fn get_call_index(&self) -> u8 {
 				match *self {
-					#( Self::#fn_name { .. } => #call_index, )*
+					#( #cfg_attrs Self::#fn_name { .. } => #call_index, )*
 					Self::__Ignore(_, _) => unreachable!("__PhantomItem cannot be used."),
 				}
 			}
 
 			fn get_call_indices() -> &'static [u8] {
-				&[ #( #call_index, )* ]
+				&[ #( #cfg_attrs #call_index, )* ]
 			}
 		}
 
@@ -416,6 +429,7 @@ pub fn expand_call(def: &mut Def) -> proc_macro2::TokenStream {
 				#frame_support::dispatch_context::run_in_context(|| {
 					match self {
 						#(
+							#cfg_attrs
 							Self::#fn_name { #( #args_name_pattern, )* } => {
 								#frame_support::__private::sp_tracing::enter_span!(
 									#frame_support::__private::sp_tracing::trace_span!(stringify!(#fn_name))

substrate/frame/support/procedural/src/pallet/expand/error.rs

@@ -16,10 +16,14 @@
 // limitations under the License.
 
 use crate::{
-	pallet::{parse::error::VariantField, Def},
+	pallet::{
+		parse::error::{VariantDef, VariantField},
+		Def,
+	},
 	COUNTER,
 };
 use frame_support_procedural_tools::get_doc_literals;
+use quote::ToTokens;
 use syn::spanned::Spanned;
 
 ///
@@ -67,20 +71,23 @@ pub fn expand_error(def: &mut Def) -> proc_macro2::TokenStream {
 		)
 	);
 
-	let as_str_matches = error.variants.iter().map(|(variant, field_ty, _)| {
+	let as_str_matches = error.variants.iter().map(
-		let variant_str = variant.to_string();
+		|VariantDef { ident: variant, field: field_ty, docs: _, cfg_attrs }| {
-		match field_ty {
+			let variant_str = variant.to_string();
-			Some(VariantField { is_named: true }) => {
+			let cfg_attrs = cfg_attrs.iter().map(|attr| attr.to_token_stream());
-				quote::quote_spanned!(error.attr_span => Self::#variant { .. } => #variant_str,)
+			match field_ty {
-			},
+				Some(VariantField { is_named: true }) => {
-			Some(VariantField { is_named: false }) => {
+					quote::quote_spanned!(error.attr_span => #( #cfg_attrs )* Self::#variant { .. } => #variant_str,)
-				quote::quote_spanned!(error.attr_span => Self::#variant(..) => #variant_str,)
+				},
-			},
+				Some(VariantField { is_named: false }) => {
-			None => {
+					quote::quote_spanned!(error.attr_span => #( #cfg_attrs )* Self::#variant(..) => #variant_str,)
-				quote::quote_spanned!(error.attr_span => Self::#variant => #variant_str,)
+				},
-			},
+				None => {
-		}
+					quote::quote_spanned!(error.attr_span => #( #cfg_attrs )* Self::#variant => #variant_str,)
-	});
+				},
+			}
+		},
+	);
 
 	let error_item = {
 		let item = &mut def.item.content.as_mut().expect("Checked by def parser").1[error.index];

substrate/frame/support/procedural/src/pallet/parse/call.rs

@@ -85,6 +85,8 @@ pub struct CallVariantDef {
 	pub docs: Vec<syn::Expr>,
 	/// Attributes annotated at the top of the dispatchable function.
 	pub attrs: Vec<syn::Attribute>,
+	/// The `cfg` attributes.
+	pub cfg_attrs: Vec<syn::Attribute>,
 	/// The optional `feeless_if` attribute on the `pallet::call`.
 	pub feeless_check: Option<syn::ExprClosure>,
 }
@@ -266,6 +268,7 @@ impl CallDef {
 					return Err(syn::Error::new(method.sig.span(), msg))
 				}
 
+				let cfg_attrs: Vec<syn::Attribute> = helper::get_item_cfg_attrs(&method.attrs);
 				let mut call_idx_attrs = vec![];
 				let mut weight_attrs = vec![];
 				let mut feeless_attrs = vec![];
@@ -442,6 +445,7 @@ impl CallDef {
 					args,
 					docs,
 					attrs: method.attrs.clone(),
+					cfg_attrs,
 					feeless_check,
 				});
 			} else {

substrate/frame/support/procedural/src/pallet/parse/error.rs

@@ -25,19 +25,31 @@ mod keyword {
 	syn::custom_keyword!(Error);
 }
 
-/// Records information about the error enum variants.
+/// Records information about the error enum variant field.
 pub struct VariantField {
 	/// Whether or not the field is named, i.e. whether it is a tuple variant or struct variant.
 	pub is_named: bool,
 }
 
+/// Records information about the error enum variants.
+pub struct VariantDef {
+	/// The variant ident.
+	pub ident: syn::Ident,
+	/// The variant field, if any.
+	pub field: Option<VariantField>,
+	/// The variant doc literals.
+	pub docs: Vec<syn::Expr>,
+	/// The `cfg` attributes.
+	pub cfg_attrs: Vec<syn::Attribute>,
+}
+
 /// This checks error declaration as a enum declaration with only variants without fields nor
 /// discriminant.
 pub struct ErrorDef {
 	/// The index of error item in pallet module.
 	pub index: usize,
-	/// Variants ident, optional field and doc literals (ordered as declaration order)
+	/// Variant definitions.
-	pub variants: Vec<(syn::Ident, Option<VariantField>, Vec<syn::Expr>)>,
+	pub variants: Vec<VariantDef>,
 	/// A set of usage of instance, must be check for consistency with trait.
 	pub instances: Vec<helper::InstanceUsage>,
 	/// The keyword error used (contains span).
@@ -87,8 +99,14 @@ impl ErrorDef {
 					let span = variant.discriminant.as_ref().unwrap().0.span();
 					return Err(syn::Error::new(span, msg))
 				}
+				let cfg_attrs: Vec<syn::Attribute> = helper::get_item_cfg_attrs(&variant.attrs);
 
-				Ok((variant.ident.clone(), field_ty, get_doc_literals(&variant.attrs)))
+				Ok(VariantDef {
+					ident: variant.ident.clone(),
+					field: field_ty,
+					docs: get_doc_literals(&variant.attrs),
+					cfg_attrs,
+				})
 			})
 			.collect::<Result<_, _>>()?;
 

substrate/frame/support/test/tests/derive_impl.rs

@@ -0,0 +1,52 @@
+// This file is part of Substrate.
+
+// Copyright (C) Parity Technologies (UK) Ltd.
+// SPDX-License-Identifier: Apache-2.0
+
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// 	http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+use frame_support::derive_impl;
+
+trait Shape {
+	fn area(&self) -> u32;
+}
+
+struct SomeRectangle {}
+
+#[frame_support::register_default_impl(SomeRectangle)]
+impl Shape for SomeRectangle {
+	#[cfg(not(feature = "feature-frame-testing"))]
+	fn area(&self) -> u32 {
+		10
+	}
+
+	#[cfg(feature = "feature-frame-testing")]
+	fn area(&self) -> u32 {
+		0
+	}
+}
+
+struct SomeSquare {}
+
+#[derive_impl(SomeRectangle)]
+impl Shape for SomeSquare {}
+
+#[test]
+fn test_feature_parsing() {
+	let square = SomeSquare {};
+	#[cfg(not(feature = "feature-frame-testing"))]
+	assert_eq!(square.area(), 10);
+
+	#[cfg(feature = "feature-frame-testing")]
+	assert_eq!(square.area(), 0);
+}

substrate/frame/support/test/tests/pallet.rs

@@ -257,6 +257,13 @@ pub mod pallet {
 		pub fn check_for_dispatch_context(_origin: OriginFor<T>) -> DispatchResult {
 			with_context::<(), _>(|_| ()).ok_or_else(|| DispatchError::Unavailable)
 		}
+
+		#[cfg(feature = "frame-feature-testing")]
+		#[pallet::call_index(5)]
+		#[pallet::weight({1})]
+		pub fn foo_feature_test(_origin: OriginFor<T>) -> DispatchResult {
+			Ok(())
+		}
 	}
 
 	#[pallet::error]
@@ -269,6 +276,8 @@ pub mod pallet {
 		#[codec(skip)]
 		Skipped(u128),
 		CompactU8(#[codec(compact)] u8),
+		#[cfg(feature = "frame-feature-testing")]
+		FeatureTest,
 	}
 
 	#[pallet::event]
@@ -796,6 +805,7 @@ fn call_expand() {
 		}
 	);
 	assert_eq!(call_foo.get_call_name(), "foo");
+	#[cfg(not(feature = "frame-feature-testing"))]
 	assert_eq!(
 		pallet::Call::<Runtime>::get_call_names(),
 		&[
@@ -806,9 +816,24 @@ fn call_expand() {
 			"check_for_dispatch_context"
 		],
 	);
+	#[cfg(feature = "frame-feature-testing")]
+	assert_eq!(
+		pallet::Call::<Runtime>::get_call_names(),
+		&[
+			"foo",
+			"foo_storage_layer",
+			"foo_index_out_of_order",
+			"foo_no_post_info",
+			"check_for_dispatch_context",
+			"foo_feature_test"
+		],
+	);
 
 	assert_eq!(call_foo.get_call_index(), 0u8);
-	assert_eq!(pallet::Call::<Runtime>::get_call_indices(), &[0u8, 1u8, 4u8, 2u8, 3u8])
+	#[cfg(not(feature = "frame-feature-testing"))]
+	assert_eq!(pallet::Call::<Runtime>::get_call_indices(), &[0u8, 1u8, 4u8, 2u8, 3u8]);
+	#[cfg(feature = "frame-feature-testing")]
+	assert_eq!(pallet::Call::<Runtime>::get_call_indices(), &[0u8, 1u8, 4u8, 2u8, 3u8, 5u8]);
 }
 
 #[test]
@@ -816,7 +841,10 @@ fn call_expand_index() {
 	let call_foo = pallet::Call::<Runtime>::foo_index_out_of_order {};
 
 	assert_eq!(call_foo.get_call_index(), 4u8);
-	assert_eq!(pallet::Call::<Runtime>::get_call_indices(), &[0u8, 1u8, 4u8, 2u8, 3u8])
+	#[cfg(not(feature = "frame-feature-testing"))]
+	assert_eq!(pallet::Call::<Runtime>::get_call_indices(), &[0u8, 1u8, 4u8, 2u8, 3u8]);
+	#[cfg(feature = "frame-feature-testing")]
+	assert_eq!(pallet::Call::<Runtime>::get_call_indices(), &[0u8, 1u8, 4u8, 2u8, 3u8, 5u8]);
 }
 
 #[test]
@@ -838,6 +866,8 @@ fn error_expand() {
 		}),
 	);
 	assert_eq!(<pallet::Error::<Runtime> as PalletError>::MAX_ENCODED_SIZE, 3);
+	#[cfg(feature = "frame-feature-testing")]
+	assert_eq!(format!("{:?}", pallet::Error::<Runtime>::FeatureTest), String::from("FeatureTest"),);
 }
 
 #[test]
@@ -2379,3 +2409,33 @@ fn test_dispatch_context() {
 			.dispatch(RuntimeOrigin::root()));
 	});
 }
+
+#[test]
+fn test_call_feature_parsing() {
+	let call = pallet::Call::<Runtime>::check_for_dispatch_context {};
+	match call {
+		pallet::Call::<Runtime>::check_for_dispatch_context {} |
+		pallet::Call::<Runtime>::foo { .. } |
+		pallet::Call::foo_storage_layer { .. } |
+		pallet::Call::foo_index_out_of_order {} |
+		pallet::Call::foo_no_post_info {} => (),
+		#[cfg(feature = "frame-feature-testing")]
+		pallet::Call::foo_feature_test {} => (),
+		pallet::Call::__Ignore(_, _) => (),
+	}
+}
+
+#[test]
+fn test_error_feature_parsing() {
+	let err = pallet::Error::<Runtime>::InsufficientProposersBalance;
+	match err {
+		pallet::Error::InsufficientProposersBalance |
+		pallet::Error::NonExistentStorageValue |
+		pallet::Error::Code(_) |
+		pallet::Error::Skipped(_) |
+		pallet::Error::CompactU8(_) => (),
+		#[cfg(feature = "frame-feature-testing")]
+		pallet::Error::FeatureTest => (),
+		pallet::Error::__Ignore(_, _) => (),
+	}
+}