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

prdoc/pr_4012.prdoc

@@ -0,0 +1,37 @@
+# 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!`: replace the use of `Self` with `Runtime`"
+
+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 visitor, similar to 
+        `CheckTraitDecl` in `decl_runtime_apis!`, `ReplaceSelfImpl`. It identifies usage of `Self` as a type argument in
+        `impl_runtime_apis!` and replaces `Self` with an explicit `Runtime` type.
+
+        For example, the following example code will be transformed before expansion:
+        ```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 will be passed to macro 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: none
+  - name: sp-api-proc-macro
+    bump: none

substrate/primitives/api/proc-macro/Cargo.toml

@@ -20,7 +20,7 @@ proc-macro = true
 
 [dependencies]
 quote = { workspace = true }
-syn = { features = ["extra-traits", "fold", "full", "visit"], workspace = true }
+syn = { features = ["extra-traits", "fold", "full", "visit", "visit-mut"], workspace = true }
 proc-macro2 = { workspace = true }
 blake2 = { workspace = true }
 proc-macro-crate = { workspace = true }

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

@@ -7,7 +7,7 @@
 // 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
+//  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,
@@ -35,7 +35,9 @@ use syn::{
 	parse::{Error, Parse, ParseStream, Result},
 	parse_macro_input, parse_quote,
 	spanned::Spanned,
-	Attribute, Ident, ImplItem, ItemImpl, LitInt, LitStr, Path, Signature, Type, TypePath,
+	visit_mut::{self, VisitMut},
+	Attribute, GenericArgument, Ident, ImplItem, ItemImpl, LitInt, LitStr, Path, Signature, Type,
+	TypePath,
 };
 
 use std::collections::HashMap;
@@ -227,34 +229,34 @@ fn generate_wasm_interface(impls: &[ItemImpl]) -> Result<TokenStream> {
 	let c = generate_crate_access();
 
 	let impl_calls =
-		generate_impl_calls(impls, &input)?
-			.into_iter()
-			.map(|(trait_, fn_name, impl_, attrs)| {
-				let fn_name =
-					Ident::new(&prefix_function_with_trait(&trait_, &fn_name), Span::call_site());
-
-				quote!(
-					#c::std_disabled! {
-						#( #attrs )*
-						#[no_mangle]
-						#[cfg_attr(any(target_arch = "riscv32", target_arch = "riscv64"), #c::__private::polkavm_export(abi = #c::__private::polkavm_abi))]
-						pub unsafe extern fn #fn_name(input_data: *mut u8, input_len: usize) -> u64 {
-							let mut #input = if input_len == 0 {
-								&[0u8; 0]
-							} else {
-								unsafe {
-									::core::slice::from_raw_parts(input_data, input_len)
-								}
-							};
-
-							#c::init_runtime_logger();
-
-							let output = (move || { #impl_ })();
-							#c::to_substrate_wasm_fn_return_value(&output)
-						}
-					}
-				)
-			});
+        generate_impl_calls(impls, &input)?
+            .into_iter()
+            .map(|(trait_, fn_name, impl_, attrs)| {
+                let fn_name =
+                    Ident::new(&prefix_function_with_trait(&trait_, &fn_name), Span::call_site());
+
+                quote!(
+                    #c::std_disabled! {
+                        #( #attrs )*
+                        #[no_mangle]
+                        #[cfg_attr(any(target_arch = "riscv32", target_arch = "riscv64"), #c::__private::polkavm_export(abi = #c::__private::polkavm_abi))]
+                        pub unsafe extern fn #fn_name(input_data: *mut u8, input_len: usize) -> u64 {
+                            let mut #input = if input_len == 0 {
+                                &[0u8; 0]
+                            } else {
+                                unsafe {
+                                    ::core::slice::from_raw_parts(input_data, input_len)
+                                }
+                            };
+
+                            #c::init_runtime_logger();
+
+                            let output = (move || { #impl_ })();
+                            #c::to_substrate_wasm_fn_return_value(&output)
+                        }
+                    }
+                )
+            });
 
 	Ok(quote!( #( #impl_calls )* ))
 }
@@ -396,10 +398,10 @@ fn generate_runtime_api_base_structures() -> Result<TokenStream> {
 			impl<Block: #crate_::BlockT, C: #crate_::CallApiAt<Block>> RuntimeApiImpl<Block, C> {
 				fn commit_or_rollback_transaction(&self, commit: bool) {
 					let proof = "\
-					We only close a transaction when we opened one ourself.
-					Other parts of the runtime that make use of transactions (state-machine)
-					also balance their transactions. The runtime cannot close client initiated
-					transactions; qed";
+                    We only close a transaction when we opened one ourself.
+                    Other parts of the runtime that make use of transactions (state-machine)
+                    also balance their transactions. The runtime cannot close client initiated
+                    transactions; qed";
 
 					let res = if commit {
 						let res = if let Some(recorder) = &self.recorder {
@@ -740,8 +742,8 @@ fn generate_runtime_api_versions(impls: &[ItemImpl]) -> Result<TokenStream> {
 			let mut error = Error::new(
 				span,
 				"Two traits with the same name detected! \
-					The trait name is used to generate its ID. \
-					Please rename one trait at the declaration!",
+                    The trait name is used to generate its ID. \
+                    Please rename one trait at the declaration!",
 			);
 
 			error.combine(Error::new(other_span, "First trait implementation."));
@@ -787,17 +789,55 @@ fn generate_runtime_api_versions(impls: &[ItemImpl]) -> Result<TokenStream> {
 	))
 }
 
+/// replaces `Self` with explicit `ItemImpl.self_ty`.
+struct ReplaceSelfImpl {
+	self_ty: Box<Type>,
+}
+
+impl ReplaceSelfImpl {
+	/// Replace `Self` with `ItemImpl.self_ty`
+	fn replace(&mut self, trait_: &mut ItemImpl) {
+		visit_mut::visit_item_impl_mut(self, trait_)
+	}
+}
+
+impl VisitMut for ReplaceSelfImpl {
+	fn visit_generic_argument_mut(&mut self, i: &mut syn::GenericArgument) {
+		let typ = match i {
+			GenericArgument::Type(Type::Path(p))
+				if p.path.get_ident().is_some_and(|ident| ident == "Self") =>
+				Some(GenericArgument::Type(*self.self_ty.clone())),
+			_ => None,
+		};
+		if let Some(typ) = typ {
+			*i = typ;
+		}
+	}
+}
+
+/// Rename `Self` to `ItemImpl.self_ty` in all items.
+fn rename_self_in_trait_impls(impls: &mut [ItemImpl]) -> Result<()> {
+	impls.iter_mut().for_each(|i| {
+		let mut checker = ReplaceSelfImpl { self_ty: i.self_ty.clone() };
+		checker.replace(i);
+	});
+
+	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
-	let RuntimeApiImpls { impls: api_impls } = parse_macro_input!(input as RuntimeApiImpls);
+	let RuntimeApiImpls { impls: mut api_impls } = parse_macro_input!(input as RuntimeApiImpls);
 
-	impl_runtime_apis_impl_inner(&api_impls)
+	impl_runtime_apis_impl_inner(&mut api_impls)
 		.unwrap_or_else(|e| e.to_compile_error())
 		.into()
 }
 
-fn impl_runtime_apis_impl_inner(api_impls: &[ItemImpl]) -> Result<TokenStream> {
+fn impl_runtime_apis_impl_inner(api_impls: &mut [ItemImpl]) -> Result<TokenStream> {
+	rename_self_in_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()?;
@@ -910,7 +950,7 @@ fn extract_api_version(attrs: &Vec<Attribute>, span: Span) -> Result<ApiVersion>
 			span,
 			format!(
 				"Found multiple #[{}] attributes for an API implementation. \
-				Each runtime API can have only one version.",
+                Each runtime API can have only one version.",
 				API_VERSION_ATTRIBUTE
 			),
 		));
@@ -945,4 +985,34 @@ mod tests {
 		assert_eq!(cfg_std, filtered[0]);
 		assert_eq!(cfg_benchmarks, filtered[1]);
 	}
+
+	#[test]
+	fn impl_trait_rename_self_param() {
+		let code = quote::quote! {
+			impl client::Core<Block> for Runtime {
+				fn initialize_block(header: &HeaderFor<Self>) -> Output<Self> {
+					let _: HeaderFor<Self> = header.clone();
+					example_fn::<Self>(header)
+				}
+			}
+		};
+		let expected = quote::quote! {
+			impl client::Core<Block> for Runtime {
+				fn initialize_block(header: &HeaderFor<Runtime>) -> Output<Runtime> {
+					let _: HeaderFor<Runtime> = header.clone();
+					example_fn::<Runtime>(header)
+				}
+			}
+		};
+
+		// Parse the items
+		let RuntimeApiImpls { impls: mut api_impls } =
+			syn::parse2::<RuntimeApiImpls>(code).unwrap();
+
+		// Run the renamer which is being run first in the `impl_runtime_apis!` macro.
+		rename_self_in_trait_impls(&mut api_impls).expect("Will succeed");
+		let result: TokenStream = quote::quote! {  #(#api_impls)* };
+
+		assert_eq!(result.to_string(), expected.to_string());
+	}
 }