Skip to content

Commit

Permalink
Warn on missing pallet::call_index (paritytech#12894)
Browse files Browse the repository at this point in the history 
* Warn on missing call_index

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Suppress camel case warning

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Simplify code

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Disallow warnings in pallet-ui tests

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Add pallet UI test

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Update Pallet UI

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* fmt

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Use module instead of function

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Update pallet-ui

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
  • Loading branch information
@ggwpez @ark0f
ggwpez authored and ark0f committed Feb 27, 2023
1 parent f4e440a commit 3bb89b0
Show file tree
Hide file tree
Showing 12 changed files with 115 additions and 27 deletions.
31 changes: 31 additions & 0 deletions frame/support/procedural/src/pallet/expand/call.rs
View file
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
View file
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
View file
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
3 changes: 2 additions & 1 deletion frame/support/test/tests/pallet_ui/call_argument_invalid_bound.rs
View file
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
18 changes: 9 additions & 9 deletions frame/support/test/tests/pallet_ui/call_argument_invalid_bound.stderr
View file
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 {
| ^^^^
3 changes: 2 additions & 1 deletion frame/support/test/tests/pallet_ui/call_argument_invalid_bound_2.rs
View file
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
24 changes: 12 additions & 12 deletions frame/support/test/tests/pallet_ui/call_argument_invalid_bound_2.stderr
View file
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
3 changes: 2 additions & 1 deletion frame/support/test/tests/pallet_ui/call_argument_invalid_bound_3.rs
View file
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
6 changes: 3 additions & 3 deletions frame/support/test/tests/pallet_ui/call_argument_invalid_bound_3.stderr
View file
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
View file
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
View file
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`
13 changes: 13 additions & 0 deletions frame/support/test/tests/pallet_ui/dev_mode_without_arg_max_encoded_len.stderr
View file
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

0 comments on commit 3bb89b0

Please sign in to comment.