Emit error when Config part is imported but without the std feature (#9225)

* Emit error when Config part is imported but without the std feature

* Add UI test for missing std feature on GenesisConfig

* Update frame/support/test/Cargo.toml

Co-authored-by: Guillaume Thiolliere <[email protected]>

* Remove unused imports

* Unify all dummy party checker macros

* Fix

* Dispaly pallet_path::GenesisConfig instead of PalletConfig in error message

* Revert changes to construct_runtime_ui.rs

* Add additional parameter for dummy part checker macro

* Apply suggestions from code review

* fix master merge: update version

* update Cargo.lock

Co-authored-by: Guillaume Thiolliere <[email protected]>

Author: KiChjang

Cargo.lock

@@ -18,7 +18,7 @@
 use crate::construct_runtime::Pallet;
 use inflector::Inflector;
 use proc_macro2::TokenStream;
-use quote::{format_ident, quote};
+use quote::{ToTokens, format_ident, quote};
 use syn::Ident;
 
 pub fn expand_outer_config(
@@ -35,6 +35,7 @@ pub fn expand_outer_config(
 		if let Some(pallet_entry) = decl.find_part("Config") {
 			let path = &decl.path;
 			let pallet_name = &decl.name;
+			let path_str = path.into_token_stream().to_string();
 			let config = format_ident!("{}Config", pallet_name);
 			let field_name = &Ident::new(
 				&pallet_name.to_string().to_snake_case(),
@@ -47,6 +48,8 @@ pub fn expand_outer_config(
 			build_storage_calls.extend(expand_config_build_storage_call(scrate, runtime, decl, &field_name));
 			query_genesis_config_part_macros.push(quote! {
 				#path::__substrate_genesis_config_check::is_genesis_config_defined!(#pallet_name);
+				#[cfg(feature = "std")]
+				#path::__substrate_genesis_config_check::is_std_enabled_for_genesis!(#pallet_name, #path_str);
 			});
 		}
 	}

frame/support/procedural/src/construct_runtime/expand/config.rs

@@ -9,96 +9,54 @@ pub fn generate_dummy_part_checker(input: TokenStream) -> TokenStream {
 
 	let count = COUNTER.with(|counter| counter.borrow_mut().inc());
 
-	let genesis_config_macro_ident = syn::Ident::new(
-		&format!("__is_genesis_config_defined_{}", count),
-		proc_macro2::Span::call_site(),
-	);
-	let event_macro_ident = syn::Ident::new(
-		&format!("__is_event_part_defined_{}", count),
-		proc_macro2::Span::call_site(),
-	);
-	let inherent_macro_ident = syn::Ident::new(
-		&format!("__is_inherent_part_defined_{}", count),
-		proc_macro2::Span::call_site(),
-	);
-	let validate_unsigned_macro_ident = syn::Ident::new(
-		&format!("__is_validate_unsigned_part_defined_{}", count),
-		proc_macro2::Span::call_site(),
-	);
-	let call_macro_ident = syn::Ident::new(
-		&format!("__is_call_part_defined_{}", count),
-		proc_macro2::Span::call_site(),
-	);
-	let origin_macro_ident = syn::Ident::new(
-		&format!("__is_origin_part_defined_{}", count),
+	let no_op_macro_ident = syn::Ident::new(
+		&format!("__dummy_part_checker_{}", count),
 		proc_macro2::Span::call_site(),
 	);
 
 	quote::quote!(
+		#[macro_export]
+		#[doc(hidden)]
+		macro_rules! #no_op_macro_ident {
+			( $( $tt:tt )* ) => {};
+		}
+
 		#[doc(hidden)]
 		pub mod __substrate_genesis_config_check {
-			#[macro_export]
 			#[doc(hidden)]
-			macro_rules! #genesis_config_macro_ident {
-				($pallet_name:ident) => {};
-			}
+			pub use #no_op_macro_ident as is_genesis_config_defined;
 			#[doc(hidden)]
-			pub use #genesis_config_macro_ident as is_genesis_config_defined;
+			pub use #no_op_macro_ident as is_std_enabled_for_genesis;
 		}
 
 		#[doc(hidden)]
 		pub mod __substrate_event_check {
-			#[macro_export]
-			#[doc(hidden)]
-			macro_rules! #event_macro_ident {
-				($pallet_name:ident) => {};
-			}
 			#[doc(hidden)]
-			pub use #event_macro_ident as is_event_part_defined;
+			pub use #no_op_macro_ident as is_event_part_defined;
 		}
 
 		#[doc(hidden)]
 		pub mod __substrate_inherent_check {
-			#[macro_export]
 			#[doc(hidden)]
-			macro_rules! #inherent_macro_ident {
-				($pallet_name:ident) => {};
-			}
-			#[doc(hidden)]
-			pub use #inherent_macro_ident as is_inherent_part_defined;
+			pub use #no_op_macro_ident as is_inherent_part_defined;
 		}
 
 		#[doc(hidden)]
 		pub mod __substrate_validate_unsigned_check {
-			#[macro_export]
-			#[doc(hidden)]
-			macro_rules! #validate_unsigned_macro_ident {
-				($pallet_name:ident) => {};
-			}
 			#[doc(hidden)]
-			pub use #validate_unsigned_macro_ident as is_validate_unsigned_part_defined;
+			pub use #no_op_macro_ident as is_validate_unsigned_part_defined;
 		}
 
 		#[doc(hidden)]
 		pub mod __substrate_call_check {
-			#[macro_export]
 			#[doc(hidden)]
-			macro_rules! #call_macro_ident {
-				($pallet_name:ident) => {};
-			}
-			#[doc(hidden)]
-			pub use #call_macro_ident as is_call_part_defined;
+			pub use #no_op_macro_ident as is_call_part_defined;
 		}
 
 		#[doc(hidden)]
 		pub mod __substrate_origin_check {
-			#[macro_export]
-			#[doc(hidden)]
-			macro_rules! #origin_macro_ident {
-				($pallet_name:ident) => {};
-			}
 			#[doc(hidden)]
-			pub use #origin_macro_ident as is_origin_part_defined;
+			pub use #no_op_macro_ident as is_origin_part_defined;
 		}
 	).into()
 }

frame/support/procedural/src/dummy_part_checker.rs

@@ -23,39 +23,60 @@ use syn::{Ident, spanned::Spanned};
 pub fn expand_genesis_config(def: &mut Def) -> proc_macro2::TokenStream {
 	let count = COUNTER.with(|counter| counter.borrow_mut().inc());
 
-	let (genesis_config, macro_ident) = if let Some(genesis_config) = &def.genesis_config {
-		let ident = Ident::new(
-			&format!("__is_genesis_config_defined_{}", count),
-			genesis_config.genesis_config.span(),
-		);
-		(genesis_config, ident)
-	} else {
-		let macro_ident = Ident::new(
-			&format!("__is_genesis_config_defined_{}", count),
-			def.item.span(),
-		);
-
-		return quote::quote! {
-			#[doc(hidden)]
-			pub mod __substrate_genesis_config_check {
-				#[macro_export]
+	let (genesis_config, def_macro_ident, std_macro_ident) =
+		if let Some(genesis_config) = &def.genesis_config {
+			let def_macro_ident = Ident::new(
+				&format!("__is_genesis_config_defined_{}", count),
+				genesis_config.genesis_config.span(),
+			);
+
+			let std_macro_ident = Ident::new(
+				&format!("__is_std_macro_defined_for_genesis_{}", count),
+				genesis_config.genesis_config.span(),
+			);
+
+			(genesis_config, def_macro_ident, std_macro_ident)
+		} else {
+			let def_macro_ident = Ident::new(
+				&format!("__is_genesis_config_defined_{}", count),
+				def.item.span(),
+			);
+
+			let std_macro_ident = Ident::new(
+				&format!("__is_std_enabled_for_genesis_{}", count),
+				def.item.span(),
+			);
+
+			return quote::quote! {
 				#[doc(hidden)]
-				macro_rules! #macro_ident {
-					($pallet_name:ident) => {
-						compile_error!(concat!(
-							"`",
-							stringify!($pallet_name),
-							"` does not have #[pallet::genesis_config] defined, perhaps you should \
-							remove `Config` from construct_runtime?",
-						));
+				pub mod __substrate_genesis_config_check {
+					#[macro_export]
+					#[doc(hidden)]
+					macro_rules! #def_macro_ident {
+						($pallet_name:ident) => {
+							compile_error!(concat!(
+								"`",
+								stringify!($pallet_name),
+								"` does not have #[pallet::genesis_config] defined, perhaps you should \
+								remove `Config` from construct_runtime?",
+							));
+						}
 					}
+
+					#[macro_export]
+					#[doc(hidden)]
+					macro_rules! #std_macro_ident {
+						($pallet_name:ident, $pallet_path:expr) => {};
+					}
+
+					#[doc(hidden)]
+					pub use #def_macro_ident as is_genesis_config_defined;
+					#[doc(hidden)]
+					pub use #std_macro_ident as is_std_enabled_for_genesis;
 				}
-	
-				#[doc(hidden)]
-				pub use #macro_ident as is_genesis_config_defined;
-			}
+			};
 		};
-	};
+
 	let frame_support = &def.frame_support;
 
 	let genesis_config_item = &mut def.item.content.as_mut()
@@ -94,12 +115,36 @@ pub fn expand_genesis_config(def: &mut Def) -> proc_macro2::TokenStream {
 		pub mod __substrate_genesis_config_check {
 			#[macro_export]
 			#[doc(hidden)]
-			macro_rules! #macro_ident {
+			macro_rules! #def_macro_ident {
 				($pallet_name:ident) => {};
 			}
-	
+
+			#[cfg(not(feature = "std"))]
+			#[macro_export]
+			#[doc(hidden)]
+			macro_rules! #std_macro_ident {
+				($pallet_name:ident, $pallet_path:expr) => {
+					compile_error!(concat!(
+						"`",
+						stringify!($pallet_name),
+						"` does not have the std feature enabled, this will cause the `",
+						$pallet_path,
+						"::GenesisConfig` type to be undefined."
+					));
+				};
+			}
+
+			#[cfg(feature = "std")]
+			#[macro_export]
+			#[doc(hidden)]
+			macro_rules! #std_macro_ident {
+				($pallet_name:ident, $pallet_path:expr) => {};
+			}
+
+			#[doc(hidden)]
+			pub use #def_macro_ident as is_genesis_config_defined;
 			#[doc(hidden)]
-			pub use #macro_ident as is_genesis_config_defined;
+			pub use #std_macro_ident as is_std_enabled_for_genesis;
 		}
 	}
 }

frame/support/procedural/src/pallet/expand/genesis_config.rs

@@ -25,6 +25,8 @@ pretty_assertions = "0.6.1"
 rustversion = "1.0.0"
 frame-metadata = { version = "14.0.0-dev", default-features = false, path = "../../metadata" }
 frame-system = { version = "4.0.0-dev", default-features = false, path = "../../system" }
+# The "std" feature for this pallet is never activated on purpose, in order to test construct_runtime error message
+test-pallet = { package = "frame-support-test-pallet", default-features = false, path = "pallet" }
 
 [features]
 default = ["std"]

frame/support/test/Cargo.toml

@@ -0,0 +1,25 @@
+[package]
+name = "frame-support-test-pallet"
+version = "4.0.0-dev"
+authors = ["Parity Technologies <[email protected]>"]
+edition = "2018"
+license = "Apache-2.0"
+publish = false
+homepage = "https://substrate.dev"
+repository = "https://github.com/paritytech/substrate/"
+
+[package.metadata.docs.rs]
+targets = ["x86_64-unknown-linux-gnu"]
+
+[dependencies]
+codec = { package = "parity-scale-codec", version = "2.0.0", default-features = false, features = ["derive"] }
+frame-support = { version = "4.0.0-dev", default-features = false, path = "../../" }
+frame-system = { version = "4.0.0-dev", default-features = false, path = "../../../system" }
+
+[features]
+default = ["std"]
+std = [
+	"codec/std",
+	"frame-support/std",
+	"frame-system/std",
+]

frame/support/test/pallet/Cargo.toml

@@ -0,0 +1,46 @@
+// This file is part of Substrate.
+
+// Copyright (C) 2021 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.
+pub use pallet::*;
+
+#[frame_support::pallet]
+pub mod pallet {
+	#[allow(unused_imports)]
+	use frame_support::pallet_prelude::*;
+	#[allow(unused_imports)]
+	use frame_system::pallet_prelude::*;
+
+	#[pallet::pallet]
+	pub struct Pallet<T>(_);
+
+	#[pallet::config]
+	pub trait Config: frame_system::Config {}
+
+	#[pallet::genesis_config]
+	pub struct GenesisConfig {}
+
+	#[cfg(feature = "std")]
+	impl Default for GenesisConfig {
+		fn default() -> Self {
+			Self {}
+		}
+	}
+
+	#[pallet::genesis_build]
+	impl<T: Config> GenesisBuild<T> for GenesisConfig {
+		fn build(&self) {}
+	}
+}

frame/support/test/pallet/src/lib.rs

@@ -0,0 +1,24 @@
+use frame_support::construct_runtime;
+use sp_runtime::{generic, traits::BlakeTwo256};
+use sp_core::sr25519;
+
+pub type Signature = sr25519::Signature;
+pub type BlockNumber = u64;
+pub type Header = generic::Header<BlockNumber, BlakeTwo256>;
+pub type Block = generic::Block<Header, UncheckedExtrinsic>;
+pub type UncheckedExtrinsic = generic::UncheckedExtrinsic<u32, Call, Signature, ()>;
+
+impl test_pallet::Config for Runtime {}
+
+construct_runtime! {
+	pub enum Runtime where
+		Block = Block,
+		NodeBlock = Block,
+		UncheckedExtrinsic = UncheckedExtrinsic
+	{
+		System: system::{Pallet, Call, Storage, Config, Event<T>},
+		Pallet: test_pallet::{Pallet, Config},
+	}
+}
+
+fn main() {}