deprecate gil-refs in `from_py_with` (Part 2) (#3972)

* deprecate `from_py_with` in `#[derive(FromPyObject)]` (NewType)

* deprecate `from_py_with` in `#[derive(FromPyObject)]` (Enum, Struct)

pyo3-macros-backend/src/frompyobject.rs

@@ -1,7 +1,7 @@
 use crate::attributes::{self, get_pyo3_options, CrateAttribute, FromPyWithAttribute};
 use crate::utils::Ctx;
 use proc_macro2::TokenStream;
-use quote::{format_ident, quote};
+use quote::{format_ident, quote, quote_spanned};
 use syn::{
     parenthesized,
     parse::{Parse, ParseStream},
@@ -44,13 +44,16 @@ impl<'a> Enum<'a> {
     }
 
     /// Build derivation body for enums.
-    fn build(&self, ctx: &Ctx) -> TokenStream {
+    fn build(&self, ctx: &Ctx) -> (TokenStream, TokenStream) {
         let Ctx { pyo3_path } = ctx;
         let mut var_extracts = Vec::new();
         let mut variant_names = Vec::new();
         let mut error_names = Vec::new();
+
+        let mut deprecations = TokenStream::new();
         for var in &self.variants {
-            let struct_derive = var.build(ctx);
+            let (struct_derive, dep) = var.build(ctx);
+            deprecations.extend(dep);
             let ext = quote!({
                 let maybe_ret = || -> #pyo3_path::PyResult<Self> {
                     #struct_derive
@@ -67,19 +70,22 @@ impl<'a> Enum<'a> {
             error_names.push(&var.err_name);
         }
         let ty_name = self.enum_ident.to_string();
-        quote!(
+        (
-            let errors = [
+            quote!(
-                #(#var_extracts),*
+                let errors = [
-            ];
+                    #(#var_extracts),*
-            ::std::result::Result::Err(
+                ];
-                #pyo3_path::impl_::frompyobject::failed_to_extract_enum(
+                ::std::result::Result::Err(
-                    obj.py(),
+                    #pyo3_path::impl_::frompyobject::failed_to_extract_enum(
-                    #ty_name,
+                        obj.py(),
-                    &[#(#variant_names),*],
+                        #ty_name,
-                    &[#(#error_names),*],
+                        &[#(#variant_names),*],
-                    &errors
+                        &[#(#error_names),*],
+                        &errors
+                    )
                 )
-            )
+            ),
+            deprecations,
         )
     }
 }
@@ -238,7 +244,7 @@ impl<'a> Container<'a> {
     }
 
     /// Build derivation body for a struct.
-    fn build(&self, ctx: &Ctx) -> TokenStream {
+    fn build(&self, ctx: &Ctx) -> (TokenStream, TokenStream) {
         match &self.ty {
             ContainerType::StructNewtype(ident, from_py_with) => {
                 self.build_newtype_struct(Some(ident), from_py_with, ctx)
@@ -256,41 +262,73 @@ impl<'a> Container<'a> {
         field_ident: Option<&Ident>,
         from_py_with: &Option<FromPyWithAttribute>,
         ctx: &Ctx,
-    ) -> TokenStream {
+    ) -> (TokenStream, TokenStream) {
         let Ctx { pyo3_path } = ctx;
         let self_ty = &self.path;
         let struct_name = self.name();
         if let Some(ident) = field_ident {
             let field_name = ident.to_string();
             match from_py_with {
-                None => quote! {
+                None => (
-                    Ok(#self_ty {
+                    quote! {
-                        #ident: #pyo3_path::impl_::frompyobject::extract_struct_field(obj, #struct_name, #field_name)?
+                        Ok(#self_ty {
-                    })
+                            #ident: #pyo3_path::impl_::frompyobject::extract_struct_field(obj, #struct_name, #field_name)?
-                },
+                        })
-                Some(FromPyWithAttribute {
+                    },
-                    value: expr_path, ..
+                    TokenStream::new(),
-                }) => quote! {
-                    Ok(#self_ty {
-                        #ident: #pyo3_path::impl_::frompyobject::extract_struct_field_with(#expr_path as fn(_) -> _, obj, #struct_name, #field_name)?
-                    })
-                },
-            }
-        } else {
-            match from_py_with {
-                None => quote!(
-                    #pyo3_path::impl_::frompyobject::extract_tuple_struct_field(obj, #struct_name, 0).map(#self_ty)
                 ),
                 Some(FromPyWithAttribute {
                     value: expr_path, ..
-                }) => quote! (
+                }) => (
-                    #pyo3_path::impl_::frompyobject::extract_tuple_struct_field_with(#expr_path as fn(_) -> _, obj, #struct_name, 0).map(#self_ty)
+                    quote! {
+                        Ok(#self_ty {
+                            #ident: #pyo3_path::impl_::frompyobject::extract_struct_field_with(#expr_path as fn(_) -> _, obj, #struct_name, #field_name)?
+                        })
+                    },
+                    quote_spanned! { expr_path.span() =>
+                        const _: () = {
+                            fn check_from_py_with() {
+                                let e = #pyo3_path::impl_::deprecations::GilRefs::new();
+                                #pyo3_path::impl_::deprecations::inspect_fn(#expr_path, &e);
+                                e.from_py_with_arg();
+                            }
+                        };
+                    },
+                ),
+            }
+        } else {
+            match from_py_with {
+                None => (
+                    quote!(
+                        #pyo3_path::impl_::frompyobject::extract_tuple_struct_field(obj, #struct_name, 0).map(#self_ty)
+                    ),
+                    TokenStream::new(),
+                ),
+                Some(FromPyWithAttribute {
+                    value: expr_path, ..
+                }) => (
+                    quote! (
+                        #pyo3_path::impl_::frompyobject::extract_tuple_struct_field_with(#expr_path as fn(_) -> _, obj, #struct_name, 0).map(#self_ty)
+                    ),
+                    quote_spanned! { expr_path.span() =>
+                        const _: () = {
+                            fn check_from_py_with() {
+                                let e = #pyo3_path::impl_::deprecations::GilRefs::new();
+                                #pyo3_path::impl_::deprecations::inspect_fn(#expr_path, &e);
+                                e.from_py_with_arg();
+                            }
+                        };
+                    },
                 ),
             }
         }
     }
 
-    fn build_tuple_struct(&self, struct_fields: &[TupleStructField], ctx: &Ctx) -> TokenStream {
+    fn build_tuple_struct(
+        &self,
+        struct_fields: &[TupleStructField],
+        ctx: &Ctx,
+    ) -> (TokenStream, TokenStream) {
         let Ctx { pyo3_path } = ctx;
         let self_ty = &self.path;
         let struct_name = &self.name();
@@ -309,15 +347,41 @@ impl<'a> Container<'a> {
                 ),
             }
         });
-        quote!(
+
-            match #pyo3_path::types::PyAnyMethods::extract(obj) {
+        let deprecations = struct_fields
-                ::std::result::Result::Ok((#(#field_idents),*)) => ::std::result::Result::Ok(#self_ty(#(#fields),*)),
+            .iter()
-                ::std::result::Result::Err(err) => ::std::result::Result::Err(err),
+            .filter_map(|field| {
-            }
+                let FromPyWithAttribute {
+                    value: expr_path, ..
+                } = field.from_py_with.as_ref()?;
+                Some(quote_spanned! { expr_path.span() =>
+                    const _: () = {
+                        fn check_from_py_with() {
+                            let e = #pyo3_path::impl_::deprecations::GilRefs::new();
+                            #pyo3_path::impl_::deprecations::inspect_fn(#expr_path, &e);
+                            e.from_py_with_arg();
+                        }
+                    };
+                })
+            })
+            .collect::<TokenStream>();
+
+        (
+            quote!(
+                match #pyo3_path::types::PyAnyMethods::extract(obj) {
+                    ::std::result::Result::Ok((#(#field_idents),*)) => ::std::result::Result::Ok(#self_ty(#(#fields),*)),
+                    ::std::result::Result::Err(err) => ::std::result::Result::Err(err),
+                }
+            ),
+            deprecations,
         )
     }
 
-    fn build_struct(&self, struct_fields: &[NamedStructField<'_>], ctx: &Ctx) -> TokenStream {
+    fn build_struct(
+        &self,
+        struct_fields: &[NamedStructField<'_>],
+        ctx: &Ctx,
+    ) -> (TokenStream, TokenStream) {
         let Ctx { pyo3_path } = ctx;
         let self_ty = &self.path;
         let struct_name = &self.name();
@@ -355,7 +419,29 @@ impl<'a> Container<'a> {
 
             fields.push(quote!(#ident: #extractor));
         }
-        quote!(::std::result::Result::Ok(#self_ty{#fields}))
+
+        let deprecations = struct_fields
+            .iter()
+            .filter_map(|field| {
+                let FromPyWithAttribute {
+                    value: expr_path, ..
+                } = field.from_py_with.as_ref()?;
+                Some(quote_spanned! { expr_path.span() =>
+                    const _: () = {
+                        fn check_from_py_with() {
+                            let e = #pyo3_path::impl_::deprecations::GilRefs::new();
+                            #pyo3_path::impl_::deprecations::inspect_fn(#expr_path, &e);
+                            e.from_py_with_arg();
+                        }
+                    };
+                })
+            })
+            .collect::<TokenStream>();
+
+        (
+            quote!(::std::result::Result::Ok(#self_ty{#fields})),
+            deprecations,
+        )
     }
 }
 
@@ -587,7 +673,7 @@ pub fn build_derive_from_pyobject(tokens: &DeriveInput) -> Result<TokenStream> {
     let ctx = &Ctx::new(&options.krate);
     let Ctx { pyo3_path } = &ctx;
 
-    let derives = match &tokens.data {
+    let (derives, from_py_with_deprecations) = match &tokens.data {
         syn::Data::Enum(en) => {
             if options.transparent || options.annotation.is_some() {
                 bail_spanned!(tokens.span() => "`transparent` or `annotation` is not supported \
@@ -617,5 +703,7 @@ pub fn build_derive_from_pyobject(tokens: &DeriveInput) -> Result<TokenStream> {
                 #derives
             }
         }
+
+        #from_py_with_deprecations
     ))
 }

tests/ui/deprecations.rs

@@ -89,6 +89,45 @@ fn pyfunction_from_py_with(
 ) {
 }
 
+#[derive(Debug, FromPyObject)]
+pub struct Zap {
+    #[pyo3(item)]
+    name: String,
+
+    #[pyo3(from_py_with = "PyAny::len", item("my_object"))]
+    some_object_length: usize,
+
+    #[pyo3(from_py_with = "extract_bound")]
+    some_number: i32,
+}
+
+#[derive(Debug, FromPyObject)]
+pub struct ZapTuple(
+    String,
+    #[pyo3(from_py_with = "PyAny::len")] usize,
+    #[pyo3(from_py_with = "extract_bound")] i32,
+);
+
+#[derive(Debug, FromPyObject, PartialEq, Eq)]
+pub enum ZapEnum {
+    Zip(#[pyo3(from_py_with = "extract_gil_ref")] i32),
+    Zap(String, #[pyo3(from_py_with = "extract_bound")] i32),
+}
+
+#[derive(Debug, FromPyObject, PartialEq, Eq)]
+#[pyo3(transparent)]
+pub struct TransparentFromPyWithGilRef {
+    #[pyo3(from_py_with = "extract_gil_ref")]
+    len: i32,
+}
+
+#[derive(Debug, FromPyObject, PartialEq, Eq)]
+#[pyo3(transparent)]
+pub struct TransparentFromPyWithBound {
+    #[pyo3(from_py_with = "extract_bound")]
+    len: i32,
+}
+
 fn test_wrap_pyfunction(py: Python<'_>, m: &Bound<'_, PyModule>) {
     // should lint
     let _ = wrap_pyfunction!(double, py);

tests/ui/deprecations.stderr

@@ -52,10 +52,34 @@ error: use of deprecated method `pyo3::deprecations::GilRefs::<T>::from_py_with_
 87 |     #[pyo3(from_py_with = "extract_gil_ref")] _gil_ref: i32,
    |                           ^^^^^^^^^^^^^^^^^
 
+error: use of deprecated method `pyo3::deprecations::GilRefs::<T>::from_py_with_arg`: use `&Bound<'_, PyAny>` as the argument for this `from_py_with` extractor
+  --> tests/ui/deprecations.rs:97:27
+   |
+97 |     #[pyo3(from_py_with = "PyAny::len", item("my_object"))]
+   |                           ^^^^^^^^^^^^
+
+error: use of deprecated method `pyo3::deprecations::GilRefs::<T>::from_py_with_arg`: use `&Bound<'_, PyAny>` as the argument for this `from_py_with` extractor
+   --> tests/ui/deprecations.rs:107:27
+    |
+107 |     #[pyo3(from_py_with = "PyAny::len")] usize,
+    |                           ^^^^^^^^^^^^
+
+error: use of deprecated method `pyo3::deprecations::GilRefs::<T>::from_py_with_arg`: use `&Bound<'_, PyAny>` as the argument for this `from_py_with` extractor
+   --> tests/ui/deprecations.rs:113:31
+    |
+113 |     Zip(#[pyo3(from_py_with = "extract_gil_ref")] i32),
+    |                               ^^^^^^^^^^^^^^^^^
+
+error: use of deprecated method `pyo3::deprecations::GilRefs::<T>::from_py_with_arg`: use `&Bound<'_, PyAny>` as the argument for this `from_py_with` extractor
+   --> tests/ui/deprecations.rs:120:27
+    |
+120 |     #[pyo3(from_py_with = "extract_gil_ref")]
+    |                           ^^^^^^^^^^^^^^^^^
+
 error: use of deprecated method `pyo3::deprecations::GilRefs::<pyo3::Python<'_>>::is_python`: use `wrap_pyfunction_bound!` instead
-  --> tests/ui/deprecations.rs:94:13
+   --> tests/ui/deprecations.rs:133:13
-   |
+    |
-94 |     let _ = wrap_pyfunction!(double, py);
+133 |     let _ = wrap_pyfunction!(double, py);
-   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-   |
+    |
-   = note: this error originates in the macro `wrap_pyfunction` (in Nightly builds, run with -Z macro-backtrace for more info)
+    = note: this error originates in the macro `wrap_pyfunction` (in Nightly builds, run with -Z macro-backtrace for more info)