From 5abe21012598cb5b8114297e12ea5c3742dc1343 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Thu, 15 Dec 2022 11:38:21 +0100 Subject: [PATCH] Warn on missing `pallet::call_index` (#12894) * Warn on missing call_index Signed-off-by: Oliver Tale-Yazdi * Suppress camel case warning Signed-off-by: Oliver Tale-Yazdi * Simplify code Signed-off-by: Oliver Tale-Yazdi * Disallow warnings in pallet-ui tests Signed-off-by: Oliver Tale-Yazdi * Add pallet UI test Signed-off-by: Oliver Tale-Yazdi * Update Pallet UI Signed-off-by: Oliver Tale-Yazdi * fmt Signed-off-by: Oliver Tale-Yazdi * Use module instead of function Signed-off-by: Oliver Tale-Yazdi * Update pallet-ui Signed-off-by: Oliver Tale-Yazdi Signed-off-by: Oliver Tale-Yazdi --- .../procedural/src/pallet/expand/call.rs | 31 +++++++++++++++++++ .../procedural/src/pallet/parse/call.rs | 4 +++ frame/support/test/tests/pallet_ui.rs | 3 ++ .../pallet_ui/call_argument_invalid_bound.rs | 3 +- .../call_argument_invalid_bound.stderr | 18 +++++------ .../call_argument_invalid_bound_2.rs | 3 +- .../call_argument_invalid_bound_2.stderr | 24 +++++++------- .../call_argument_invalid_bound_3.rs | 3 +- .../call_argument_invalid_bound_3.stderr | 6 ++-- .../tests/pallet_ui/call_missing_index.rs | 22 +++++++++++++ .../tests/pallet_ui/call_missing_index.stderr | 12 +++++++ ...ev_mode_without_arg_max_encoded_len.stderr | 13 ++++++++ 12 files changed, 115 insertions(+), 27 deletions(-) create mode 100644 frame/support/test/tests/pallet_ui/call_missing_index.rs create mode 100644 frame/support/test/tests/pallet_ui/call_missing_index.stderr diff --git a/frame/support/procedural/src/pallet/expand/call.rs b/frame/support/procedural/src/pallet/expand/call.rs index 6b166e6726d38..30ef1a8fca31d 100644 --- 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::>(); + 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: + and + .")] + #[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::>(); @@ -178,6 +201,14 @@ pub fn expand_call(def: &mut Def) -> proc_macro2::TokenStream { .collect::>(); 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 index fbca9a52c767c..9620038edfd6d 100644 --- 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, /// 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 index 2db1d3cb0543a..26d016c5a7796 100644 --- 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 index ee9d692eba9b3..4f18f7281817a 100644 --- 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 Pallet { #[pallet::weight(0)] - pub fn foo(origin: OriginFor, bar: T::Bar) -> DispatchResultWithPostInfo { + #[pallet::call_index(0)] + pub fn foo(origin: OriginFor, _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 index 62d8649f8af49..aed53b07b5e63 100644 --- 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]: `::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, bar: T::Bar) -> DispatchResultWithPostInfo { - | ^^^ `::Bar` cannot be formatted using `{:?}` because it doesn't implement `std::fmt::Debug` +21 | pub fn foo(origin: OriginFor, _bar: T::Bar) -> DispatchResultWithPostInfo { + | ^^^^ `::Bar` cannot be formatted using `{:?}` because it doesn't implement `std::fmt::Debug` | = help: the trait `std::fmt::Debug` is not implemented for `::Bar` = note: required for `&::Bar` to implement `std::fmt::Debug` = note: required for the cast from `&::Bar` to the object type `dyn std::fmt::Debug` error[E0277]: the trait bound `::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, bar: T::Bar) -> DispatchResultWithPostInfo { - | ^^^ the trait `Clone` is not implemented for `::Bar` +21 | pub fn foo(origin: OriginFor, _bar: T::Bar) -> DispatchResultWithPostInfo { + | ^^^^ the trait `Clone` is not implemented for `::Bar` error[E0369]: binary operation `==` cannot be applied to type `&::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, bar: T::Bar) -> DispatchResultWithPostInfo { - | ^^^ +21 | pub fn foo(origin: OriginFor, _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 index d981b55c48620..20568908e72b2 100644 --- 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 Pallet { #[pallet::weight(0)] - pub fn foo(origin: OriginFor, bar: T::Bar) -> DispatchResultWithPostInfo { + #[pallet::call_index(0)] + pub fn foo(origin: OriginFor, _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 index f486c071631ea..3e2e70432a9c9 100644 --- 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]: `::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, bar: T::Bar) -> DispatchResultWithPostInfo { - | ^^^ `::Bar` cannot be formatted using `{:?}` because it doesn't implement `std::fmt::Debug` +21 | pub fn foo(origin: OriginFor, _bar: T::Bar) -> DispatchResultWithPostInfo { + | ^^^^ `::Bar` cannot be formatted using `{:?}` because it doesn't implement `std::fmt::Debug` | = help: the trait `std::fmt::Debug` is not implemented for `::Bar` = note: required for `&::Bar` to implement `std::fmt::Debug` = note: required for the cast from `&::Bar` to the object type `dyn std::fmt::Debug` error[E0277]: the trait bound `::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, bar: T::Bar) -> DispatchResultWithPostInfo { - | ^^^ the trait `Clone` is not implemented for `::Bar` +21 | pub fn foo(origin: OriginFor, _bar: T::Bar) -> DispatchResultWithPostInfo { + | ^^^^ the trait `Clone` is not implemented for `::Bar` error[E0369]: binary operation `==` cannot be applied to type `&::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, bar: T::Bar) -> DispatchResultWithPostInfo { - | ^^^ +21 | pub fn foo(origin: OriginFor, _bar: T::Bar) -> DispatchResultWithPostInfo { + | ^^^^ error[E0277]: the trait bound `::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 `::Bar: WrapperTypeEncode` is 17 | | #[pallet::call] | |__________________- required by a bound introduced by this call ... -20 | pub fn foo(origin: OriginFor, bar: T::Bar) -> DispatchResultWithPostInfo { - | ^^^ the trait `WrapperTypeEncode` is not implemented for `::Bar` +21 | pub fn foo(origin: OriginFor, _bar: T::Bar) -> DispatchResultWithPostInfo { + | ^^^^ the trait `WrapperTypeEncode` is not implemented for `::Bar` | = note: required for `::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 index 1cdfb369feadb..64b6642b0a878 100644 --- 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 Pallet { #[pallet::weight(0)] - pub fn foo(origin: OriginFor, bar: Bar) -> DispatchResultWithPostInfo { + #[pallet::call_index(0)] + pub fn foo(origin: OriginFor, _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 index 6e51bf2dbf862..395da09cb0d6d 100644 --- 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, bar: Bar) -> DispatchResultWithPostInfo { - | ^^^ `Bar` cannot be formatted using `{:?}` +23 | pub fn foo(origin: OriginFor, _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 new file mode 100644 index 0000000000000..7e305a573d622 --- /dev/null +++ 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(core::marker::PhantomData); + + #[pallet::call] + impl Pallet { + #[pallet::weight(0)] + pub fn foo(_: OriginFor) -> 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 new file mode 100644 index 0000000000000..9d00204979211 --- /dev/null +++ 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: + and + . + --> tests/pallet_ui/call_missing_index.rs:15:10 + | +15 | pub fn foo(_: OriginFor) -> 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 index a5ec31a9bb4e7..170555665d877 100644 --- 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: + and + . + --> tests/pallet_ui/dev_mode_without_arg_max_encoded_len.rs:25:10 + | +25 | pub fn my_call(_origin: OriginFor) -> DispatchResult { + | ^^^^^^^ + | + = note: `-D deprecated` implied by `-D warnings` + error[E0277]: the trait bound `Vec: MaxEncodedLen` is not satisfied --> tests/pallet_ui/dev_mode_without_arg_max_encoded_len.rs:11:12 |