Bug 1664525 - Part 3: Improve errors produced by #[derive(xpcom)], r=emilio

Differential Revision: https://phabricator.services.mozilla.com/D89948

xpcom/rust/xpcom/xpcom_macros/src/lib.rs

@@ -149,13 +149,27 @@ use proc_macro::TokenStream;
 use proc_macro2::{Span, TokenStream as TokenStream2};
 use quote::{quote, ToTokens};
 use std::collections::{HashMap, HashSet};
-use std::error::Error;
 use syn::punctuated::Punctuated;
 use syn::{
-    parse, parse_quote, Attribute, Data, DataStruct, DeriveInput, Field, Fields, Ident, Lit, Meta,
-    NestedMeta, Token, Type,
+    parse_macro_input, parse_quote, Attribute, Data, DataStruct, DeriveInput, Field, Fields, Ident,
+    Lit, Meta, NestedMeta, Token, Type,
 };
 
+macro_rules! bail {
+    (@($t:expr), $s:expr) => {
+        return Err(syn::Error::new_spanned(&$t, &$s[..]));
+    };
+    (@($t:expr), $f:expr, $($e:expr),*) => {
+        return Err(syn::Error::new_spanned(&$t, &format!($f, $($e),*)[..]));
+    };
+    ($s:expr) => {
+        return Err(syn::Error::new(Span::call_site(), &$s[..]));
+    };
+    ($f:expr, $($e:expr),*) => {
+        return Err(syn::Error::new(Span::call_site(), &format!($f, $($e),*)[..]));
+    };
+}
+
 /* These are the structs generated by the rust_macros.py script */
 
 /// A single parameter to an XPCOM method.
@@ -183,16 +197,22 @@ struct Interface {
 }
 
 impl Interface {
-    fn base(&self) -> Result<Option<&'static Interface>, Box<dyn Error>> {
-        Ok(if let Some(base) = self.base {
-            Some(
-                *IFACES
-                    .get(base)
-                    .ok_or_else(|| format!("Base interface {} does not exist", base))?,
-            )
-        } else {
-            None
-        })
+    fn base(&self) -> Option<&'static Interface> {
+        Some(IFACES[self.base?])
+    }
+
+    fn methods(&self) -> Result<&'static [Method], syn::Error> {
+        match self.methods {
+            Ok(methods) => Ok(methods),
+            Err(reason) => Err(syn::Error::new(
+                Span::call_site(),
+                &format!(
+                    "Interface {} cannot be implemented in rust \
+                     because {} is not supported yet",
+                    self.name, reason
+                ),
+            )),
+        }
     }
 }
 
@@ -231,7 +251,7 @@ impl ToTokens for RefcntKind {
 }
 
 /// Scans through the attributes on a struct, and extracts the type of the refcount to use.
-fn get_refcnt_kind(attrs: &[Attribute]) -> Result<RefcntKind, Box<dyn Error>> {
+fn get_refcnt_kind(attrs: &[Attribute]) -> Result<RefcntKind, syn::Error> {
     for attr in attrs {
         if let Meta::NameValue(syn::MetaNameValue {
             ref path, ref lit, ..
@@ -244,7 +264,7 @@ fn get_refcnt_kind(attrs: &[Attribute]) -> Result<RefcntKind, Box<dyn Error>> {
             let value = if let Lit::Str(ref s) = lit {
                 s.value()
             } else {
-                Err("Unexpected non-string value in #[refcnt]")?
+                bail!(@(attr), "Unexpected non-string value in #[refcnt]");
             };
 
             return if value == "nonatomic" {
@@ -252,19 +272,19 @@ fn get_refcnt_kind(attrs: &[Attribute]) -> Result<RefcntKind, Box<dyn Error>> {
             } else if value == "atomic" {
                 Ok(RefcntKind::Atomic)
             } else {
-                Err("Unexpected value in #[refcnt]. \
-                     Expected `nonatomic`, or `atomic`")?
+                bail!(@(attr), "Unexpected value in #[refcnt]. \
+                                Expected `nonatomic`, or `atomic`");
             };
         }
     }
 
-    Err("Expected #[refcnt] attribute")?
+    bail!("Expected #[refcnt] attribute")
 }
 
 /// Scan the attributes looking for an #[xpimplements] attribute. The identifier
 /// arguments passed to this attribute are the interfaces which the type wants to
 /// directly implement.
-fn get_bases(attrs: &[Attribute]) -> Result<Vec<&'static Interface>, Box<dyn Error>> {
+fn get_bases(attrs: &[Attribute]) -> Result<Vec<&'static Interface>, syn::Error> {
     let mut inherits = Vec::new();
     for attr in attrs {
         if let Meta::List(syn::MetaList {
@@ -280,20 +300,16 @@ fn get_bases(attrs: &[Attribute]) -> Result<Vec<&'static Interface>, Box<dyn Err
             for item in nested.iter() {
                 let iface = match *item {
                     NestedMeta::Meta(syn::Meta::Path(ref iface)) => iface,
-                    _ => Err("Unexpected non-identifier in #[xpimplements(..)]")?,
+                    _ => bail!(@(attr), "Unexpected non-identifier in #[xpimplements(..)]"),
                 };
                 let ident = match iface.get_ident() {
                     Some(ref iface) => iface.to_string(),
-                    _ => Err("Too many components in xpimplements path")?,
+                    _ => bail!(@(attr), "Too many components in xpimplements path"),
                 };
                 if let Some(&iface) = IFACES.get(ident.as_str()) {
                     inherits.push(iface);
                 } else {
-                    Err(format!(
-                        "Unexpected invalid base interface `{}` in \
-                                 #[xpimplements(..)]",
-                        ident
-                    ))?
+                    bail!(@(attr), "Unexpected invalid base interface `{}` in #[xpimplements(..)]", ident);
                 }
             }
         }
@@ -302,15 +318,16 @@ fn get_bases(attrs: &[Attribute]) -> Result<Vec<&'static Interface>, Box<dyn Err
 }
 
 /// Extract the fields list from the input struct.
-fn get_fields(di: &DeriveInput) -> Result<&Punctuated<Field, Token![,]>, Box<dyn Error>> {
+fn get_fields(di: &DeriveInput) -> Result<&Punctuated<Field, Token![,]>, syn::Error> {
     match di.data {
         Data::Struct(DataStruct {
             fields: Fields::Named(ref named),
             ..
         }) => Ok(&named.named),
-        _ => Err("The initializer struct must be a standard \
-                  named value struct definition"
-            .into()),
+        _ => {
+            bail!(@(di), "The initializer struct must be a standard named \
+                          value struct definition")
+        }
     }
 }
 
@@ -319,11 +336,11 @@ fn gen_real_struct(
     init: &DeriveInput,
     bases: &[&Interface],
     refcnt_ty: RefcntKind,
-) -> Result<DeriveInput, Box<dyn Error>> {
+) -> Result<DeriveInput, syn::Error> {
     // Determine the name for the real struct based on the name of the
     // initializer struct's name.
     if !init.ident.to_string().starts_with("Init") {
-        Err("The target struct's name must begin with Init")?
+        bail!(@(init.ident), "The target struct's name must begin with Init");
     }
     let name: Ident = Ident::new(&init.ident.to_string()[4..], Span::call_site());
     let vis = &init.vis;
@@ -351,25 +368,17 @@ fn gen_real_struct(
 /// These methods attempt to invoke the `recover_self` method to translate from
 /// the passed-in raw pointer to the actual `&self` value, and it is expected to
 /// be in scope.
-fn gen_vtable_methods(iface: &Interface) -> Result<TokenStream2, Box<dyn Error>> {
+fn gen_vtable_methods(iface: &Interface) -> Result<TokenStream2, syn::Error> {
     let base_ty = Ident::new(iface.name, Span::call_site());
 
-    let base_methods = if let Some(base) = iface.base()? {
+    let base_methods = if let Some(base) = iface.base() {
         gen_vtable_methods(base)?
     } else {
         quote! {}
     };
 
-    let methods = iface.methods.map_err(|reason| {
-        format!(
-            "Interface {} cannot be implemented in rust \
-                                   because {} is not supported yet",
-            iface.name, reason
-        )
-    })?;
-
     let mut method_defs = Vec::new();
-    for method in methods {
+    for method in iface.methods()? {
         let name = Ident::new(method.name, Span::call_site());
         let ret = syn::parse_str::<Type>(method.ret)?;
 
@@ -399,19 +408,11 @@ fn gen_vtable_methods(iface: &Interface) -> Result<TokenStream2, Box<dyn Error>>
 
 /// Generates the VTable for a given base interface. This assumes that the
 /// implementations of each of the `extern "system"` methods are in scope.
-fn gen_inner_vtable(iface: &Interface) -> Result<TokenStream2, Box<dyn Error>> {
+fn gen_inner_vtable(iface: &Interface) -> Result<TokenStream2, syn::Error> {
     let vtable_ty = Ident::new(&format!("{}VTable", iface.name), Span::call_site());
 
-    let methods = iface.methods.map_err(|reason| {
-        format!(
-            "Interface {} cannot be implemented in rust \
-                                   because {} is not supported yet",
-            iface.name, reason
-        )
-    })?;
-
     // Generate the vtable for the base interface.
-    let base_vtable = if let Some(base) = iface.base()? {
+    let base_vtable = if let Some(base) = iface.base() {
         let vt = gen_inner_vtable(base)?;
         quote! {__base: #vt,}
     } else {
@@ -419,7 +420,8 @@ fn gen_inner_vtable(iface: &Interface) -> Result<TokenStream2, Box<dyn Error>> {
     };
 
     // Include each of the method definitions for this interface.
-    let vtable_init = methods
+    let vtable_init = iface
+        .methods()?
         .into_iter()
         .map(|method| {
             let name = Ident::new(method.name, Span::call_site());
@@ -433,7 +435,7 @@ fn gen_inner_vtable(iface: &Interface) -> Result<TokenStream2, Box<dyn Error>> {
     }))
 }
 
-fn gen_root_vtable(name: &Ident, base: &Interface) -> Result<TokenStream2, Box<dyn Error>> {
+fn gen_root_vtable(name: &Ident, base: &Interface) -> Result<TokenStream2, syn::Error> {
     let field = Ident::new(&format!("__base_{}", base.name), Span::call_site());
     let vtable_ty = Ident::new(&format!("{}VTable", base.name), Span::call_site());
     let methods = gen_vtable_methods(base)?;
@@ -480,13 +482,13 @@ fn gen_casts(
     name: &Ident,
     coerce_name: &Ident,
     vtable_field: &Ident,
-) -> Result<(TokenStream2, TokenStream2), Box<dyn Error>> {
+) -> Result<(TokenStream2, TokenStream2), syn::Error> {
     if !seen.insert(iface.name) {
         return Ok((quote! {}, quote! {}));
     }
 
     // Generate the cast implementations for the base interfaces.
-    let (base_qi, base_coerce) = if let Some(base) = iface.base()? {
+    let (base_qi, base_coerce) = if let Some(base) = iface.base() {
         gen_casts(seen, base, name, coerce_name, vtable_field)?
     } else {
         (quote! {}, quote! {})
@@ -530,42 +532,41 @@ fn gen_casts(
 }
 
 /// The root xpcom procedural macro definition.
-fn xpcom(init: DeriveInput) -> Result<TokenStream2, Box<dyn Error>> {
+fn xpcom(init: DeriveInput) -> Result<TokenStream2, syn::Error> {
     if !init.generics.params.is_empty() || !init.generics.where_clause.is_none() {
-        return Err("Cannot #[derive(xpcom)] on a generic type, due to \
-                    rust limitations. It is not possible to instantiate \
-                    a static with a generic type parameter, meaning that \
-                    generic types cannot have their VTables instantiated \
-                    correctly."
-            .into());
+        bail!(
+            "Cannot #[derive(xpcom)] on a generic type, due to \
+             rust limitations. It is not possible to instantiate \
+             a static with a generic type parameter, meaning that \
+             generic types cannot have their VTables instantiated \
+             correctly."
+        )
     }
 
     let bases = get_bases(&init.attrs)?;
     if bases.is_empty() {
-        return Err("Types with #[derive(xpcom)] must implement at least one \
-                    interface. Interfaces can be implemented by adding the \
-                    #[xpimplements(nsIFoo, nsIBar)] attribute to the struct \
-                    declaration."
-            .into());
+        bail!(
+            "Types with #[derive(xpcom)] must implement at least one \
+             interface. Interfaces can be implemented by adding the \
+             #[xpimplements(nsIFoo, nsIBar)] attribute to the struct \
+             declaration."
+        );
     }
 
     // Ensure that all our base interface methods have unique names.
-    if bases.len() > 1 {
-        let mut names = HashMap::new();
-        for base in &bases {
-            if let Ok(methods) = base.methods {
-                for method in methods {
-                    if let Some(existing) = names.insert(method.name, base.name) {
-                        return Err(format!(
-                            "The method `{0}` is declared on both `{1}` and `{2}`, \
-                             but a Rust type cannot implement two methods with the same name. \
-                             You can add the `[binaryname(Renamed{0})]` XPIDL attribute to one of \
-                             the declarations to rename it.",
-                            method.name, existing, base.name
-                        )
-                        .into());
-                    }
-                }
+    let mut method_names = HashMap::new();
+    for base in &bases {
+        for method in base.methods()? {
+            if let Some(existing) = method_names.insert(method.name, base.name) {
+                bail!(
+                    "The method `{0}` is declared on both `{1}` and `{2}`,
+                     but a Rust type cannot implement two methods with the \
+                     same name. You can add the `[binaryname(Renamed{0})]` \
+                     XPIDL attribute to one of the declarations to rename it.",
+                    method.name,
+                    existing,
+                    base.name
+                );
             }
         }
     }
@@ -722,7 +723,9 @@ fn xpcom(init: DeriveInput) -> Result<TokenStream2, Box<dyn Error>> {
 
 #[proc_macro_derive(xpcom, attributes(xpimplements, refcnt))]
 pub fn xpcom_internal(input: TokenStream) -> TokenStream {
-    xpcom(parse(input).expect("Invalid derive input"))
-        .expect("#[derive(xpcom)] failed")
-        .into()
+    let input = parse_macro_input!(input as DeriveInput);
+    match xpcom(input) {
+        Ok(ts) => ts.into(),
+        Err(err) => err.to_compile_error().into(),
+    }
 }