From f455d4c8ab11d4fdb8d00fa9329b94842bdc3862 Mon Sep 17 00:00:00 2001 From: Devin Ragotzy Date: Sat, 8 Aug 2020 21:01:19 -0400 Subject: [PATCH] Remove Response lifetime generation code in ruma_api macro MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit … and enforce that there are no lifetimes in response {} --- ruma-api-macros/src/api.rs | 66 +++++++----------- ruma-api-macros/src/api/request.rs | 6 +- ruma-api-macros/src/api/response.rs | 103 +++++----------------------- ruma-api-macros/src/util.rs | 65 +++++++++++++++++- ruma-api/tests/ruma_api_lifetime.rs | 10 ++- 5 files changed, 114 insertions(+), 136 deletions(-) diff --git a/ruma-api-macros/src/api.rs b/ruma-api-macros/src/api.rs index 3eb9683f..63a2b173 100644 --- a/ruma-api-macros/src/api.rs +++ b/ruma-api-macros/src/api.rs @@ -7,6 +7,7 @@ use quote::{quote, ToTokens}; use syn::{ braced, parse::{Parse, ParseStream}, + spanned::Spanned, Field, FieldValue, Token, Type, }; @@ -100,12 +101,6 @@ impl ToTokens for Api { quote!(Request) }; - let response_try_from_type = if self.response.contains_lifetimes() { - quote!(IncomingResponse) - } else { - quote!(Response) - }; - let extract_request_path = if self.request.has_path_fields() { quote! { let path_segments: ::std::vec::Vec<&::std::primitive::str> = @@ -195,29 +190,19 @@ impl ToTokens for Api { TokenStream::new() }; - let typed_response_body_decl = if self.response.has_body_fields() - || self.response.newtype_body_field().is_some() - { - let body_lifetimes = if self.response.has_body_lifetimes() { - // duplicate the anonymous lifetime as many times as needed - let lifetimes = - std::iter::repeat(quote! { '_ }).take(self.response.body_lifetime_count()); - quote! { < #( #lifetimes, )* >} + let typed_response_body_decl = + if self.response.has_body_fields() || self.response.newtype_body_field().is_some() { + quote! { + let response_body: ::Incoming = + ::ruma_api::try_deserialize!( + response, + ::ruma_api::exports::serde_json::from_slice(response.body().as_slice()), + ); + } } else { TokenStream::new() }; - quote! { - let response_body: ::Incoming = - ::ruma_api::try_deserialize!( - response, - ::ruma_api::exports::serde_json::from_slice(response.body().as_slice()), - ); - } - } else { - TokenStream::new() - }; - let response_init_fields = self.response.init_fields(); let serialize_response_headers = self.response.apply_header_fields(); @@ -233,7 +218,6 @@ impl ToTokens for Api { let error = &self.error; - let response_lifetimes = self.response.combine_lifetimes(); let request_lifetimes = self.request.combine_lifetimes(); let non_auth_endpoint_impl = if requires_authentication.value { @@ -250,9 +234,7 @@ impl ToTokens for Api { #[doc = #request_doc] #request_type - impl ::std::convert::TryFrom<::ruma_api::exports::http::Request>> - for #request_try_from_type - { + impl ::std::convert::TryFrom<::ruma_api::exports::http::Request>> for #request_try_from_type { type Error = ::ruma_api::error::FromHttpRequestError; #[allow(unused_variables)] @@ -276,15 +258,11 @@ impl ToTokens for Api { #[doc = #response_doc] #response_type - impl #response_lifetimes ::std::convert::TryFrom - for ::ruma_api::exports::http::Response> - { + impl ::std::convert::TryFrom for ::ruma_api::exports::http::Response> { type Error = ::ruma_api::error::IntoHttpError; #[allow(unused_variables)] - fn try_from( - response: Response #response_lifetimes, - ) -> ::std::result::Result { + fn try_from(response: Response) -> ::std::result::Result { let response = ::ruma_api::exports::http::Response::builder() .header(::ruma_api::exports::http::header::CONTENT_TYPE, "application/json") #serialize_response_headers @@ -296,9 +274,7 @@ impl ToTokens for Api { } } - impl ::std::convert::TryFrom<::ruma_api::exports::http::Response>> - for #response_try_from_type - { + impl ::std::convert::TryFrom<::ruma_api::exports::http::Response>> for Response { type Error = ::ruma_api::error::FromHttpResponseError<#error>; #[allow(unused_variables)] @@ -325,7 +301,7 @@ impl ToTokens for Api { } impl #request_lifetimes ::ruma_api::Endpoint for Request #request_lifetimes { - type Response = Response #response_lifetimes; + type Response = Response; type ResponseError = #error; /// Metadata for the `#name` endpoint. @@ -465,7 +441,17 @@ impl Parse for RawResponse { fields: fields .parse_terminated::(Field::parse_named)? .into_iter() - .collect(), + .map(|f| { + if util::has_lifetime(&f.ty) { + Err(syn::Error::new( + f.ident.span(), + "Lifetimes on Response fields cannot be supported until GAT are stable", + )) + } else { + Ok(f) + } + }) + .collect::, _>>()?, }) } } diff --git a/ruma-api-macros/src/api/request.rs b/ruma-api-macros/src/api/request.rs index 29a98022..9b991bec 100644 --- a/ruma-api-macros/src/api/request.rs +++ b/ruma-api-macros/src/api/request.rs @@ -137,7 +137,7 @@ impl Request { /// The combination of every fields unique lifetime annotation. pub fn combine_lifetimes(&self) -> TokenStream { - util::generics_to_tokens( + util::unique_lifetimes_to_tokens( self.lifetimes .body .iter() @@ -151,12 +151,12 @@ impl Request { /// The lifetimes on fields with the `query` attribute. pub fn query_lifetimes(&self) -> TokenStream { - util::generics_to_tokens(self.lifetimes.query.iter()) + util::unique_lifetimes_to_tokens(self.lifetimes.query.iter()) } /// The lifetimes on fields with the `body` attribute. pub fn body_lifetimes(&self) -> TokenStream { - util::generics_to_tokens(self.lifetimes.body.iter()) + util::unique_lifetimes_to_tokens(self.lifetimes.body.iter()) } // /// The lifetimes on fields with the `header` attribute. diff --git a/ruma-api-macros/src/api/response.rs b/ruma-api-macros/src/api/response.rs index 789e601f..47e9f05e 100644 --- a/ruma-api-macros/src/api/response.rs +++ b/ruma-api-macros/src/api/response.rs @@ -1,10 +1,10 @@ //! Details of the `response` section of the procedural macro. -use std::{collections::BTreeSet, convert::TryFrom, mem}; +use std::{convert::TryFrom, mem}; use proc_macro2::TokenStream; use quote::{quote, quote_spanned, ToTokens}; -use syn::{spanned::Spanned, Field, Ident, Lifetime}; +use syn::{spanned::Spanned, Field, Ident}; use crate::{ api::{ @@ -14,19 +14,10 @@ use crate::{ util, }; -#[derive(Debug, Default)] -pub struct ResponseLifetimes { - body: BTreeSet, - header: BTreeSet, -} - /// The result of processing the `response` section of the macro. pub struct Response { /// The fields of the response. fields: Vec, - - /// The collected lifetime identifiers from the declared fields. - lifetimes: ResponseLifetimes, } impl Response { @@ -40,36 +31,6 @@ impl Response { self.fields.iter().any(|field| field.is_header()) } - /// Whether any `body` field has a lifetime annotation. - pub fn has_body_lifetimes(&self) -> bool { - !self.lifetimes.body.is_empty() - } - - /// The number of unique lifetime annotations for `body` fields. - pub fn body_lifetime_count(&self) -> usize { - self.lifetimes.body.len() - } - - /// Whether any field has a lifetime annotation. - pub fn contains_lifetimes(&self) -> bool { - !(self.lifetimes.body.is_empty() && self.lifetimes.header.is_empty()) - } - - pub fn combine_lifetimes(&self) -> TokenStream { - util::generics_to_tokens( - self.lifetimes - .body - .iter() - .chain(self.lifetimes.header.iter()) - .collect::>() - .into_iter(), - ) - } - - pub fn body_lifetimes(&self) -> TokenStream { - util::generics_to_tokens(self.lifetimes.body.iter()) - } - /// Produces code for a response struct initializer. pub fn init_fields(&self) -> TokenStream { let mut fields = vec![]; @@ -196,7 +157,6 @@ impl TryFrom for Response { fn try_from(raw: RawResponse) -> syn::Result { let mut newtype_body_field = None; - let mut lifetimes = ResponseLifetimes::default(); let fields = raw .fields @@ -247,22 +207,12 @@ impl TryFrom for Response { } Ok(match field_kind.unwrap_or(ResponseFieldKind::Body) { - ResponseFieldKind::Body => { - util::collect_lifetime_ident(&mut lifetimes.body, &field.ty); - ResponseField::Body(field) - } + ResponseFieldKind::Body => ResponseField::Body(field), ResponseFieldKind::Header => { - util::collect_lifetime_ident(&mut lifetimes.header, &field.ty); ResponseField::Header(field, header.expect("missing header name")) } - ResponseFieldKind::NewtypeBody => { - util::collect_lifetime_ident(&mut lifetimes.body, &field.ty); - ResponseField::NewtypeBody(field) - } - ResponseFieldKind::NewtypeRawBody => { - util::collect_lifetime_ident(&mut lifetimes.body, &field.ty); - ResponseField::NewtypeRawBody(field) - } + ResponseFieldKind::NewtypeBody => ResponseField::NewtypeBody(field), + ResponseFieldKind::NewtypeRawBody => ResponseField::NewtypeRawBody(field), }) }) .collect::>>()?; @@ -275,7 +225,7 @@ impl TryFrom for Response { )); } - Ok(Self { fields, lifetimes }) + Ok(Self { fields }) } } @@ -290,52 +240,35 @@ 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 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() }; - // Though we don't track the difference between new type body and body - // for lifetimes, the outer check and the macro failing if it encounters - // an illegal combination of field attributes, is enough to guarantee - // `body_lifetimes` correctness. - let (derive_deserialize, lifetimes) = if self.has_body_lifetimes() { - (TokenStream::new(), self.body_lifetimes()) - } else { - (quote!(::ruma_api::exports::serde::Deserialize), TokenStream::new()) - }; + quote! { (#field); } + } else if self.has_body_fields() { + let fields = self.fields.iter().filter(|f| f.is_body()); - (derive_deserialize, quote! { #lifetimes (#field); }) - } else if self.has_body_fields() { - let fields = self.fields.iter().filter(|f| f.is_body()); - let (derive_deserialize, lifetimes) = if self.has_body_lifetimes() { - (TokenStream::new(), self.body_lifetimes()) - } else { - (quote!(::ruma_api::exports::serde::Deserialize), TokenStream::new()) - }; + let fields = fields.map(ResponseField::field); - let fields = fields.map(ResponseField::field); - - (derive_deserialize, quote!( #lifetimes { #(#fields),* })) - } else { - (TokenStream::new(), quote!({})) - }; + 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_generics = self.combine_lifetimes(); let response = quote! { #[derive(Debug, Clone, ::ruma_api::Outgoing)] #[incoming_no_deserialize] - pub struct Response #response_generics #response_def + pub struct Response #response_def #response_body_struct }; diff --git a/ruma-api-macros/src/util.rs b/ruma-api-macros/src/util.rs index c20b1501..bb649e82 100644 --- a/ruma-api-macros/src/util.rs +++ b/ruma-api-macros/src/util.rs @@ -68,8 +68,10 @@ pub fn collect_lifetime_ident(lifetimes: &mut BTreeSet, ty: &Type) { } /// Generates a `TokenStream` of lifetime identifiers `<'lifetime>`. -pub fn generics_to_tokens<'a, I: Iterator>(lifetimes: I) -> TokenStream { - let lifetimes = lifetimes.collect::>(); +pub fn unique_lifetimes_to_tokens<'a, I: Iterator>( + lifetimes: I, +) -> TokenStream { + let lifetimes = lifetimes.collect::>(); if lifetimes.is_empty() { TokenStream::new() } else { @@ -78,6 +80,65 @@ pub fn generics_to_tokens<'a, I: Iterator>(lifetimes: I) -> } } +pub fn has_lifetime(ty: &Type) -> bool { + match ty { + Type::Path(TypePath { path, .. }) => { + let mut found = false; + for seg in &path.segments { + match &seg.arguments { + PathArguments::AngleBracketed(AngleBracketedGenericArguments { + args, .. + }) => { + for gen in args { + if let GenericArgument::Type(ty) = gen { + if has_lifetime(&ty) { + found = true; + }; + } else if let GenericArgument::Lifetime(_) = gen { + return true; + } + } + } + PathArguments::Parenthesized(ParenthesizedGenericArguments { + inputs, .. + }) => { + for ty in inputs { + if has_lifetime(ty) { + found = true; + } + } + } + _ => {} + } + } + found + } + Type::Reference(TypeReference { elem, lifetime, .. }) => { + if lifetime.is_some() { + true + } else { + has_lifetime(&elem) + } + } + Type::Tuple(TypeTuple { elems, .. }) => { + let mut found = false; + for ty in elems { + if has_lifetime(ty) { + found = true; + } + } + found + } + Type::Paren(TypeParen { elem, .. }) => has_lifetime(&elem), + Type::Group(TypeGroup { elem, .. }) => has_lifetime(&*elem), + Type::Ptr(TypePtr { elem, .. }) => has_lifetime(&*elem), + Type::Slice(TypeSlice { elem, .. }) => has_lifetime(&*elem), + Type::Array(TypeArray { elem, .. }) => has_lifetime(&*elem), + Type::BareFn(TypeBareFn { lifetimes: Some(syn::BoundLifetimes { .. }), .. }) => true, + _ => false, + } +} + /// The first item in the tuple generates code for the request path from /// the `Metadata` and `Request` structs. The second item in the returned tuple /// is the code to generate a Request struct field created from any segments diff --git a/ruma-api/tests/ruma_api_lifetime.rs b/ruma-api/tests/ruma_api_lifetime.rs index 9cec8f82..7a35003c 100644 --- a/ruma-api/tests/ruma_api_lifetime.rs +++ b/ruma-api/tests/ruma_api_lifetime.rs @@ -85,9 +85,9 @@ mod full_request_response { response: { #[ruma_api(body)] - pub thing: OtherThing<'a>, + pub thing: Vec, #[ruma_api(header = CONTENT_TYPE)] - pub stuff: &'a str, + pub stuff: String, } } } @@ -95,8 +95,6 @@ mod full_request_response { mod full_request_response_with_query_map { use ruma_api::ruma_api; - use super::{IncomingOtherThing, OtherThing}; - ruma_api! { metadata: { description: "Does something.", @@ -119,9 +117,9 @@ mod full_request_response_with_query_map { response: { #[ruma_api(body)] - pub thing: OtherThing<'a>, + pub thing: String, #[ruma_api(header = CONTENT_TYPE)] - pub stuff: &'a str, + pub stuff: String, } } }