Skip to content

Commit

Permalink
fix deprecation warning for trailing optional on #[setter] functions (
Browse files Browse the repository at this point in the history 
#4304)

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

* add comment
  • Loading branch information
@davidhewitt
davidhewitt authored Jul 4, 2024
1 parent ee9123a commit 0af0227
Show file tree
Hide file tree
Showing 6 changed files with 88 additions and 44 deletions.
1 change: 1 addition & 0 deletions newsfragments/4304.fixed.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix invalid deprecation warning for trailing optional on `#[setter]` function.
1 change: 1 addition & 0 deletions pyo3-macros-backend/src/deprecations.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
19 changes: 18 additions & 1 deletion pyo3-macros-backend/src/method.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,20 @@ impl FnType {
}
}

pub fn signature_attribute_allowed(&self) -> bool {
match self {
FnType::Fn(_)
| FnType::FnNew
| FnType::FnStatic
| FnType::FnClass(_)
| FnType::FnNewClass(_)
| FnType::FnModule(_) => true,
// Setter, Getter and ClassAttribute all have fixed signatures (either take 0 or 1
// arguments) so cannot have a `signature = (...)` attribute.
FnType::Getter(_) | FnType::Setter(_) | FnType::ClassAttribute => false,
}
}

pub fn self_arg(
&self,
cls: Option<&syn::Type>,
Expand Down Expand Up @@ -1096,15 +1110,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 {
Expand Down
36 changes: 36 additions & 0 deletions tests/test_getter_setter.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
);
})
}
3 changes: 0 additions & 3 deletions tests/ui/deprecations.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
72 changes: 32 additions & 40 deletions tests/ui/deprecations.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
Expand Down Expand Up @@ -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)

0 comments on commit 0af0227

Please sign in to comment.