From df7914de1a6bbbdf6786a9fa85408a73a97faf61 Mon Sep 17 00:00:00 2001 From: Jonas Platte Date: Wed, 22 Apr 2020 12:54:47 +0200 Subject: [PATCH] Remove Outgoing trait we won't need it anymore once `EventResult` is replaced with `EventJson` --- Cargo.toml | 2 +- ruma-api-macros/src/api.rs | 4 +- ruma-api-macros/src/api/request.rs | 24 +- ruma-api-macros/src/api/response.rs | 38 +--- ruma-api-macros/src/derive_outgoing.rs | 214 ------------------ .../src/derive_outgoing/wrap_incoming.rs | 58 ----- ruma-api-macros/src/lib.rs | 53 +---- src/lib.rs | 46 +--- tests/ruma_api_macros.rs | 27 +-- 9 files changed, 34 insertions(+), 432 deletions(-) delete mode 100644 ruma-api-macros/src/derive_outgoing.rs delete mode 100644 ruma-api-macros/src/derive_outgoing/wrap_incoming.rs diff --git a/Cargo.toml b/Cargo.toml index cd9e86af..b418e679 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -26,7 +26,7 @@ serde_urlencoded = "0.6.1" strum = "0.18.0" [dev-dependencies] -ruma-events = "0.19.0" +ruma-events = { git = "https://github.com/ruma/ruma-events", branch = "event-json" } [features] default = ["with-ruma-api-macros"] diff --git a/ruma-api-macros/src/api.rs b/ruma-api-macros/src/api.rs index 3bcd71ba..8ebbbeb9 100644 --- a/ruma-api-macros/src/api.rs +++ b/ruma-api-macros/src/api.rs @@ -298,7 +298,7 @@ impl ToTokens for Api { let extract_request_body = if self.request.has_body_fields() || self.request.newtype_body_field().is_some() { quote! { - let request_body: ::Incoming = + let request_body: RequestBody = match ruma_api::exports::serde_json::from_slice(request.body().as_slice()) { Ok(body) => body, Err(err) => { @@ -368,7 +368,7 @@ impl ToTokens for Api { || self.response.newtype_body_field().is_some() { quote! { - let response_body: ::Incoming = + let response_body: ResponseBody = match ruma_api::exports::serde_json::from_slice(response.body().as_slice()) { Ok(body) => body, Err(err) => { diff --git a/ruma-api-macros/src/api/request.rs b/ruma-api-macros/src/api/request.rs index c95bf4fc..408060fc 100644 --- a/ruma-api-macros/src/api/request.rs +++ b/ruma-api-macros/src/api/request.rs @@ -310,34 +310,21 @@ impl ToTokens for Request { let request_body_struct = if let Some(body_field) = self.fields.iter().find(|f| f.is_newtype_body()) { let field = Field { ident: None, colon_token: None, ..body_field.field().clone() }; - let derive_deserialize = if body_field.has_wrap_incoming_attr() { - TokenStream::new() - } else { - quote!(ruma_api::exports::serde::Deserialize) - }; - - Some((derive_deserialize, quote! { (#field); })) + Some(quote! { (#field); }) } else if self.has_body_fields() { let fields = self.fields.iter().filter(|f| f.is_body()); - let derive_deserialize = if fields.clone().any(|f| f.has_wrap_incoming_attr()) { - TokenStream::new() - } else { - quote!(ruma_api::exports::serde::Deserialize) - }; let fields = fields.map(RequestField::field); - - Some((derive_deserialize, quote! { { #(#fields),* } })) + Some(quote! { { #(#fields),* } }) } else { None } - .map(|(derive_deserialize, def)| { + .map(|def| { quote! { /// Data in the request body. #[derive( Debug, - ruma_api::Outgoing, + ruma_api::exports::serde::Deserialize, ruma_api::exports::serde::Serialize, - #derive_deserialize )] struct RequestBody #def } @@ -374,8 +361,7 @@ impl ToTokens for Request { }; let request = quote! { - #[derive(Debug, Clone, ruma_api::Outgoing)] - #[incoming_no_deserialize] + #[derive(Debug, Clone)] pub struct Request #request_def #request_body_struct diff --git a/ruma-api-macros/src/api/response.rs b/ruma-api-macros/src/api/response.rs index 94a38921..2bdef543 100644 --- a/ruma-api-macros/src/api/response.rs +++ b/ruma-api-macros/src/api/response.rs @@ -244,44 +244,28 @@ impl ToTokens for Response { quote! { { #(#fields),* } } }; - let (derive_deserialize, def) = - if let Some(body_field) = self.fields.iter().find(|f| f.is_newtype_body()) { - let field = Field { ident: None, colon_token: None, ..body_field.field().clone() }; - let derive_deserialize = if body_field.has_wrap_incoming_attr() { - TokenStream::new() - } else { - quote!(ruma_api::exports::serde::Deserialize) - }; - - (derive_deserialize, quote! { (#field); }) - } else if self.has_body_fields() { - let fields = self.fields.iter().filter(|f| f.is_body()); - let derive_deserialize = if fields.clone().any(|f| f.has_wrap_incoming_attr()) { - TokenStream::new() - } else { - quote!(ruma_api::exports::serde::Deserialize) - }; - let fields = fields.map(ResponseField::field); - - (derive_deserialize, quote!({ #(#fields),* })) - } else { - (TokenStream::new(), quote!({})) - }; + let def = if let Some(body_field) = self.fields.iter().find(|f| f.is_newtype_body()) { + let field = Field { ident: None, colon_token: None, ..body_field.field().clone() }; + quote! { (#field); } + } else if self.has_body_fields() { + let fields = self.fields.iter().filter_map(|f| f.as_body_field()); + quote!({ #(#fields),* }) + } else { + quote!({}) + }; let response_body_struct = quote! { /// Data in the response body. #[derive( Debug, - ruma_api::Outgoing, + ruma_api::exports::serde::Deserialize, ruma_api::exports::serde::Serialize, - #derive_deserialize )] struct ResponseBody #def }; let response = quote! { - #[derive(Debug, Clone, ruma_api::Outgoing)] - #[incoming_no_deserialize] + #[derive(Debug, Clone)] pub struct Response #response_def #response_body_struct diff --git a/ruma-api-macros/src/derive_outgoing.rs b/ruma-api-macros/src/derive_outgoing.rs deleted file mode 100644 index 0b1b0f51..00000000 --- a/ruma-api-macros/src/derive_outgoing.rs +++ /dev/null @@ -1,214 +0,0 @@ -use std::mem; - -use proc_macro2::{Ident, Span, TokenStream}; -use quote::{format_ident, quote, ToTokens}; -use syn::{ - parse_quote, punctuated::Pair, spanned::Spanned, Attribute, Data, DeriveInput, Fields, - GenericArgument, Path, PathArguments, Type, TypePath, -}; - -mod wrap_incoming; - -use wrap_incoming::Meta; - -enum StructKind { - Struct, - Tuple, -} - -pub fn expand_derive_outgoing(input: DeriveInput) -> syn::Result { - if !input.generics.params.is_empty() { - return Err(syn::Error::new_spanned( - input.generics, - "derive(Outgoing) doesn't currently support types with generics!", - )); - } - - let derive_deserialize = if no_deserialize_in_attrs(&input.attrs) { - TokenStream::new() - } else { - quote!(ruma_api::exports::serde::Deserialize) - }; - - let (mut fields, struct_kind): (Vec<_>, _) = match input.data { - Data::Enum(_) | Data::Union(_) => { - panic!("#[derive(Outgoing)] is only supported for structs") - } - Data::Struct(s) => match s.fields { - Fields::Named(fs) => { - (fs.named.into_pairs().map(Pair::into_value).collect(), StructKind::Struct) - } - Fields::Unnamed(fs) => { - (fs.unnamed.into_pairs().map(Pair::into_value).collect(), StructKind::Tuple) - } - Fields::Unit => return Ok(impl_outgoing_with_incoming_self(input.ident)), - }, - }; - - let mut any_attribute = false; - - for field in &mut fields { - let mut field_meta = None; - - let mut remaining_attrs = Vec::new(); - for attr in mem::replace(&mut field.attrs, Vec::new()) { - if let Some(meta) = Meta::from_attribute(&attr)? { - if field_meta.is_some() { - return Err(syn::Error::new_spanned( - attr, - "duplicate #[wrap_incoming] attribute", - )); - } - field_meta = Some(meta); - any_attribute = true; - } else { - remaining_attrs.push(attr); - } - } - field.attrs = remaining_attrs; - - if let Some(attr) = field_meta { - if let Some(type_to_wrap) = attr.type_to_wrap { - wrap_generic_arg(&type_to_wrap, &mut field.ty, attr.wrapper_type.as_ref())?; - } else { - wrap_ty(&mut field.ty, attr.wrapper_type)?; - } - } - } - - if !any_attribute { - return Ok(impl_outgoing_with_incoming_self(input.ident)); - } - - let vis = input.vis; - let doc = format!("'Incoming' variant of [{ty}](struct.{ty}.html).", ty = input.ident); - let original_ident = input.ident; - let incoming_ident = format_ident!("Incoming{}", original_ident, span = Span::call_site()); - - let struct_def = match struct_kind { - StructKind::Struct => quote! { { #(#fields,)* } }, - StructKind::Tuple => quote! { ( #(#fields,)* ); }, - }; - - Ok(quote! { - #[doc = #doc] - #[derive(Debug, #derive_deserialize)] - #vis struct #incoming_ident #struct_def - - impl ruma_api::Outgoing for #original_ident { - type Incoming = #incoming_ident; - } - }) -} - -fn no_deserialize_in_attrs(attrs: &[Attribute]) -> bool { - for attr in attrs { - match &attr.path { - Path { leading_colon: None, segments } - if segments.len() == 1 && segments[0].ident == "incoming_no_deserialize" => - { - return true - } - _ => {} - } - } - - false -} - -fn impl_outgoing_with_incoming_self(ident: Ident) -> TokenStream { - quote! { - impl ruma_api::Outgoing for #ident { - type Incoming = Self; - } - } -} - -fn wrap_ty(ty: &mut Type, path: Option) -> syn::Result<()> { - if let Some(wrap_ty) = path { - *ty = parse_quote!(#wrap_ty<#ty>); - } else { - match ty { - Type::Path(TypePath { path, .. }) => { - let ty_ident = &mut path.segments.last_mut().unwrap().ident; - let ident = format_ident!("Incoming{}", ty_ident, span = Span::call_site()); - *ty_ident = parse_quote!(#ident); - } - _ => return Err(syn::Error::new_spanned(ty, "Can't wrap this type")), - } - } - - Ok(()) -} - -fn wrap_generic_arg(type_to_wrap: &Type, of: &mut Type, with: Option<&Path>) -> syn::Result<()> { - let mut span = None; - wrap_generic_arg_impl(type_to_wrap, of, with, &mut span)?; - - if span.is_some() { - Ok(()) - } else { - Err(syn::Error::new_spanned( - of, - format!( - "Couldn't find generic argument `{}` in this type", - type_to_wrap.to_token_stream() - ), - )) - } -} - -fn wrap_generic_arg_impl( - type_to_wrap: &Type, - of: &mut Type, - with: Option<&Path>, - span: &mut Option, -) -> syn::Result<()> { - // TODO: Support things like array types? - let ty_path = match of { - Type::Path(TypePath { path, .. }) => path, - _ => return Ok(()), - }; - - let args = match &mut ty_path.segments.last_mut().unwrap().arguments { - PathArguments::AngleBracketed(ab) => &mut ab.args, - _ => return Ok(()), - }; - - for arg in args.iter_mut() { - let ty = match arg { - GenericArgument::Type(ty) => ty, - _ => continue, - }; - - if ty == type_to_wrap { - if let Some(s) = span { - let mut error = syn::Error::new( - *s, - format!( - "`{}` found multiple times, this is not currently supported", - type_to_wrap.to_token_stream() - ), - ); - error.combine(syn::Error::new_spanned(ty, "second occurrence")); - return Err(error); - } - - *span = Some(ty.span()); - - if let Some(wrapper_type) = with { - *ty = parse_quote!(#wrapper_type<#ty>); - } else if let Type::Path(TypePath { path, .. }) = ty { - let ty_ident = &mut path.segments.last_mut().unwrap().ident; - let ident = format_ident!("Incoming{}", ty_ident, span = Span::call_site()); - *ty_ident = parse_quote!(#ident); - } else { - return Err(syn::Error::new_spanned(ty, "Can't wrap this type")); - } - } else { - wrap_generic_arg_impl(type_to_wrap, ty, with, span)?; - } - } - - Ok(()) -} diff --git a/ruma-api-macros/src/derive_outgoing/wrap_incoming.rs b/ruma-api-macros/src/derive_outgoing/wrap_incoming.rs deleted file mode 100644 index f13c6f3c..00000000 --- a/ruma-api-macros/src/derive_outgoing/wrap_incoming.rs +++ /dev/null @@ -1,58 +0,0 @@ -use syn::{ - parse::{Parse, ParseStream}, - Ident, Path, Type, -}; - -mod kw { - use syn::custom_keyword; - custom_keyword!(with); -} - -/// The inside of a `#[wrap_incoming]` attribute -#[derive(Default)] -pub struct Meta { - pub type_to_wrap: Option, - pub wrapper_type: Option, -} - -impl Meta { - /// Check if the given attribute is a wrap_incoming attribute. If it is, parse it. - pub fn from_attribute(attr: &syn::Attribute) -> syn::Result> { - if attr.path.is_ident("wrap_incoming") { - if attr.tokens.is_empty() { - Ok(Some(Self::default())) - } else { - attr.parse_args().map(Some) - } - } else { - Ok(None) - } - } -} - -impl Parse for Meta { - fn parse(input: ParseStream) -> syn::Result { - let mut type_to_wrap = None; - let mut wrapper_type = try_parse_wrapper_type(input)?; - - if wrapper_type.is_none() && input.peek(Ident) { - type_to_wrap = Some(input.parse()?); - wrapper_type = try_parse_wrapper_type(input)?; - } - - if input.is_empty() { - Ok(Self { type_to_wrap, wrapper_type }) - } else { - Err(input.error("expected end of attribute args")) - } - } -} - -fn try_parse_wrapper_type(input: ParseStream) -> syn::Result> { - if input.peek(kw::with) { - input.parse::()?; - Ok(Some(input.parse()?)) - } else { - Ok(None) - } -} diff --git a/ruma-api-macros/src/lib.rs b/ruma-api-macros/src/lib.rs index 570578e7..b8e39bde 100644 --- a/ruma-api-macros/src/lib.rs +++ b/ruma-api-macros/src/lib.rs @@ -14,15 +14,11 @@ use std::convert::TryFrom as _; use proc_macro::TokenStream; use quote::ToTokens; -use syn::{parse_macro_input, DeriveInput}; +use syn::parse_macro_input; -use self::{ - api::{Api, RawApi}, - derive_outgoing::expand_derive_outgoing, -}; +use self::api::{Api, RawApi}; mod api; -mod derive_outgoing; #[proc_macro] pub fn ruma_api(input: TokenStream) -> TokenStream { @@ -32,48 +28,3 @@ pub fn ruma_api(input: TokenStream) -> TokenStream { Err(err) => err.to_compile_error().into(), } } - -/// Derive the `Outgoing` trait, possibly generating an 'Incoming' version of the struct this -/// derive macro is used on. Specifically, if no `#[wrap_incoming]` attribute is used on any of the -/// fields of the struct, this simple implementation will be generated: -/// -/// ```ignore -/// impl Outgoing for MyType { -/// type Incoming = Self; -/// } -/// ``` -/// -/// If, however, `#[wrap_incoming]` is used (which is the only reason you should ever use this -/// derive macro manually), a new struct `IncomingT` (where `T` is the type this derive is used on) -/// is generated, with all of the fields with `#[wrap_incoming]` replaced: -/// -/// ```ignore -/// #[derive(Outgoing)] -/// struct MyType { -/// pub foo: Foo, -/// #[wrap_incoming] -/// pub bar: Bar, -/// #[wrap_incoming(Baz)] -/// pub baz: Option, -/// #[wrap_incoming(with EventResult)] -/// pub x: XEvent, -/// #[wrap_incoming(YEvent with EventResult)] -/// pub ys: Vec, -/// } -/// -/// // generated -/// struct IncomingMyType { -/// pub foo: Foo, -/// pub bar: IncomingBar, -/// pub baz: Option, -/// pub x: EventResult, -/// pub ys: Vec>, -/// } -/// ``` -// TODO: Make it clear that `#[wrap_incoming]` and `#[wrap_incoming(Type)]` without the "with" part -// are (only) useful for fallible deserialization of nested structures. -#[proc_macro_derive(Outgoing, attributes(wrap_incoming, incoming_no_deserialize))] -pub fn derive_outgoing(input: TokenStream) -> TokenStream { - let input = parse_macro_input!(input as DeriveInput); - expand_derive_outgoing(input).unwrap_or_else(|err| err.to_compile_error()).into() -} diff --git a/src/lib.rs b/src/lib.rs index 0c8e8ad4..0d0f1069 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -191,19 +191,9 @@ use http::Method; /// } /// } /// ``` -/// -/// ## Fallible deserialization -/// -/// All request and response types also derive [`Outgoing`][Outgoing]. As such, to allow fallible -/// deserialization, you can use the `#[wrap_incoming]` attribute. For details, see the -/// documentation for [the derive macro](derive.Outgoing.html). -// TODO: Explain the concept of fallible deserialization before jumping to `ruma_api::Outgoing` #[cfg(feature = "with-ruma-api-macros")] pub use ruma_api_macros::ruma_api; -#[cfg(feature = "with-ruma-api-macros")] -pub use ruma_api_macros::Outgoing; - pub mod error; /// This module is used to support the generated code from ruma-api-macros. /// It is not considered part of ruma-api's public API. @@ -219,18 +209,6 @@ pub mod exports { use error::{FromHttpRequestError, FromHttpResponseError, IntoHttpError}; -/// A type that can be sent to another party that understands the matrix protocol. If any of the -/// fields of `Self` don't implement serde's `Deserialize`, you can derive this trait to generate a -/// corresponding 'Incoming' type that supports deserialization. This is useful for things like -/// ruma_events' `EventResult` type. For more details, see the [derive macro's documentation][doc]. -/// -/// [doc]: derive.Outgoing.html -// TODO: Better explain how this trait relates to serde's traits -pub trait Outgoing { - /// The 'Incoming' variant of `Self`. - type Incoming; -} - /// Gives users the ability to define their own serializable/deserializable errors. pub trait EndpointError: Sized { /// Tries to construct `Self` from an `http::Response`. @@ -245,16 +223,14 @@ pub trait EndpointError: Sized { /// A Matrix API endpoint. /// /// The type implementing this trait contains any data needed to make a request to the endpoint. -pub trait Endpoint: Outgoing + TryInto>, Error = IntoHttpError> -where - ::Incoming: TryFrom>, Error = FromHttpRequestError>, - ::Incoming: TryFrom< - http::Response>, - Error = FromHttpResponseError<::ResponseError>, - >, +pub trait Endpoint: + TryInto>, Error = IntoHttpError> + + TryFrom>, Error = FromHttpRequestError> { /// Data returned in a successful response from the endpoint. - type Response: Outgoing + TryInto>, Error = IntoHttpError>; + type Response: TryInto>, Error = IntoHttpError> + + TryFrom>, Error = FromHttpResponseError>; + /// Error type returned when response from endpoint fails. type ResponseError: EndpointError; @@ -300,7 +276,7 @@ mod tests { FromHttpRequestError, FromHttpResponseError, IntoHttpError, RequestDeserializationError, ServerError, Void, }, - Endpoint, Metadata, Outgoing, + Endpoint, Metadata, }; /// A request to create a new room alias. @@ -310,10 +286,6 @@ mod tests { pub room_alias: RoomAliasId, // path } - impl Outgoing for Request { - type Incoming = Self; - } - impl Endpoint for Request { type Response = Response; type ResponseError = Void; @@ -394,10 +366,6 @@ mod tests { #[derive(Clone, Copy, Debug)] pub struct Response; - impl Outgoing for Response { - type Incoming = Self; - } - impl TryFrom>> for Response { type Error = FromHttpResponseError; diff --git a/tests/ruma_api_macros.rs b/tests/ruma_api_macros.rs index 5b4f9948..b4bf6434 100644 --- a/tests/ruma_api_macros.rs +++ b/tests/ruma_api_macros.rs @@ -1,7 +1,6 @@ pub mod some_endpoint { - use ruma_api::{ruma_api, Outgoing}; - use ruma_events::{collections::all, sticker::StickerEvent, tag::TagEvent, EventResult}; - use serde::Serialize; + use ruma_api::ruma_api; + use ruma_events::{collections::all, tag::TagEvent, EventJson}; ruma_api! { metadata { @@ -43,27 +42,13 @@ pub mod some_endpoint { #[serde(skip_serializing_if = "Option::is_none")] pub optional_flag: Option, - // This is how you usually use `#[wrap_incoming]` with event types - #[wrap_incoming(with EventResult)] - pub event: TagEvent, + // Use `EventJson` instead of the actual event to allow additional fields to be sent... + pub event: EventJson, - // Same for lists of events - #[wrap_incoming(all::RoomEvent with EventResult)] - pub list_of_events: Vec, - - // This is how `#[wrap_incoming]` is used with nested `EventResult`s - #[wrap_incoming] - pub object: ObjectContainingEvents, + // ... and to allow unknown events when the endpoint deals with event collections. + pub list_of_events: Vec>, } } - - #[derive(Clone, Debug, Serialize, Outgoing)] - pub struct ObjectContainingEvents { - #[wrap_incoming(TagEvent with EventResult)] - pub event_list_1: Vec, - #[wrap_incoming(StickerEvent with EventResult)] - pub event_list_2: Vec, - } } pub mod newtype_body_endpoint {