fix: adding tests for pyclass hygiene cases (#4359)

* fix: updated pyclass heighten check to validate for eq and ord, fixing Ok issue in eq implementation. identified similar issues in Complex enum and tuple enum, resolved serveral cases, but working on current error coming from traits not being in scope * fix: fully qualified clone and from calls for enums. * update: added changelog entry --------- Co-authored-by: MG <[email protected]>
PyO3 · Jul 17, 2024 · c1f524f · c1f524f
1 parent f635afc
commit c1f524f
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 12 deletions.
diff --git a/newsfragments/4359.fixed.md b/newsfragments/4359.fixed.md
@@ -0,0 +1 @@
+Fixed `pyclass` macro hygiene issues for structs and enums and added tests.
diff --git a/pyo3-macros-backend/src/pyclass.rs b/pyo3-macros-backend/src/pyclass.rs
@@ -923,7 +923,7 @@ fn impl_complex_enum(
                 let variant_cls = gen_complex_enum_variant_class_ident(cls, variant.get_ident());
                 quote! {
                     #cls::#variant_ident { .. } => {
-                        let pyclass_init = #pyo3_path::PyClassInitializer::from(self).add_subclass(#variant_cls);
+                        let pyclass_init = <#pyo3_path::PyClassInitializer<Self> as ::std::convert::From<Self>>::from(self).add_subclass(#variant_cls);
                         let variant_value = #pyo3_path::Py::new(py, pyclass_init).unwrap();
                         #pyo3_path::IntoPy::into_py(variant_value, py)
                     }
@@ -1091,8 +1091,8 @@ fn impl_complex_enum_struct_variant_cls(
         let field_getter_impl = quote! {
             fn #field_name(slf: #pyo3_path::PyRef<Self>) -> #pyo3_path::PyResult<#field_type> {
                 match &*slf.into_super() {
-                    #enum_name::#variant_ident { #field_name, .. } => Ok(#field_name.clone()),
-                    _ => unreachable!("Wrong complex enum variant found in variant wrapper PyClass"),
+                    #enum_name::#variant_ident { #field_name, .. } => ::std::result::Result::Ok(::std::clone::Clone::clone(&#field_name)),
+                    _ => ::core::unreachable!("Wrong complex enum variant found in variant wrapper PyClass"),
                 }
             }
         };
@@ -1114,7 +1114,7 @@ fn impl_complex_enum_struct_variant_cls(
         impl #variant_cls {
             fn __pymethod_constructor__(py: #pyo3_path::Python<'_>, #(#fields_with_types,)*) -> #pyo3_path::PyClassInitializer<#variant_cls> {
                 let base_value = #enum_name::#variant_ident { #(#field_names,)* };
-                #pyo3_path::PyClassInitializer::from(base_value).add_subclass(#variant_cls)
+                <#pyo3_path::PyClassInitializer<#enum_name> as ::std::convert::From<#enum_name>>::from(base_value).add_subclass(#variant_cls)
             }
 
             #match_args_const_impl
@@ -1157,12 +1157,11 @@ fn impl_complex_enum_tuple_variant_field_getters(
                 }
             })
             .collect();
-
         let field_getter_impl: syn::ImplItemFn = parse_quote! {
             fn #field_name(slf: #pyo3_path::PyRef<Self>) -> #pyo3_path::PyResult<#field_type> {
                 match &*slf.into_super() {
-                    #enum_name::#variant_ident ( #(#field_access_tokens), *) => Ok(val.clone()),
-                    _ => unreachable!("Wrong complex enum variant found in variant wrapper PyClass"),
+                    #enum_name::#variant_ident ( #(#field_access_tokens), *) => ::std::result::Result::Ok(::std::clone::Clone::clone(&val)),
+                    _ => ::core::unreachable!("Wrong complex enum variant found in variant wrapper PyClass"),
                 }
             }
         };
@@ -1186,7 +1185,7 @@ fn impl_complex_enum_tuple_variant_len(
 
     let mut len_method_impl: syn::ImplItemFn = parse_quote! {
         fn __len__(slf: #pyo3_path::PyRef<Self>) -> #pyo3_path::PyResult<usize> {
-            Ok(#num_fields)
+            ::std::result::Result::Ok(#num_fields)
         }
     };
 
@@ -1208,7 +1207,7 @@ fn impl_complex_enum_tuple_variant_getitem(
         .map(|i| {
             let field_access = format_ident!("_{}", i);
             quote! {
-            #i => Ok(
+            #i => ::std::result::Result::Ok(
                 #pyo3_path::IntoPy::into_py(
                     #variant_cls::#field_access(slf)?
                     , py)
@@ -1223,7 +1222,7 @@ fn impl_complex_enum_tuple_variant_getitem(
             let py = slf.py();
             match idx {
                 #( #match_arms, )*
-                _ => Err(pyo3::exceptions::PyIndexError::new_err("tuple index out of range")),
+                _ => ::std::result::Result::Err(#pyo3_path::exceptions::PyIndexError::new_err("tuple index out of range")),
             }
         }
     };
@@ -1287,7 +1286,7 @@ fn impl_complex_enum_tuple_variant_cls(
         impl #variant_cls {
             fn __pymethod_constructor__(py: #pyo3_path::Python<'_>, #(#field_names : #field_types,)*) -> #pyo3_path::PyClassInitializer<#variant_cls> {
                 let base_value = #enum_name::#variant_ident ( #(#field_names,)* );
-                #pyo3_path::PyClassInitializer::from(base_value).add_subclass(#variant_cls)
+                <#pyo3_path::PyClassInitializer<#enum_name> as ::std::convert::From<#enum_name>>::from(base_value).add_subclass(#variant_cls)
             }
 
             #len_method_impl
@@ -1828,7 +1827,7 @@ fn pyclass_richcmp(
                 op: #pyo3_path::pyclass::CompareOp
             ) -> #pyo3_path::PyResult<#pyo3_path::PyObject> {
                 let self_val = self;
-                if let Ok(other) = #pyo3_path::types::PyAnyMethods::downcast::<Self>(other) {
+                if let ::std::result::Result::Ok(other) = #pyo3_path::types::PyAnyMethods::downcast::<Self>(other) {
                     let other = &*other.borrow();
                     match op {
                         #arms

diff --git a/src/tests/hygiene/pyclass.rs b/src/tests/hygiene/pyclass.rs
@@ -66,3 +66,28 @@ pub struct Foo4 {
     #[cfg(not(any()))]
     field: u32,
 }
+
+#[crate::pyclass(eq, ord)]
+#[pyo3(crate = "crate")]
+#[derive(PartialEq, PartialOrd)]
+pub struct PointEqOrd {
+    x: u32,
+    y: u32,
+    z: u32,
+}
+
+#[crate::pyclass(eq, ord)]
+#[pyo3(crate = "crate")]
+#[derive(PartialEq, PartialOrd)]
+pub enum ComplexEnumEqOrd {
+    Variant1 { a: u32, b: u32 },
+    Variant2 { c: u32 },
+}
+
+#[crate::pyclass(eq, ord)]
+#[pyo3(crate = "crate")]
+#[derive(PartialEq, PartialOrd)]
+pub enum TupleEnumEqOrd {
+    Variant1(u32, u32),
+    Variant2(u32),
+}