From d828e26e57ddd29d0c3d1538e0c11cd3f4cd82ad Mon Sep 17 00:00:00 2001 From: Niklas Adolfsson Date: Thu, 9 Sep 2021 16:57:48 +0200 Subject: [PATCH] fix(proc macros): subscriptions must return result (#455) Similar to #435 that adds the same restrictions to subscriptions too. To avoid having faulty trait bounds on when the subcription actually returns Result. --- examples/proc_macro.rs | 10 ++++--- proc-macros/src/helpers.rs | 2 +- proc-macros/src/lib.rs | 8 +++--- proc-macros/src/render_server.rs | 2 +- proc-macros/src/rpc_macro.rs | 3 --- .../ui/correct/alias_doesnt_use_namespace.rs | 2 +- proc-macros/tests/ui/correct/basic.rs | 16 ++++++------ proc-macros/tests/ui/correct/only_server.rs | 8 +++--- .../tests/ui/correct/rpc_deny_missing_docs.rs | 2 +- .../ui/incorrect/sub/sub_conflicting_alias.rs | 4 +-- .../sub/sub_conflicting_alias.stderr | 2 +- .../tests/ui/incorrect/sub/sub_return_type.rs | 10 ------- .../ui/incorrect/sub/sub_return_type.stderr | 6 ----- tests/tests/proc_macros.rs | 26 ++++++++++--------- 14 files changed, 44 insertions(+), 57 deletions(-) delete mode 100644 proc-macros/tests/ui/incorrect/sub/sub_return_type.rs delete mode 100644 proc-macros/tests/ui/incorrect/sub/sub_return_type.stderr diff --git a/examples/proc_macro.rs b/examples/proc_macro.rs index fcfadf13dd..dd1922f692 100644 --- a/examples/proc_macro.rs +++ b/examples/proc_macro.rs @@ -46,7 +46,7 @@ where /// Subscription that takes a `StorageKey` as input and produces a `Vec`. #[subscription(name = "subscribeStorage", item = Vec)] - fn subscribe_storage(&self, keys: Option>); + fn subscribe_storage(&self, keys: Option>) -> Result<(), Error>; } pub struct RpcServerImpl; @@ -61,8 +61,12 @@ impl RpcServer for RpcServerImpl { Ok(vec![storage_key]) } - fn subscribe_storage(&self, mut sink: SubscriptionSink, _keys: Option>) { - sink.send(&vec![[0; 32]]).unwrap(); + fn subscribe_storage( + &self, + mut sink: SubscriptionSink, + _keys: Option>, + ) -> Result<(), Error> { + sink.send(&vec![[0; 32]]) } } diff --git a/proc-macros/src/helpers.rs b/proc-macros/src/helpers.rs index 8f204c3c6a..246bdfbf65 100644 --- a/proc-macros/src/helpers.rs +++ b/proc-macros/src/helpers.rs @@ -78,7 +78,7 @@ fn find_jsonrpsee_crate(http_name: &str, ws_name: &str) -> Result JsonRpcResult; /// /// #[subscription(name = "sub", item = Vec)] -/// fn sub(&self); +/// fn sub(&self) -> JsonRpcResult<()>; /// } /// ``` /// diff --git a/proc-macros/src/lib.rs b/proc-macros/src/lib.rs index df3dfbaa62..21795931fc 100644 --- a/proc-macros/src/lib.rs +++ b/proc-macros/src/lib.rs @@ -215,7 +215,7 @@ pub(crate) mod visitor; /// fn sync_method(&self) -> JsonRpcResult; /// /// #[subscription(name = "sub", item = String)] -/// fn sub(&self); +/// fn sub(&self) -> JsonRpcResult<()>; /// } /// /// // Structure that will implement the `MyRpcServer` trait. @@ -236,9 +236,9 @@ pub(crate) mod visitor; /// // We could've spawned a `tokio` future that yields values while our program works, /// // but for simplicity of the example we will only send two values and then close /// // the subscription. -/// fn sub(&self, mut sink: SubscriptionSink) { -/// sink.send(&"Response_A").unwrap(); -/// sink.send(&"Response_B").unwrap(); +/// fn sub(&self, mut sink: SubscriptionSink) -> JsonRpcResult<()> { +/// sink.send(&"Response_A")?; +/// sink.send(&"Response_B") /// } /// } /// } diff --git a/proc-macros/src/render_server.rs b/proc-macros/src/render_server.rs index e93c3477cd..c65ede5cfe 100644 --- a/proc-macros/src/render_server.rs +++ b/proc-macros/src/render_server.rs @@ -168,7 +168,7 @@ impl RpcDescription { handle_register_result(quote! { rpc.register_subscription(#rpc_sub_name, #rpc_unsub_name, |params, sink, context| { #parsing - Ok(context.as_ref().#rust_method_name(sink, #params_seq)) + context.as_ref().#rust_method_name(sink, #params_seq) }) }) }) diff --git a/proc-macros/src/rpc_macro.rs b/proc-macros/src/rpc_macro.rs index f4f3d0b128..d4f3247232 100644 --- a/proc-macros/src/rpc_macro.rs +++ b/proc-macros/src/rpc_macro.rs @@ -180,9 +180,6 @@ impl RpcDescription { if method.sig.asyncness.is_some() { return Err(syn::Error::new_spanned(&method, "Subscription methods must not be `async`")); } - if !matches!(method.sig.output, syn::ReturnType::Default) { - return Err(syn::Error::new_spanned(&method, "Subscription methods must not return anything")); - } let sub_data = RpcSubscription::from_item(method.clone())?; subscriptions.push(sub_data); diff --git a/proc-macros/tests/ui/correct/alias_doesnt_use_namespace.rs b/proc-macros/tests/ui/correct/alias_doesnt_use_namespace.rs index 83c10b641c..b385a64b63 100644 --- a/proc-macros/tests/ui/correct/alias_doesnt_use_namespace.rs +++ b/proc-macros/tests/ui/correct/alias_doesnt_use_namespace.rs @@ -7,7 +7,7 @@ pub trait Rpc { async fn async_method(&self, param_a: u8, param_b: String) -> JsonRpcResult; #[subscription(name = "getFood", item = String, aliases = "getFood", unsubscribe_aliases = "unsubscribegetFood")] - fn sub(&self); + fn sub(&self) -> JsonRpcResult<()>; } fn main() {} diff --git a/proc-macros/tests/ui/correct/basic.rs b/proc-macros/tests/ui/correct/basic.rs index 51bd1bf0cf..1e06e62d30 100644 --- a/proc-macros/tests/ui/correct/basic.rs +++ b/proc-macros/tests/ui/correct/basic.rs @@ -26,10 +26,10 @@ pub trait Rpc { fn sync_method(&self) -> JsonRpcResult; #[subscription(name = "sub", item = String)] - fn sub(&self); + fn sub(&self) -> JsonRpcResult<()>; #[subscription(name = "echo", aliases = "ECHO", item = u32, unsubscribe_aliases = "NotInterested, listenNoMore")] - fn sub_with_params(&self, val: u32); + fn sub_with_params(&self, val: u32) -> JsonRpcResult<()>; } pub struct RpcServerImpl; @@ -58,14 +58,14 @@ impl RpcServer for RpcServerImpl { Ok(10u16) } - fn sub(&self, mut sink: SubscriptionSink) { - sink.send(&"Response_A").unwrap(); - sink.send(&"Response_B").unwrap(); + fn sub(&self, mut sink: SubscriptionSink) -> JsonRpcResult<()> { + sink.send(&"Response_A")?; + sink.send(&"Response_B") } - fn sub_with_params(&self, mut sink: SubscriptionSink, val: u32) { - sink.send(&val).unwrap(); - sink.send(&val).unwrap(); + fn sub_with_params(&self, mut sink: SubscriptionSink, val: u32) -> JsonRpcResult<()> { + sink.send(&val)?; + sink.send(&val) } } diff --git a/proc-macros/tests/ui/correct/only_server.rs b/proc-macros/tests/ui/correct/only_server.rs index 429d461403..0db34a391d 100644 --- a/proc-macros/tests/ui/correct/only_server.rs +++ b/proc-macros/tests/ui/correct/only_server.rs @@ -14,7 +14,7 @@ pub trait Rpc { fn sync_method(&self) -> JsonRpcResult; #[subscription(name = "sub", item = String)] - fn sub(&self); + fn sub(&self) -> JsonRpcResult<()>; } pub struct RpcServerImpl; @@ -29,9 +29,9 @@ impl RpcServer for RpcServerImpl { Ok(10u16) } - fn sub(&self, mut sink: SubscriptionSink) { - sink.send(&"Response_A").unwrap(); - sink.send(&"Response_B").unwrap(); + fn sub(&self, mut sink: SubscriptionSink) -> JsonRpcResult<()> { + sink.send(&"Response_A")?; + sink.send(&"Response_B") } } diff --git a/proc-macros/tests/ui/correct/rpc_deny_missing_docs.rs b/proc-macros/tests/ui/correct/rpc_deny_missing_docs.rs index 94044540ff..a8476f254d 100644 --- a/proc-macros/tests/ui/correct/rpc_deny_missing_docs.rs +++ b/proc-macros/tests/ui/correct/rpc_deny_missing_docs.rs @@ -12,7 +12,7 @@ pub trait ApiWithoutDocumentation { /// Subscription docs. #[subscription(name = "sub", item = String)] - fn sub(&self); + fn sub(&self) -> jsonrpsee::types::JsonRpcResult<()>; } fn main() {} diff --git a/proc-macros/tests/ui/incorrect/sub/sub_conflicting_alias.rs b/proc-macros/tests/ui/incorrect/sub/sub_conflicting_alias.rs index dd05c6b1df..2dc9d65759 100644 --- a/proc-macros/tests/ui/incorrect/sub/sub_conflicting_alias.rs +++ b/proc-macros/tests/ui/incorrect/sub/sub_conflicting_alias.rs @@ -1,9 +1,9 @@ -use jsonrpsee::proc_macros::rpc; +use jsonrpsee::{proc_macros::rpc, types::JsonRpcResult}; #[rpc(client, server)] pub trait DuplicatedSubAlias { #[subscription(name = "alias", item = String, aliases = "hello_is_goodbye", unsubscribe_aliases = "hello_is_goodbye")] - fn async_method(&self); + fn async_method(&self) -> JsonRpcResult<()>; } fn main() {} diff --git a/proc-macros/tests/ui/incorrect/sub/sub_conflicting_alias.stderr b/proc-macros/tests/ui/incorrect/sub/sub_conflicting_alias.stderr index 7c08822b38..8344c1b2fb 100644 --- a/proc-macros/tests/ui/incorrect/sub/sub_conflicting_alias.stderr +++ b/proc-macros/tests/ui/incorrect/sub/sub_conflicting_alias.stderr @@ -1,5 +1,5 @@ error: "hello_is_goodbye" is already defined --> $DIR/sub_conflicting_alias.rs:6:5 | -6 | fn async_method(&self); +6 | fn async_method(&self) -> JsonRpcResult<()>; | ^^^^^^^^^^^^ diff --git a/proc-macros/tests/ui/incorrect/sub/sub_return_type.rs b/proc-macros/tests/ui/incorrect/sub/sub_return_type.rs deleted file mode 100644 index 14d77b422e..0000000000 --- a/proc-macros/tests/ui/incorrect/sub/sub_return_type.rs +++ /dev/null @@ -1,10 +0,0 @@ -use jsonrpsee::proc_macros::rpc; - -// Subscription method must not have return type. -#[rpc(client, server)] -pub trait SubWithReturnType { - #[subscription(name = "sub", item = u8)] - fn sub(&self) -> u8; -} - -fn main() {} diff --git a/proc-macros/tests/ui/incorrect/sub/sub_return_type.stderr b/proc-macros/tests/ui/incorrect/sub/sub_return_type.stderr deleted file mode 100644 index 1e690ebb54..0000000000 --- a/proc-macros/tests/ui/incorrect/sub/sub_return_type.stderr +++ /dev/null @@ -1,6 +0,0 @@ -error: Subscription methods must not return anything - --> $DIR/sub_return_type.rs:6:2 - | -6 | / #[subscription(name = "sub", item = u8)] -7 | | fn sub(&self) -> u8; - | |________________________^ diff --git a/tests/tests/proc_macros.rs b/tests/tests/proc_macros.rs index 97a81ec442..6618f45f0a 100644 --- a/tests/tests/proc_macros.rs +++ b/tests/tests/proc_macros.rs @@ -48,10 +48,10 @@ mod rpc_impl { fn sync_method(&self) -> JsonRpcResult; #[subscription(name = "sub", item = String)] - fn sub(&self); + fn sub(&self) -> JsonRpcResult<()>; #[subscription(name = "echo", aliases = "alias_echo", item = u32)] - fn sub_with_params(&self, val: u32); + fn sub_with_params(&self, val: u32) -> JsonRpcResult<()>; #[method(name = "params")] fn params(&self, a: u8, b: &str) -> JsonRpcResult { @@ -102,7 +102,7 @@ mod rpc_impl { /// All head subscription #[subscription(name = "subscribeAllHeads", item = Header)] - fn subscribe_all_heads(&self, hash: Hash); + fn subscribe_all_heads(&self, hash: Hash) -> JsonRpcResult<()>; } /// Trait to ensure that the trait bounds are correct. @@ -117,7 +117,7 @@ mod rpc_impl { pub trait OnlyGenericSubscription { /// Get header of a relay chain block. #[subscription(name = "sub", item = Vec)] - fn sub(&self, hash: Input); + fn sub(&self, hash: Input) -> JsonRpcResult<()>; } /// Trait to ensure that the trait bounds are correct. @@ -154,14 +154,16 @@ mod rpc_impl { Ok(10u16) } - fn sub(&self, mut sink: SubscriptionSink) { - sink.send(&"Response_A").unwrap(); - sink.send(&"Response_B").unwrap(); + fn sub(&self, mut sink: SubscriptionSink) -> JsonRpcResult<()> { + sink.send(&"Response_A")?; + sink.send(&"Response_B")?; + Ok(()) } - fn sub_with_params(&self, mut sink: SubscriptionSink, val: u32) { - sink.send(&val).unwrap(); - sink.send(&val).unwrap(); + fn sub_with_params(&self, mut sink: SubscriptionSink, val: u32) -> JsonRpcResult<()> { + sink.send(&val)?; + sink.send(&val)?; + Ok(()) } } @@ -174,8 +176,8 @@ mod rpc_impl { #[async_trait] impl OnlyGenericSubscriptionServer for RpcServerImpl { - fn sub(&self, mut sink: SubscriptionSink, _: String) { - sink.send(&"hello").unwrap(); + fn sub(&self, mut sink: SubscriptionSink, _: String) -> JsonRpcResult<()> { + sink.send(&"hello") } } }