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

[dastansam]

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 compiles fine:

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:

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)
		}
		// ...
         }
// ...

Output:

$ 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 {

[paritytech-cicd-pr]

The CI pipeline was cancelled due to failure one of the required jobs.
Job name: cargo-clippy
Logs: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/5811316

[dastansam]

@kianenigma just pinging here :)

[bkchr]

IMO, it would make more sense to replace Self with the impl type of the trait. So, the user can just use Self and doesn't see an error.

[kianenigma]

I would be happy with the existing approach, but maybe we should explore @bkchr's suggestion? at least see how much harder it is?

in any case, this is great and we will submit a tip for it, thank you!

[dastansam]

I would be happy with the existing approach, but maybe we should explore @bkchr's suggestion? at least see how much harder it is?

in any case, this is great and we will submit a tip for it, thank you!

sure, I will give it a try

[kianenigma]

any updates?

[dastansam]

any updates?

working on it

[kianenigma]

again? :)

[dastansam]

again? :)

will try to push it through this weekend, if not, I am ok if someone takes over)

[jsdw]

ie would it currently work if I had something like this?:

let foo: Self = ...

Or is that sort of thing nonsensical anyways.

[pkhry]

i'd keep things conservative in this regard

[jsdw]

Looks good to me but I wonder if we can simplify the code a touch and whether we should replace Self more generally everywhere or else tweak the comment :)

[pkhry]

/cmd fmt

[github-actions]

Command "fmt" has started 🚀 See logs here

[github-actions]

Command "fmt" has finished ✅ See logs here

[pkhry]

/cmd fmt

[github-actions]

Command "fmt" has started 🚀 See logs here

[github-actions]

Command "fmt" has finished ✅ See logs here