Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow trait bounds to be overridden in macro #808

Merged
merged 13 commits into from
Jul 4, 2022
Merged
90 changes: 90 additions & 0 deletions examples/examples/proc_macro_bounds.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
// Copyright 2019-2021 Parity Technologies (UK) Ltd.
//
// Permission is hereby granted, free of charge, to any
// person obtaining a copy of this software and associated
// documentation files (the "Software"), to deal in the
// Software without restriction, including without
// limitation the rights to use, copy, modify, merge,
// publish, distribute, sublicense, and/or sell copies of
// the Software, and to permit persons to whom the Software
// is furnished to do so, subject to the following
// conditions:
//
// The above copyright notice and this permission notice
// shall be included in all copies or substantial portions
// of the Software.
//
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF
// ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED
// TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A
// PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT
// SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY
// CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION
// OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR
// IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
// DEALINGS IN THE SOFTWARE.

use std::net::SocketAddr;

use jsonrpsee::core::{async_trait, Error};
use jsonrpsee::proc_macros::rpc;
use jsonrpsee::ws_client::WsClientBuilder;
use jsonrpsee::ws_server::{WsServerBuilder, WsServerHandle};

type ExampleHash = [u8; 32];

pub trait Config {
type Hash: Send + Sync + 'static;
Copy link
Member

@niklasad1 niklasad1 Jun 28, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC: before/"currently" we add the trait bound the entire trait right?

Can you just verify that we don't add trait bounds on associated types that are not used in the RPC trait?

such as:

pub trait Config {
   type Hash: Send + Sync + 'static;
   type NotUsed;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Debugging the proc-macro, it seems that "currently" (without custom bounds attribute):

pub trait Config {
	type Hash: Send + Sync + 'static;
	type NotUsed;
}

#[rpc(server, namespace = "foo")]
pub trait Rpc<Conf: Config> {
	#[method(name = "bar")]
	fn method(&self) -> Result<Conf::Hash, Error>;
}

The rpc macro implies just: Conf : Send + Sync + 'static + jsonrpsee :: core :: Serialize.

Although, this fails to compile due to:

55  | #[rpc(server, client, namespace = "foo", client_bounds(Conf::Hash: jsonrpsee::core::DeserializeOwned))]
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Serialize` is not implemented for `<Conf as Config>::Hash`

Copy link
Member

@niklasad1 niklasad1 Jun 28, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that's fine I meant with the overrides for the Hash

pub trait Config {
	type Hash: Send + Sync + 'static;
	type NotUsed;
}

#[rpc(server, client, client_bounds(Conf::Hash: jsonrpsee::core::Serialize))]
pub trait Rpc<Conf: Config> {
	#[method(name = "bar")]
	fn method(&self) -> Result<Conf::Hash, Error>;
}

Such that it doesn't expand to something like:

pub trait Rpc<T: Client, C: Config> 
where 
        C::Hash: Serialize,
        // NotUsed is not used so the trait bounds are needless
        // and should not be added.
        C::NotUsed: Send + Sync + Serialize + DeserializeOwned
{
	#[method(name = "bar")]
	fn method(&self) -> Result<Conf::Hash, Error>;
}

But as it works in subxt all fine, IIRC we only inspect the parameters and return types anyway and add bounds on them so all good.

}

impl Config for ExampleHash {
type Hash = Self;
}

/// The RPC macro requires `DeserializeOwned` for output types for the client implementation, while the
/// server implementation requires output types to be bounded by `Serialize`.
///
/// In this example, we don't want the `Conf` to be bounded by default to
/// `Conf : Send + Sync + 'static + jsonrpsee::core::DeserializeOwned` for client implementation and
/// `Conf : Send + Sync + 'static + jsonrpsee::core::Serialize` for server implementation.
///
/// Explicitly, specify client and server bounds to handle the `Serialize` and `DeserializeOwned` cases
/// just for the `Conf::hash` part.
#[rpc(server, client, namespace = "foo", client_bounds(T::Hash: jsonrpsee::core::DeserializeOwned), server_bounds(T::Hash: jsonrpsee::core::Serialize))]
pub trait Rpc<T: Config> {
#[method(name = "bar")]
fn method(&self) -> Result<T::Hash, Error>;
}

pub struct RpcServerImpl;

#[async_trait]
impl RpcServer<ExampleHash> for RpcServerImpl {
fn method(&self) -> Result<<ExampleHash as Config>::Hash, Error> {
Ok([0u8; 32])
}
}

#[tokio::main]
async fn main() -> anyhow::Result<()> {
tracing_subscriber::FmtSubscriber::builder()
.with_env_filter(tracing_subscriber::EnvFilter::from_default_env())
.try_init()
.expect("setting default subscriber failed");

let (server_addr, _handle) = run_server().await?;
let url = format!("ws://{}", server_addr);

let client = WsClientBuilder::default().build(&url).await?;
assert_eq!(RpcClient::<ExampleHash>::method(&client).await.unwrap(), [0u8; 32]);

Ok(())
}

async fn run_server() -> anyhow::Result<(SocketAddr, WsServerHandle)> {
let server = WsServerBuilder::default().build("127.0.0.1:0").await?;

let addr = server.local_addr()?;
let handle = server.start(RpcServerImpl.into_rpc())?;
Ok((addr, handle))
}
13 changes: 12 additions & 1 deletion proc-macros/src/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ use crate::visitor::{FindAllParams, FindSubscriptionParams};
use proc_macro2::{Span, TokenStream as TokenStream2};
use proc_macro_crate::{crate_name, FoundCrate};
use quote::quote;
use syn::{parse_quote, punctuated::Punctuated, visit::Visit, Token};
use syn::{parse_quote, punctuated::Punctuated, token::Comma, visit::Visit, Token, WherePredicate};

/// Search for client-side `jsonrpsee` in `Cargo.toml`.
pub(crate) fn find_jsonrpsee_client_crate() -> Result<proc_macro2::TokenStream, syn::Error> {
Expand Down Expand Up @@ -91,10 +91,21 @@ pub(crate) fn generate_where_clause(
item_trait: &syn::ItemTrait,
sub_tys: &[syn::Type],
is_client: bool,
bounds: Option<&Punctuated<WherePredicate, Comma>>,
) -> Vec<syn::WherePredicate> {
let visitor = visit_trait(item_trait, sub_tys);
let additional_where_clause = item_trait.generics.where_clause.clone();

if let Some(custom_bounds) = bounds {
let mut bounds = additional_where_clause
.map(|where_clause| where_clause.predicates.into_iter().collect())
.unwrap_or(Vec::new());

bounds.extend(custom_bounds.iter().cloned());

return bounds;
}

item_trait
.generics
.type_params()
Expand Down
6 changes: 5 additions & 1 deletion proc-macros/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,10 @@ pub(crate) mod visitor;
/// implementation's methods conveniently.
/// - `namespace`: add a prefix to all the methods and subscriptions in this RPC. For example, with namespace `foo` and
/// method `spam`, the resulting method name will be `foo_spam`.
/// - `server_bounds`: replace *all* auto-generated trait bounds with the user-defined ones for the server
/// implementation.
/// - `client_bounds`: replace *all* auto-generated trait bounds with the user-defined ones for the client
/// implementation.
///
/// **Trait requirements:**
///
Expand Down Expand Up @@ -287,7 +291,7 @@ pub(crate) mod visitor;
/// // The stream API can be used to pipe items from the underlying stream
/// // as subscription responses.
/// fn sub_override_notif_method(&self, pending: PendingSubscription) {
/// let mut sink = pending.accept().unwrap();
/// let mut sink = pending.accept().unwrap();
/// tokio::spawn(async move {
/// let stream = futures_util::stream::iter(["one", "two", "three"]);
/// sink.pipe_from_stream(stream).await;
Expand Down
4 changes: 2 additions & 2 deletions proc-macros/src/render_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ impl RpcDescription {
let sub_tys: Vec<syn::Type> = self.subscriptions.clone().into_iter().map(|s| s.item).collect();

let trait_name = quote::format_ident!("{}Client", &self.trait_def.ident);
let where_clause = generate_where_clause(&self.trait_def, &sub_tys, true);
let where_clause = generate_where_clause(&self.trait_def, &sub_tys, true, self.client_bounds.as_ref());
let type_idents = self.trait_def.generics.type_params().collect::<Vec<&TypeParam>>();
let (impl_generics, type_generics, _) = self.trait_def.generics.split_for_impl();

Expand All @@ -63,7 +63,7 @@ impl RpcDescription {
#(#sub_impls)*
}

impl<T #(,#type_idents)*> #trait_name #type_generics for T where T: #super_trait #(,#where_clause)* {}
impl<TypeJsonRpseeInteral #(,#type_idents)*> #trait_name #type_generics for TypeJsonRpseeInteral where TypeJsonRpseeInteral: #super_trait #(,#where_clause)* {}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

};

Ok(trait_impl)
Expand Down
2 changes: 1 addition & 1 deletion proc-macros/src/render_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ impl RpcDescription {
and adds them into a single `RpcModule`.";

let sub_tys: Vec<syn::Type> = self.subscriptions.clone().into_iter().map(|s| s.item).collect();
let where_clause = generate_where_clause(&self.trait_def, &sub_tys, false);
let where_clause = generate_where_clause(&self.trait_def, &sub_tys, false, self.server_bounds.as_ref());

// NOTE(niklasad1): empty where clause is valid rust syntax.
Ok(quote! {
Expand Down
25 changes: 24 additions & 1 deletion proc-macros/src/rpc_macro.rs
Original file line number Diff line number Diff line change
Expand Up @@ -220,20 +220,41 @@ pub struct RpcDescription {
pub(crate) methods: Vec<RpcMethod>,
/// List of RPC subscriptions defined in the trait.
pub(crate) subscriptions: Vec<RpcSubscription>,
/// Optional user defined trait bounds for the client implementation.
pub(crate) client_bounds: Option<Punctuated<syn::WherePredicate, Token![,]>>,
/// Optional user defined trait bounds for the server implementation.
pub(crate) server_bounds: Option<Punctuated<syn::WherePredicate, Token![,]>>,
}

impl RpcDescription {
pub fn from_item(attr: Attribute, mut item: syn::ItemTrait) -> syn::Result<Self> {
let [client, server, namespace] = AttributeMeta::parse(attr)?.retain(["client", "server", "namespace"])?;
let [client, server, namespace, client_bounds, server_bounds] =
AttributeMeta::parse(attr)?.retain(["client", "server", "namespace", "client_bounds", "server_bounds"])?;

let needs_server = optional(server, Argument::flag)?.is_some();
let needs_client = optional(client, Argument::flag)?.is_some();
let namespace = optional(namespace, Argument::string)?;
let client_bounds = optional(client_bounds, Argument::group)?;
let server_bounds = optional(server_bounds, Argument::group)?;

if !needs_server && !needs_client {
return Err(syn::Error::new_spanned(&item.ident, "Either 'server' or 'client' attribute must be applied"));
}

if client_bounds.is_some() && !needs_client {
return Err(syn::Error::new_spanned(
&item.ident,
"Attribute 'client' must be specified with 'client_bounds'",
));
}

if server_bounds.is_some() && !needs_server {
return Err(syn::Error::new_spanned(
&item.ident,
"Attribute 'server' must be specified with 'server_bounds'",
));
}

let jsonrpsee_client_path = crate::helpers::find_jsonrpsee_client_crate().ok();
let jsonrpsee_server_path = crate::helpers::find_jsonrpsee_server_crate().ok();

Expand Down Expand Up @@ -313,6 +334,8 @@ impl RpcDescription {
trait_def: item,
methods,
subscriptions,
client_bounds,
server_bounds,
})
}

Expand Down
27 changes: 27 additions & 0 deletions proc-macros/tests/ui/correct/rpc_bounds.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
use jsonrpsee::proc_macros::rpc;
use jsonrpsee::core::RpcResult;

pub trait Config {
type Hash: Send + Sync + 'static;
type NotUsed;
}

#[rpc(client, namespace = "foo", client_bounds(Conf::Hash: jsonrpsee::core::DeserializeOwned))]
pub trait MyRpcClient<Conf: Config> {
#[method(name = "bar")]
fn method(&self) -> RpcResult<Conf::Hash>;
}

#[rpc(server, namespace = "foo", server_bounds(Conf::Hash: jsonrpsee::core::Serialize))]
pub trait MyRpcServer<Conf: Config> {
#[method(name = "bar")]
fn method(&self) -> RpcResult<Conf::Hash>;
}

#[rpc(server, client, namespace = "foo", client_bounds(Conf::Hash: jsonrpsee::core::DeserializeOwned), server_bounds(Conf::Hash: jsonrpsee::core::Serialize))]
pub trait MyRpcServerClient<Conf: Config> {
#[method(name = "bar")]
fn method(&self) -> RpcResult<Conf::Hash>;
}

fn main() {}
lexnv marked this conversation as resolved.
Show resolved Hide resolved
19 changes: 19 additions & 0 deletions proc-macros/tests/ui/incorrect/rpc/rpc_bounds_without_impl.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
use jsonrpsee::proc_macros::rpc;

pub trait Config {
type Hash: Send + Sync + 'static;
}

#[rpc(server, client_bounds(), server_bounds(Conf::Hash: jsonrpsee::core::Serialize))]
pub trait ClientBoundsForbidden<Conf: Config> {
#[method(name = "bar")]
fn method(&self) -> Result<Conf::Hash, Error>;
}

#[rpc(client, server_bounds(), client_bounds(Conf::Hash: jsonrpsee::core::DeserializeOwned))]
pub trait ServerBoundsForbidden<Conf: Config> {
#[method(name = "bar")]
fn method(&self) -> Result<Conf::Hash, Error>;
}

fn main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
error: Attribute 'client' must be specified with 'client_bounds'
--> tests/ui/incorrect/rpc/rpc_bounds_without_impl.rs:8:11
|
8 | pub trait ClientBoundsForbidden<Conf: Config> {
| ^^^^^^^^^^^^^^^^^^^^^

error: Attribute 'server' must be specified with 'server_bounds'
--> tests/ui/incorrect/rpc/rpc_bounds_without_impl.rs:14:11
|
14 | pub trait ServerBoundsForbidden<Conf: Config> {
| ^^^^^^^^^^^^^^^^^^^^^
16 changes: 16 additions & 0 deletions proc-macros/tests/ui/incorrect/rpc/rpc_empty_bounds.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
use jsonrpsee::proc_macros::rpc;
use jsonrpsee::core::Error;

pub trait Config {
type Hash: Send + Sync + 'static;
}

/// Client bound must be `Conf::Hash: jsonrpsee::core::DeserializeOwned`
/// Server bound must be `Conf::Hash: jsonrpsee::core::Serialize`
#[rpc(server, client, namespace = "foo", client_bounds(), server_bounds())]
pub trait EmptyBounds<Conf: Config> {
#[method(name = "bar")]
fn method(&self) -> Result<Conf::Hash, Error>;
}

fn main() {}
26 changes: 26 additions & 0 deletions proc-macros/tests/ui/incorrect/rpc/rpc_empty_bounds.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
error[E0277]: the trait bound `<Conf as Config>::Hash: Serialize` is not satisfied
--> tests/ui/incorrect/rpc/rpc_empty_bounds.rs:10:1
|
10 | #[rpc(server, client, namespace = "foo", client_bounds(), server_bounds())]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Serialize` is not implemented for `<Conf as Config>::Hash`
|
note: required by a bound in `RpcModule::<Context>::register_method`
--> $WORKSPACE/core/src/server/rpc_module.rs
|
| R: Serialize,
| ^^^^^^^^^ required by this bound in `RpcModule::<Context>::register_method`
= note: this error originates in the attribute macro `rpc` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: the trait bound `for<'de> <Conf as Config>::Hash: Deserialize<'de>` is not satisfied
--> tests/ui/incorrect/rpc/rpc_empty_bounds.rs:10:1
|
10 | #[rpc(server, client, namespace = "foo", client_bounds(), server_bounds())]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `for<'de> Deserialize<'de>` is not implemented for `<Conf as Config>::Hash`
|
= note: required because of the requirements on the impl of `DeserializeOwned` for `<Conf as Config>::Hash`
note: required by a bound in `request`
--> $WORKSPACE/core/src/client/mod.rs
|
| R: DeserializeOwned;
| ^^^^^^^^^^^^^^^^ required by this bound in `request`
= note: this error originates in the attribute macro `rpc` (in Nightly builds, run with -Z macro-backtrace for more info)