From 3b17ab0529d155ea05824c284e3d3a62159e6c27 Mon Sep 17 00:00:00 2001
From: Yuji Kanagawa <yuji.kngw.80s.revive@gmail.com>
Date: Sun, 22 Mar 2020 18:48:22 +0900
Subject: [PATCH] Apply suggestions from code review

Co-Authored-By: Georg Brandl <georg@python.org>
---
 pyo3-derive-backend/src/pyclass.rs   | 32 +++++++++++-----------------
 tests/ui/invalid_pyclass_args.rs     |  3 +++
 tests/ui/invalid_pyclass_args.stderr | 14 ++++++++----
 3 files changed, 25 insertions(+), 24 deletions(-)

diff --git a/pyo3-derive-backend/src/pyclass.rs b/pyo3-derive-backend/src/pyclass.rs
index 3dfe1294fae..23697a76009 100644
--- a/pyo3-derive-backend/src/pyclass.rs
+++ b/pyo3-derive-backend/src/pyclass.rs
@@ -74,12 +74,12 @@ impl PyClassArgs {
 
         macro_rules! expected {
             ($expected: literal) => {
+                expected!($expected, right)
+            };
+            ($expected: literal, $span: ident) => {
                 return Err(syn::Error::new_spanned(
-                    right,
-                    format!(
-                        concat!("Expected ", $expected, ", but got {}"),
-                        quote! { #right }
-                    ),
+                    $span,
+                    concat!("Expected ", $expected),
                 ));
             };
         }
@@ -93,7 +93,7 @@ impl PyClassArgs {
                 syn::Expr::Path(exp) if exp.path.segments.len() == 1 => {
                     self.name = Some(exp.clone().into());
                 }
-                _ => expected!("single type path(e.g., Name)"),
+                _ => expected!("type name (e.g., Name)"),
             },
             "extends" => match &**right {
                 syn::Expr::Path(exp) => {
@@ -103,7 +103,7 @@ impl PyClassArgs {
                     };
                     self.has_extends = true;
                 }
-                _ => expected!("type path(e.g., my_mod::MyClass)"),
+                _ => expected!("type path (e.g., my_mod::BaseClass)"),
             },
             "module" => match &**right {
                 syn::Expr::Lit(syn::ExprLit {
@@ -112,17 +112,9 @@ impl PyClassArgs {
                 }) => {
                     self.module = Some(lit.clone());
                 }
-                _ => expected!(r#"string literal(e.g., "mymod")"#),
+                _ => expected!(r#"string literal (e.g., "my_mod")"#),
             },
-            x => {
-                return Err(syn::Error::new_spanned(
-                    left,
-                    format!(
-                        "Expected one of freelist/name/extends/module, but got {}",
-                        x
-                    ),
-                ));
-            }
+            _ => expected!("one of freelist/name/extends/module", left),
         };
 
         Ok(())
@@ -144,10 +136,10 @@ impl PyClassArgs {
             "dict" => {
                 parse_quote! {pyo3::type_flags::DICT}
             }
-            x => {
+            _ => {
                 return Err(syn::Error::new_spanned(
-                    exp.path.clone(),
-                    format!("Expected one of gc/weakref/subclass/dict, but got {}", x),
+                    &exp.path,
+                    "Expected one of gc/weakref/subclass/dict",
                 ))
             }
         };
diff --git a/tests/ui/invalid_pyclass_args.rs b/tests/ui/invalid_pyclass_args.rs
index 1b998b1d05a..57aa3a09a91 100644
--- a/tests/ui/invalid_pyclass_args.rs
+++ b/tests/ui/invalid_pyclass_args.rs
@@ -12,4 +12,7 @@ struct InvalidName {}
 #[pyclass(module = my_module)]
 struct InvalidModule {}
 
+#[pyclass(weakrev)]
+struct InvalidArg {}
+
 fn main() {}
diff --git a/tests/ui/invalid_pyclass_args.stderr b/tests/ui/invalid_pyclass_args.stderr
index 37132f1d46c..72373cd6d3e 100644
--- a/tests/ui/invalid_pyclass_args.stderr
+++ b/tests/ui/invalid_pyclass_args.stderr
@@ -1,23 +1,29 @@
-error: Expected one of freelist/name/extends/module, but got extend
+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::MyClass), but got "PyDict"
+error: Expected type path (e.g., my_mod::BaseClass)
  --> $DIR/invalid_pyclass_args.rs:6:21
   |
 6 | #[pyclass(extends = "PyDict")]
   |                     ^^^^^^^^
 
-error: Expected single type path(e.g., Name), but got m :: MyClass
+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., "mymod"), but got my_module
+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)]
+   |           ^^^^^^^