Address comments from PR#692

PyO3 · Dec 18, 2019 · b245e71 · b245e71
1 parent 0032508
commit b245e71
Show file tree

Hide file tree

Showing 3 changed files with 28 additions and 59 deletions.
diff --git a/pyo3-derive-backend/src/method.rs b/pyo3-derive-backend/src/method.rs
@@ -37,8 +37,8 @@ pub struct FnSpec<'a> {
     pub tp: FnType,
     // Rust function name
     pub name: &'a syn::Ident,
-    // Wrapped python name. This should have been sent through syn::IdentExt::unraw()
-    // to ensure that any leading r# is removed.
+    // Wrapped python name. This should not have any leading r#.
+    // r# can be removed by syn::ext::IdentExt::unraw()
     pub python_name: syn::Ident,
     pub attrs: Vec<Argument>,
     pub args: Vec<FnArg<'a>>,
@@ -162,14 +162,8 @@ impl<'a> FnSpec<'a> {
                 "text_signature not allowed on __new__; if you want to add a signature on \
                  __new__, put it on the struct definition instead",
             )?,
-            FnType::FnCall => {
-                parse_erroneous_text_signature("text_signature not allowed on __call__")?
-            }
-            FnType::Getter => {
-                parse_erroneous_text_signature("text_signature not allowed on getter")?
-            }
-            FnType::Setter => {
-                parse_erroneous_text_signature("text_signature not allowed on setter")?
+            FnType::FnCall | FnType::Getter | FnType::Setter => {
+                parse_erroneous_text_signature("text_signature not allowed with this attribute")?
             }
         };
 
@@ -497,28 +491,10 @@ fn parse_method_name_attribute(
     // Reject some invalid combinations
     if let Some(name) = &name {
         match ty {
-            FnType::FnNew => {
-                return Err(syn::Error::new_spanned(
-                    name,
-                    "name can not be specified with #[new]",
-                ))
-            }
-            FnType::FnCall => {
-                return Err(syn::Error::new_spanned(
-                    name,
-                    "name can not be specified with #[call]",
-                ))
-            }
-            FnType::Getter => {
-                return Err(syn::Error::new_spanned(
-                    name,
-                    "name can not be specified for getter",
-                ))
-            }
-            FnType::Setter => {
+            FnType::FnNew | FnType::FnCall | FnType::Getter | FnType::Setter => {
                 return Err(syn::Error::new_spanned(
                     name,
-                    "name can not be specified for setter",
+                    "name not allowed with this attribute",
                 ))
             }
             _ => {}

diff --git a/pyo3-derive-backend/src/pyfunction.rs b/pyo3-derive-backend/src/pyfunction.rs
@@ -209,33 +209,26 @@ pub fn parse_name_attribute(attrs: &mut Vec<syn::Attribute>) -> syn::Result<Opti
         _ => true,
     });
 
-    let mut name = None;
-
-    for (lit, span) in name_attrs {
-        if name.is_some() {
-            return Err(syn::Error::new(
-                span,
-                "#[name] can not be specified multiple times",
-            ));
+    match &*name_attrs {
+        [] => Ok(None),
+        [(syn::Lit::Str(s), span)] => {
+            let mut ident: syn::Ident = s.parse()?;
+            // This span is the whole attribute span, which is nicer for reporting errors.
+            ident.set_span(*span);
+            Ok(Some(ident))
         }
-
-        name = match lit {
-            syn::Lit::Str(s) => {
-                let mut ident: syn::Ident = s.parse()?;
-                // This span is the whole attribute span, which is nicer for reporting errors.
-                ident.set_span(span);
-                Some(ident)
-            }
-            _ => {
-                return Err(syn::Error::new(
-                    span,
-                    "Expected string literal for #[name] argument",
-                ))
-            }
-        };
+        [(_, span)] => Err(syn::Error::new(
+            *span,
+            "Expected string literal for #[name] argument",
+        )),
+        // TODO: The below pattern is unstable, so instead we match the wildcard.
+        // slice_patterns due to be stable soon: https://github.com/rust-lang/rust/issues/62254
+        // [(_, span), _, ..] => {
+        _ => Err(syn::Error::new(
+            name_attrs[0].1,
+            "#[name] can not be specified multiple times",
+        )),
     }
-
-    Ok(name)
 }
 
 pub fn build_py_function(ast: &mut syn::ItemFn, args: PyFunctionAttr) -> syn::Result<TokenStream> {

diff --git a/tests/ui/invalid_pymethod_names.stderr b/tests/ui/invalid_pymethod_names.stderr
@@ -1,16 +1,16 @@
-error: name can not be specified for getter
+error: name not allowed with this attribute
   --> $DIR/invalid_pymethod_names.rs:10:5
    |
 10 |     #[name = "num"]
    |     ^^^^^^^^^^^^^^^
 
 error: #[name] can not be specified multiple times
-  --> $DIR/invalid_pymethod_names.rs:18:5
+  --> $DIR/invalid_pymethod_names.rs:17:5
    |
-18 |     #[name = "bar"]
+17 |     #[name = "foo"]
    |     ^^^^^^^^^^^^^^^
 
-error: name can not be specified with #[new]
+error: name not allowed with this attribute
   --> $DIR/invalid_pymethod_names.rs:24:5
    |
 24 |     #[name = "makenew"]