emit deprecation on pyclasses that are affected by the FromPyObject blanket impl (#5550)

Author: Icxolu

guide/src/class.md

@@ -893,7 +893,7 @@ Classes can also be passed by value if they can be cloned, i.e. they automatical
 ```rust,no_run
 # #![allow(dead_code)]
 # use pyo3::prelude::*;
-#[pyclass]
+#[pyclass(from_py_object)]
 #[derive(Clone)]
 struct MyClass {
     my_field: Box<i32>,

guide/src/faq.md

@@ -99,7 +99,7 @@ You may have a nested struct similar to this:
 
 ```rust,no_run
 # use pyo3::prelude::*;
-#[pyclass]
+#[pyclass(from_py_object)]
 #[derive(Clone)]
 struct Inner {/* fields omitted */}
 

guide/src/migration.md

@@ -3,6 +3,18 @@
 This guide can help you upgrade code through breaking changes from one PyO3 version to the next.
 For a detailed list of all changes, see the [CHANGELOG](changelog.md).
 
+## from 0.27.* to 0.28
+
+### Deprecation of automatic `FromPyObject` for `#[pyclass]` types which implement `Clone`
+
+`#[pyclass]` types which implement `Clone` used to also implement `FromPyObject` automatically.
+This behaviour is phased out and replaced by an explicit opt-in.
+Affected types will by marked by a deprecation message.
+To migrate use either
+
+- `from_py_object` to keep the automatic derive, or
+- `skip_from_py_object` to accept the new behaviour
+
 ## from 0.26.* to 0.27
 
 ### `FromPyObject` reworked for flexibility and efficiency

newsfragments/5550.changed.md

@@ -0,0 +1 @@
+deprecate implicit by value extraction of pyclasses

pyo3-macros-backend/src/pyclass.rs

@@ -2511,6 +2511,20 @@ impl<'a> PyClassImplsBuilder<'a> {
             }
         };
 
+        let deprecation = if self.attr.options.skip_from_py_object.is_none()
+            && self.attr.options.from_py_object.is_none()
+        {
+            quote! {
+                const _: () = {
+                    #[allow(unused_import)]
+                    use #pyo3_path::impl_::pyclass::Probe as _;
+                    #pyo3_path::impl_::deprecated::HasAutomaticFromPyObject::<{ #pyo3_path::impl_::pyclass::IsClone::<#cls>::VALUE }>::MSG
+                };
+            }
+        } else {
+            TokenStream::new()
+        };
+
         let extract_pyclass_with_clone = if let Some(from_py_object) =
             self.attr.options.from_py_object
         {
@@ -2540,6 +2554,8 @@ impl<'a> PyClassImplsBuilder<'a> {
         };
 
         Ok(quote! {
+            #deprecation
+
             #extract_pyclass_with_clone
 
             #assertions

pytests/src/pyclasses.rs

@@ -4,7 +4,7 @@ use pyo3::exceptions::{PyStopIteration, PyValueError};
 use pyo3::prelude::*;
 use pyo3::types::PyType;
 
-#[pyclass]
+#[pyclass(from_py_object)]
 #[derive(Clone, Default)]
 struct EmptyClass {}
 
@@ -70,7 +70,7 @@ impl PyClassThreadIter {
 }
 
 /// Demonstrates a base class which can operate on the relevant subclass in its constructor.
-#[pyclass(subclass)]
+#[pyclass(subclass, skip_from_py_object)]
 #[derive(Clone, Debug)]
 struct AssertingBaseClass;
 
@@ -104,7 +104,7 @@ impl ClassWithDict {
     }
 }
 
-#[pyclass]
+#[pyclass(skip_from_py_object)]
 #[derive(Clone)]
 struct ClassWithDecorators {
     attr: usize,

src/impl_.rs

@@ -10,6 +10,7 @@ pub mod callback;
 pub mod concat;
 #[cfg(feature = "experimental-async")]
 pub mod coroutine;
+pub mod deprecated;
 pub mod exceptions;
 pub mod extract_argument;
 pub mod freelist;

src/impl_/deprecated.rs

@@ -0,0 +1,13 @@
+pub struct HasAutomaticFromPyObject<const IS_CLONE: bool> {}
+
+impl HasAutomaticFromPyObject<true> {
+    #[deprecated(
+        since = "0.28.0",
+        note = "The automatically derived `FromPyObject` implementation for `#[pyclass]` types which implement `Clone` is being phased out. Use `from_py_object` to keep the automatic derive or `skip_from_py_object` to accept the new behaviour."
+    )]
+    pub const MSG: () = ();
+}
+
+impl HasAutomaticFromPyObject<false> {
+    pub const MSG: () = ();
+}

src/impl_/pyclass/probes.rs

@@ -73,6 +73,12 @@ impl<T: super::doc::PyClassNewTextSignature> HasNewTextSignature<T> {
     pub const VALUE: bool = true;
 }
 
+probe!(IsClone);
+
+impl<T: Clone> IsClone<T> {
+    pub const VALUE: bool = true;
+}
+
 #[cfg(test)]
 macro_rules! value_of {
     ($probe:ident, $ty:ty) => {{

src/marker.rs

@@ -1007,7 +1007,7 @@ mod tests {
     fn test_py_run_inserts_globals_2() {
         use std::ffi::CString;
 
-        #[crate::pyclass(crate = "crate")]
+        #[crate::pyclass(crate = "crate", skip_from_py_object)]
         #[derive(Clone)]
         struct CodeRunner {
             code: CString,