Switch to span-based errors for pyclass and pyimpl

derive/src/error.rs

@@ -0,0 +1,158 @@
+// Taken from https://github.com/rustwasm/wasm-bindgen/blob/master/crates/backend/src/error.rs
+//
+// Copyright (c) 2014 Alex Crichton
+//
+// Permission is hereby granted, free of charge, to any
+// person obtaining a copy of this software and associated
+// documentation files (the "Software"), to deal in the
+// Software without restriction, including without
+// limitation the rights to use, copy, modify, merge,
+// publish, distribute, sublicense, and/or sell copies of
+// the Software, and to permit persons to whom the Software
+// is furnished to do so, subject to the following
+// conditions:
+//
+// The above copyright notice and this permission notice
+// shall be included in all copies or substantial portions
+// of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF
+// ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED
+// TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A
+// PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT
+// SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY
+// CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION
+// OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR
+// IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+// DEALINGS IN THE SOFTWARE.
+#![allow(dead_code)]
+
+use proc_macro2::*;
+use quote::{ToTokens, TokenStreamExt};
+use syn::parse::Error;
+
+macro_rules! err_span {
+    ($span:expr, $($msg:tt)*) => (
+        $crate::Diagnostic::spanned_error(&$span, format!($($msg)*))
+    )
+}
+
+macro_rules! bail_span {
+    ($($t:tt)*) => (
+        return Err(err_span!($($t)*).into())
+    )
+}
+
+macro_rules! push_err_span {
+    ($diagnostics:expr, $($t:tt)*) => {
+        $diagnostics.push(err_span!($($t)*))
+    };
+}
+
+#[derive(Debug)]
+pub struct Diagnostic {
+    inner: Repr,
+}
+
+#[derive(Debug)]
+enum Repr {
+    Single {
+        text: String,
+        span: Option<(Span, Span)>,
+    },
+    SynError(Error),
+    Multi {
+        diagnostics: Vec<Diagnostic>,
+    },
+}
+
+impl Diagnostic {
+    pub fn error<T: Into<String>>(text: T) -> Diagnostic {
+        Diagnostic {
+            inner: Repr::Single {
+                text: text.into(),
+                span: None,
+            },
+        }
+    }
+
+    pub fn span_error<T: Into<String>>(span: Span, text: T) -> Diagnostic {
+        Diagnostic {
+            inner: Repr::Single {
+                text: text.into(),
+                span: Some((span, span)),
+            },
+        }
+    }
+
+    pub fn spanned_error<T: Into<String>>(node: &ToTokens, text: T) -> Diagnostic {
+        Diagnostic {
+            inner: Repr::Single {
+                text: text.into(),
+                span: extract_spans(node),
+            },
+        }
+    }
+
+    pub fn from_vec(diagnostics: Vec<Diagnostic>) -> Result<(), Diagnostic> {
+        if diagnostics.len() == 0 {
+            Ok(())
+        } else {
+            Err(Diagnostic {
+                inner: Repr::Multi { diagnostics },
+            })
+        }
+    }
+
+    #[allow(unconditional_recursion)]
+    pub fn panic(&self) -> ! {
+        match &self.inner {
+            Repr::Single { text, .. } => panic!("{}", text),
+            Repr::SynError(error) => panic!("{}", error),
+            Repr::Multi { diagnostics } => diagnostics[0].panic(),
+        }
+    }
+}
+
+impl From<Error> for Diagnostic {
+    fn from(err: Error) -> Diagnostic {
+        Diagnostic {
+            inner: Repr::SynError(err),
+        }
+    }
+}
+
+fn extract_spans(node: &ToTokens) -> Option<(Span, Span)> {
+    let mut t = TokenStream::new();
+    node.to_tokens(&mut t);
+    let mut tokens = t.into_iter();
+    let start = tokens.next().map(|t| t.span());
+    let end = tokens.last().map(|t| t.span());
+    start.map(|start| (start, end.unwrap_or(start)))
+}
+
+impl ToTokens for Diagnostic {
+    fn to_tokens(&self, dst: &mut TokenStream) {
+        match &self.inner {
+            Repr::Single { text, span } => {
+                let cs2 = (Span::call_site(), Span::call_site());
+                let (start, end) = span.unwrap_or(cs2);
+                dst.append(Ident::new("compile_error", start));
+                dst.append(Punct::new('!', Spacing::Alone));
+                let mut message = TokenStream::new();
+                message.append(Literal::string(text));
+                let mut group = Group::new(Delimiter::Brace, message);
+                group.set_span(end);
+                dst.append(group);
+            }
+            Repr::Multi { diagnostics } => {
+                for diagnostic in diagnostics {
+                    diagnostic.to_tokens(dst);
+                }
+            }
+            Repr::SynError(err) => {
+                err.to_compile_error().to_tokens(dst);
+            }
+        }
+    }
+}

derive/src/lib.rs

@@ -1,11 +1,23 @@
 extern crate proc_macro;
 
-use proc_macro::TokenStream;
-use syn::{parse_macro_input, AttributeArgs, DeriveInput, Item};
-
+#[macro_use]
+mod error;
 mod from_args;
 mod pyclass;
 
+use error::Diagnostic;
+use proc_macro::TokenStream;
+use proc_macro2::TokenStream as TokenStream2;
+use quote::ToTokens;
+use syn::{parse_macro_input, AttributeArgs, DeriveInput, Item};
+
+fn result_to_tokens(result: Result<TokenStream2, Diagnostic>) -> TokenStream {
+    match result {
+        Ok(tokens) => tokens.into(),
+        Err(diagnostic) => diagnostic.into_token_stream().into(),
+    }
+}
+
 #[proc_macro_derive(FromArgs, attributes(pyarg))]
 pub fn derive_from_args(input: TokenStream) -> TokenStream {
     let ast: DeriveInput = syn::parse(input).unwrap();
@@ -17,12 +29,12 @@ pub fn derive_from_args(input: TokenStream) -> TokenStream {
 pub fn pyclass(attr: TokenStream, item: TokenStream) -> TokenStream {
     let attr = parse_macro_input!(attr as AttributeArgs);
     let item = parse_macro_input!(item as Item);
-    pyclass::impl_pyclass(attr, item).into()
+    result_to_tokens(pyclass::impl_pyclass(attr, item))
 }
 
 #[proc_macro_attribute]
 pub fn pyimpl(attr: TokenStream, item: TokenStream) -> TokenStream {
     let attr = parse_macro_input!(attr as AttributeArgs);
     let item = parse_macro_input!(item as Item);
-    pyclass::impl_pyimpl(attr, item).into()
+    result_to_tokens(pyclass::impl_pyimpl(attr, item))
 }

derive/src/pyclass.rs

@@ -1,3 +1,4 @@
+use super::Diagnostic;
 use proc_macro2::TokenStream as TokenStream2;
 use quote::quote;
 use std::collections::HashMap;
@@ -15,19 +16,21 @@ enum ClassItem {
     },
 }
 
-fn meta_to_vec(meta: Meta) -> Option<Vec<NestedMeta>> {
+fn meta_to_vec(meta: Meta) -> Result<Vec<NestedMeta>, Meta> {
     match meta {
-        Meta::Word(_) => Some(Vec::new()),
-        Meta::List(list) => Some(list.nested.into_iter().collect()),
-        Meta::NameValue(_) => None,
+        Meta::Word(_) => Ok(Vec::new()),
+        Meta::List(list) => Ok(list.nested.into_iter().collect()),
+        Meta::NameValue(_) => Err(meta),
     }
 }
 
 impl ClassItem {
-    fn extract_from_syn(attrs: &mut Vec<Attribute>, sig: &MethodSig) -> Option<ClassItem> {
+    fn extract_from_syn(
+        attrs: &mut Vec<Attribute>,
+        sig: &MethodSig,
+    ) -> Result<Option<ClassItem>, Diagnostic> {
         let mut item = None;
         let mut attr_idx = None;
-        // TODO: better error handling throughout this
         for (i, meta) in attrs
             .iter()
             .filter_map(|attr| attr.parse_meta().ok())
@@ -36,12 +39,18 @@ impl ClassItem {
             let name = meta.name();
             if name == "pymethod" {
                 if item.is_some() {
-                    panic!("You can only have one #[py*] attribute on an impl item")
+                    bail_span!(
+                        sig.ident,
+                        "You can only have one #[py*] attribute on an impl item"
+                    )
                 }
-                let nesteds = meta_to_vec(meta).expect(
-                    "#[pyproperty = \"...\"] cannot be a name/value, you probably meant \
-                     #[pyproperty(name = \"...\")]",
-                );
+                let nesteds = meta_to_vec(meta).map_err(|meta| {
+                    err_span!(
+                        meta,
+                        "#[pyproperty = \"...\"] cannot be a name/value, you probably meant \
+                         #[pyproperty(name = \"...\")]",
+                    )
+                })?;
                 let mut py_name = None;
                 for meta in nesteds {
                     let meta = match meta {
@@ -54,7 +63,10 @@ impl ClassItem {
                                 if let Lit::Str(s) = &name_value.lit {
                                     py_name = Some(s.value());
                                 } else {
-                                    panic!("#[pymethod(name = ...)] must be a string");
+                                    bail_span!(
+                                        &sig.ident,
+                                        "#[pymethod(name = ...)] must be a string"
+                                    );
                                 }
                             }
                         }
@@ -68,12 +80,18 @@ impl ClassItem {
                 attr_idx = Some(i);
             } else if name == "pyproperty" {
                 if item.is_some() {
-                    panic!("You can only have one #[py*] attribute on an impl item")
+                    bail_span!(
+                        sig.ident,
+                        "You can only have one #[py*] attribute on an impl item"
+                    )
                 }
-                let nesteds = meta_to_vec(meta).expect(
-                    "#[pyproperty = \"...\"] cannot be a name/value, you probably meant \
-                     #[pyproperty(name = \"...\")]",
-                );
+                let nesteds = meta_to_vec(meta).map_err(|meta| {
+                    err_span!(
+                        meta,
+                        "#[pyproperty = \"...\"] cannot be a name/value, you probably meant \
+                         #[pyproperty(name = \"...\")]"
+                    )
+                })?;
                 let mut setter = false;
                 let mut py_name = None;
                 for meta in nesteds {
@@ -87,7 +105,10 @@ impl ClassItem {
                                 if let Lit::Str(s) = &name_value.lit {
                                     py_name = Some(s.value());
                                 } else {
-                                    panic!("#[pyproperty(name = ...)] must be a string");
+                                    bail_span!(
+                                        &sig.ident,
+                                        "#[pyproperty(name = ...)] must be a string"
+                                    );
                                 }
                             }
                         }
@@ -99,29 +120,34 @@ impl ClassItem {
                         _ => {}
                     }
                 }
-                let py_name = py_name.unwrap_or_else(|| {
-                    let item_ident = sig.ident.to_string();
-                    if setter {
-                        if item_ident.starts_with("set_") {
-                            let name = &item_ident["set_".len()..];
-                            if name.is_empty() {
-                                panic!(
-                                    "A #[pyproperty(setter)] fn with a set_* name have something \
-                                     after \"set_\""
-                                )
+                let py_name = match py_name {
+                    Some(py_name) => py_name,
+                    None => {
+                        let item_ident = sig.ident.to_string();
+                        if setter {
+                            if item_ident.starts_with("set_") {
+                                let name = &item_ident["set_".len()..];
+                                if name.is_empty() {
+                                    bail_span!(
+                                        &sig.ident,
+                                        "A #[pyproperty(setter)] fn with a set_* name must \
+                                         have something after \"set_\""
+                                    )
+                                } else {
+                                    name.to_string()
+                                }
                             } else {
-                                name.to_string()
+                                bail_span!(
+                                    &sig.ident,
+                                    "A #[pyproperty(setter)] fn must either have a `name` \
+                                     parameter or a fn name along the lines of \"set_*\""
+                                )
                             }
                         } else {
-                            panic!(
-                            "A #[pyproperty(setter)] fn must either have a `name` parameter or a \
-                             fn name along the lines of \"set_*\""
-                        )
+                            item_ident
                         }
-                    } else {
-                        item_ident
                     }
-                });
+                };
                 item = Some(ClassItem::Property {
                     py_name,
                     item_ident: sig.ident.clone(),
@@ -133,23 +159,27 @@ impl ClassItem {
         if let Some(attr_idx) = attr_idx {
             attrs.remove(attr_idx);
         }
-        item
+        Ok(item)
     }
 }
 
-pub fn impl_pyimpl(_attr: AttributeArgs, item: Item) -> TokenStream2 {
+pub fn impl_pyimpl(_attr: AttributeArgs, item: Item) -> Result<TokenStream2, Diagnostic> {
     let mut imp = if let Item::Impl(imp) = item {
         imp
     } else {
-        return quote!(#item);
+        return Ok(quote!(#item));
     };
 
+    let mut diagnostics: Vec<Diagnostic> = Vec::new();
+
     let items = imp
         .items
         .iter_mut()
         .filter_map(|item| {
             if let ImplItem::Method(meth) = item {
                 ClassItem::extract_from_syn(&mut meth.attrs, &meth.sig)
+                    .map_err(|err| diagnostics.push(err))
+                    .unwrap_or_default()
             } else {
                 None
             }
@@ -160,14 +190,18 @@ pub fn impl_pyimpl(_attr: AttributeArgs, item: Item) -> TokenStream2 {
     for item in items.iter() {
         match item {
             ClassItem::Property {
-                item_ident,
-                py_name,
+                ref item_ident,
+                ref py_name,
                 setter,
             } => {
                 let entry = properties.entry(py_name).or_default();
                 let func = if *setter { &mut entry.1 } else { &mut entry.0 };
                 if func.is_some() {
-                    panic!("Multiple property accessors with name {:?}", py_name)
+                    bail_span!(
+                        item_ident,
+                        "Multiple property accessors with name {:?}",
+                        py_name
+                    )
                 }
                 *func = Some(item_ident);
             }
@@ -187,24 +221,37 @@ pub fn impl_pyimpl(_attr: AttributeArgs, item: Item) -> TokenStream2 {
             None
         }
     });
-    let properties = properties.iter().map(|(name, prop)| {
-        let getter = match prop.0 {
-            Some(getter) => getter,
-            None => panic!("Property {:?} is missing a getter", name),
-        };
-        let add_setter = prop.1.map(|setter| quote!(.add_setter(Self::#setter)));
-        quote! {
-            class.set_str_attr(
-                #name,
-                ::rustpython_vm::obj::objproperty::PropertyBuilder::new(ctx)
-                    .add_getter(Self::#getter)
-                    #add_setter
-                    .create(),
-            );
-        }
-    });
+    let properties = properties
+        .iter()
+        .map(|(name, prop)| {
+            let getter = match prop.0 {
+                Some(getter) => getter,
+                None => {
+                    push_err_span!(
+                        diagnostics,
+                        prop.1.unwrap(),
+                        "Property {:?} is missing a getter",
+                        name
+                    );
+                    return TokenStream2::new();
+                }
+            };
+            let add_setter = prop.1.map(|setter| quote!(.add_setter(Self::#setter)));
+            quote! {
+                class.set_str_attr(
+                    #name,
+                    ::rustpython_vm::obj::objproperty::PropertyBuilder::new(ctx)
+                        .add_getter(Self::#getter)
+                        #add_setter
+                        .create(),
+                );
+            }
+        })
+        .collect::<Vec<_>>();
 
-    quote! {
+    Diagnostic::from_vec(diagnostics)?;
+
+    let ret = quote! {
         #imp
         impl ::rustpython_vm::pyobject::PyClassImpl for #ty {
             fn impl_extend_class(
@@ -215,14 +262,18 @@ pub fn impl_pyimpl(_attr: AttributeArgs, item: Item) -> TokenStream2 {
                 #(#properties)*
             }
         }
-    }
+    };
+    Ok(ret)
 }
 
-pub fn impl_pyclass(attr: AttributeArgs, item: Item) -> TokenStream2 {
+pub fn impl_pyclass(attr: AttributeArgs, item: Item) -> Result<TokenStream2, Diagnostic> {
     let (item, ident, attrs) = match item {
         Item::Struct(struc) => (quote!(#struc), struc.ident, struc.attrs),
         Item::Enum(enu) => (quote!(#enu), enu.ident, enu.attrs),
-        _ => panic!("#[pyclass] can only be on a struct or enum declaration"),
+        other => bail_span!(
+            other,
+            "#[pyclass] can only be on a struct or enum declaration"
+        ),
     };
 
     let mut class_name = None;
@@ -233,7 +284,7 @@ pub fn impl_pyclass(attr: AttributeArgs, item: Item) -> TokenStream2 {
                     if let Lit::Str(s) = name_value.lit {
                         class_name = Some(s.value());
                     } else {
-                        panic!("#[pyclass(name = ...)] must be a string");
+                        bail_span!(name_value.lit, "#[pyclass(name = ...)] must be a string");
                     }
                 }
             }
@@ -252,8 +303,6 @@ pub fn impl_pyclass(attr: AttributeArgs, item: Item) -> TokenStream2 {
                         Some(ref mut doc) => doc.push(val),
                         None => doc = Some(vec![val]),
                     }
-                } else {
-                    panic!("expected #[doc = ...] to be a string")
                 }
             }
         }
@@ -266,11 +315,12 @@ pub fn impl_pyclass(attr: AttributeArgs, item: Item) -> TokenStream2 {
         None => quote!(None),
     };
 
-    quote! {
+    let ret = quote! {
         #item
         impl ::rustpython_vm::pyobject::PyClassDef for #ident {
             const NAME: &'static str = #class_name;
             const DOC: Option<&'static str> = #doc;
         }
-    }
+    };
+    Ok(ret)
 }