From 0cfb93c5296a86978c514c34f2d831befa2e6c8e Mon Sep 17 00:00:00 2001 From: Icxolu <10486322+Icxolu@users.noreply.github.com> Date: Thu, 9 May 2024 12:07:01 +0200 Subject: [PATCH] generate individual deprecation messages per function --- guide/src/function/signature.md | 2 +- pyo3-macros-backend/src/deprecations.rs | 53 +++++++++++++++++- pyo3-macros-backend/src/method.rs | 7 +++ pyo3-macros-backend/src/params.rs | 38 +------------ pyo3-macros-backend/src/pymethod.rs | 5 +- src/impl_/deprecations.rs | 10 ---- tests/test_pyfunction.rs | 1 + tests/ui/deprecations.rs | 18 ++++++ tests/ui/deprecations.stderr | 74 +++++++++++++++---------- 9 files changed, 132 insertions(+), 76 deletions(-) diff --git a/guide/src/function/signature.md b/guide/src/function/signature.md index 936fa95e261..69949220be6 100644 --- a/guide/src/function/signature.md +++ b/guide/src/function/signature.md @@ -123,7 +123,7 @@ num=-1
-⚠️ Warning: This behaviour is phased out 🛠️ +⚠️ Warning: This behaviour is being phased out 🛠️ The special casing of trailing optional arguments is deprecated. In a future `pyo3` version, arguments of type `Option<..>` will share the same behaviour as other arguments, they are required unless a default is set using `#[pyo3(signature = (...))]`. diff --git a/pyo3-macros-backend/src/deprecations.rs b/pyo3-macros-backend/src/deprecations.rs index 3f1f34144f6..4db40cc86f7 100644 --- a/pyo3-macros-backend/src/deprecations.rs +++ b/pyo3-macros-backend/src/deprecations.rs @@ -1,4 +1,7 @@ -use crate::utils::Ctx; +use crate::{ + method::{FnArg, FnSpec}, + utils::Ctx, +}; use proc_macro2::{Span, TokenStream}; use quote::{quote_spanned, ToTokens}; @@ -45,3 +48,51 @@ impl<'ctx> ToTokens for Deprecations<'ctx> { } } } + +pub(crate) fn deprecate_trailing_option_default(spec: &FnSpec<'_>) -> TokenStream { + if spec.signature.attribute.is_none() + && spec.signature.arguments.iter().any(|arg| { + if let FnArg::Regular(arg) = arg { + arg.option_wrapped_type.is_some() + } else { + false + } + }) + { + use std::fmt::Write; + let mut deprecation_msg = String::from( + "This function has implicit defaults for the trailing `Option` arguments. \ + These implicit defaults are being phased out. Add `#[pyo3(signature = (", + ); + spec.signature.arguments.iter().for_each(|arg| { + match arg { + FnArg::Regular(arg) => { + if arg.option_wrapped_type.is_some() { + write!(deprecation_msg, "{}=None, ", arg.name) + } else { + write!(deprecation_msg, "{}, ", arg.name) + } + } + FnArg::VarArgs(arg) => write!(deprecation_msg, "{}, ", arg.name), + FnArg::KwArgs(arg) => write!(deprecation_msg, "{}, ", arg.name), + FnArg::Py(_) | FnArg::CancelHandle(_) => Ok(()), + } + .expect("writing to `String` should not fail"); + }); + + //remove trailing space and comma + deprecation_msg.pop(); + deprecation_msg.pop(); + + deprecation_msg + .push_str(")]` to this function to silence this warning and keep the current behavior"); + quote_spanned! { spec.name.span() => + #[deprecated(note = #deprecation_msg)] + #[allow(dead_code)] + const SIGNATURE: () = (); + const _: () = SIGNATURE; + } + } else { + TokenStream::new() + } +} diff --git a/pyo3-macros-backend/src/method.rs b/pyo3-macros-backend/src/method.rs index cb86e8ec606..1d898038c01 100644 --- a/pyo3-macros-backend/src/method.rs +++ b/pyo3-macros-backend/src/method.rs @@ -5,6 +5,7 @@ use proc_macro2::{Span, TokenStream}; use quote::{format_ident, quote, quote_spanned, ToTokens}; use syn::{ext::IdentExt, spanned::Spanned, Ident, Result}; +use crate::deprecations::deprecate_trailing_option_default; use crate::utils::Ctx; use crate::{ attributes::{FromPyWithAttribute, TextSignatureAttribute, TextSignatureAttributeValue}, @@ -708,6 +709,8 @@ impl<'a> FnSpec<'a> { quote!(#func_name) }; + let deprecation = deprecate_trailing_option_default(self); + Ok(match self.convention { CallingConvention::Noargs => { let mut holders = Holders::new(); @@ -730,6 +733,7 @@ impl<'a> FnSpec<'a> { py: #pyo3_path::Python<'py>, _slf: *mut #pyo3_path::ffi::PyObject, ) -> #pyo3_path::PyResult<*mut #pyo3_path::ffi::PyObject> { + #deprecation let _slf_ref = &_slf; let function = #rust_name; // Shadow the function name to avoid #3017 #init_holders @@ -754,6 +758,7 @@ impl<'a> FnSpec<'a> { _nargs: #pyo3_path::ffi::Py_ssize_t, _kwnames: *mut #pyo3_path::ffi::PyObject ) -> #pyo3_path::PyResult<*mut #pyo3_path::ffi::PyObject> { + #deprecation let _slf_ref = &_slf; let function = #rust_name; // Shadow the function name to avoid #3017 #arg_convert @@ -778,6 +783,7 @@ impl<'a> FnSpec<'a> { _args: *mut #pyo3_path::ffi::PyObject, _kwargs: *mut #pyo3_path::ffi::PyObject ) -> #pyo3_path::PyResult<*mut #pyo3_path::ffi::PyObject> { + #deprecation let _slf_ref = &_slf; let function = #rust_name; // Shadow the function name to avoid #3017 #arg_convert @@ -805,6 +811,7 @@ impl<'a> FnSpec<'a> { _kwargs: *mut #pyo3_path::ffi::PyObject ) -> #pyo3_path::PyResult<*mut #pyo3_path::ffi::PyObject> { use #pyo3_path::callback::IntoPyCallbackOutput; + #deprecation let _slf_ref = &_slf; let function = #rust_name; // Shadow the function name to avoid #3017 #arg_convert diff --git a/pyo3-macros-backend/src/params.rs b/pyo3-macros-backend/src/params.rs index 469b59a10fa..cab9d2a7d29 100644 --- a/pyo3-macros-backend/src/params.rs +++ b/pyo3-macros-backend/src/params.rs @@ -134,16 +134,7 @@ pub fn impl_arg_params( .arguments .iter() .enumerate() - .map(|(i, arg)| { - impl_arg_param( - arg, - spec.signature.attribute.is_some(), - i, - &mut 0, - holders, - ctx, - ) - }) + .map(|(i, arg)| impl_arg_param(arg, i, &mut 0, holders, ctx)) .collect(); return ( quote! { @@ -183,16 +174,7 @@ pub fn impl_arg_params( .arguments .iter() .enumerate() - .map(|(i, arg)| { - impl_arg_param( - arg, - spec.signature.attribute.is_some(), - i, - &mut option_pos, - holders, - ctx, - ) - }) + .map(|(i, arg)| impl_arg_param(arg, i, &mut option_pos, holders, ctx)) .collect(); let args_handler = if spec.signature.python_signature.varargs.is_some() { @@ -255,7 +237,6 @@ pub fn impl_arg_params( fn impl_arg_param( arg: &FnArg<'_>, - has_signature_attr: bool, pos: usize, option_pos: &mut usize, holders: &mut Holders, @@ -269,14 +250,7 @@ fn impl_arg_param( let from_py_with = format_ident!("from_py_with_{}", pos); let arg_value = quote!(#args_array[#option_pos].as_deref()); *option_pos += 1; - let tokens = impl_regular_arg_param( - arg, - has_signature_attr, - from_py_with, - arg_value, - holders, - ctx, - ); + let tokens = impl_regular_arg_param(arg, from_py_with, arg_value, holders, ctx); check_arg_for_gil_refs(tokens, holders.push_gil_refs_checker(arg.ty.span()), ctx) } FnArg::VarArgs(arg) => { @@ -311,7 +285,6 @@ fn impl_arg_param( /// index and the index in option diverge when using py: Python pub(crate) fn impl_regular_arg_param( arg: &RegularArg<'_>, - has_signature_attr: bool, from_py_with: syn::Ident, arg_value: TokenStream, // expected type: Option<&'a Bound<'py, PyAny>> holders: &mut Holders, @@ -362,11 +335,6 @@ pub(crate) fn impl_regular_arg_param( } } else if arg.option_wrapped_type.is_some() { let holder = holders.push_holder(arg.name.span()); - let arg_value = if !has_signature_attr { - quote_arg_span! { #pyo3_path::impl_::deprecations::deprecate_implicit_option(#arg_value) } - } else { - quote!(#arg_value) - }; quote_arg_span! { #pyo3_path::impl_::extract_argument::extract_optional_argument( #arg_value, diff --git a/pyo3-macros-backend/src/pymethod.rs b/pyo3-macros-backend/src/pymethod.rs index 99f6f87bcc5..d8e03b5460e 100644 --- a/pyo3-macros-backend/src/pymethod.rs +++ b/pyo3-macros-backend/src/pymethod.rs @@ -1,6 +1,7 @@ use std::borrow::Cow; use crate::attributes::{NameAttribute, RenamingRule}; +use crate::deprecations::deprecate_trailing_option_default; use crate::method::{CallingConvention, ExtractErrorMode, PyArg}; use crate::params::{check_arg_for_gil_refs, impl_regular_arg_param, Holders}; use crate::utils::Ctx; @@ -630,7 +631,6 @@ pub fn impl_py_setter_def( let tokens = impl_regular_arg_param( arg, - spec.signature.attribute.is_some(), ident, quote!(::std::option::Option::Some(_value.into())), &mut holders, @@ -638,7 +638,10 @@ pub fn impl_py_setter_def( ); let extract = check_arg_for_gil_refs(tokens, holders.push_gil_refs_checker(arg.ty.span()), ctx); + + let deprecation = deprecate_trailing_option_default(spec); quote! { + #deprecation #from_py_with let _val = #extract; } diff --git a/src/impl_/deprecations.rs b/src/impl_/deprecations.rs index acd1064f2dc..9eb1da05c92 100644 --- a/src/impl_/deprecations.rs +++ b/src/impl_/deprecations.rs @@ -85,13 +85,3 @@ impl std::ops::Deref for OptionGilRefs { &self.0 } } - -#[deprecated( - since = "0.22.0", - note = "Implicit default for trailing optional arguments is phased out. Add an explicit \ - `#[pyo3(signature = (...))]` attribute a to silence this warning. In a future \ - pyo3 version `Option<..>` arguments will be treated the same as any other argument." -)] -pub fn deprecate_implicit_option(t: T) -> T { - t -} diff --git a/tests/test_pyfunction.rs b/tests/test_pyfunction.rs index 6eaf1a8f11b..4a90f3f9d99 100644 --- a/tests/test_pyfunction.rs +++ b/tests/test_pyfunction.rs @@ -182,6 +182,7 @@ fn test_from_py_with_defaults() { // issue 2280 combination of from_py_with and Option did not compile #[pyfunction] + #[pyo3(signature = (int=None))] fn from_py_with_option(#[pyo3(from_py_with = "optional_int")] int: Option) -> i32 { int.unwrap_or(0) } diff --git a/tests/ui/deprecations.rs b/tests/ui/deprecations.rs index f59a8167352..fc9e8687cae 100644 --- a/tests/ui/deprecations.rs +++ b/tests/ui/deprecations.rs @@ -39,6 +39,9 @@ impl MyClass { #[setter] fn set_bar_bound(&self, _value: &Bound<'_, PyAny>) {} + #[setter] + fn set_option(&self, _value: Option) {} + fn __eq__(&self, #[pyo3(from_py_with = "extract_gil_ref")] _other: i32) -> bool { true } @@ -103,6 +106,10 @@ fn extract_bound(obj: &Bound<'_, PyAny>) -> PyResult { obj.extract() } +fn extract_options(obj: &Bound<'_, PyAny>) -> PyResult> { + obj.extract() +} + #[pyfunction] fn pyfunction_from_py_with( #[pyo3(from_py_with = "extract_gil_ref")] _gil_ref: i32, @@ -124,6 +131,17 @@ fn pyfunction_option_1(_i: u32, _any: Option) {} #[pyfunction] fn pyfunction_option_2(_i: u32, _any: Option) {} +#[pyfunction] +fn pyfunction_option_3(_i: u32, _any: Option, _foo: Option) {} + +#[pyfunction] +fn pyfunction_option_4( + _i: u32, + #[pyo3(from_py_with = "extract_options")] _any: Option, + _foo: Option, +) { +} + #[derive(Debug, FromPyObject)] pub struct Zap { #[pyo3(item)] diff --git a/tests/ui/deprecations.stderr b/tests/ui/deprecations.stderr index 326d56a93f8..d014a06bbcc 100644 --- a/tests/ui/deprecations.stderr +++ b/tests/ui/deprecations.stderr @@ -10,11 +10,29 @@ note: the lint level is defined here 1 | #![deny(deprecated)] | ^^^^^^^^^^ -error: use of deprecated function `pyo3::deprecations::deprecate_implicit_option`: Implicit default for trailing optional arguments is phased out. Add an explicit `#[pyo3(signature = (...))]` attribute a to silence this warning. In a future pyo3 version `Option<..>` arguments will be treated the same as any other argument. - --> tests/ui/deprecations.rs:125:39 +error: use of deprecated constant `MyClass::__pymethod_set_set_option__::SIGNATURE`: This function has implicit defaults for the trailing `Option` arguments. These implicit defaults are being phased out. 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) {} + | ^^^^^^^^^^ + +error: use of deprecated constant `__pyfunction_pyfunction_option_2::SIGNATURE`: This function has implicit defaults for the trailing `Option` arguments. These implicit defaults are being phased out. Add `#[pyo3(signature = (_i, _any=None)]` to this function to silence this warning and keep the current behavior + --> tests/ui/deprecations.rs:132:4 + | +132 | fn pyfunction_option_2(_i: u32, _any: Option) {} + | ^^^^^^^^^^^^^^^^^^^ + +error: use of deprecated constant `__pyfunction_pyfunction_option_3::SIGNATURE`: This function has implicit defaults for the trailing `Option` arguments. These implicit defaults are being phased out. 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 + | +135 | fn pyfunction_option_3(_i: u32, _any: Option, _foo: Option) {} + | ^^^^^^^^^^^^^^^^^^^ + +error: use of deprecated constant `__pyfunction_pyfunction_option_4::SIGNATURE`: This function has implicit defaults for the trailing `Option` arguments. These implicit defaults are being phased out. 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 | -125 | fn pyfunction_option_2(_i: u32, _any: Option) {} - | ^^^^^^ +138 | fn pyfunction_option_4( + | ^^^^^^^^^^^^^^^^^^^ error: use of deprecated struct `pyo3::PyCell`: `PyCell` was merged into `Bound`, use that instead; see the migration guide for more info --> tests/ui/deprecations.rs:23:30 @@ -23,9 +41,9 @@ error: use of deprecated struct `pyo3::PyCell`: `PyCell` was merged into `Bound` | ^^^^^^ error: use of deprecated method `pyo3::deprecations::GilRefs::::from_py_with_arg`: use `&Bound<'_, PyAny>` as the argument for this `from_py_with` extractor - --> tests/ui/deprecations.rs:42:44 + --> tests/ui/deprecations.rs:45:44 | -42 | fn __eq__(&self, #[pyo3(from_py_with = "extract_gil_ref")] _other: i32) -> bool { +45 | fn __eq__(&self, #[pyo3(from_py_with = "extract_gil_ref")] _other: i32) -> bool { | ^^^^^^^^^^^^^^^^^ error: use of deprecated method `pyo3::deprecations::GilRefs::::function_arg`: use `&Bound<'_, T>` instead for this function argument @@ -59,69 +77,69 @@ error: use of deprecated method `pyo3::deprecations::GilRefs::::function_arg` | ^ error: use of deprecated method `pyo3::deprecations::GilRefs::::function_arg`: use `&Bound<'_, T>` instead for this function argument - --> tests/ui/deprecations.rs:61:44 + --> tests/ui/deprecations.rs:64:44 | -61 | fn pyfunction_with_module_gil_ref(_module: &PyModule) -> PyResult<&str> { +64 | fn pyfunction_with_module_gil_ref(_module: &PyModule) -> PyResult<&str> { | ^ error: use of deprecated method `pyo3::deprecations::GilRefs::::function_arg`: use `&Bound<'_, T>` instead for this function argument - --> tests/ui/deprecations.rs:71:19 + --> tests/ui/deprecations.rs:74:19 | -71 | fn module_gil_ref(_m: &PyModule) -> PyResult<()> { +74 | fn module_gil_ref(_m: &PyModule) -> PyResult<()> { | ^^ error: use of deprecated method `pyo3::deprecations::GilRefs::::function_arg`: use `&Bound<'_, T>` instead for this function argument - --> tests/ui/deprecations.rs:76:57 + --> tests/ui/deprecations.rs:79:57 | -76 | fn module_gil_ref_with_explicit_py_arg(_py: Python<'_>, _m: &PyModule) -> PyResult<()> { +79 | fn module_gil_ref_with_explicit_py_arg(_py: Python<'_>, _m: &PyModule) -> PyResult<()> { | ^^ error: use of deprecated method `pyo3::deprecations::GilRefs::::from_py_with_arg`: use `&Bound<'_, PyAny>` as the argument for this `from_py_with` extractor - --> tests/ui/deprecations.rs:108:27 + --> tests/ui/deprecations.rs:115:27 | -108 | #[pyo3(from_py_with = "extract_gil_ref")] _gil_ref: i32, +115 | #[pyo3(from_py_with = "extract_gil_ref")] _gil_ref: i32, | ^^^^^^^^^^^^^^^^^ error: use of deprecated method `pyo3::deprecations::GilRefs::::function_arg`: use `&Bound<'_, T>` instead for this function argument - --> tests/ui/deprecations.rs:114:29 + --> tests/ui/deprecations.rs:121:29 | -114 | fn pyfunction_gil_ref(_any: &PyAny) {} +121 | fn pyfunction_gil_ref(_any: &PyAny) {} | ^ error: use of deprecated method `pyo3::deprecations::OptionGilRefs::>::function_arg`: use `Option<&Bound<'_, T>>` instead for this function argument - --> tests/ui/deprecations.rs:118:36 + --> tests/ui/deprecations.rs:125:36 | -118 | fn pyfunction_option_gil_ref(_any: Option<&PyAny>) {} +125 | fn pyfunction_option_gil_ref(_any: Option<&PyAny>) {} | ^^^^^^ error: use of deprecated method `pyo3::deprecations::GilRefs::::from_py_with_arg`: use `&Bound<'_, PyAny>` as the argument for this `from_py_with` extractor - --> tests/ui/deprecations.rs:132:27 + --> tests/ui/deprecations.rs:150:27 | -132 | #[pyo3(from_py_with = "PyAny::len", item("my_object"))] +150 | #[pyo3(from_py_with = "PyAny::len", item("my_object"))] | ^^^^^^^^^^^^ error: use of deprecated method `pyo3::deprecations::GilRefs::::from_py_with_arg`: use `&Bound<'_, PyAny>` as the argument for this `from_py_with` extractor - --> tests/ui/deprecations.rs:142:27 + --> tests/ui/deprecations.rs:160:27 | -142 | #[pyo3(from_py_with = "PyAny::len")] usize, +160 | #[pyo3(from_py_with = "PyAny::len")] usize, | ^^^^^^^^^^^^ error: use of deprecated method `pyo3::deprecations::GilRefs::::from_py_with_arg`: use `&Bound<'_, PyAny>` as the argument for this `from_py_with` extractor - --> tests/ui/deprecations.rs:148:31 + --> tests/ui/deprecations.rs:166:31 | -148 | Zip(#[pyo3(from_py_with = "extract_gil_ref")] i32), +166 | Zip(#[pyo3(from_py_with = "extract_gil_ref")] i32), | ^^^^^^^^^^^^^^^^^ error: use of deprecated method `pyo3::deprecations::GilRefs::::from_py_with_arg`: use `&Bound<'_, PyAny>` as the argument for this `from_py_with` extractor - --> tests/ui/deprecations.rs:155:27 + --> tests/ui/deprecations.rs:173:27 | -155 | #[pyo3(from_py_with = "extract_gil_ref")] +173 | #[pyo3(from_py_with = "extract_gil_ref")] | ^^^^^^^^^^^^^^^^^ error: use of deprecated method `pyo3::deprecations::GilRefs::>::is_python`: use `wrap_pyfunction_bound!` instead - --> tests/ui/deprecations.rs:168:13 + --> tests/ui/deprecations.rs:186:13 | -168 | let _ = wrap_pyfunction!(double, py); +186 | 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)