use `extract_argument` for `#[setter]` extraction (#3998)

* use `extract_argument` for `#[setter]` extraction

* add newsfragment

newsfragments/3998.changed.md

@@ -0,0 +1 @@
+Warn on uses of GIL Refs for `#[setter]` function arguments.

newsfragments/3998.fixed.md

@@ -0,0 +1 @@
+Allow extraction of `&Bound` in `#[setter]` methods.

pyo3-macros-backend/src/pymethod.rs

@@ -586,6 +586,8 @@ pub fn impl_py_setter_def(
         }
     };
 
+    // TODO: rework this to make use of `impl_::params::impl_arg_param` which
+    // handles all these cases already.
     let extract = if let PropertyType::Function { spec, .. } = &property_type {
         Some(spec)
     } else {
@@ -609,8 +611,21 @@ pub fn impl_py_setter_def(
         })
     })
     .unwrap_or_else(|| {
+        let (span, name) = match &property_type {
+            PropertyType::Descriptor { field, .. } => (field.ty.span(), field.ident.as_ref().map(|i|i.to_string()).unwrap_or_default()),
+            PropertyType::Function { spec, .. } => {
+                let (_, args) = split_off_python_arg(&spec.signature.arguments);
+                (args[0].ty.span(), args[0].name.to_string())
+            }
+        };
+
+        let holder = holders.push_holder(span);
+        let gil_refs_checker = holders.push_gil_refs_checker(span);
         quote! {
-            let _val = #pyo3_path::FromPyObject::extract_bound(_value.into())?;
+            let _val = #pyo3_path::impl_::deprecations::inspect_type(
+                #pyo3_path::impl_::extract_argument::extract_argument(_value.into(), &mut #holder, #name)?,
+                &#gil_refs_checker
+            );
         }
     });
 
@@ -639,8 +654,8 @@ pub fn impl_py_setter_def(
                 .ok_or_else(|| {
                     #pyo3_path::exceptions::PyAttributeError::new_err("can't delete attribute")
                 })?;
-            #extract
             #init_holders
+            #extract
             let result = #setter_impl;
             #check_gil_refs
             #pyo3_path::callback::convert(py, result)

tests/test_getter_setter.rs

@@ -46,6 +46,12 @@ impl ClassWithProperties {
         self.num = value;
     }
 
+    #[setter]
+    fn set_from_any(&mut self, value: &Bound<'_, PyAny>) -> PyResult<()> {
+        self.num = value.extract()?;
+        Ok(())
+    }
+
     #[getter]
     fn get_data_list<'py>(&self, py: Python<'py>) -> Bound<'py, PyList> {
         PyList::new_bound(py, [self.num])
@@ -76,6 +82,9 @@ fn class_with_properties() {
         py_run!(py, inst, "inst.from_len = [0, 0, 0]");
         py_run!(py, inst, "assert inst.get_num() == 3");
 
+        py_run!(py, inst, "inst.from_any = 15");
+        py_run!(py, inst, "assert inst.get_num() == 15");
+
         let d = [("C", py.get_type_bound::<ClassWithProperties>())].into_py_dict_bound(py);
         py_assert!(py, *d, "C.DATA.__doc__ == 'a getter for data'");
     });

tests/ui/deprecations.rs

@@ -32,6 +32,12 @@ impl MyClass {
 
     #[setter]
     fn set_foo_bound(&self, #[pyo3(from_py_with = "extract_bound")] _value: i32) {}
+
+    #[setter]
+    fn set_bar_gil_ref(&self, _value: &PyAny) {}
+
+    #[setter]
+    fn set_bar_bound(&self, _value: &Bound<'_, PyAny>) {}
 }
 
 fn main() {}

tests/ui/deprecations.stderr

@@ -41,69 +41,75 @@ error: use of deprecated method `pyo3::deprecations::GilRefs::<T>::from_py_with_
    |                                                     ^^^^^^^^^^^^^^^^^
 
 error: use of deprecated method `pyo3::deprecations::GilRefs::<T>::function_arg`: use `&Bound<'_, T>` instead for this function argument
-  --> tests/ui/deprecations.rs:47:43
+  --> tests/ui/deprecations.rs:37:39
    |
-47 | fn pyfunction_with_module_gil_ref(module: &PyModule) -> PyResult<&str> {
+37 |     fn set_bar_gil_ref(&self, _value: &PyAny) {}
+   |                                       ^
+
+error: use of deprecated method `pyo3::deprecations::GilRefs::<T>::function_arg`: use `&Bound<'_, T>` instead for this function argument
+  --> tests/ui/deprecations.rs:53:43
+   |
+53 | fn pyfunction_with_module_gil_ref(module: &PyModule) -> PyResult<&str> {
    |                                           ^
 
 error: use of deprecated method `pyo3::deprecations::GilRefs::<T>::function_arg`: use `&Bound<'_, T>` instead for this function argument
-  --> tests/ui/deprecations.rs:57:19
+  --> tests/ui/deprecations.rs:63:19
    |
-57 | fn module_gil_ref(m: &PyModule) -> PyResult<()> {
+63 | fn module_gil_ref(m: &PyModule) -> PyResult<()> {
    |                   ^
 
 error: use of deprecated method `pyo3::deprecations::GilRefs::<T>::function_arg`: use `&Bound<'_, T>` instead for this function argument
-  --> tests/ui/deprecations.rs:63:57
+  --> tests/ui/deprecations.rs:69:57
    |
-63 | fn module_gil_ref_with_explicit_py_arg(_py: Python<'_>, m: &PyModule) -> PyResult<()> {
+69 | fn module_gil_ref_with_explicit_py_arg(_py: Python<'_>, m: &PyModule) -> PyResult<()> {
    |                                                         ^
 
 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:96:27
+   --> tests/ui/deprecations.rs:102:27
-   |
+    |
-96 |     #[pyo3(from_py_with = "extract_gil_ref")] _gil_ref: i32,
+102 |     #[pyo3(from_py_with = "extract_gil_ref")] _gil_ref: i32,
-   |                           ^^^^^^^^^^^^^^^^^
+    |                           ^^^^^^^^^^^^^^^^^
 
 error: use of deprecated method `pyo3::deprecations::GilRefs::<T>::function_arg`: use `&Bound<'_, T>` instead for this function argument
-   --> tests/ui/deprecations.rs:102:29
+   --> tests/ui/deprecations.rs:108:29
     |
-102 | fn pyfunction_gil_ref(_any: &PyAny) {}
+108 | fn pyfunction_gil_ref(_any: &PyAny) {}
     |                             ^
 
 error: use of deprecated method `pyo3::deprecations::OptionGilRefs::<std::option::Option<T>>::function_arg`: use `Option<&Bound<'_, T>>` instead for this function argument
-   --> tests/ui/deprecations.rs:105:36
+   --> tests/ui/deprecations.rs:111:36
     |
-105 | fn pyfunction_option_gil_ref(_any: Option<&PyAny>) {}
+111 | fn pyfunction_option_gil_ref(_any: Option<&PyAny>) {}
     |                                    ^^^^^^
 
 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:112:27
+   --> tests/ui/deprecations.rs:118:27
     |
-112 |     #[pyo3(from_py_with = "PyAny::len", item("my_object"))]
+118 |     #[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:122:27
+   --> tests/ui/deprecations.rs:128:27
     |
-122 |     #[pyo3(from_py_with = "PyAny::len")] usize,
+128 |     #[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:128:31
+   --> tests/ui/deprecations.rs:134:31
     |
-128 |     Zip(#[pyo3(from_py_with = "extract_gil_ref")] i32),
+134 |     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:135:27
+   --> tests/ui/deprecations.rs:141:27
     |
-135 |     #[pyo3(from_py_with = "extract_gil_ref")]
+141 |     #[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:148:13
+   --> tests/ui/deprecations.rs:154:13
     |
-148 |     let _ = wrap_pyfunction!(double, py);
+154 |     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)