frompyobject: improve error message for tuple case

pyo3-macros-backend/src/frompyobject.rs

@@ -3,7 +3,7 @@ use crate::{
     utils::get_pyo3_crate,
 };
 use proc_macro2::TokenStream;
-use quote::quote;
+use quote::{quote, format_ident};
 use syn::{
     parenthesized,
     parse::{Parse, ParseStream},
@@ -244,48 +244,33 @@ impl<'a> Container<'a> {
 
     fn build_tuple_struct(&self, tups: &[FieldPyO3Attributes]) -> TokenStream {
         let self_ty = &self.path;
-        let mut fields: Punctuated<TokenStream, syn::Token![,]> = Punctuated::new();
+        let field_idents: Vec<_> = (0..tups.len()).into_iter().map(|i| format_ident!("arg{}", i)).collect();
-        for (index, attrs) in tups.iter().enumerate() {
+        let fields = tups.iter().zip(&field_idents).enumerate().map(|(index, (attrs, ident))| {
             let error_msg = format!("failed to extract field {}.{}", quote!(#self_ty), index);
 
             let parsed_item = match &attrs.from_py_with {
                 None => quote!(
-                    obj.get_item(#index)?.extract()
+                    _pyo3::PyAny::extract(#ident)
                 ),
                 Some(FromPyWithAttribute {
                     value: expr_path, ..
                 }) => quote! (
-                    #expr_path(obj.get_item(#index)?)
+                    #expr_path(#ident)
                 ),
             };
 
-            let extractor = quote!(
+            quote!(
                 #parsed_item.map_err(|inner| {
                     let py = _pyo3::PyNativeType::py(obj);
                     let new_err = _pyo3::exceptions::PyTypeError::new_err(#error_msg);
                     new_err.set_cause(py, ::std::option::Option::Some(inner));
                     new_err
                 })?
-            );
+            )
-
+        });
-            fields.push(quote!(#extractor));
-        }
-        let len = tups.len();
-        let msg = if self.is_enum_variant {
-            quote!(::std::format!(
-                "expected tuple of length {}, but got length {}",
-                #len,
-                s.len()
-            ))
-        } else {
-            quote!("")
-        };
         quote!(
-            let s = <_pyo3::types::PyTuple as _pyo3::conversion::PyTryFrom>::try_from(obj)?;
+            let (#(#field_idents),*) = obj.extract()?;
-            if s.len() != #len {
+            ::std::result::Result::Ok(#self_ty(#(#fields),*))
-                return ::std::result::Result::Err(_pyo3::exceptions::PyValueError::new_err(#msg))
-            }
-            ::std::result::Result::Ok(#self_ty(#fields))
         )
     }
 

src/types/tuple.rs

@@ -291,9 +291,10 @@ impl<'a> IntoIterator for &'a PyTuple {
     }
 }
 
+#[cold]
 fn wrong_tuple_length(t: &PyTuple, expected_length: usize) -> PyErr {
     let msg = format!(
-        "Expected tuple of length {}, but got tuple of length {}.",
+        "expected tuple of length {}, but got tuple of length {}",
         expected_length,
         t.len()
     );

tests/test_frompyobject.rs

@@ -399,6 +399,21 @@ TypeError: failed to extract enum Foo ('TupleVar | StructVar | TransparentTuple
 - variant StructWithGetItem (StructWithGetItem): 'a'
 - variant StructWithGetItemArg (StructWithGetItemArg): 'foo'"
         );
+
+        let tup = PyTuple::empty(py);
+        let err = Foo::extract(tup.as_ref()).unwrap_err();
+        assert_eq!(
+            err.to_string(),
+            "\
+TypeError: failed to extract enum Foo ('TupleVar | StructVar | TransparentTuple | TransparentStructVar | StructVarGetAttrArg | StructWithGetItem | StructWithGetItemArg')
+- variant TupleVar (TupleVar): expected tuple of length 2, but got tuple of length 0
+- variant StructVar (StructVar): 'tuple' object has no attribute 'test'
+- variant TransparentTuple (TransparentTuple): 'tuple' object cannot be interpreted as an integer
+- variant TransparentStructVar (TransparentStructVar): failed to extract field Foo :: TransparentStructVar.a
+- variant StructVarGetAttrArg (StructVarGetAttrArg): 'tuple' object has no attribute 'bla'
+- variant StructWithGetItem (StructWithGetItem): tuple indices must be integers or slices, not str
+- variant StructWithGetItemArg (StructWithGetItemArg): tuple indices must be integers or slices, not str"
+        );
     });
 }
 
@@ -500,6 +515,23 @@ fn test_from_py_with_tuple_struct() {
     });
 }
 
+#[test]
+fn test_from_py_with_tuple_struct_error() {
+    Python::with_gil(|py| {
+        let py_zap = py
+            .eval(r#"("whatever", [1, 2, 3], "third")"#, None, None)
+            .expect("failed to create tuple");
+
+        let f = ZapTuple::extract(py_zap);
+
+        assert!(f.is_err());
+        assert_eq!(
+            f.unwrap_err().to_string(),
+            "ValueError: expected tuple of length 2, but got tuple of length 3"
+        );
+    });
+}
+
 #[derive(Debug, FromPyObject, PartialEq)]
 pub enum ZapEnum {
     Zip(#[pyo3(from_py_with = "PyAny::len")] usize),