ruma-api-macros: Improve error handling for invalid metadata

ruma-api-macros/src/api/metadata.rs

@@ -1,7 +1,10 @@
 //! Details of the `metadata` section of the procedural macro.
 
-use proc_macro2::Ident;
+use std::convert::TryFrom;
-use syn::{Expr, ExprLit, FieldValue, Lit, LitBool, LitStr, Member};
+
+use syn::{Expr, ExprLit, ExprPath, Ident, Lit, LitBool, LitStr, Member};
+
+use crate::api::RawMetadata;
 
 /// The result of processing the `metadata` section of the macro.
 pub struct Metadata {
@@ -19,8 +22,10 @@ pub struct Metadata {
     pub requires_authentication: LitBool,
 }
 
-impl From<Vec<FieldValue>> for Metadata {
+impl TryFrom<RawMetadata> for Metadata {
-    fn from(field_values: Vec<FieldValue>) -> Self {
+    type Error = syn::Error;
+
+    fn try_from(raw: RawMetadata) -> syn::Result<Self> {
         let mut description = None;
         let mut method = None;
         let mut name = None;
@@ -28,84 +33,81 @@ impl From<Vec<FieldValue>> for Metadata {
         let mut rate_limited = None;
         let mut requires_authentication = None;
 
-        for field_value in field_values {
+        for field_value in raw.field_values {
-            let identifier = match field_value.member {
+            let identifier = match field_value.member.clone() {
                 Member::Named(identifier) => identifier,
                 _ => panic!("expected Member::Named"),
             };
+            let expr = field_value.expr.clone();
 
             match &identifier.to_string()[..] {
-                "description" => {
+                "description" => match expr {
-                    let literal = match field_value.expr {
+                    Expr::Lit(ExprLit {
-                        Expr::Lit(ExprLit {
+                        lit: Lit::Str(literal),
-                            lit: Lit::Str(s), ..
+                        ..
-                        }) => s,
+                    }) => {
-                        _ => panic!("expected string literal"),
+                        description = Some(literal);
-                    };
+                    }
-                    description = Some(literal);
+                    _ => return Err(syn::Error::new_spanned(expr, "expected a string literal")),
-                }
+                },
-                "method" => {
+                "method" => match expr {
-                    let expr_path = match field_value.expr {
+                    Expr::Path(ExprPath { path, .. }) if path.segments.len() == 1 => {
-                        Expr::Path(expr_path) => expr_path,
+                        method = Some(path.segments[0].ident.clone());
-                        _ => panic!("expected Expr::Path"),
+                    }
-                    };
+                    _ => return Err(syn::Error::new_spanned(expr, "expected an identifier")),
-                    let path = expr_path.path;
+                },
-                    let mut segments = path.segments.iter();
+                "name" => match expr {
-                    let method_name = segments.next().expect("expected non-empty path");
+                    Expr::Lit(ExprLit {
-                    assert!(
+                        lit: Lit::Str(literal),
-                        segments.next().is_none(),
+                        ..
-                        "ruma_api! expects a one-component path for `metadata` `method`"
+                    }) => {
-                    );
+                        name = Some(literal);
-                    method = Some(method_name.ident.clone());
+                    }
-                }
+                    _ => return Err(syn::Error::new_spanned(expr, "expected a string literal")),
-                "name" => {
+                },
-                    let literal = match field_value.expr {
+                "path" => match expr {
-                        Expr::Lit(ExprLit {
+                    Expr::Lit(ExprLit {
-                            lit: Lit::Str(s), ..
+                        lit: Lit::Str(literal),
-                        }) => s,
+                        ..
-                        _ => panic!("expected string literal"),
+                    }) => {
-                    };
+                        path = Some(literal);
-                    name = Some(literal);
+                    }
-                }
+                    _ => return Err(syn::Error::new_spanned(expr, "expected a string literal")),
-                "path" => {
+                },
-                    let literal = match field_value.expr {
+                "rate_limited" => match expr {
-                        Expr::Lit(ExprLit {
+                    Expr::Lit(ExprLit {
-                            lit: Lit::Str(s), ..
+                        lit: Lit::Bool(literal),
-                        }) => s,
+                        ..
-                        _ => panic!("expected string literal"),
+                    }) => {
-                    };
+                        rate_limited = Some(literal);
-                    path = Some(literal);
+                    }
-                }
+                    _ => return Err(syn::Error::new_spanned(expr, "expected a bool literal")),
-                "rate_limited" => {
+                },
-                    let literal = match field_value.expr {
+                "requires_authentication" => match expr {
-                        Expr::Lit(ExprLit {
+                    Expr::Lit(ExprLit {
-                            lit: Lit::Bool(b), ..
+                        lit: Lit::Bool(literal),
-                        }) => b,
+                        ..
-                        _ => panic!("expected Expr::Lit"),
+                    }) => {
-                    };
+                        requires_authentication = Some(literal);
-                    rate_limited = Some(literal)
+                    }
-                }
+                    _ => return Err(syn::Error::new_spanned(expr, "expected a bool literal")),
-                "requires_authentication" => {
+                },
-                    let literal = match field_value.expr {
+                _ => return Err(syn::Error::new_spanned(field_value, "unexpected field")),
-                        Expr::Lit(ExprLit {
-                            lit: Lit::Bool(b), ..
-                        }) => b,
-                        _ => panic!("expected Expr::Lit"),
-                    };
-                    requires_authentication = Some(literal)
-                }
-                _ => panic!("ruma_api! metadata included unexpected field"),
             }
         }
 
-        Self {
+        let metadata_kw = raw.metadata_kw;
-            description: description.expect("ruma_api! `metadata` is missing `description`"),
+        let missing_field =
-            method: method.expect("ruma_api! `metadata` is missing `method`"),
+            |name| syn::Error::new_spanned(metadata_kw, format!("missing field `{}`", name));
-            name: name.expect("ruma_api! `metadata` is missing `name`"),
+
-            path: path.expect("ruma_api! `metadata` is missing `path`"),
+        Ok(Self {
-            rate_limited: rate_limited.expect("ruma_api! `metadata` is missing `rate_limited`"),
+            description: description.ok_or_else(|| missing_field("description"))?,
+            method: method.ok_or_else(|| missing_field("method"))?,
+            name: name.ok_or_else(|| missing_field("name"))?,
+            path: path.ok_or_else(|| missing_field("path"))?,
+            rate_limited: rate_limited.ok_or_else(|| missing_field("rate_limited"))?,
             requires_authentication: requires_authentication
-                .expect("ruma_api! `metadata` is missing `requires_authentication`"),
+                .ok_or_else(|| missing_field("requires_authentication"))?,
-        }
+        })
     }
 }

ruma-api-macros/src/api/mod.rs

@@ -1,10 +1,12 @@
 //! Details of the `ruma_api` procedural macro.
 
+use std::convert::{TryFrom, TryInto as _};
+
 use proc_macro2::{Span, TokenStream};
 use quote::{quote, ToTokens};
 use syn::{
     braced,
-    parse::{Parse, ParseStream, Result},
+    parse::{Parse, ParseStream},
     Field, FieldValue, Ident, Token,
 };
 
@@ -34,12 +36,14 @@ pub struct Api {
     response: Response,
 }
 
-impl From<RawApi> for Api {
+impl TryFrom<RawApi> for Api {
-    fn from(raw_api: RawApi) -> Self {
+    type Error = syn::Error;
+
+    fn try_from(raw_api: RawApi) -> syn::Result<Self> {
         let res = Self {
-            metadata: raw_api.metadata.into(),
+            metadata: raw_api.metadata.try_into()?,
-            request: raw_api.request.into(),
+            request: raw_api.request.try_into()?,
-            response: raw_api.response.into(),
+            response: raw_api.response.try_into()?,
         };
 
         assert!(
@@ -48,7 +52,7 @@ impl From<RawApi> for Api {
             "GET endpoints can't have body fields"
         );
 
-        res
+        Ok(res)
     }
 }
 
@@ -303,7 +307,7 @@ mod kw {
 /// The entire `ruma_api!` macro structure directly as it appears in the source code..
 pub struct RawApi {
     /// The `metadata` section of the macro.
-    pub metadata: Vec<FieldValue>,
+    pub metadata: RawMetadata,
     /// The `request` section of the macro.
     pub request: Vec<Field>,
     /// The `response` section of the macro.
@@ -311,10 +315,8 @@ pub struct RawApi {
 }
 
 impl Parse for RawApi {
-    fn parse(input: ParseStream<'_>) -> Result<Self> {
+    fn parse(input: ParseStream<'_>) -> syn::Result<Self> {
-        input.parse::<kw::metadata>()?;
+        let metadata = input.parse::<RawMetadata>()?;
-        let metadata;
-        braced!(metadata in input);
 
         input.parse::<kw::request>()?;
         let request;
@@ -325,10 +327,7 @@ impl Parse for RawApi {
         braced!(response in input);
 
         Ok(Self {
-            metadata: metadata
+            metadata,
-                .parse_terminated::<FieldValue, Token![,]>(FieldValue::parse)?
-                .into_iter()
-                .collect(),
             request: request
                 .parse_terminated::<Field, Token![,]>(Field::parse_named)?
                 .into_iter()
@@ -340,3 +339,24 @@ impl Parse for RawApi {
         })
     }
 }
+
+pub struct RawMetadata {
+    pub metadata_kw: kw::metadata,
+    pub field_values: Vec<FieldValue>,
+}
+
+impl Parse for RawMetadata {
+    fn parse(input: ParseStream<'_>) -> syn::Result<Self> {
+        let metadata_kw = input.parse::<kw::metadata>()?;
+        let field_values;
+        braced!(field_values in input);
+
+        Ok(Self {
+            metadata_kw,
+            field_values: field_values
+                .parse_terminated::<FieldValue, Token![,]>(FieldValue::parse)?
+                .into_iter()
+                .collect(),
+        })
+    }
+}

ruma-api-macros/src/api/request.rs

@@ -1,5 +1,7 @@
 //! Details of the `request` section of the procedural macro.
 
+use std::convert::TryFrom;
+
 use proc_macro2::TokenStream;
 use quote::{quote, quote_spanned, ToTokens};
 use syn::{spanned::Spanned, Field, Ident};
@@ -119,8 +121,10 @@ impl Request {
     }
 }
 
-impl From<Vec<Field>> for Request {
+impl TryFrom<Vec<Field>> for Request {
-    fn from(fields: Vec<Field>) -> Self {
+    type Error = syn::Error;
+
+    fn try_from(fields: Vec<Field>) -> syn::Result<Self> {
         let fields: Vec<_> = fields.into_iter().map(|mut field| {
             let mut field_kind = None;
             let mut header = None;
@@ -179,7 +183,7 @@ impl From<Vec<Field>> for Request {
             );
         }
 
-        Self { fields }
+        Ok(Self { fields })
     }
 }
 

ruma-api-macros/src/api/response.rs

@@ -1,5 +1,7 @@
 //! Details of the `response` section of the procedural macro.
 
+use std::convert::TryFrom;
+
 use proc_macro2::TokenStream;
 use quote::{quote, quote_spanned, ToTokens};
 use syn::{spanned::Spanned, Field, Ident};
@@ -89,8 +91,10 @@ impl Response {
     }
 }
 
-impl From<Vec<Field>> for Response {
+impl TryFrom<Vec<Field>> for Response {
-    fn from(fields: Vec<Field>) -> Self {
+    type Error = syn::Error;
+
+    fn try_from(fields: Vec<Field>) -> syn::Result<Self> {
         let fields: Vec<_> = fields
             .into_iter()
             .map(|mut field| {
@@ -157,7 +161,7 @@ impl From<Vec<Field>> for Response {
             );
         }
 
-        Self { fields }
+        Ok(Self { fields })
     }
 }
 

ruma-api-macros/src/lib.rs

@@ -15,6 +15,8 @@
 
 extern crate proc_macro;
 
+use std::convert::TryFrom as _;
+
 use proc_macro::TokenStream;
 use quote::ToTokens;
 
@@ -191,8 +193,8 @@ mod api;
 #[proc_macro]
 pub fn ruma_api(input: TokenStream) -> TokenStream {
     let raw_api = syn::parse_macro_input!(input as RawApi);
-
+    match Api::try_from(raw_api) {
-    let api = Api::from(raw_api);
+        Ok(api) => api.into_token_stream().into(),
-
+        Err(err) => err.to_compile_error().into(),
-    api.into_token_stream().into()
+    }
 }