From a86959607cfc855c5ec3812124f6f033db4bfbed Mon Sep 17 00:00:00 2001
From: "paritytech-cmd-bot-polkadot-sdk[bot]"
<179002856+paritytech-cmd-bot-polkadot-sdk[bot]@users.noreply.github.com>
Date: Tue, 17 Sep 2024 12:10:19 +0200
Subject: [PATCH] [stable2409] Backport #5580 (#5733)
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 <gui.thiolliere@gmail.com>
---
prdoc/pr_5580.prdoc | 13 +++++++
.../procedural/src/pallet/expand/call.rs | 36 +++++++++++++-----
.../procedural/src/pallet/parse/call.rs | 11 ++----
.../procedural/src/pallet/parse/helper.rs | 23 +++++++++---
.../tests/pallet_ui/call_span_for_error.rs | 37 +++++++++++++++++++
.../pallet_ui/call_span_for_error.stderr | 26 +++++++++++++
6 files changed, 125 insertions(+), 21 deletions(-)
create mode 100644 prdoc/pr_5580.prdoc
create mode 100644 substrate/frame/support/test/tests/pallet_ui/call_span_for_error.rs
create mode 100644 substrate/frame/support/test/tests/pallet_ui/call_span_for_error.stderr
diff --git a/prdoc/pr_5580.prdoc b/prdoc/pr_5580.prdoc
new file mode 100644
index 00000000000..e03b946070a
--- /dev/null
+++ b/prdoc/pr_5580.prdoc
@@ -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
diff --git a/substrate/frame/support/procedural/src/pallet/expand/call.rs b/substrate/frame/support/procedural/src/pallet/expand/call.rs
index f395872c8a8..5dc8dc3146c 100644
--- a/substrate/frame/support/procedural/src/pallet/expand/call.rs
+++ b/substrate/frame/support/procedural/src/pallet/expand/call.rs
@@ -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,
@@ -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
+ )
}};
}
});
diff --git a/substrate/frame/support/procedural/src/pallet/parse/call.rs b/substrate/frame/support/procedural/src/pallet/parse/call.rs
index 4e09b86fdde..68c2cb8bd1b 100644
--- a/substrate/frame/support/procedural/src/pallet/parse/call.rs
+++ b/substrate/frame/support/procedural/src/pallet/parse/call.rs
@@ -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.
@@ -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![];
@@ -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";
diff --git a/substrate/frame/support/procedural/src/pallet/parse/helper.rs b/substrate/frame/support/procedural/src/pallet/parse/helper.rs
index d4f58a4c56d..d5ae607d90f 100644
--- a/substrate/frame/support/procedural/src/pallet/parse/helper.rs
+++ b/substrate/frame/support/procedural/src/pallet/parse/helper.rs
@@ -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 {
diff --git a/substrate/frame/support/test/tests/pallet_ui/call_span_for_error.rs b/substrate/frame/support/test/tests/pallet_ui/call_span_for_error.rs
new file mode 100644
index 00000000000..08b42c29a68
--- /dev/null
+++ b/substrate/frame/support/test/tests/pallet_ui/call_span_for_error.rs
@@ -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() {}
diff --git a/substrate/frame/support/test/tests/pallet_ui/call_span_for_error.stderr b/substrate/frame/support/test/tests/pallet_ui/call_span_for_error.stderr
new file mode 100644
index 00000000000..8f3003c0222
--- /dev/null
+++ b/substrate/frame/support/test/tests/pallet_ui/call_span_for_error.stderr
@@ -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());
+ | +++++++
--
GitLab