Skip to content

Commit

Permalink
[stable2409] Backport #5580 (#5733)
Browse files Browse the repository at this point in the history
Backport #5580 into `stable2409` from gui1117.

See the
[documentation](https://github.com/paritytech/polkadot-sdk/blob/master/docs/BACKPORT.md)
on how to use this bot.

<!--
  # To be used by other automation, do not modify:
  original-pr-number: #${pull_number}
-->

Co-authored-by: Guillaume Thiolliere <[email protected]>
  • Loading branch information
1 parent a45d203 commit a869596
Show file tree
Hide file tree
Showing 6 changed files with 125 additions and 21 deletions.
13 changes: 13 additions & 0 deletions prdoc/pr_5580.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0
# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json

title: Fix error message on pallet macro

doc:
- audience: Runtime Dev
description: |
Improve error message for pallet macro generated code.

crates:
- name: frame-support-procedural
bump: patch
36 changes: 27 additions & 9 deletions substrate/frame/support/procedural/src/pallet/expand/call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
use crate::{
pallet::{
expand::warnings::{weight_constant_warning, weight_witness_warning},
parse::call::CallWeightDef,
parse::{call::CallWeightDef, helper::CallReturnType},
Def,
},
COUNTER,
Expand Down Expand Up @@ -197,18 +197,36 @@ pub fn expand_call(def: &mut Def) -> proc_macro2::TokenStream {
let capture_docs = if cfg!(feature = "no-metadata-docs") { "never" } else { "always" };

// Wrap all calls inside of storage layers
if let Some(syn::Item::Impl(item_impl)) = def
.call
.as_ref()
.map(|c| &mut def.item.content.as_mut().expect("Checked by def parser").1[c.index])
{
item_impl.items.iter_mut().for_each(|i| {
if let syn::ImplItem::Fn(method) = i {
if let Some(call) = def.call.as_ref() {
let item_impl =
&mut def.item.content.as_mut().expect("Checked by def parser").1[call.index];
let syn::Item::Impl(item_impl) = item_impl else {
unreachable!("Checked by def parser");
};

item_impl.items.iter_mut().enumerate().for_each(|(i, item)| {
if let syn::ImplItem::Fn(method) = item {
let return_type =
&call.methods.get(i).expect("def should be consistent with item").return_type;

let (ok_type, err_type) = match return_type {
CallReturnType::DispatchResult => (
quote::quote!(()),
quote::quote!(#frame_support::pallet_prelude::DispatchError),
),
CallReturnType::DispatchResultWithPostInfo => (
quote::quote!(#frame_support::dispatch::PostDispatchInfo),
quote::quote!(#frame_support::dispatch::DispatchErrorWithPostInfo),
),
};

let block = &method.block;
method.block = syn::parse_quote! {{
// We execute all dispatchable in a new storage layer, allowing them
// to return an error at any point, and undoing any storage changes.
#frame_support::storage::with_storage_layer(|| #block)
#frame_support::storage::with_storage_layer::<#ok_type, #err_type, _>(
|| #block
)
}};
}
});
Expand Down
11 changes: 4 additions & 7 deletions substrate/frame/support/procedural/src/pallet/parse/call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,8 @@ pub struct CallVariantDef {
pub cfg_attrs: Vec<syn::Attribute>,
/// The optional `feeless_if` attribute on the `pallet::call`.
pub feeless_check: Option<syn::ExprClosure>,
/// The return type of the call: `DispatchInfo` or `DispatchResultWithPostInfo`.
pub return_type: helper::CallReturnType,
}

/// Attributes for functions in call impl block.
Expand Down Expand Up @@ -260,13 +262,7 @@ impl CallDef {
},
}

if let syn::ReturnType::Type(_, type_) = &method.sig.output {
helper::check_pallet_call_return_type(type_)?;
} else {
let msg = "Invalid pallet::call, require return type \
DispatchResultWithPostInfo";
return Err(syn::Error::new(method.sig.span(), msg))
}
let return_type = helper::check_pallet_call_return_type(&method.sig)?;

let cfg_attrs: Vec<syn::Attribute> = helper::get_item_cfg_attrs(&method.attrs);
let mut call_idx_attrs = vec![];
Expand Down Expand Up @@ -447,6 +443,7 @@ impl CallDef {
attrs: method.attrs.clone(),
cfg_attrs,
feeless_check,
return_type,
});
} else {
let msg = "Invalid pallet::call, only method accepted";
Expand Down
23 changes: 18 additions & 5 deletions substrate/frame/support/procedural/src/pallet/parse/helper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -597,25 +597,38 @@ pub fn check_type_value_gen(
Ok(i)
}

/// The possible return type of a dispatchable.
#[derive(Clone)]
pub enum CallReturnType {
DispatchResult,
DispatchResultWithPostInfo,
}

/// Check the keyword `DispatchResultWithPostInfo` or `DispatchResult`.
pub fn check_pallet_call_return_type(type_: &syn::Type) -> syn::Result<()> {
pub struct Checker;
pub fn check_pallet_call_return_type(sig: &syn::Signature) -> syn::Result<CallReturnType> {
let syn::ReturnType::Type(_, type_) = &sig.output else {
let msg = "Invalid pallet::call, require return type \
DispatchResultWithPostInfo";
return Err(syn::Error::new(sig.span(), msg))
};

pub struct Checker(CallReturnType);
impl syn::parse::Parse for Checker {
fn parse(input: syn::parse::ParseStream) -> syn::Result<Self> {
let lookahead = input.lookahead1();
if lookahead.peek(keyword::DispatchResultWithPostInfo) {
input.parse::<keyword::DispatchResultWithPostInfo>()?;
Ok(Self)
Ok(Self(CallReturnType::DispatchResultWithPostInfo))
} else if lookahead.peek(keyword::DispatchResult) {
input.parse::<keyword::DispatchResult>()?;
Ok(Self)
Ok(Self(CallReturnType::DispatchResult))
} else {
Err(lookahead.error())
}
}
}

syn::parse2::<Checker>(type_.to_token_stream()).map(|_| ())
syn::parse2::<Checker>(type_.to_token_stream()).map(|c| c.0)
}

pub(crate) fn two128_str(s: &str) -> TokenStream {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// This file is part of Substrate.

// Copyright (C) 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.

#[frame_support::pallet(dev_mode)]
mod pallet {
use frame_support::pallet_prelude::*;
use frame_system::pallet_prelude::*;

#[pallet::config]
pub trait Config: frame_system::Config {}

#[pallet::pallet]
pub struct Pallet<T>(_);

#[pallet::call]
impl <T: Config> Pallet<T> {
pub fn foo(origin: OriginFor<T>) -> DispatchResultWithPostInfo {
return Err(DispatchError::BadOrigin);
}
}
}

fn main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
error[E0308]: mismatched types
--> tests/pallet_ui/call_span_for_error.rs:32:15
|
32 | return Err(DispatchError::BadOrigin);
| --- ^^^^^^^^^^^^^^^^^^^^^^^^ expected `DispatchErrorWithPostInfo<PostDispatchInfo>`, found `DispatchError`
| |
| arguments to this enum variant are incorrect
|
= note: expected struct `DispatchErrorWithPostInfo<PostDispatchInfo>`
found enum `frame_support::pallet_prelude::DispatchError`
help: the type constructed contains `frame_support::pallet_prelude::DispatchError` due to the type of the argument passed
--> tests/pallet_ui/call_span_for_error.rs:32:11
|
32 | return Err(DispatchError::BadOrigin);
| ^^^^------------------------^
| |
| this argument influences the type of `Err`
note: tuple variant defined here
--> $RUST/core/src/result.rs
|
| Err(#[stable(feature = "rust1", since = "1.0.0")] E),
| ^^^
help: call `Into::into` on this expression to convert `frame_support::pallet_prelude::DispatchError` into `DispatchErrorWithPostInfo<PostDispatchInfo>`
|
32 | return Err(DispatchError::BadOrigin.into());
| +++++++

0 comments on commit a869596

Please sign in to comment.