diff --git a/substrate/primitives/api/proc-macro/src/decl_runtime_apis.rs b/substrate/primitives/api/proc-macro/src/decl_runtime_apis.rs index e9f3087912e26f322214fa22cdbca397a4a2c332..ef50bd840a7cccc8849f9741c30e51b9ce660f20 100644 --- a/substrate/primitives/api/proc-macro/src/decl_runtime_apis.rs +++ b/substrate/primitives/api/proc-macro/src/decl_runtime_apis.rs @@ -876,6 +876,53 @@ struct CheckTraitDecl { errors: Vec<Error>, } +impl CheckTraitDecl { + /// Check the given trait. + /// + /// All errors will be collected in `self.errors`. + fn check(&mut self, trait_: &ItemTrait) { + self.check_method_declarations(trait_.items.iter().filter_map(|i| match i { + TraitItem::Method(method) => Some(method), + _ => None, + })); + + visit::visit_item_trait(self, trait_); + } + + /// Check that the given method declarations are correct. + /// + /// Any error is stored in `self.errors`. + fn check_method_declarations<'a>(&mut self, methods: impl Iterator<Item = &'a TraitItemMethod>) { + let mut method_to_signature_changed = HashMap::<Ident, Vec<Option<u64>>>::new(); + + methods.into_iter().for_each(|method| { + let attributes = remove_supported_attributes(&mut method.attrs.clone()); + + let changed_in = match get_changed_in(&attributes) { + Ok(r) => r, + Err(e) => { self.errors.push(e); return; }, + }; + + method_to_signature_changed + .entry(method.sig.ident.clone()) + .or_default() + .push(changed_in); + }); + + method_to_signature_changed.into_iter().for_each(|(f, changed)| { + // If `changed_in` is `None`, it means it is the current "default" method that calls + // into the latest implementation. + if changed.iter().filter(|c| c.is_none()).count() == 0 { + self.errors.push(Error::new( + f.span(), + "There is no 'default' method with this name (without `changed_in` attribute).\n\ + The 'default' method is used to call into the latest implementation.", + )); + } + }); + } +} + impl<'ast> Visit<'ast> for CheckTraitDecl { fn visit_fn_arg(&mut self, input: &'ast FnArg) { if let FnArg::Receiver(_) = input { @@ -923,7 +970,7 @@ impl<'ast> Visit<'ast> for CheckTraitDecl { /// Check that the trait declarations are in the format we expect. fn check_trait_decls(decls: &[ItemTrait]) -> Result<()> { let mut checker = CheckTraitDecl { errors: Vec::new() }; - decls.iter().for_each(|decl| visit::visit_item_trait(&mut checker, &decl)); + decls.iter().for_each(|decl| checker.check(decl)); if let Some(err) = checker.errors.pop() { Err(checker.errors.into_iter().fold(err, |mut err, other| { diff --git a/substrate/primitives/api/src/lib.rs b/substrate/primitives/api/src/lib.rs index 74bcf19a9949e43316001d34b7ccfe3bda2b73cb..80a36a904c39431f8e608c5dc3f5f1d348d300c4 100644 --- a/substrate/primitives/api/src/lib.rs +++ b/substrate/primitives/api/src/lib.rs @@ -113,7 +113,9 @@ use std::{panic::UnwindSafe, cell::RefCell}; /// change is highlighted with the `#[changed_in(2)]` attribute above a method. A method that is /// tagged with this attribute is callable by the name `METHOD_before_version_VERSION`. This /// method will only support calling into wasm, trying to call into native will fail (change the -/// spec version!). Such a method also does not need to be implemented in the runtime. +/// spec version!). Such a method also does not need to be implemented in the runtime. It is +/// required that there exist the "default" of the method without the `#[changed_in(_)]` attribute, +/// this method will be used to call the current default implementation. /// /// ```rust /// sp_api::decl_runtime_apis! { diff --git a/substrate/primitives/api/test/tests/ui/changed_in_no_default_method.rs b/substrate/primitives/api/test/tests/ui/changed_in_no_default_method.rs new file mode 100644 index 0000000000000000000000000000000000000000..6af183a4cde910248575e93ca57e437a24950fbf --- /dev/null +++ b/substrate/primitives/api/test/tests/ui/changed_in_no_default_method.rs @@ -0,0 +1,19 @@ +use sp_runtime::traits::GetNodeBlockType; +use substrate_test_runtime_client::runtime::Block; + +/// The declaration of the `Runtime` type and the implementation of the `GetNodeBlockType` +/// trait are done by the `construct_runtime!` macro in a real runtime. +struct Runtime {} +impl GetNodeBlockType for Runtime { + type NodeBlock = Block; +} + +sp_api::decl_runtime_apis! { + #[api_version(2)] + pub trait Api { + #[changed_in(2)] + fn test(data: u64); + } +} + +fn main() {} diff --git a/substrate/primitives/api/test/tests/ui/changed_in_no_default_method.stderr b/substrate/primitives/api/test/tests/ui/changed_in_no_default_method.stderr new file mode 100644 index 0000000000000000000000000000000000000000..ed4c0f9088573e78211f6e9560fc781e24936c02 --- /dev/null +++ b/substrate/primitives/api/test/tests/ui/changed_in_no_default_method.stderr @@ -0,0 +1,6 @@ +error: There is no 'default' method with this name (without `changed_in` attribute). +The 'default' method is used to call into the latest implementation. + --> $DIR/changed_in_no_default_method.rs:15:6 + | +15 | fn test(data: u64); + | ^^^^