sp-api: impl_runtime_apis! replace the use of Self as a type argument

prdoc/pr_4012.prdoc

@@ -0,0 +1,35 @@
+# 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: "`impl_runtime_apis!`: prevent the use of `Self` as type argument"
+
+doc:
+  - audience: Runtime Dev
+    description: |
+        Currently, if there is a type alias similar to `type HeaderFor<T>` in the scope, it makes sense to expect that
+        `HeaderFor<Runtime>` and `HeaderFor<Self>` are equivalent. However, this is not the case. It currently leads to 
+        a compilation error that `Self is not in scope`, which is confusing. This PR Introduces a checker struct, similar to 
+        `CheckTraitDecl` in `decl_runtime_apis!`, `CheckTraitImpl`. It identifies usage of `Self` as a type argument in
+        `impl_runtime_apis!` and emits an error message. The error message suggests replacing `Self` with `Runtime`.
+
+        For example, the following example code is not allowed:
+        ```rust
+        impl apis::Core<Block> for Runtime {
+            fn initialize_block(header: &HeaderFor<Self>) -> ExtrinsicInclusionMode {
+                let _: HeaderFor<Self> = header.clone();
+                RuntimeExecutive::initialize_block(header)
+            }
+        }
+        ``` 
+        Instead, it should be written as:
+        ```rust
+        impl apis::Core<Block> for Runtime {
+            fn initialize_block(header: &HeaderFor<Runtime>) -> ExtrinsicInclusionMode {
+                let _: HeaderFor<Runtime> = header.clone();
+                RuntimeExecutive::initialize_block(header)
+            }
+        }
+        ```
+crates:
+  - name: sp-api
+    bump: minor

substrate/primitives/api/proc-macro/src/impl_runtime_apis.rs

@@ -18,7 +18,8 @@
 use crate::{
 	common::API_VERSION_ATTRIBUTE,
 	utils::{
-		extract_all_signature_types, extract_block_type_from_trait_path, extract_impl_trait,
+		extract_all_signature_types, extract_angle_bracketed_idents_from_type_path,
+		extract_block_type_from_trait_path, extract_impl_trait,
 		extract_parameter_names_types_and_borrows, generate_crate_access,
 		generate_runtime_mod_name_for_trait, parse_runtime_api_version, prefix_function_with_trait,
 		versioned_trait_name, AllowSelfRefInParameters, RequireQualifiedTraitPath,
@@ -35,6 +36,7 @@ use syn::{
 	parse::{Error, Parse, ParseStream, Result},
 	parse_macro_input, parse_quote,
 	spanned::Spanned,
+	visit::{self, Visit},
 	Attribute, Ident, ImplItem, ItemImpl, LitInt, LitStr, Path, Signature, Type, TypePath,
 };
 
@@ -803,6 +805,49 @@ fn generate_runtime_api_versions(impls: &[ItemImpl]) -> Result<TokenStream> {
 	))
 }
 
+/// Checks that a trait implementation is in the format we expect.
+struct CheckTraitImpl {
+	errors: Vec<Error>,
+}
+
+impl CheckTraitImpl {
+	/// Check the given trait implementation.
+	///
+	/// All errors will be collected in `self.errors`.
+	fn check(&mut self, trait_: &ItemImpl) {
+		visit::visit_item_impl(self, trait_)
+	}
+}
+
+impl<'ast> Visit<'ast> for CheckTraitImpl {
+	fn visit_type_path(&mut self, i: &'ast syn::TypePath) {
+		if extract_angle_bracketed_idents_from_type_path(i).iter().any(|i| *i == "Self") {
+			self.errors.push(Error::new(
+				i.span(),
+				"`Self` can not be used as a type argument in the scope of `impl_runtime_apis!`. Use `Runtime` instead.",
+			));
+		}
+
+		visit::visit_type_path(self, i)
+	}
+}
+
+/// Check all trait implementations are in the format we expect.
+fn check_trait_impls(impls: &[ItemImpl]) -> Result<()> {
+	let mut checker = CheckTraitImpl { errors: Vec::new() };
+
+	impls.iter().for_each(|i| checker.check(i));
+
+	if let Some(err) = checker.errors.pop() {
+		Err(checker.errors.into_iter().fold(err, |mut acc, e| {
+			acc.combine(e);
+			acc
+		}))
+	} else {
+		Ok(())
+	}
+}
+
 /// The implementation of the `impl_runtime_apis!` macro.
 pub fn impl_runtime_apis_impl(input: proc_macro::TokenStream) -> proc_macro::TokenStream {
 	// Parse all impl blocks
@@ -814,6 +859,8 @@ pub fn impl_runtime_apis_impl(input: proc_macro::TokenStream) -> proc_macro::Tok
 }
 
 fn impl_runtime_apis_impl_inner(api_impls: &[ItemImpl]) -> Result<TokenStream> {
+	check_trait_impls(api_impls)?;
+
 	let dispatch_impl = generate_dispatch_function(api_impls)?;
 	let api_impls_for_runtime = generate_api_impl_for_runtime(api_impls)?;
 	let base_runtime_api = generate_runtime_api_base_structures()?;

substrate/primitives/api/proc-macro/src/utils.rs

@@ -20,6 +20,7 @@ use inflector::Inflector;
 use proc_macro2::{Span, TokenStream};
 use proc_macro_crate::{crate_name, FoundCrate};
 use quote::{format_ident, quote};
+use std::collections::HashSet;
 use syn::{
 	parse_quote, spanned::Spanned, token::And, Attribute, Error, FnArg, GenericArgument, Ident,
 	ImplItem, ItemImpl, Pat, Path, PathArguments, Result, ReturnType, Signature, Type, TypePath,
@@ -240,6 +241,25 @@ pub fn extract_impl_trait(impl_: &ItemImpl, require: RequireQualifiedTraitPath)
 		})
 }
 
+/// Extracts all unique `Ident`'s from the given `TypePath`
+pub fn extract_angle_bracketed_idents_from_type_path(type_path: &TypePath) -> HashSet<&Ident> {
+	let mut idents = HashSet::new();
+
+	for segment in &type_path.path.segments {
+		if let PathArguments::AngleBracketed(args) = &segment.arguments {
+			args.args.iter().for_each(|arg| {
+				if let GenericArgument::Type(Type::Path(p)) = arg {
+					if let Some(ident) = p.path.get_ident() {
+						idents.insert(ident);
+					}
+				}
+			});
+		}
+	}
+
+	idents
+}
+
 /// Parse the given attribute as `API_VERSION_ATTRIBUTE`.
 pub fn parse_runtime_api_version(version: &Attribute) -> Result<u64> {
 	let version = version.parse_args::<syn::LitInt>().map_err(|_| {
@@ -328,4 +348,13 @@ mod tests {
 		assert_eq!(cfg_std, filtered[0]);
 		assert_eq!(cfg_benchmarks, filtered[1]);
 	}
+
+	#[test]
+	fn check_extract_angle_bracketed_idents_from_type_path() {
+		let type_path: TypePath = parse_quote!(polkadot_primitives::types::HeaderFor<Self>);
+		let idents = extract_angle_bracketed_idents_from_type_path(&type_path);
+
+		assert_eq!(idents.len(), 1);
+		assert!(idents.contains(&&format_ident!("Self")));
+	}
 }