fix deprecation warning for trailing optional on #[setter] functions

Author: davidhewitt

newsfragments/4304.fixed.md

@@ -0,0 +1 @@
+Fix invalid deprecation warning for trailing optional on `#[setter]` function.

pyo3-macros-backend/src/deprecations.rs

@@ -51,6 +51,7 @@ impl<'ctx> ToTokens for Deprecations<'ctx> {
 
 pub(crate) fn deprecate_trailing_option_default(spec: &FnSpec<'_>) -> TokenStream {
     if spec.signature.attribute.is_none()
+        && spec.tp.signature_attribute_allowed()
         && spec.signature.arguments.iter().any(|arg| {
             if let FnArg::Regular(arg) = arg {
                 arg.option_wrapped_type.is_some()

pyo3-macros-backend/src/method.rs

@@ -228,6 +228,18 @@ impl FnType {
         }
     }
 
+    pub fn signature_attribute_allowed(&self) -> bool {
+        match self {
+            FnType::Fn(_)
+            | FnType::FnNew
+            | FnType::FnStatic
+            | FnType::FnClass(_)
+            | FnType::FnNewClass(_)
+            | FnType::FnModule(_) => true,
+            FnType::Getter(_) | FnType::Setter(_) | FnType::ClassAttribute => false,
+        }
+    }
+
     pub fn self_arg(
         &self,
         cls: Option<&syn::Type>,
@@ -1096,15 +1108,18 @@ fn ensure_signatures_on_valid_method(
     if let Some(signature) = signature {
         match fn_type {
             FnType::Getter(_) => {
+                debug_assert!(!fn_type.signature_attribute_allowed());
                 bail_spanned!(signature.kw.span() => "`signature` not allowed with `getter`")
             }
             FnType::Setter(_) => {
+                debug_assert!(!fn_type.signature_attribute_allowed());
                 bail_spanned!(signature.kw.span() => "`signature` not allowed with `setter`")
             }
             FnType::ClassAttribute => {
+                debug_assert!(!fn_type.signature_attribute_allowed());
                 bail_spanned!(signature.kw.span() => "`signature` not allowed with `classattr`")
             }
-            _ => {}
+            _ => debug_assert!(fn_type.signature_attribute_allowed()),
         }
     }
     if let Some(text_signature) = text_signature {

tests/test_getter_setter.rs

@@ -280,3 +280,39 @@ fn frozen_py_field_get() {
         py_run!(py, inst, "assert inst.value == 'value'");
     });
 }
+
+#[test]
+fn test_optional_setter() {
+    #[pyclass]
+    struct SimpleClass {
+        field: Option<u32>,
+    }
+
+    #[pymethods]
+    impl SimpleClass {
+        #[getter]
+        fn get_field(&self) -> Option<u32> {
+            self.field
+        }
+
+        #[setter]
+        fn set_field(&mut self, field: Option<u32>) {
+            self.field = field;
+        }
+    }
+
+    Python::with_gil(|py| {
+        let instance = Py::new(py, SimpleClass { field: None }).unwrap();
+        py_run!(py, instance, "assert instance.field is None");
+        py_run!(
+            py,
+            instance,
+            "instance.field = 42; assert instance.field == 42"
+        );
+        py_run!(
+            py,
+            instance,
+            "instance.field = None; assert instance.field is None"
+        );
+    })
+}

tests/ui/deprecations.rs

@@ -39,9 +39,6 @@ impl MyClass {
     #[setter]
     fn set_bar_bound(&self, _value: &Bound<'_, PyAny>) {}
 
-    #[setter]
-    fn set_option(&self, _value: Option<i32>) {}
-
     fn __eq__(&self, #[pyo3(from_py_with = "extract_gil_ref")] _other: i32) -> bool {
         true
     }

tests/ui/deprecations.stderr

@@ -10,42 +10,34 @@ note: the lint level is defined here
 1  | #![deny(deprecated)]
    |         ^^^^^^^^^^
 
-error: use of deprecated constant `MyClass::__pymethod_set_set_option__::SIGNATURE`: this function has implicit defaults for the trailing `Option<T>` arguments
-       = note: these implicit defaults are being phased out
-       = help: add `#[pyo3(signature = (_value=None))]` to this function to silence this warning and keep the current behavior
-  --> tests/ui/deprecations.rs:43:8
-   |
-43 |     fn set_option(&self, _value: Option<i32>) {}
-   |        ^^^^^^^^^^
-
 error: use of deprecated constant `__pyfunction_pyfunction_option_2::SIGNATURE`: this function has implicit defaults for the trailing `Option<T>` arguments
        = note: these implicit defaults are being phased out
        = help: add `#[pyo3(signature = (_i, _any=None))]` to this function to silence this warning and keep the current behavior
-   --> tests/ui/deprecations.rs:132:4
+   --> tests/ui/deprecations.rs:129:4
     |
-132 | fn pyfunction_option_2(_i: u32, _any: Option<i32>) {}
+129 | fn pyfunction_option_2(_i: u32, _any: Option<i32>) {}
     |    ^^^^^^^^^^^^^^^^^^^
 
 error: use of deprecated constant `__pyfunction_pyfunction_option_3::SIGNATURE`: this function has implicit defaults for the trailing `Option<T>` arguments
        = note: these implicit defaults are being phased out
        = help: add `#[pyo3(signature = (_i, _any=None, _foo=None))]` to this function to silence this warning and keep the current behavior
-   --> tests/ui/deprecations.rs:135:4
+   --> tests/ui/deprecations.rs:132:4
     |
-135 | fn pyfunction_option_3(_i: u32, _any: Option<i32>, _foo: Option<String>) {}
+132 | fn pyfunction_option_3(_i: u32, _any: Option<i32>, _foo: Option<String>) {}
     |    ^^^^^^^^^^^^^^^^^^^
 
 error: use of deprecated constant `__pyfunction_pyfunction_option_4::SIGNATURE`: this function has implicit defaults for the trailing `Option<T>` arguments
        = note: these implicit defaults are being phased out
        = help: add `#[pyo3(signature = (_i, _any=None, _foo=None))]` to this function to silence this warning and keep the current behavior
-   --> tests/ui/deprecations.rs:138:4
+   --> tests/ui/deprecations.rs:135:4
     |
-138 | fn pyfunction_option_4(
+135 | fn pyfunction_option_4(
     |    ^^^^^^^^^^^^^^^^^^^
 
 error: use of deprecated constant `SimpleEnumWithoutEq::__pyo3__generated____richcmp__::DEPRECATION`: Implicit equality for simple enums is deprecated. Use `#[pyclass(eq, eq_int)` to keep the current behavior.
-   --> tests/ui/deprecations.rs:200:1
+   --> tests/ui/deprecations.rs:197:1
     |
-200 | #[pyclass]
+197 | #[pyclass]
     | ^^^^^^^^^^
     |
     = note: this error originates in the attribute macro `pyclass` (in Nightly builds, run with -Z macro-backtrace for more info)
@@ -57,9 +49,9 @@ error: use of deprecated struct `pyo3::PyCell`: `PyCell` was merged into `Bound`
    |                              ^^^^^^
 
 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:45:44
+  --> tests/ui/deprecations.rs:42:44
    |
-45 |     fn __eq__(&self, #[pyo3(from_py_with = "extract_gil_ref")] _other: i32) -> bool {
+42 |     fn __eq__(&self, #[pyo3(from_py_with = "extract_gil_ref")] _other: i32) -> bool {
    |                                            ^^^^^^^^^^^^^^^^^
 
 error: use of deprecated method `pyo3::deprecations::GilRefs::<T>::function_arg`: use `&Bound<'_, T>` instead for this function argument
@@ -93,69 +85,69 @@ error: use of deprecated method `pyo3::deprecations::GilRefs::<T>::function_arg`
    |                                       ^
 
 error: use of deprecated method `pyo3::deprecations::GilRefs::<T>::function_arg`: use `&Bound<'_, T>` instead for this function argument
-  --> tests/ui/deprecations.rs:64:44
+  --> tests/ui/deprecations.rs:61:44
    |
-64 | fn pyfunction_with_module_gil_ref(_module: &PyModule) -> PyResult<&str> {
+61 | 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:74:19
+  --> tests/ui/deprecations.rs:71:19
    |
-74 | fn module_gil_ref(_m: &PyModule) -> PyResult<()> {
+71 | 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:79:57
+  --> tests/ui/deprecations.rs:76:57
    |
-79 | fn module_gil_ref_with_explicit_py_arg(_py: Python<'_>, _m: &PyModule) -> PyResult<()> {
+76 | 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:115:27
+   --> tests/ui/deprecations.rs:112:27
     |
-115 |     #[pyo3(from_py_with = "extract_gil_ref")] _gil_ref: i32,
+112 |     #[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:121:29
+   --> tests/ui/deprecations.rs:118:29
     |
-121 | fn pyfunction_gil_ref(_any: &PyAny) {}
+118 | 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:125:36
+   --> tests/ui/deprecations.rs:122:36
     |
-125 | fn pyfunction_option_gil_ref(_any: Option<&PyAny>) {}
+122 | 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:150:27
+   --> tests/ui/deprecations.rs:147:27
     |
-150 |     #[pyo3(from_py_with = "PyAny::len", item("my_object"))]
+147 |     #[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:160:27
+   --> tests/ui/deprecations.rs:157:27
     |
-160 |     #[pyo3(from_py_with = "PyAny::len")] usize,
+157 |     #[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:166:31
+   --> tests/ui/deprecations.rs:163:31
     |
-166 |     Zip(#[pyo3(from_py_with = "extract_gil_ref")] i32),
+163 |     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:173:27
+   --> tests/ui/deprecations.rs:170:27
     |
-173 |     #[pyo3(from_py_with = "extract_gil_ref")]
+170 |     #[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:186:13
+   --> tests/ui/deprecations.rs:183:13
     |
-186 |     let _ = wrap_pyfunction!(double, py);
+183 |     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)