feat: alias attribute for proc macros

[niklasad1]

Closing #440

The new format was suggested by @dvdplm:

#[rpc(server, namespace = "state")]
pub trait StateApi<Hash> {
    …
    #[subscription(
      name = "subscribeRuntimeVersion", 
      item = RuntimeVersion, 
      /// may one or several separated by comma
      aliases = "chain_subscribeRuntimeVersion", 
      /// may one or several separated by comma
      unsubscribe_aliases = "state_unsubscribeRuntimeVersion, chain_unsubscribeRuntimeVersion"
    )]
    fn subscribe_runtime_version(&self, keys: Option<Vec<u8>>);
}

The unsubscribe method naming is handled by jsonrpsee and is unsubscribeRuntimeVersion.
If the subscribe method starts with subscribe we just push un to the string otherwise unsubscribe
Then users can provide their own aliases if they don't like default unsubscribe name.

After some discussion we agreed that aliases should be kept outside the namespace (if one exist) because aliases are often mistake and might be a PITA to refactor the entire trait just to manually add the namespace to each method to work properly.

[dvdplm]

My preference would be to let the macro build the unsubscribe method and any aliases itself, so that it'd look like this:

#[subscription(
  name = "subscribeStorage", 
  item = Vec<usize>, 
  alias = "foo", 
)]
fn subscribe_storage(&self, keys: Option<Vec<u8>>);

The unsubscribe method name would be unsubscribe_subscribeStorage (or possibly unsubscribeSubscribeStorage) and the alias unsubscribe_foo. Is that possible?

[niklasad1]

The unsubscribe method name would be unsubscribe_subscribeStorage (or possibly unsubscribeSubscribeStorage) and the alias unsubscribe_foo. Is that possible?

I like it it's much more readable and I like it but it won't work with this example without doing some clever parsing (strip the name space identifier) state_ in this example

The whole point of this PR is make it possible to port everything in substrate to work with polkadot UI

In addition for you example to work the current: if the name starts with subscribeFoo we would need to rename it unsubscribeFoo instead of unsubscribesubscribeFoo ^^

[dvdplm]

like it it's much more readable and I like it but it won't work with this example without doing some clever parsing (strip the name space identifier) state_ in this example

The whole point of this PR is make it possible to port everything in substrate to work with polkadot UI

Maybe we can solve the backwards compatibility using aliases? It would look something like this:

#[rpc(server, namespace = "state")]
pub trait StateApi<Hash> {
    …
    #[subscription(
      name = "subscribeRuntimeVersion", 
      item = RuntimeVersion, 
      alias = "chain_subscribeRuntimeVersion", 
      unsubscribe_alias = "state_unsubscribeRuntimeVersion, chain_unsubscribeRuntimeVersion"
    )]
    fn subscribe_runtime_version(&self, keys: Option<Vec<u8>>);

}

(here the aliases would be used verbatim and not get the namespace prepended)

EDIT: I guess the point I'm trying to make is: it's ok if the current RPCs look ugly but we should strive to make new code look as neat as we can.

[niklasad1]

Yes, I totally agree in the long-term to make it "not-ugly" but short term I want it compatible with polkadot UI (so not really jsonrpc related) but I think your last example should quite straight-forward to implement.

[niklasad1]

@dvdplm @jsdw @maciejhirsz This is now ready for review

[dvdplm]

lgtm, let's see how this works in substrate.

[dvdplm]

This tracks method names right? Maybe name it registered_methods or seen_method_names? And that brings the question, do we need to check aliases too? I guess it's not a big problem if users define duplicate aliases but might be good to keep things tight.
EDIT: nvm, you do check aliases too.

[niklasad1]

Exactly, the code is little !DRY but that's mainly because of that inner closure check_names, should be "ok" here

[niklasad1]

I think it makes sense to have this check otherwise it would panic with duplicate names or something, also good to have if you unintentionally register the same alias for two different methods.

[TarikGul]

Just for clarification, you are just appending anything after "subscribe" to the "unsubscribe" new method, and this method name originates from the subscriptions attributes, which can technically be any string?

[niklasad1]

in a nutshell if the existing string is named subscribeTarik the unsubscribe method is named unsubscribeTarik hence the skip(9)

otherwise only unsubcribe is pushed at the start of the subscribe method for example someSub -> unsubscribesomeSub

It can be any string but if a duplicate string is used then it will end-up in compile-time error

[TarikGul]

Cool, makes sense!

[TarikGul]

LGTM