From 777e9c4c70ef00b8e132142ec1df277e39f7c509 Mon Sep 17 00:00:00 2001 From: Jonas Platte Date: Sat, 20 Jul 2019 12:41:54 +0200 Subject: [PATCH 1/4] Stop throwing away span information when parsing metadata --- src/api/metadata.rs | 98 ++++++++++++++++++++------------------------- src/api/mod.rs | 11 +++-- 2 files changed, 50 insertions(+), 59 deletions(-) diff --git a/src/api/metadata.rs b/src/api/metadata.rs index 69fc7523..3d7b6a29 100644 --- a/src/api/metadata.rs +++ b/src/api/metadata.rs @@ -1,21 +1,22 @@ //! Details of the `metadata` section of the procedural macro. -use syn::{punctuated::Pair, Expr, FieldValue, Lit, Member}; +use proc_macro2::Ident; +use syn::{Expr, ExprLit, FieldValue, Lit, LitBool, LitStr, Member}; /// The result of processing the `metadata` section of the macro. pub struct Metadata { /// The description field. - pub description: String, + pub description: LitStr, /// The method field. - pub method: String, + pub method: Ident, /// The name field. - pub name: String, + pub name: LitStr, /// The path field. - pub path: String, + pub path: LitStr, /// The rate_limited field. - pub rate_limited: bool, + pub rate_limited: LitBool, /// The description field. - pub requires_authentication: bool, + pub requires_authentication: LitBool, } impl From> for Metadata { @@ -35,15 +36,13 @@ impl From> for Metadata { match &identifier.to_string()[..] { "description" => { - let expr_lit = match field_value.expr { - Expr::Lit(expr_lit) => expr_lit, - _ => panic!("expected Expr::Lit"), + let literal = match field_value.expr { + Expr::Lit(ExprLit { + lit: Lit::Str(s), .. + }) => s, + _ => panic!("expected string literal"), }; - let lit_str = match expr_lit.lit { - Lit::Str(lit_str) => lit_str, - _ => panic!("expected Lit::Str"), - }; - description = Some(lit_str.value()); + description = Some(literal); } "method" => { let expr_path = match field_value.expr { @@ -51,60 +50,49 @@ impl From> for Metadata { _ => panic!("expected Expr::Path"), }; let path = expr_path.path; - let segments = path.segments; - if segments.len() != 1 { - panic!("ruma_api! expects a one component path for `metadata` `method`"); - } - let pair = segments.first().unwrap(); // safe because we just checked - let method_name = match pair { - Pair::End(method_name) => method_name, - _ => panic!("expected Pair::End"), - }; - method = Some(method_name.ident.to_string()); + let mut segments = path.segments.iter(); + let method_name = segments.next().expect("expected non-empty path"); + assert!( + segments.next().is_none(), + "ruma_api! expects a one-component path for `metadata` `method`" + ); + method = Some(method_name.ident.clone()); } "name" => { - let expr_lit = match field_value.expr { - Expr::Lit(expr_lit) => expr_lit, - _ => panic!("expected Expr::Lit"), + let literal = match field_value.expr { + Expr::Lit(ExprLit { + lit: Lit::Str(s), .. + }) => s, + _ => panic!("expected string literal"), }; - let lit_str = match expr_lit.lit { - Lit::Str(lit_str) => lit_str, - _ => panic!("expected Lit::Str"), - }; - name = Some(lit_str.value()); + name = Some(literal); } "path" => { - let expr_lit = match field_value.expr { - Expr::Lit(expr_lit) => expr_lit, - _ => panic!("expected Expr::Lit"), + let literal = match field_value.expr { + Expr::Lit(ExprLit { + lit: Lit::Str(s), .. + }) => s, + _ => panic!("expected string literal"), }; - let lit_str = match expr_lit.lit { - Lit::Str(lit_str) => lit_str, - _ => panic!("expected Lit::Str"), - }; - path = Some(lit_str.value()); + path = Some(literal); } "rate_limited" => { - let expr_lit = match field_value.expr { - Expr::Lit(expr_lit) => expr_lit, + let literal = match field_value.expr { + Expr::Lit(ExprLit { + lit: Lit::Bool(b), .. + }) => b, _ => panic!("expected Expr::Lit"), }; - let lit_bool = match expr_lit.lit { - Lit::Bool(lit_bool) => lit_bool, - _ => panic!("expected Lit::Bool"), - }; - rate_limited = Some(lit_bool.value) + rate_limited = Some(literal) } "requires_authentication" => { - let expr_lit = match field_value.expr { - Expr::Lit(expr_lit) => expr_lit, + let literal = match field_value.expr { + Expr::Lit(ExprLit { + lit: Lit::Bool(b), .. + }) => b, _ => panic!("expected Expr::Lit"), }; - let lit_bool = match expr_lit.lit { - Lit::Bool(lit_bool) => lit_bool, - _ => panic!("expected Lit::Bool"), - }; - requires_authentication = Some(lit_bool.value) + requires_authentication = Some(literal) } _ => panic!("ruma_api! metadata included unexpected field"), } diff --git a/src/api/mod.rs b/src/api/mod.rs index 0fc84677..00d6eac8 100644 --- a/src/api/mod.rs +++ b/src/api/mod.rs @@ -65,8 +65,11 @@ impl From for Api { impl ToTokens for Api { fn to_tokens(&self, tokens: &mut TokenStream) { let description = &self.metadata.description; - let method = Ident::new(self.metadata.method.as_ref(), Span::call_site()); - let name = &self.metadata.name; + let method = &self.metadata.method; + // We don't (currently) use this literal as a literal in the generated code. Instead we just + // put it into doc comments, for which the span information is irrelevant. So we can work + // with only the literal's value from here on. + let name = &self.metadata.name.value(); let path = &self.metadata.path; let rate_limited = &self.metadata.rate_limited; let requires_authentication = &self.metadata.requires_authentication; @@ -85,7 +88,7 @@ impl ToTokens for Api { }; let (set_request_path, parse_request_path) = if self.request.has_path_fields() { - let path_str = path.as_str(); + let path_str = path.value(); assert!(path_str.starts_with('/'), "path needs to start with '/'"); assert!( @@ -313,7 +316,7 @@ impl ToTokens for Api { } }; - let endpoint_doc = format!("The `{}` API endpoint.\n\n{}", name, description); + let endpoint_doc = format!("The `{}` API endpoint.\n\n{}", name, description.value()); let request_doc = format!("Data for a request to the `{}` API endpoint.", name); let response_doc = format!("Data in the response from the `{}` API endpoint.", name); From 8f3b141db5918a840943ec54e13587e867b65d57 Mon Sep 17 00:00:00 2001 From: Jonas Platte Date: Thu, 25 Jul 2019 22:00:24 +0200 Subject: [PATCH 2/4] Replace string literals by identifiers in #[ruma_api] attributes --- README.md | 4 +- src/api/attribute.rs | 63 ++++++++++++++++++++++++++++ src/api/mod.rs | 3 +- src/api/request.rs | 91 ++++++++++++++++------------------------ src/api/response.rs | 91 ++++++++++++++++------------------------ src/lib.rs | 4 +- tests/ruma_api_macros.rs | 4 +- 7 files changed, 143 insertions(+), 117 deletions(-) create mode 100644 src/api/attribute.rs diff --git a/README.md b/README.md index 74d14c40..1024dfe7 100644 --- a/README.md +++ b/README.md @@ -39,7 +39,7 @@ pub mod some_endpoint { pub foo: String, // This value will be put into the "Content-Type" HTTP header. - #[ruma_api(header = "CONTENT_TYPE")] + #[ruma_api(header = CONTENT_TYPE)] pub content_type: String // This value will be put into the query string of the request's URL. @@ -54,7 +54,7 @@ pub mod some_endpoint { response { // This value will be extracted from the "Content-Type" HTTP header. - #[ruma_api(header = "CONTENT_TYPE")] + #[ruma_api(header = CONTENT_TYPE)] pub content_type: String // With no attribute on the field, it will be extracted from the body of the response. diff --git a/src/api/attribute.rs b/src/api/attribute.rs new file mode 100644 index 00000000..0e4cf33f --- /dev/null +++ b/src/api/attribute.rs @@ -0,0 +1,63 @@ +//! Details of the `#[ruma_api(...)]` attributes. + +use syn::{ + parenthesized, + parse::{Parse, ParseStream}, + Ident, Token, +}; + +/// Like syn::Meta, but only parses ruma_api attributes +pub enum Meta { + /// A single word, like `query` in `#[ruma_api(query)]` + Word(Ident), + /// A name-value pair, like `header = CONTENT_TYPE` in `#[ruma_api(header = CONTENT_TYPE)]` + NameValue(MetaNameValue), +} + +impl Meta { + pub fn from_attribute(attr: syn::Attribute) -> Result { + match &attr.path { + syn::Path { + leading_colon: None, + segments, + } => { + if segments.len() == 1 && segments[0].ident == "ruma_api" { + Ok( + syn::parse2(attr.tts) + .expect("ruma_api! could not parse request field attributes"), + ) + } else { + Err(attr) + } + } + _ => Err(attr), + } + } +} + +/// Like syn::MetaNameValue, but expects an identifier as the value. Also, we don't care about the +/// the span of the equals sign, so we don't have the `eq_token` field from syn::MetaNameValue. +pub struct MetaNameValue { + /// The part left of the equals sign + pub name: Ident, + /// The part right of the equals sign + pub value: Ident, +} + +impl Parse for Meta { + fn parse(input: ParseStream) -> syn::Result { + let content; + let _ = parenthesized!(content in input); + let ident = content.parse()?; + + if content.peek(Token![=]) { + let _ = content.parse::(); + Ok(Meta::NameValue(MetaNameValue { + name: ident, + value: content.parse()?, + })) + } else { + Ok(Meta::Word(ident)) + } + } +} diff --git a/src/api/mod.rs b/src/api/mod.rs index 00d6eac8..a656becb 100644 --- a/src/api/mod.rs +++ b/src/api/mod.rs @@ -1,4 +1,4 @@ -//! Details of the `ruma-api` procedural macro. +//! Details of the `ruma_api` procedural macro. use proc_macro2::{Span, TokenStream}; use quote::{quote, ToTokens}; @@ -8,6 +8,7 @@ use syn::{ Field, FieldValue, Ident, Meta, Token, }; +mod attribute; mod metadata; mod request; mod response; diff --git a/src/api/request.rs b/src/api/request.rs index 67d26968..865f040e 100644 --- a/src/api/request.rs +++ b/src/api/request.rs @@ -1,10 +1,13 @@ //! Details of the `request` section of the procedural macro. -use proc_macro2::{Span, TokenStream}; +use proc_macro2::TokenStream; use quote::{quote, quote_spanned, ToTokens}; -use syn::{spanned::Spanned, Field, Ident, Lit, Meta, NestedMeta}; +use syn::{spanned::Spanned, Field, Ident}; -use crate::api::strip_serde_attrs; +use crate::api::{ + attribute::{Meta, MetaNameValue}, + strip_serde_attrs, +}; /// The result of processing the `request` section of the macro. pub struct Request { @@ -16,13 +19,12 @@ impl Request { /// Produces code to add necessary HTTP headers to an `http::Request`. pub fn add_headers_to_request(&self) -> TokenStream { let append_stmts = self.header_fields().map(|request_field| { - let (field, header_name_string) = match request_field { - RequestField::Header(field, header_name_string) => (field, header_name_string), + let (field, header_name) = match request_field { + RequestField::Header(field, header_name) => (field, header_name), _ => panic!("expected request field to be header variant"), }; let field_name = &field.ident; - let header_name = Ident::new(header_name_string.as_ref(), Span::call_site()); quote! { headers.append( @@ -41,13 +43,13 @@ impl Request { /// Produces code to extract fields from the HTTP headers in an `http::Request`. pub fn parse_headers_from_request(&self) -> TokenStream { let fields = self.header_fields().map(|request_field| { - let (field, header_name_string) = match request_field { - RequestField::Header(field, header_name_string) => (field, header_name_string), + let (field, header_name) = match request_field { + RequestField::Header(field, header_name) => (field, header_name), _ => panic!("expected request field to be header variant"), }; let field_name = &field.ident; - let header_name = Ident::new(header_name_string.as_ref(), Span::call_site()); + let header_name_string = header_name.to_string(); quote! { #field_name: headers.get(::http::header::#header_name) @@ -180,57 +182,36 @@ impl From> for Request { let mut field_kind = RequestFieldKind::Body; let mut header = None; - field.attrs = field.attrs.into_iter().filter(|attr| { - let meta = attr.interpret_meta() - .expect("ruma_api! could not parse request field attributes"); - - let meta_list = match meta { - Meta::List(meta_list) => meta_list, - _ => return true, + field.attrs = field.attrs.into_iter().filter_map(|attr| { + let meta = match Meta::from_attribute(attr) { + Ok(meta) => meta, + Err(attr) => return Some(attr), }; - if &meta_list.ident.to_string() != "ruma_api" { - return true; - } - - for nested_meta_item in meta_list.nested { - match nested_meta_item { - NestedMeta::Meta(meta_item) => { - match meta_item { - Meta::Word(ident) => { - match &ident.to_string()[..] { - "body" => { - has_newtype_body = true; - field_kind = RequestFieldKind::NewtypeBody; - } - "path" => field_kind = RequestFieldKind::Path, - "query" => field_kind = RequestFieldKind::Query, - _ => panic!("ruma_api! single-word attribute on requests must be: body, path, or query"), - } - } - Meta::NameValue(name_value) => { - match &name_value.ident.to_string()[..] { - "header" => { - match name_value.lit { - Lit::Str(lit_str) => header = Some(lit_str.value()), - _ => panic!("ruma_api! header attribute's value must be a string literal"), - } - - field_kind = RequestFieldKind::Header; - } - _ => panic!("ruma_api! name/value pair attribute on requests must be: header"), - } - } - _ => panic!("ruma_api! attributes on requests must be a single word or a name/value pair"), + match meta { + Meta::Word(ident) => { + match &ident.to_string()[..] { + "body" => { + has_newtype_body = true; + field_kind = RequestFieldKind::NewtypeBody; } + "path" => field_kind = RequestFieldKind::Path, + "query" => field_kind = RequestFieldKind::Query, + _ => panic!("ruma_api! single-word attribute on requests must be: body, path, or query"), } - NestedMeta::Literal(_) => panic!( - "ruma_api! attributes on requests must be: body, header, path, or query" - ), + } + Meta::NameValue(MetaNameValue { name, value }) => { + assert!( + name == "header", + "ruma_api! name/value pair attribute on requests must be: header" + ); + + header = Some(value); + field_kind = RequestFieldKind::Header; } } - false + None }).collect(); if field_kind == RequestFieldKind::Body { @@ -370,7 +351,7 @@ pub enum RequestField { /// JSON data in the body of the request. Body(Field), /// Data in an HTTP header. - Header(Field, String), + Header(Field, Ident), /// A specific data type in the body of the request. NewtypeBody(Field), /// Data that appears in the URL path. @@ -381,7 +362,7 @@ pub enum RequestField { impl RequestField { /// Creates a new `RequestField`. - fn new(kind: RequestFieldKind, field: Field, header: Option) -> Self { + fn new(kind: RequestFieldKind, field: Field, header: Option) -> Self { match kind { RequestFieldKind::Body => RequestField::Body(field), RequestFieldKind::Header => { diff --git a/src/api/response.rs b/src/api/response.rs index 7405338a..32263702 100644 --- a/src/api/response.rs +++ b/src/api/response.rs @@ -1,10 +1,13 @@ //! Details of the `response` section of the procedural macro. -use proc_macro2::{Span, TokenStream}; +use proc_macro2::TokenStream; use quote::{quote, quote_spanned, ToTokens}; -use syn::{spanned::Spanned, Field, Ident, Lit, Meta, NestedMeta}; +use syn::{spanned::Spanned, Field, Ident}; -use crate::api::strip_serde_attrs; +use crate::api::{ + attribute::{Meta, MetaNameValue}, + strip_serde_attrs, +}; /// The result of processing the `request` section of the macro. pub struct Response { @@ -50,12 +53,11 @@ impl Response { #field_name: response_body.#field_name } } - ResponseField::Header(ref field, ref header) => { + ResponseField::Header(ref field, ref header_name) => { let field_name = field .ident .clone() .expect("expected field to have an identifier"); - let header_name = Ident::new(header.as_ref(), Span::call_site()); let span = field.span(); quote_spanned! {span=> @@ -87,12 +89,11 @@ impl Response { /// Produces code to add necessary HTTP headers to an `http::Response`. pub fn apply_header_fields(&self) -> TokenStream { let header_calls = self.fields.iter().filter_map(|response_field| { - if let ResponseField::Header(ref field, ref header) = *response_field { + if let ResponseField::Header(ref field, ref header_name) = *response_field { let field_name = field .ident .as_ref() .expect("expected field to have an identifier"); - let header_name = Ident::new(header.as_ref(), Span::call_site()); let span = field.span(); Some(quote_spanned! {span=> @@ -165,64 +166,44 @@ impl From> for Response { let mut field_kind = ResponseFieldKind::Body; let mut header = None; - field.attrs = field.attrs.into_iter().filter(|attr| { - let meta = attr.interpret_meta() - .expect("ruma_api! could not parse response field attributes"); - - let meta_list = match meta { - Meta::List(meta_list) => meta_list, - _ => return true, + field.attrs = field.attrs.into_iter().filter_map(|attr| { + let meta = match Meta::from_attribute(attr) { + Ok(meta) => meta, + Err(attr) => return Some(attr), }; - if &meta_list.ident.to_string() != "ruma_api" { - return true; - } + match meta { + Meta::Word(ident) => { + assert!( + ident == "body", + "ruma_api! single-word attribute on responses must be: body" + ); - for nested_meta_item in meta_list.nested { - match nested_meta_item { - NestedMeta::Meta(meta_item) => { - match meta_item { - Meta::Word(ident) => { - match &ident.to_string()[..] { - "body" => { - has_newtype_body = true; - field_kind = ResponseFieldKind::NewtypeBody; - } - _ => panic!("ruma_api! single-word attribute on responses must be: body"), - } - } - Meta::NameValue(name_value) => { - match &name_value.ident.to_string()[..] { - "header" => { - match name_value.lit { - Lit::Str(lit_str) => header = Some(lit_str.value()), - _ => panic!("ruma_api! header attribute's value must be a string literal"), - } + has_newtype_body = true; + field_kind = ResponseFieldKind::NewtypeBody; + } + Meta::NameValue(MetaNameValue { name, value }) => { + assert!( + name == "header", + "ruma_api! name/value pair attribute on requests must be: header" + ); - field_kind = ResponseFieldKind::Header; - } - _ => panic!("ruma_api! name/value pair attribute on requests must be: header"), - } - } - _ => panic!("ruma_api! attributes on responses must be a single word or a name/value pair"), - } - } - NestedMeta::Literal(_) => panic!( - "ruma_api! attribute meta item on responses must be: header" - ), + header = Some(value); + field_kind = ResponseFieldKind::Header; } } - false + None }).collect(); match field_kind { ResponseFieldKind::Body => { - if has_newtype_body { - panic!("ruma_api! responses cannot have both normal body fields and a newtype body field"); - } else { - ResponseField::Body(field) - } + assert!( + !has_newtype_body, + "ruma_api! responses cannot have both normal body fields and a newtype body field" + ); + + ResponseField::Body(field) } ResponseFieldKind::Header => ResponseField::Header(field, header.expect("missing header name")), ResponseFieldKind::NewtypeBody => ResponseField::NewtypeBody(field), @@ -307,7 +288,7 @@ pub enum ResponseField { /// JSON data in the body of the response. Body(Field), /// Data in an HTTP header. - Header(Field, String), + Header(Field, Ident), /// A specific data type in the body of the response. NewtypeBody(Field), } diff --git a/src/lib.rs b/src/lib.rs index e260765e..909463c5 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -158,7 +158,7 @@ mod api; /// request { /// pub foo: String, /// -/// #[ruma_api(header = "CONTENT_TYPE")] +/// #[ruma_api(header = CONTENT_TYPE)] /// pub content_type: String, /// /// #[ruma_api(query)] @@ -169,7 +169,7 @@ mod api; /// } /// /// response { -/// #[ruma_api(header = "CONTENT_TYPE")] +/// #[ruma_api(header = CONTENT_TYPE)] /// pub content_type: String, /// /// pub value: String, diff --git a/tests/ruma_api_macros.rs b/tests/ruma_api_macros.rs index 55a34917..292f1c7a 100644 --- a/tests/ruma_api_macros.rs +++ b/tests/ruma_api_macros.rs @@ -17,7 +17,7 @@ pub mod some_endpoint { pub foo: String, // This value will be put into the "Content-Type" HTTP header. - #[ruma_api(header = "CONTENT_TYPE")] + #[ruma_api(header = CONTENT_TYPE)] pub content_type: String, // This value will be put into the query string of the request's URL. @@ -32,7 +32,7 @@ pub mod some_endpoint { response { // This value will be extracted from the "Content-Type" HTTP header. - #[ruma_api(header = "CONTENT_TYPE")] + #[ruma_api(header = CONTENT_TYPE)] pub content_type: String, // With no attribute on the field, it will be extracted from the body of the response. From 53cf6c562d6ff1aa27ca231ed6170d4e416279e2 Mon Sep 17 00:00:00 2001 From: Jonas Platte Date: Thu, 25 Jul 2019 22:42:47 +0200 Subject: [PATCH 3/4] Re-run rustfmt --- src/api/attribute.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/api/attribute.rs b/src/api/attribute.rs index 0e4cf33f..03d3edd8 100644 --- a/src/api/attribute.rs +++ b/src/api/attribute.rs @@ -22,10 +22,8 @@ impl Meta { segments, } => { if segments.len() == 1 && segments[0].ident == "ruma_api" { - Ok( - syn::parse2(attr.tts) - .expect("ruma_api! could not parse request field attributes"), - ) + Ok(syn::parse2(attr.tts) + .expect("ruma_api! could not parse request field attributes")) } else { Err(attr) } From a89f69e4f35ec50a040fd2c26a2333b842689fb5 Mon Sep 17 00:00:00 2001 From: Jonas Platte Date: Thu, 25 Jul 2019 23:59:35 +0200 Subject: [PATCH 4/4] Add documentation to Meta::from_attribute --- src/api/attribute.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/api/attribute.rs b/src/api/attribute.rs index 03d3edd8..70016d12 100644 --- a/src/api/attribute.rs +++ b/src/api/attribute.rs @@ -15,6 +15,8 @@ pub enum Meta { } impl Meta { + /// Check if the given attribute is a ruma_api attribute. If it is, parse it, if not, return + /// it unchanged. Panics if the argument is an invalid ruma_api attribute. pub fn from_attribute(attr: syn::Attribute) -> Result { match &attr.path { syn::Path {