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

…ment (#4012)

closes #1890 

### Overview 

Introduces similar checker struct to `CheckTraitDecls` in
`decl_runtime_apis!` - `CheckTraitImpls`. Overrides
`visit::visit_type_path` to detect usage of `Self` as a type argument
within the scope of `impl_runtime_apis!`.

**Note**: only prevents the usage of `Self` as a type argument in an
angle bracket `<>`, as it is the only use case that fails to compile.
For example, the code
[below](https://github.com/paritytech/polkadot-sdk/blob/master/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/src/lib.rs#L1002)
compiles fine:

```rs
impl BridgeMessagesConfig<WithBridgeHubRococoMessagesInstance> for Runtime {
  fn is_relayer_rewarded(relayer: &Self::AccountId) -> bool {
	  let bench_lane_id = <Self as BridgeMessagesConfig<WithBridgeHubRococoMessagesInstance>>::bench_lane_id();
	  // ...
```

### Result

Given a block of code like this:

```rs
impl_runtime_apis! {
	impl apis::Core<Block> for Runtime {
		fn initialize_block(header: &HeaderFor<Self>) -> ExtrinsicInclusionMode {
			let _: HeaderFor<Self> = header.clone();
			RuntimeExecutive::initialize_block(header)
		}
		// ...
         }
// ...
```

<details open>

<summary>Output:</summary>

```bash
$ cargo build --release -p minimal-template-node
  error: `Self` can not be used as type argument in the scope of `impl_runtime_apis!`. Use `Runtime` instead.
     --> /polkadot-sdk/templates/minimal/runtime/src/lib.rs:133:11
      |
  133 |             let _: HeaderFor<Self> = header.clone();
      |                    ^^^^^^^^^^^^^^^

  error: `Self` can not be used as type argument in the scope of `impl_runtime_apis!`. Use `Runtime` instead.
     --> /polkadot-sdk/templates/minimal/runtime/src/lib.rs:132:32
      |
  132 |         fn initialize_block(header: &HeaderFor<Self>) -> ExtrinsicInclusionMode {
```

</details>

---------

Co-authored-by: Pavlo Khrystenko <p.khrystenko@gmail.com>
Co-authored-by: Pavlo Khrystenko <45178695+pkhry@users.noreply.github.com>
Co-authored-by: Alexandru Vasile <60601340+lexnv@users.noreply.github.com>
Co-authored-by: Bastian Köcher <git@kchr.de>

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

@@ -31,6 +31,7 @@ use syn::{
 	parse::{Error, Parse, ParseStream, Result},
 	parse_macro_input, parse_quote,
 	spanned::Spanned,
+	visit_mut::{self, VisitMut},
 	Attribute, Ident, ImplItem, ItemImpl, Path, Signature, Type, TypePath,
 };
 
@@ -223,34 +224,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 )* ))
 }
@@ -392,10 +393,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 {
@@ -736,8 +737,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."));
@@ -783,17 +784,50 @@ 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_type_mut(&mut self, ty: &mut syn::Type) {
+		match ty {
+			Type::Path(p) if p.path.is_ident("Self") => {
+				*ty = *self.self_ty.clone();
+			},
+			ty => syn::visit_mut::visit_type_mut(self, ty),
+		}
+	}
+}
+
+/// Rename `Self` to `ItemImpl.self_ty` in all items.
+fn rename_self_in_trait_impls(impls: &mut [ItemImpl]) {
+	impls.iter_mut().for_each(|i| {
+		let mut checker = ReplaceSelfImpl { self_ty: i.self_ty.clone() };
+		checker.replace(i);
+	});
+}
+
 /// 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()?;
@@ -859,4 +893,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);
+		let result: TokenStream = quote::quote! {  #(#api_impls)* };
+
+		assert_eq!(result.to_string(), expected.to_string());
+	}
 }