Merge pull request #826 from kngwyu/pyclass-err-improve

More ergonomic error messages for invalid #[pyclass] args

pyo3-derive-backend/src/pyclass.rs

@@ -56,72 +56,65 @@ impl PyClassArgs {
         match expr {
             syn::Expr::Path(ref exp) if exp.path.segments.len() == 1 => self.add_path(exp),
             syn::Expr::Assign(ref assign) => self.add_assign(assign),
-            _ => Err(syn::Error::new_spanned(expr, "Could not parse arguments")),
+            _ => Err(syn::Error::new_spanned(expr, "Failed to parse arguments")),
         }
     }
 
     /// Match a single flag
     fn add_assign(&mut self, assign: &syn::ExprAssign) -> syn::Result<()> {
-        let key = match *assign.left {
+        let syn::ExprAssign { left, right, .. } = assign;
-            syn::Expr::Path(ref exp) if exp.path.segments.len() == 1 => {
+        let key = match &**left {
+            syn::Expr::Path(exp) if exp.path.segments.len() == 1 => {
                 exp.path.segments.first().unwrap().ident.to_string()
             }
             _ => {
-                return Err(syn::Error::new_spanned(assign, "could not parse argument"));
+                return Err(syn::Error::new_spanned(assign, "Failed to parse arguments"));
             }
         };
 
+        macro_rules! expected {
+            ($expected: literal) => {
+                expected!($expected, right)
+            };
+            ($expected: literal, $span: ident) => {
+                return Err(syn::Error::new_spanned(
+                    $span,
+                    concat!("Expected ", $expected),
+                ));
+            };
+        }
+
         match key.as_str() {
             "freelist" => {
                 // We allow arbitrary expressions here so you can e.g. use `8*64`
-                self.freelist = Some(*assign.right.clone());
+                self.freelist = Some(syn::Expr::clone(right));
             }
-            "name" => match *assign.right {
+            "name" => match &**right {
-                syn::Expr::Path(ref exp) if exp.path.segments.len() == 1 => {
+                syn::Expr::Path(exp) if exp.path.segments.len() == 1 => {
                     self.name = Some(exp.clone().into());
                 }
-                _ => {
+                _ => expected!("type name (e.g., Name)"),
-                    return Err(syn::Error::new_spanned(
-                        *assign.right.clone(),
-                        "Wrong 'name' format",
-                    ));
-                }
             },
-            "extends" => match *assign.right {
+            "extends" => match &**right {
-                syn::Expr::Path(ref exp) => {
+                syn::Expr::Path(exp) => {
                     self.base = syn::TypePath {
                         path: exp.path.clone(),
                         qself: None,
                     };
                     self.has_extends = true;
                 }
-                _ => {
+                _ => expected!("type path (e.g., my_mod::BaseClass)"),
-                    return Err(syn::Error::new_spanned(
-                        *assign.right.clone(),
-                        "Wrong format for extends",
-                    ));
-                }
             },
-            "module" => match *assign.right {
+            "module" => match &**right {
                 syn::Expr::Lit(syn::ExprLit {
-                    lit: syn::Lit::Str(ref lit),
+                    lit: syn::Lit::Str(lit),
                     ..
                 }) => {
                     self.module = Some(lit.clone());
                 }
-                _ => {
+                _ => expected!(r#"string literal (e.g., "my_mod")"#),
-                    return Err(syn::Error::new_spanned(
-                        *assign.right.clone(),
-                        "Wrong format for module",
-                    ));
-                }
             },
-            _ => {
+            _ => expected!("one of freelist/name/extends/module", left),
-                return Err(syn::Error::new_spanned(
-                    *assign.left.clone(),
-                    "Unsupported parameter",
-                ));
-            }
         };
 
         Ok(())
@@ -145,9 +138,9 @@ impl PyClassArgs {
             }
             _ => {
                 return Err(syn::Error::new_spanned(
-                    exp.path.clone(),
+                    &exp.path,
-                    "Unsupported parameter",
+                    "Expected one of gc/weakref/subclass/dict",
-                ));
+                ))
             }
         };
 

tests/test_compile_error.rs

@@ -3,6 +3,7 @@ fn test_compile_errors() {
     let t = trybuild::TestCases::new();
     t.compile_fail("tests/ui/invalid_macro_args.rs");
     t.compile_fail("tests/ui/invalid_property_args.rs");
+    t.compile_fail("tests/ui/invalid_pyclass_args.rs");
     t.compile_fail("tests/ui/invalid_pymethod_names.rs");
     t.compile_fail("tests/ui/missing_clone.rs");
     t.compile_fail("tests/ui/reject_generics.rs");

tests/ui/invalid_pyclass_args.rs

@@ -0,0 +1,18 @@
+use pyo3::prelude::*;
+
+#[pyclass(extend=pyo3::types::PyDict)]
+struct TypoIntheKey {}
+
+#[pyclass(extends = "PyDict")]
+struct InvalidExtends {}
+
+#[pyclass(name = m::MyClass)]
+struct InvalidName {}
+
+#[pyclass(module = my_module)]
+struct InvalidModule {}
+
+#[pyclass(weakrev)]
+struct InvalidArg {}
+
+fn main() {}

tests/ui/invalid_pyclass_args.stderr

@@ -0,0 +1,29 @@
+error: Expected one of freelist/name/extends/module
+ --> $DIR/invalid_pyclass_args.rs:3:11
+  |
+3 | #[pyclass(extend=pyo3::types::PyDict)]
+  |           ^^^^^^
+
+error: Expected type path (e.g., my_mod::BaseClass)
+ --> $DIR/invalid_pyclass_args.rs:6:21
+  |
+6 | #[pyclass(extends = "PyDict")]
+  |                     ^^^^^^^^
+
+error: Expected type name (e.g., Name)
+ --> $DIR/invalid_pyclass_args.rs:9:18
+  |
+9 | #[pyclass(name = m::MyClass)]
+  |                  ^^^^^^^^^^
+
+error: Expected string literal (e.g., "my_mod")
+  --> $DIR/invalid_pyclass_args.rs:12:20
+   |
+12 | #[pyclass(module = my_module)]
+   |                    ^^^^^^^^^
+
+error: Expected one of gc/weakref/subclass/dict
+  --> $DIR/invalid_pyclass_args.rs:15:11
+   |
+15 | #[pyclass(weakrev)]
+   |           ^^^^^^^