Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Warn on missing pallet::call_index #12894

Merged
merged 10 commits into from
Dec 15, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions frame/support/procedural/src/pallet/expand/call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<_>>();
Expand Down Expand Up @@ -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]
Expand Down
4 changes: 4 additions & 0 deletions frame/support/procedural/src/pallet/parse/call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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(),
Expand Down
3 changes: 3 additions & 0 deletions frame/support/test/tests/pallet_ui.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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 {
| ^^^^
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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 {
Expand All @@ -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`

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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`
Expand Down
22 changes: 22 additions & 0 deletions frame/support/test/tests/pallet_ui/call_missing_index.rs
Original file line number Diff line number Diff line change
@@ -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() {
}
12 changes: 12 additions & 0 deletions frame/support/test/tests/pallet_ui/call_missing_index.stderr
Original file line number Diff line number Diff line change
@@ -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`
Original file line number Diff line number Diff line change
@@ -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
|
Expand Down