From 325dab19cc2a7c4b508fc6cc41857407e2b1c13f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= <bkchr@users.noreply.github.com> Date: Thu, 18 Jun 2020 09:35:18 +0200 Subject: [PATCH] `decl_module!` print better error on duplicate reserved keyword (#6384) * `decl_module!` print better error on duplicate reserved keyword This prints a better error message on duplicated reserved keywords, instead of complaining because of missing `origin`. * Review feedback --- substrate/frame/support/src/dispatch.rs | 140 +++++++++++++++++- .../support/test/tests/decl_module_ui.rs | 26 ++++ ...served_keyword_two_times_integrity_test.rs | 7 + ...ed_keyword_two_times_integrity_test.stderr | 25 ++++ ...eserved_keyword_two_times_on_initialize.rs | 11 ++ ...ved_keyword_two_times_on_initialize.stderr | 25 ++++ 6 files changed, 233 insertions(+), 1 deletion(-) create mode 100644 substrate/frame/support/test/tests/decl_module_ui.rs create mode 100644 substrate/frame/support/test/tests/decl_module_ui/reserved_keyword_two_times_integrity_test.rs create mode 100644 substrate/frame/support/test/tests/decl_module_ui/reserved_keyword_two_times_integrity_test.stderr create mode 100644 substrate/frame/support/test/tests/decl_module_ui/reserved_keyword_two_times_on_initialize.rs create mode 100644 substrate/frame/support/test/tests/decl_module_ui/reserved_keyword_two_times_on_initialize.stderr diff --git a/substrate/frame/support/src/dispatch.rs b/substrate/frame/support/src/dispatch.rs index 094cbce2634..edb6e626396 100644 --- a/substrate/frame/support/src/dispatch.rs +++ b/substrate/frame/support/src/dispatch.rs @@ -399,6 +399,29 @@ macro_rules! decl_module { "`deposit_event` function is reserved and must follow the syntax: `$vis:vis fn deposit_event() = default;`" ); }; + // Compile error on `deposit_event` being added a second time. + (@normalize + $(#[$attr:meta])* + pub struct $mod_type:ident< + $trait_instance:ident: $trait_name:ident$(<I>, I: $instantiable:path $(= $module_default_instance:path)?)? + > + for enum $call_type:ident where origin: $origin_type:ty, system = $system:ident + { $( $other_where_bounds:tt )* } + { $( $deposit_event:tt )+ } + { $( $on_initialize:tt )* } + { $( $on_runtime_upgrade:tt )* } + { $( $on_finalize:tt )* } + { $( $offchain:tt )* } + { $( $constants:tt )* } + { $( $error_type:tt )* } + { $( $integrity_test:tt )* } + [ $( $dispatchables:tt )* ] + $(#[doc = $doc_attr:tt])* + $vis:vis fn deposit_event() = default; + $($rest:tt)* + ) => { + compile_error!("`deposit_event` can only be passed once as input."); + }; // Add on_finalize (@normalize $(#[$attr:meta])* @@ -462,6 +485,30 @@ macro_rules! decl_module { `on_initialize` or `on_runtime_upgrade` instead" ); }; + // Compile error on `on_finalize` being added a second time. + (@normalize + $(#[$attr:meta])* + pub struct $mod_type:ident< + $trait_instance:ident: $trait_name:ident$(<I>, I: $instantiable:path $(= $module_default_instance:path)?)? + > + for enum $call_type:ident where origin: $origin_type:ty, system = $system:ident + { $( $other_where_bounds:tt )* } + { $( $deposit_event:tt )* } + { $( $on_initialize:tt )* } + { $( $on_runtime_upgrade:tt )* } + { $( $on_finalize:tt )+ } + { $( $offchain:tt )* } + { $( $constants:tt )* } + { $( $error_type:tt )* } + { $( $integrity_test:tt )* } + [ $( $dispatchables:tt )* ] + $(#[doc = $doc_attr:tt])* + #[weight = $weight:expr] + fn on_finalize( $( $param_name:ident : $param:ty ),* $(,)? ) { $( $impl:tt )* } + $($rest:tt)* + ) => { + compile_error!("`on_finalize` can only be passed once as input."); + }; // compile_error on_runtime_upgrade, without a given weight removed syntax. (@normalize $(#[$attr:meta])* @@ -554,6 +601,29 @@ macro_rules! decl_module { $($rest)* ); }; + // Compile error on `on_runtime_upgrade` being added a second time. + (@normalize + $(#[$attr:meta])* + pub struct $mod_type:ident< + $trait_instance:ident: $trait_name:ident$(<I>, I: $instantiable:path $(= $module_default_instance:path)?)? + > + for enum $call_type:ident where origin: $origin_type:ty, system = $system:ident + { $( $other_where_bounds:tt )* } + { $( $deposit_event:tt )* } + { $( $on_initialize:tt )* } + { $( $on_runtime_upgrade:tt )+ } + { $( $on_finalize:tt )* } + { $( $offchain:tt )* } + { $( $constants:tt )* } + { $( $error_type:tt )* } + { $( $integrity_test:tt )* } + [ $( $dispatchables:tt )* ] + $(#[doc = $doc_attr:tt])* + fn on_runtime_upgrade( $( $param_name:ident : $param:ty ),* $(,)? ) -> $return:ty { $( $impl:tt )* } + $($rest:tt)* + ) => { + compile_error!("`on_runtime_upgrade` can only be passed once as input."); + }; // Add integrity_test (@normalize $(#[$attr:meta])* @@ -595,6 +665,29 @@ macro_rules! decl_module { $($rest)* ); }; + // Compile error on `integrity_test` being added a second time. + (@normalize + $(#[$attr:meta])* + pub struct $mod_type:ident< + $trait_instance:ident: $trait_name:ident$(<I>, I: $instantiable:path $(= $module_default_instance:path)?)? + > + for enum $call_type:ident where origin: $origin_type:ty, system = $system:ident + { $( $other_where_bounds:tt )* } + { $( $deposit_event:tt )* } + { $( $on_initialize:tt )* } + { $( $on_runtime_upgrade:tt )* } + { $( $on_finalize:tt )* } + { $( $offchain:tt )* } + { $( $constants:tt )* } + { $( $error_type:tt )* } + { $( $integrity_test:tt )+ } + [ $( $dispatchables:tt )* ] + $(#[doc = $doc_attr:tt])* + fn integrity_test() { $( $impl:tt )* } + $($rest:tt)* + ) => { + compile_error!("`integrity_test` can only be passed once as input."); + }; // compile_error on_initialize, without a given weight removed syntax. (@normalize $(#[$attr:meta])* @@ -687,6 +780,29 @@ macro_rules! decl_module { $($rest)* ); }; + // Compile error on trying to add a second `on_initialize`. + (@normalize + $(#[$attr:meta])* + pub struct $mod_type:ident< + $trait_instance:ident: $trait_name:ident$(<I>, I: $instantiable:path $(= $module_default_instance:path)?)? + > + for enum $call_type:ident where origin: $origin_type:ty, system = $system:ident + { $( $other_where_bounds:tt )* } + { $( $deposit_event:tt )* } + { $( $on_initialize:tt )+ } + { $( $on_runtime_upgrade:tt )* } + { $( $on_finalize:tt )* } + { $( $offchain:tt )* } + { $( $constants:tt )* } + { $( $error_type:tt )* } + { $( $integrity_test:tt )* } + [ $( $dispatchables:tt )* ] + $(#[doc = $doc_attr:tt])* + fn on_initialize( $( $param_name:ident : $param:ty ),* $(,)? ) -> $return:ty { $( $impl:tt )* } + $($rest:tt)* + ) => { + compile_error!("`on_initialize` can only be passed once as input."); + }; (@normalize $(#[$attr:meta])* pub struct $mod_type:ident< @@ -727,7 +843,29 @@ macro_rules! decl_module { $($rest)* ); }; - + // Compile error on trying to add a second `offchain_worker`. + (@normalize + $(#[$attr:meta])* + pub struct $mod_type:ident< + $trait_instance:ident: $trait_name:ident$(<I>, I: $instantiable:path $(= $module_default_instance:path)?)? + > + for enum $call_type:ident where origin: $origin_type:ty, system = $system:ident + { $( $other_where_bounds:tt )* } + { $( $deposit_event:tt )* } + { $( $on_initialize:tt )* } + { $( $on_runtime_upgrade:tt )* } + { $( $on_finalize:tt )* } + { $( $offchain:tt )+ } + { $( $constants:tt )* } + { $( $error_type:tt )* } + { $( $integrity_test:tt )* } + [ $( $dispatchables:tt )* ] + $(#[doc = $doc_attr:tt])* + fn offchain_worker( $( $param_name:ident : $param:ty ),* $(,)? ) -> $return:ty { $( $impl:tt )* } + $($rest:tt)* + ) => { + compile_error!("`offchain_worker` can only be passed once as input."); + }; // This puts a constant in the parsed constants list. (@normalize $(#[$attr:meta])* diff --git a/substrate/frame/support/test/tests/decl_module_ui.rs b/substrate/frame/support/test/tests/decl_module_ui.rs new file mode 100644 index 00000000000..90d105e7cfa --- /dev/null +++ b/substrate/frame/support/test/tests/decl_module_ui.rs @@ -0,0 +1,26 @@ +// This file is part of Substrate. + +// Copyright (C) 2020 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. + +//#[rustversion::attr(not(stable), ignore)] +#[test] +fn decl_module_ui() { + // As trybuild is using `cargo check`, we don't need the real WASM binaries. + std::env::set_var("BUILD_DUMMY_WASM_BINARY", "1"); + + let t = trybuild::TestCases::new(); + t.compile_fail("tests/decl_module_ui/*.rs"); +} diff --git a/substrate/frame/support/test/tests/decl_module_ui/reserved_keyword_two_times_integrity_test.rs b/substrate/frame/support/test/tests/decl_module_ui/reserved_keyword_two_times_integrity_test.rs new file mode 100644 index 00000000000..4dbae05f07f --- /dev/null +++ b/substrate/frame/support/test/tests/decl_module_ui/reserved_keyword_two_times_integrity_test.rs @@ -0,0 +1,7 @@ +frame_support::decl_module! { + pub struct Module<T: Trait> for enum Call where origin: T::Origin { + fn integrity_test() {} + + fn integrity_test() {} + } +} diff --git a/substrate/frame/support/test/tests/decl_module_ui/reserved_keyword_two_times_integrity_test.stderr b/substrate/frame/support/test/tests/decl_module_ui/reserved_keyword_two_times_integrity_test.stderr new file mode 100644 index 00000000000..d6498961d31 --- /dev/null +++ b/substrate/frame/support/test/tests/decl_module_ui/reserved_keyword_two_times_integrity_test.stderr @@ -0,0 +1,25 @@ +error: `integrity_test` can only be passed once as input. + --> $DIR/reserved_keyword_two_times_integrity_test.rs:1:1 + | +1 | / frame_support::decl_module! { +2 | | pub struct Module<T: Trait> for enum Call where origin: T::Origin { +3 | | fn integrity_test() {} +4 | | +5 | | fn integrity_test() {} +6 | | } +7 | | } + | |_^ + | + = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) + +error[E0601]: `main` function not found in crate `$CRATE` + --> $DIR/reserved_keyword_two_times_integrity_test.rs:1:1 + | +1 | / frame_support::decl_module! { +2 | | pub struct Module<T: Trait> for enum Call where origin: T::Origin { +3 | | fn integrity_test() {} +4 | | +5 | | fn integrity_test() {} +6 | | } +7 | | } + | |_^ consider adding a `main` function to `$DIR/tests/decl_module_ui/reserved_keyword_two_times_integrity_test.rs` diff --git a/substrate/frame/support/test/tests/decl_module_ui/reserved_keyword_two_times_on_initialize.rs b/substrate/frame/support/test/tests/decl_module_ui/reserved_keyword_two_times_on_initialize.rs new file mode 100644 index 00000000000..4f05134997e --- /dev/null +++ b/substrate/frame/support/test/tests/decl_module_ui/reserved_keyword_two_times_on_initialize.rs @@ -0,0 +1,11 @@ +frame_support::decl_module! { + pub struct Module<T: Trait> for enum Call where origin: T::Origin { + fn on_initialize() -> Weight { + 0 + } + + fn on_initialize() -> Weight { + 0 + } + } +} diff --git a/substrate/frame/support/test/tests/decl_module_ui/reserved_keyword_two_times_on_initialize.stderr b/substrate/frame/support/test/tests/decl_module_ui/reserved_keyword_two_times_on_initialize.stderr new file mode 100644 index 00000000000..8a9f025046b --- /dev/null +++ b/substrate/frame/support/test/tests/decl_module_ui/reserved_keyword_two_times_on_initialize.stderr @@ -0,0 +1,25 @@ +error: `on_initialize` can only be passed once as input. + --> $DIR/reserved_keyword_two_times_on_initialize.rs:1:1 + | +1 | / frame_support::decl_module! { +2 | | pub struct Module<T: Trait> for enum Call where origin: T::Origin { +3 | | fn on_initialize() -> Weight { +4 | | 0 +... | +10 | | } +11 | | } + | |_^ + | + = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) + +error[E0601]: `main` function not found in crate `$CRATE` + --> $DIR/reserved_keyword_two_times_on_initialize.rs:1:1 + | +1 | / frame_support::decl_module! { +2 | | pub struct Module<T: Trait> for enum Call where origin: T::Origin { +3 | | fn on_initialize() -> Weight { +4 | | 0 +... | +10 | | } +11 | | } + | |_^ consider adding a `main` function to `$DIR/tests/decl_module_ui/reserved_keyword_two_times_on_initialize.rs` -- GitLab