Warn on missing pallet::call_index (paritytech#12894)

* Warn on missing call_index Signed-off-by: Oliver Tale-Yazdi <[email protected]> * Suppress camel case warning Signed-off-by: Oliver Tale-Yazdi <[email protected]> * Simplify code Signed-off-by: Oliver Tale-Yazdi <[email protected]> * Disallow warnings in pallet-ui tests Signed-off-by: Oliver Tale-Yazdi <[email protected]> * Add pallet UI test Signed-off-by: Oliver Tale-Yazdi <[email protected]> * Update Pallet UI Signed-off-by: Oliver Tale-Yazdi <[email protected]> * fmt Signed-off-by: Oliver Tale-Yazdi <[email protected]> * Use module instead of function Signed-off-by: Oliver Tale-Yazdi <[email protected]> * Update pallet-ui Signed-off-by: Oliver Tale-Yazdi <[email protected]> Signed-off-by: Oliver Tale-Yazdi <[email protected]>
gear-tech · Feb 27, 2023 · 3bb89b0 · 3bb89b0
1 parent f4e440a
commit 3bb89b0
Show file tree

Hide file tree

Showing 12 changed files with 115 additions and 27 deletions.
diff --git a/frame/support/procedural/src/pallet/expand/call.rs b/frame/support/procedural/src/pallet/expand/call.rs
@@ -54,6 +54,29 @@ pub fn expand_call(def: &mut Def) -> proc_macro2::TokenStream {
 		.map(|fn_name| format!("Create a call with the variant `{}`.", fn_name))
 		.collect::<Vec<_>>();
 
+	let mut warning_structs = Vec::new();
+	let mut warning_names = Vec::new();
+	// Emit a warning for each call that is missing `call_index` when not in dev-mode.
+	for method in &methods {
+		if method.explicit_call_index || def.dev_mode {
+			continue
+		}
+
+		let name = syn::Ident::new(&format!("{}", method.name), method.name.span());
+		let warning: syn::ItemStruct = syn::parse_quote!(
+			#[deprecated(note = r"
+			Implicit call indices are deprecated in favour of explicit ones.
+			Please ensure that all calls have the `pallet::call_index` attribute or that the
+			`dev-mode` of the pallet is enabled. For more info see:
+			<https://github.com/paritytech/substrate/pull/12891> and
+			<https://github.com/paritytech/substrate/pull/11381>.")]
+			#[allow(non_camel_case_types)]
+			struct #name;
+		);
+		warning_names.push(name);
+		warning_structs.push(warning);
+	}
+
 	let fn_weight = methods.iter().map(|method| &method.weight);
 
 	let fn_doc = methods.iter().map(|method| &method.docs).collect::<Vec<_>>();
@@ -178,6 +201,14 @@ pub fn expand_call(def: &mut Def) -> proc_macro2::TokenStream {
 		.collect::<Vec<_>>();
 
 	quote::quote_spanned!(span =>
+		mod warnings {
+			#(
+				#warning_structs
+				// This triggers each deprecated warning once.
+				const _: Option<#warning_names> = None;
+			)*
+		}
+
 		#[doc(hidden)]
 		pub mod __substrate_call_check {
 			#[macro_export]

diff --git a/frame/support/procedural/src/pallet/parse/call.rs b/frame/support/procedural/src/pallet/parse/call.rs
@@ -59,6 +59,8 @@ pub struct CallVariantDef {
 	pub weight: syn::Expr,
 	/// Call index of the dispatchable.
 	pub call_index: u8,
+	/// Whether an explicit call index was specified.
+	pub explicit_call_index: bool,
 	/// Docs, used for metadata.
 	pub docs: Vec<syn::Lit>,
 	/// Attributes annotated at the top of the dispatchable function.
@@ -243,6 +245,7 @@ impl CallDef {
 					FunctionAttr::CallIndex(idx) => idx,
 					_ => unreachable!("checked during creation of the let binding"),
 				});
+				let explicit_call_index = call_index.is_some();
 
 				let final_index = match call_index {
 					Some(i) => i,
@@ -296,6 +299,7 @@ impl CallDef {
 					name: method.sig.ident.clone(),
 					weight,
 					call_index: final_index,
+					explicit_call_index,
 					args,
 					docs,
 					attrs: method.attrs.clone(),

diff --git a/frame/support/test/tests/pallet_ui.rs b/frame/support/test/tests/pallet_ui.rs
@@ -27,6 +27,9 @@ fn pallet_ui() {
 	// As trybuild is using `cargo check`, we don't need the real WASM binaries.
 	std::env::set_var("SKIP_WASM_BUILD", "1");
 
+	// Deny all warnings since we emit warnings as part of a Pallet's UI.
+	std::env::set_var("RUSTFLAGS", "--deny warnings");
+
 	let t = trybuild::TestCases::new();
 	t.compile_fail("tests/pallet_ui/*.rs");
 	t.pass("tests/pallet_ui/pass/*.rs");

diff --git a/frame/support/test/tests/pallet_ui/call_argument_invalid_bound.rs b/frame/support/test/tests/pallet_ui/call_argument_invalid_bound.rs
@@ -17,7 +17,8 @@ mod pallet {
 	#[pallet::call]
 	impl<T: Config> Pallet<T> {
 		#[pallet::weight(0)]
-		pub fn foo(origin: OriginFor<T>, bar: T::Bar) -> DispatchResultWithPostInfo {
+		#[pallet::call_index(0)]
+		pub fn foo(origin: OriginFor<T>, _bar: T::Bar) -> DispatchResultWithPostInfo {
 			Ok(().into())
 		}
 	}

diff --git a/frame/support/test/tests/pallet_ui/call_argument_invalid_bound.stderr b/frame/support/test/tests/pallet_ui/call_argument_invalid_bound.stderr
@@ -1,21 +1,21 @@
 error[E0277]: `<T as pallet::Config>::Bar` doesn't implement `std::fmt::Debug`
-  --> tests/pallet_ui/call_argument_invalid_bound.rs:20:36
+  --> tests/pallet_ui/call_argument_invalid_bound.rs:21:36
    |
-20 |         pub fn foo(origin: OriginFor<T>, bar: T::Bar) -> DispatchResultWithPostInfo {
-   |                                          ^^^ `<T as pallet::Config>::Bar` cannot be formatted using `{:?}` because it doesn't implement `std::fmt::Debug`
+21 |         pub fn foo(origin: OriginFor<T>, _bar: T::Bar) -> DispatchResultWithPostInfo {
+   |                                          ^^^^ `<T as pallet::Config>::Bar` cannot be formatted using `{:?}` because it doesn't implement `std::fmt::Debug`
    |
    = help: the trait `std::fmt::Debug` is not implemented for `<T as pallet::Config>::Bar`
    = note: required for `&<T as pallet::Config>::Bar` to implement `std::fmt::Debug`
    = note: required for the cast from `&<T as pallet::Config>::Bar` to the object type `dyn std::fmt::Debug`
 
 error[E0277]: the trait bound `<T as pallet::Config>::Bar: Clone` is not satisfied
-  --> tests/pallet_ui/call_argument_invalid_bound.rs:20:36
+  --> tests/pallet_ui/call_argument_invalid_bound.rs:21:36
    |
-20 |         pub fn foo(origin: OriginFor<T>, bar: T::Bar) -> DispatchResultWithPostInfo {
-   |                                          ^^^ the trait `Clone` is not implemented for `<T as pallet::Config>::Bar`
+21 |         pub fn foo(origin: OriginFor<T>, _bar: T::Bar) -> DispatchResultWithPostInfo {
+   |                                          ^^^^ the trait `Clone` is not implemented for `<T as pallet::Config>::Bar`
 
 error[E0369]: binary operation `==` cannot be applied to type `&<T as pallet::Config>::Bar`
-  --> tests/pallet_ui/call_argument_invalid_bound.rs:20:36
+  --> tests/pallet_ui/call_argument_invalid_bound.rs:21:36
    |
-20 |         pub fn foo(origin: OriginFor<T>, bar: T::Bar) -> DispatchResultWithPostInfo {
-   |                                          ^^^
+21 |         pub fn foo(origin: OriginFor<T>, _bar: T::Bar) -> DispatchResultWithPostInfo {
+   |                                          ^^^^
diff --git a/frame/support/test/tests/pallet_ui/call_argument_invalid_bound_2.rs b/frame/support/test/tests/pallet_ui/call_argument_invalid_bound_2.rs
@@ -17,7 +17,8 @@ mod pallet {
 	#[pallet::call]
 	impl<T: Config> Pallet<T> {
 		#[pallet::weight(0)]
-		pub fn foo(origin: OriginFor<T>, bar: T::Bar) -> DispatchResultWithPostInfo {
+		#[pallet::call_index(0)]
+		pub fn foo(origin: OriginFor<T>, _bar: T::Bar) -> DispatchResultWithPostInfo {
 			Ok(().into())
 		}
 	}

diff --git a/frame/support/test/tests/pallet_ui/call_argument_invalid_bound_2.stderr b/frame/support/test/tests/pallet_ui/call_argument_invalid_bound_2.stderr
@@ -1,27 +1,27 @@
 error[E0277]: `<T as pallet::Config>::Bar` doesn't implement `std::fmt::Debug`
-  --> tests/pallet_ui/call_argument_invalid_bound_2.rs:20:36
+  --> tests/pallet_ui/call_argument_invalid_bound_2.rs:21:36
    |
-20 |         pub fn foo(origin: OriginFor<T>, bar: T::Bar) -> DispatchResultWithPostInfo {
-   |                                          ^^^ `<T as pallet::Config>::Bar` cannot be formatted using `{:?}` because it doesn't implement `std::fmt::Debug`
+21 |         pub fn foo(origin: OriginFor<T>, _bar: T::Bar) -> DispatchResultWithPostInfo {
+   |                                          ^^^^ `<T as pallet::Config>::Bar` cannot be formatted using `{:?}` because it doesn't implement `std::fmt::Debug`
    |
    = help: the trait `std::fmt::Debug` is not implemented for `<T as pallet::Config>::Bar`
    = note: required for `&<T as pallet::Config>::Bar` to implement `std::fmt::Debug`
    = note: required for the cast from `&<T as pallet::Config>::Bar` to the object type `dyn std::fmt::Debug`
 
 error[E0277]: the trait bound `<T as pallet::Config>::Bar: Clone` is not satisfied
-  --> tests/pallet_ui/call_argument_invalid_bound_2.rs:20:36
+  --> tests/pallet_ui/call_argument_invalid_bound_2.rs:21:36
    |
-20 |         pub fn foo(origin: OriginFor<T>, bar: T::Bar) -> DispatchResultWithPostInfo {
-   |                                          ^^^ the trait `Clone` is not implemented for `<T as pallet::Config>::Bar`
+21 |         pub fn foo(origin: OriginFor<T>, _bar: T::Bar) -> DispatchResultWithPostInfo {
+   |                                          ^^^^ the trait `Clone` is not implemented for `<T as pallet::Config>::Bar`
 
 error[E0369]: binary operation `==` cannot be applied to type `&<T as pallet::Config>::Bar`
-  --> tests/pallet_ui/call_argument_invalid_bound_2.rs:20:36
+  --> tests/pallet_ui/call_argument_invalid_bound_2.rs:21:36
    |
-20 |         pub fn foo(origin: OriginFor<T>, bar: T::Bar) -> DispatchResultWithPostInfo {
-   |                                          ^^^
+21 |         pub fn foo(origin: OriginFor<T>, _bar: T::Bar) -> DispatchResultWithPostInfo {
+   |                                          ^^^^
 
 error[E0277]: the trait bound `<T as pallet::Config>::Bar: WrapperTypeEncode` is not satisfied
-  --> tests/pallet_ui/call_argument_invalid_bound_2.rs:20:36
+  --> tests/pallet_ui/call_argument_invalid_bound_2.rs:21:36
    |
 1  | / #[frame_support::pallet]
 2  | | mod pallet {
@@ -32,8 +32,8 @@ error[E0277]: the trait bound `<T as pallet::Config>::Bar: WrapperTypeEncode` is
 17 | |     #[pallet::call]
    | |__________________- required by a bound introduced by this call
 ...
-20 |           pub fn foo(origin: OriginFor<T>, bar: T::Bar) -> DispatchResultWithPostInfo {
-   |                                            ^^^ the trait `WrapperTypeEncode` is not implemented for `<T as pallet::Config>::Bar`
+21 |           pub fn foo(origin: OriginFor<T>, _bar: T::Bar) -> DispatchResultWithPostInfo {
+   |                                            ^^^^ the trait `WrapperTypeEncode` is not implemented for `<T as pallet::Config>::Bar`
    |
    = note: required for `<T as pallet::Config>::Bar` to implement `Encode`
 

diff --git a/frame/support/test/tests/pallet_ui/call_argument_invalid_bound_3.rs b/frame/support/test/tests/pallet_ui/call_argument_invalid_bound_3.rs
@@ -19,7 +19,8 @@ mod pallet {
 	#[pallet::call]
 	impl<T: Config> Pallet<T> {
 		#[pallet::weight(0)]
-		pub fn foo(origin: OriginFor<T>, bar: Bar) -> DispatchResultWithPostInfo {
+		#[pallet::call_index(0)]
+		pub fn foo(origin: OriginFor<T>, _bar: Bar) -> DispatchResultWithPostInfo {
 			Ok(().into())
 		}
 	}

diff --git a/frame/support/test/tests/pallet_ui/call_argument_invalid_bound_3.stderr b/frame/support/test/tests/pallet_ui/call_argument_invalid_bound_3.stderr
@@ -1,8 +1,8 @@
 error[E0277]: `Bar` doesn't implement `std::fmt::Debug`
-  --> tests/pallet_ui/call_argument_invalid_bound_3.rs:22:36
+  --> tests/pallet_ui/call_argument_invalid_bound_3.rs:23:36
    |
-22 |         pub fn foo(origin: OriginFor<T>, bar: Bar) -> DispatchResultWithPostInfo {
-   |                                          ^^^ `Bar` cannot be formatted using `{:?}`
+23 |         pub fn foo(origin: OriginFor<T>, _bar: Bar) -> DispatchResultWithPostInfo {
+   |                                          ^^^^ `Bar` cannot be formatted using `{:?}`
    |
    = help: the trait `std::fmt::Debug` is not implemented for `Bar`
    = note: add `#[derive(Debug)]` to `Bar` or manually `impl std::fmt::Debug for Bar`

diff --git a/frame/support/test/tests/pallet_ui/call_missing_index.rs b/frame/support/test/tests/pallet_ui/call_missing_index.rs
@@ -0,0 +1,22 @@
+#[frame_support::pallet]
+mod pallet {
+	use frame_support::pallet_prelude::DispatchResult;
+	use frame_system::pallet_prelude::OriginFor;
+
+	#[pallet::config]
+	pub trait Config: frame_system::Config {}
+
+	#[pallet::pallet]
+	pub struct Pallet<T>(core::marker::PhantomData<T>);
+
+	#[pallet::call]
+	impl<T: Config> Pallet<T> {
+		#[pallet::weight(0)]
+		pub fn foo(_: OriginFor<T>) -> DispatchResult {
+			Ok(())
+		}
+	}
+}
+
+fn main() {
+}
diff --git a/frame/support/test/tests/pallet_ui/call_missing_index.stderr b/frame/support/test/tests/pallet_ui/call_missing_index.stderr
@@ -0,0 +1,12 @@
+error: use of deprecated struct `pallet::warnings::foo`:
+                   Implicit call indices are deprecated in favour of explicit ones.
+                   Please ensure that all calls have the `pallet::call_index` attribute or that the
+                   `dev-mode` of the pallet is enabled. For more info see:
+                   <https://github.com/paritytech/substrate/pull/12891> and
+                   <https://github.com/paritytech/substrate/pull/11381>.
+  --> tests/pallet_ui/call_missing_index.rs:15:10
+   |
+15 |         pub fn foo(_: OriginFor<T>) -> DispatchResult {
+   |                ^^^
+   |
+   = note: `-D deprecated` implied by `-D warnings`
diff --git a/frame/support/test/tests/pallet_ui/dev_mode_without_arg_max_encoded_len.stderr b/frame/support/test/tests/pallet_ui/dev_mode_without_arg_max_encoded_len.stderr
@@ -1,3 +1,16 @@
+error: use of deprecated struct `pallet::warnings::my_call`:
+                   Implicit call indices are deprecated in favour of explicit ones.
+                   Please ensure that all calls have the `pallet::call_index` attribute or that the
+                   `dev-mode` of the pallet is enabled. For more info see:
+                   <https://github.com/paritytech/substrate/pull/12891> and
+                   <https://github.com/paritytech/substrate/pull/11381>.
+  --> tests/pallet_ui/dev_mode_without_arg_max_encoded_len.rs:25:10
+   |
+25 |         pub fn my_call(_origin: OriginFor<T>) -> DispatchResult {
+   |                ^^^^^^^
+   |
+   = note: `-D deprecated` implied by `-D warnings`
+
 error[E0277]: the trait bound `Vec<u8>: MaxEncodedLen` is not satisfied
   --> tests/pallet_ui/dev_mode_without_arg_max_encoded_len.rs:11:12
    |