Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

frompyobject: improve error messages of derived impls #2422

Merged
merged 1 commit into from
Jun 2, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 19 additions & 29 deletions pyo3-macros-backend/src/frompyobject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,7 @@ impl<'a> Enum<'a> {
.map(|variant| {
let attrs = ContainerOptions::from_attrs(&variant.attrs)?;
let var_ident = &variant.ident;
Container::new(
&variant.fields,
parse_quote!(#ident::#var_ident),
attrs,
true,
)
Container::new(&variant.fields, parse_quote!(#ident::#var_ident), attrs)
})
.collect::<Result<Vec<_>>>()?;

Expand Down Expand Up @@ -123,19 +118,13 @@ struct Container<'a> {
path: syn::Path,
ty: ContainerType<'a>,
err_name: String,
is_enum_variant: bool,
}

impl<'a> Container<'a> {
/// Construct a container based on fields, identifier and attributes.
///
/// Fails if the variant has no fields or incompatible attributes.
fn new(
fields: &'a Fields,
path: syn::Path,
options: ContainerOptions,
is_enum_variant: bool,
) -> Result<Self> {
fn new(fields: &'a Fields, path: syn::Path, options: ContainerOptions) -> Result<Self> {
ensure_spanned!(
!fields.is_empty(),
fields.span() => "cannot derive FromPyObject for empty structs and variants"
Expand Down Expand Up @@ -195,11 +184,21 @@ impl<'a> Container<'a> {
path,
ty: style,
err_name,
is_enum_variant,
};
Ok(v)
}

fn name(&self) -> String {
let mut value = String::new();
for segment in &self.path.segments {
if !value.is_empty() {
value.push_str("::");
}
value.push_str(&segment.ident.to_string());
}
value
}

/// Build derivation body for a struct.
fn build(&self) -> TokenStream {
match &self.ty {
Expand All @@ -212,37 +211,28 @@ impl<'a> Container<'a> {

fn build_newtype_struct(&self, field_ident: Option<&Ident>) -> TokenStream {
let self_ty = &self.path;
let struct_name = self.name();
if let Some(ident) = field_ident {
let struct_name = quote!(#self_ty).to_string();
let field_name = ident.to_string();
quote!(
::std::result::Result::Ok(#self_ty{
#ident: _pyo3::impl_::frompyobject::extract_struct_field(obj, #struct_name, #field_name)?
})
)
} else if !self.is_enum_variant {
let error_msg = format!("failed to extract inner field of {}", quote!(#self_ty));
} else {
quote!(
::std::result::Result::Ok(#self_ty(obj.extract().map_err(|err| {
let py = _pyo3::PyNativeType::py(obj);
let err_msg = ::std::format!("{}: {}",
#error_msg,
err.value(py).str().unwrap());
_pyo3::exceptions::PyTypeError::new_err(err_msg)
})?))
_pyo3::impl_::frompyobject::extract_tuple_struct_field(obj, #struct_name, 0).map(#self_ty)
)
} else {
quote!(obj.extract().map(#self_ty))
}
}

fn build_tuple_struct(&self, tups: &[FieldPyO3Attributes]) -> TokenStream {
let self_ty = &self.path;
let struct_name = &self.name();
let field_idents: Vec<_> = (0..tups.len())
.into_iter()
.map(|i| format_ident!("arg{}", i))
.collect();
let struct_name = &quote!(#self_ty).to_string();
let fields = tups.iter().zip(&field_idents).enumerate().map(|(index, (attrs, ident))| {
match &attrs.from_py_with {
None => quote!(
Expand All @@ -265,9 +255,9 @@ impl<'a> Container<'a> {

fn build_struct(&self, tups: &[(&Ident, FieldPyO3Attributes)]) -> TokenStream {
let self_ty = &self.path;
let struct_name = &self.name();
let mut fields: Punctuated<TokenStream, syn::Token![,]> = Punctuated::new();
for (ident, attrs) in tups {
let struct_name = quote!(#self_ty).to_string();
let field_name = ident.to_string();
let getter = match &attrs.getter {
FieldGetter::GetAttr(Some(name)) => {
Expand Down Expand Up @@ -523,7 +513,7 @@ pub fn build_derive_from_pyobject(tokens: &DeriveInput) -> Result<TokenStream> {
bail_spanned!(lit_str.span() => "`annotation` is unsupported for structs");
}
let ident = &tokens.ident;
let st = Container::new(&st.fields, parse_quote!(#ident), options, false)?;
let st = Container::new(&st.fields, parse_quote!(#ident), options)?;
st.build()
}
syn::Data::Union(_) => bail_spanned!(
Expand Down
14 changes: 13 additions & 1 deletion src/impl_/frompyobject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ pub fn failed_to_extract_enum(
error_names: &[&str],
errors: &[PyErr],
) -> PyErr {
// TODO maybe use ExceptionGroup on Python 3.11+ ?
let mut err_msg = format!(
"failed to extract enum {} ('{}')",
type_name,
Expand All @@ -19,12 +20,23 @@ pub fn failed_to_extract_enum(
"- variant {variant_name} ({error_name}): {error_msg}",
variant_name = variant_name,
error_name = error_name,
error_msg = error.value(py).str().unwrap().to_str().unwrap(),
error_msg = extract_traceback(py, error.clone_ref(py)),
));
}
PyTypeError::new_err(err_msg)
}

/// Flattens a chain of errors into a single string.
fn extract_traceback(py: Python<'_>, mut error: PyErr) -> String {
let mut error_msg = error.to_string();
while let Some(cause) = error.cause(py) {
error_msg.push_str(", caused by ");
error_msg.push_str(&cause.to_string());
error = cause
}
error_msg
}

pub fn extract_struct_field<'py, T>(
obj: &'py PyAny,
struct_name: &str,
Expand Down
38 changes: 19 additions & 19 deletions tests/test_frompyobject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ fn test_transparent_tuple_error_message() {
assert!(tup.is_err());
assert_eq!(
extract_traceback(py, tup.unwrap_err()),
"TypeError: failed to extract inner field of TransparentTuple: 'int' object \
"TypeError: failed to extract field TransparentTuple.0: TypeError: 'int' object \
cannot be converted to 'PyString'",
);
});
Expand Down Expand Up @@ -391,13 +391,13 @@ fn test_enum_error() {
err.to_string(),
"\
TypeError: failed to extract enum Foo ('TupleVar | StructVar | TransparentTuple | TransparentStructVar | StructVarGetAttrArg | StructWithGetItem | StructWithGetItemArg')
- variant TupleVar (TupleVar): 'dict' object cannot be converted to 'PyTuple'
- variant StructVar (StructVar): 'dict' object has no attribute 'test'
- variant TransparentTuple (TransparentTuple): 'dict' object cannot be interpreted as an integer
- variant TransparentStructVar (TransparentStructVar): failed to extract field Foo :: TransparentStructVar.a
- variant StructVarGetAttrArg (StructVarGetAttrArg): 'dict' object has no attribute 'bla'
- variant StructWithGetItem (StructWithGetItem): 'a'
- variant StructWithGetItemArg (StructWithGetItemArg): 'foo'"
- variant TupleVar (TupleVar): TypeError: 'dict' object cannot be converted to 'PyTuple'
- variant StructVar (StructVar): AttributeError: 'dict' object has no attribute 'test'
- variant TransparentTuple (TransparentTuple): TypeError: failed to extract field Foo::TransparentTuple.0, caused by TypeError: 'dict' object cannot be interpreted as an integer
- variant TransparentStructVar (TransparentStructVar): TypeError: failed to extract field Foo::TransparentStructVar.a, caused by TypeError: 'dict' object cannot be converted to 'PyString'
- variant StructVarGetAttrArg (StructVarGetAttrArg): AttributeError: 'dict' object has no attribute 'bla'
- variant StructWithGetItem (StructWithGetItem): KeyError: 'a'
- variant StructWithGetItemArg (StructWithGetItemArg): KeyError: 'foo'"
);

let tup = PyTuple::empty(py);
Expand All @@ -406,13 +406,13 @@ TypeError: failed to extract enum Foo ('TupleVar | StructVar | TransparentTuple
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"
- variant TupleVar (TupleVar): ValueError: expected tuple of length 2, but got tuple of length 0
- variant StructVar (StructVar): AttributeError: 'tuple' object has no attribute 'test'
- variant TransparentTuple (TransparentTuple): TypeError: failed to extract field Foo::TransparentTuple.0, caused by TypeError: 'tuple' object cannot be interpreted as an integer
- variant TransparentStructVar (TransparentStructVar): TypeError: failed to extract field Foo::TransparentStructVar.a, caused by TypeError: 'tuple' object cannot be converted to 'PyString'
- variant StructVarGetAttrArg (StructVarGetAttrArg): AttributeError: 'tuple' object has no attribute 'bla'
- variant StructWithGetItem (StructWithGetItem): TypeError: tuple indices must be integers or slices, not str
- variant StructWithGetItemArg (StructWithGetItemArg): TypeError: tuple indices must be integers or slices, not str"
);
});
}
Expand Down Expand Up @@ -463,10 +463,10 @@ fn test_err_rename() {
assert_eq!(
f.unwrap_err().to_string(),
"\
TypeError: failed to extract enum Bar (\'str | uint | int\')
- variant A (str): \'dict\' object cannot be converted to \'PyString\'
- variant B (uint): \'dict\' object cannot be interpreted as an integer
- variant C (int): \'dict\' object cannot be interpreted as an integer"
TypeError: failed to extract enum Bar ('str | uint | int')
- variant A (str): TypeError: failed to extract field Bar::A.0, caused by TypeError: 'dict' object cannot be converted to 'PyString'
- variant B (uint): TypeError: failed to extract field Bar::B.0, caused by TypeError: 'dict' object cannot be interpreted as an integer
- variant C (int): TypeError: failed to extract field Bar::C.0, caused by TypeError: 'dict' object cannot be interpreted as an integer"
);
});
}
Expand Down