Collecting multiple attribute error

newsfragments/4243.changed.md

@@ -0,0 +1 @@
+Report multiple errors from `#[pyclass]` and `#[pyo3(..)]` attributes.

pyo3-macros-backend/src/attributes.rs

@@ -233,13 +233,41 @@ pub fn take_attributes(
 
 pub fn take_pyo3_options<T: Parse>(attrs: &mut Vec<syn::Attribute>) -> Result<Vec<T>> {
     let mut out = Vec::new();
-    take_attributes(attrs, |attr| {
-        if let Some(options) = get_pyo3_options(attr)? {
-            out.extend(options);
+    let mut all_errors = ErrorCombiner(None);
+    take_attributes(attrs, |attr| match get_pyo3_options(attr) {
+        Ok(result) => {
+            if let Some(options) = result {
+                out.extend(options);
+                Ok(true)
+            } else {
+                Ok(false)
+            }
+        }
+        Err(err) => {
+            all_errors.combine(err);
             Ok(true)
-        } else {
-            Ok(false)
         }
     })?;
+    all_errors.ensure_empty()?;
     Ok(out)
 }
+
+pub struct ErrorCombiner(pub Option<syn::Error>);
+
+impl ErrorCombiner {
+    pub fn combine(&mut self, error: syn::Error) {
+        if let Some(existing) = &mut self.0 {
+            existing.combine(error);
+        } else {
+            self.0 = Some(error);
+        }
+    }
+
+    pub fn ensure_empty(self) -> Result<()> {
+        if let Some(error) = self.0 {
+            Err(error)
+        } else {
+            Ok(())
+        }
+    }
+}

pyo3-macros-backend/src/pyclass.rs

@@ -2,8 +2,8 @@ use std::borrow::Cow;
 
 use crate::attributes::kw::frozen;
 use crate::attributes::{
-    self, kw, take_pyo3_options, CrateAttribute, ExtendsAttribute, FreelistAttribute,
-    ModuleAttribute, NameAttribute, NameLitStr, RenameAllAttribute,
+    self, kw, take_pyo3_options, CrateAttribute, ErrorCombiner, ExtendsAttribute,
+    FreelistAttribute, ModuleAttribute, NameAttribute, NameLitStr, RenameAllAttribute,
 };
 use crate::deprecations::Deprecations;
 use crate::konst::{ConstAttributes, ConstSpec};
@@ -230,23 +230,35 @@ pub fn build_py_class(
             For an explanation, see https://pyo3.rs/latest/class.html#no-generic-parameters"
     );
 
+    let mut all_errors = ErrorCombiner(None);
+
     let mut field_options: Vec<(&syn::Field, FieldPyO3Options)> = match &mut class.fields {
-        syn::Fields::Named(fields) => fields
-            .named
-            .iter_mut()
-            .map(|field| {
-                FieldPyO3Options::take_pyo3_options(&mut field.attrs)
-                    .map(move |options| (&*field, options))
-            })
-            .collect::<Result<_>>()?,
         syn::Fields::Unnamed(fields) => fields
             .unnamed
             .iter_mut()
-            .map(|field| {
-                FieldPyO3Options::take_pyo3_options(&mut field.attrs)
-                    .map(move |options| (&*field, options))
-            })
-            .collect::<Result<_>>()?,
+            .filter_map(
+                |field| match FieldPyO3Options::take_pyo3_options(&mut field.attrs) {
+                    Ok(options) => Some((&*field, options)),
+                    Err(e) => {
+                        all_errors.combine(e);
+                        None
+                    }
+                },
+            )
+            .collect::<Vec<_>>(),
+        syn::Fields::Named(fields) => fields
+            .named
+            .iter_mut()
+            .filter_map(
+                |field| match FieldPyO3Options::take_pyo3_options(&mut field.attrs) {
+                    Ok(options) => Some((&*field, options)),
+                    Err(e) => {
+                        all_errors.combine(e);
+                        None
+                    }
+                },
+            )
+            .collect::<Vec<_>>(),
         syn::Fields::Unit => {
             if let Some(attr) = args.options.set_all {
                 return Err(syn::Error::new_spanned(attr, UNIT_SET));
@@ -259,6 +271,8 @@ pub fn build_py_class(
         }
     };
 
+    all_errors.ensure_empty()?;
+
     if let Some(attr) = args.options.get_all {
         for (_, FieldPyO3Options { get, .. }) in &mut field_options {
             if let Some(old_get) = get.replace(Annotated::Struct(attr)) {

tests/ui/invalid_pyclass_args.rs

@@ -76,4 +76,13 @@ struct InvalidOrderedStruct {
     inner: i32
 }
 
+#[pyclass]
+struct MultipleErrors {
+    #[pyo3(foo)]
+    #[pyo3(blah)]
+    x: i32,
+    #[pyo3(pop)]
+    y: i32,
+}
+
 fn main() {}

tests/ui/invalid_pyclass_args.stderr

@@ -82,6 +82,24 @@ error: The `ord` option requires the `eq` option.
 74 | #[pyclass(ord)]
    |           ^^^
 
+error: expected one of: `get`, `set`, `name`
+  --> tests/ui/invalid_pyclass_args.rs:81:12
+   |
+81 |     #[pyo3(foo)]
+   |            ^^^
+
+error: expected one of: `get`, `set`, `name`
+  --> tests/ui/invalid_pyclass_args.rs:82:12
+   |
+82 |     #[pyo3(blah)]
+   |            ^^^^
+
+error: expected one of: `get`, `set`, `name`
+  --> tests/ui/invalid_pyclass_args.rs:84:12
+   |
+84 |     #[pyo3(pop)]
+   |            ^^^
+
 error[E0592]: duplicate definitions with name `__pymethod___richcmp____`
   --> tests/ui/invalid_pyclass_args.rs:36:1
    |