From ce87ac5d6913102cb3086cd911c4f5a5603ae139 Mon Sep 17 00:00:00 2001 From: Nikhil Gupta <17176722+gupnik@users.noreply.github.com> Date: Thu, 30 May 2024 10:49:55 +0530 Subject: [PATCH 1/4] Better error for missing index in CRV2 --- .../procedural/src/runtime/parse/mod.rs | 14 +++++++--- .../tests/runtime_ui/missing_pallet_index.rs | 27 +++++++++++++++++++ .../runtime_ui/missing_pallet_index.stderr | 5 ++++ 3 files changed, 43 insertions(+), 3 deletions(-) create mode 100644 substrate/frame/support/test/tests/runtime_ui/missing_pallet_index.rs create mode 100644 substrate/frame/support/test/tests/runtime_ui/missing_pallet_index.stderr diff --git a/substrate/frame/support/procedural/src/runtime/parse/mod.rs b/substrate/frame/support/procedural/src/runtime/parse/mod.rs index 893cb4726e2b..79e9a380ce00 100644 --- a/substrate/frame/support/procedural/src/runtime/parse/mod.rs +++ b/substrate/frame/support/procedural/src/runtime/parse/mod.rs @@ -153,7 +153,9 @@ impl Def { for item in items.iter_mut() { let mut pallet_item = None; - let mut pallet_index = 0; + let mut pallet_index = None; + + let mut is_item_runtime_struct = false; let mut disable_call = false; let mut disable_unsigned = false; @@ -165,12 +167,13 @@ impl Def { RuntimeAttr::Runtime(span) if runtime_struct.is_none() => { let p = runtime_struct::RuntimeStructDef::try_from(span, item)?; runtime_struct = Some(p); + is_item_runtime_struct = true; }, RuntimeAttr::Derive(_, types) if runtime_types.is_none() => { runtime_types = Some(types); }, RuntimeAttr::PalletIndex(span, index) => { - pallet_index = index; + pallet_index = Some(index); pallet_item = if let syn::Item::Type(item) = item { Some(item.clone()) } else { @@ -187,6 +190,11 @@ impl Def { } } + if !is_item_runtime_struct && pallet_index.is_none() { + let msg = "Missing pallet index for pallet declaration. Please add `#[runtime::pallet_index(...)]`"; + return Err(syn::Error::new(item.span(), &msg)) + } + if let Some(pallet_item) = pallet_item { match *pallet_item.ty.clone() { syn::Type::Path(ref path) => { @@ -209,7 +217,7 @@ impl Def { let pallet = Pallet::try_from( item.span(), &pallet_item, - pallet_index, + pallet_index.expect("Pallet item is present only for valid pallet index").into(), disable_call, disable_unsigned, &bounds, diff --git a/substrate/frame/support/test/tests/runtime_ui/missing_pallet_index.rs b/substrate/frame/support/test/tests/runtime_ui/missing_pallet_index.rs new file mode 100644 index 000000000000..469a7833e5af --- /dev/null +++ b/substrate/frame/support/test/tests/runtime_ui/missing_pallet_index.rs @@ -0,0 +1,27 @@ +// 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::runtime] +mod runtime { + #[runtime::runtime] + #[runtime::derive(RuntimeCall)] + pub struct Runtime; + + pub type System = frame_system; +} + +fn main() {} diff --git a/substrate/frame/support/test/tests/runtime_ui/missing_pallet_index.stderr b/substrate/frame/support/test/tests/runtime_ui/missing_pallet_index.stderr new file mode 100644 index 000000000000..a2cbaa48199d --- /dev/null +++ b/substrate/frame/support/test/tests/runtime_ui/missing_pallet_index.stderr @@ -0,0 +1,5 @@ +error: Missing pallet index for pallet declaration. Please add `#[runtime::pallet_index(...)]` + --> tests/runtime_ui/missing_pallet_index.rs:24:5 + | +24 | pub type System = frame_system; + | ^^^ From eaf293f3785beeae407f3223d5ea89c46b294958 Mon Sep 17 00:00:00 2001 From: Nikhil Gupta <17176722+gupnik@users.noreply.github.com> Date: Thu, 30 May 2024 11:13:33 +0530 Subject: [PATCH 2/4] Minor fix --- .../frame/support/procedural/src/runtime/parse/mod.rs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/substrate/frame/support/procedural/src/runtime/parse/mod.rs b/substrate/frame/support/procedural/src/runtime/parse/mod.rs index 79e9a380ce00..a9cacb73626e 100644 --- a/substrate/frame/support/procedural/src/runtime/parse/mod.rs +++ b/substrate/frame/support/procedural/src/runtime/parse/mod.rs @@ -155,8 +155,6 @@ impl Def { let mut pallet_item = None; let mut pallet_index = None; - let mut is_item_runtime_struct = false; - let mut disable_call = false; let mut disable_unsigned = false; @@ -167,7 +165,6 @@ impl Def { RuntimeAttr::Runtime(span) if runtime_struct.is_none() => { let p = runtime_struct::RuntimeStructDef::try_from(span, item)?; runtime_struct = Some(p); - is_item_runtime_struct = true; }, RuntimeAttr::Derive(_, types) if runtime_types.is_none() => { runtime_types = Some(types); @@ -190,9 +187,11 @@ impl Def { } } - if !is_item_runtime_struct && pallet_index.is_none() { - let msg = "Missing pallet index for pallet declaration. Please add `#[runtime::pallet_index(...)]`"; - return Err(syn::Error::new(item.span(), &msg)) + if pallet_index.is_none() { + if let syn::Item::Type(item) = item { + let msg = "Missing pallet index for pallet declaration. Please add `#[runtime::pallet_index(...)]`"; + return Err(syn::Error::new(item.span(), &msg)) + } } if let Some(pallet_item) = pallet_item { From 8729b9eae4420b3939fb0d7045a903e47e484367 Mon Sep 17 00:00:00 2001 From: command-bot <> Date: Fri, 31 May 2024 04:31:51 +0000 Subject: [PATCH 3/4] ".git/.scripts/commands/fmt/fmt.sh" --- substrate/frame/support/procedural/src/runtime/parse/mod.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/substrate/frame/support/procedural/src/runtime/parse/mod.rs b/substrate/frame/support/procedural/src/runtime/parse/mod.rs index a9cacb73626e..e1766a0d4bbd 100644 --- a/substrate/frame/support/procedural/src/runtime/parse/mod.rs +++ b/substrate/frame/support/procedural/src/runtime/parse/mod.rs @@ -216,7 +216,9 @@ impl Def { let pallet = Pallet::try_from( item.span(), &pallet_item, - pallet_index.expect("Pallet item is present only for valid pallet index").into(), + pallet_index + .expect("Pallet item is present only for valid pallet index") + .into(), disable_call, disable_unsigned, &bounds, From 37f123579599ff5781a0ac1b0ae87ee76dabab53 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Sun, 2 Jun 2024 19:59:39 +0200 Subject: [PATCH 4/4] Remove the `expect` --- .../procedural/src/runtime/parse/mod.rs | 26 +++++++------------ 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/substrate/frame/support/procedural/src/runtime/parse/mod.rs b/substrate/frame/support/procedural/src/runtime/parse/mod.rs index e1766a0d4bbd..dd83cd0da90a 100644 --- a/substrate/frame/support/procedural/src/runtime/parse/mod.rs +++ b/substrate/frame/support/procedural/src/runtime/parse/mod.rs @@ -152,8 +152,7 @@ impl Def { let mut pallets = vec![]; for item in items.iter_mut() { - let mut pallet_item = None; - let mut pallet_index = None; + let mut pallet_index_and_item = None; let mut disable_call = false; let mut disable_unsigned = false; @@ -170,9 +169,8 @@ impl Def { runtime_types = Some(types); }, RuntimeAttr::PalletIndex(span, index) => { - pallet_index = Some(index); - pallet_item = if let syn::Item::Type(item) = item { - Some(item.clone()) + pallet_index_and_item = if let syn::Item::Type(item) = item { + Some((index, item.clone())) } else { let msg = "Invalid runtime::pallet_index, expected type definition"; return Err(syn::Error::new(span, msg)) @@ -187,14 +185,7 @@ impl Def { } } - if pallet_index.is_none() { - if let syn::Item::Type(item) = item { - let msg = "Missing pallet index for pallet declaration. Please add `#[runtime::pallet_index(...)]`"; - return Err(syn::Error::new(item.span(), &msg)) - } - } - - if let Some(pallet_item) = pallet_item { + if let Some((pallet_index, pallet_item)) = pallet_index_and_item { match *pallet_item.ty.clone() { syn::Type::Path(ref path) => { let pallet_decl = @@ -216,9 +207,7 @@ impl Def { let pallet = Pallet::try_from( item.span(), &pallet_item, - pallet_index - .expect("Pallet item is present only for valid pallet index") - .into(), + pallet_index, disable_call, disable_unsigned, &bounds, @@ -239,6 +228,11 @@ impl Def { }, _ => continue, } + } else { + if let syn::Item::Type(item) = item { + let msg = "Missing pallet index for pallet declaration. Please add `#[runtime::pallet_index(...)]`"; + return Err(syn::Error::new(item.span(), &msg)) + } } }