Skip to content

Commit

Permalink
generate individual deprecation messages per function
Browse files Browse the repository at this point in the history
  • Loading branch information
Icxolu committed May 9, 2024
1 parent c6d253f commit bc2f105
Show file tree
Hide file tree
Showing 9 changed files with 131 additions and 76 deletions.
2 changes: 1 addition & 1 deletion guide/src/function/signature.md
Expand Up @@ -123,7 +123,7 @@ num=-1

<div class="warning">

⚠️ 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 = (...))]`.

Expand Down
52 changes: 51 additions & 1 deletion 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};

Expand Down Expand Up @@ -45,3 +48,50 @@ 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<T>` 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)]
const SIGNATURE: () = ();
const _: () = SIGNATURE;
}
} else {
TokenStream::new()
}
}
7 changes: 7 additions & 0 deletions pyo3-macros-backend/src/method.rs
Expand Up @@ -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},
Expand Down Expand Up @@ -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();
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
38 changes: 3 additions & 35 deletions pyo3-macros-backend/src/params.rs
Expand Up @@ -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! {
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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,
Expand All @@ -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) => {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
5 changes: 4 additions & 1 deletion 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;
Expand Down Expand Up @@ -630,15 +631,17 @@ 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,
ctx,
);
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;
}
Expand Down
10 changes: 0 additions & 10 deletions src/impl_/deprecations.rs
Expand Up @@ -85,13 +85,3 @@ impl<T> std::ops::Deref for OptionGilRefs<T> {
&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 {
t
}
1 change: 1 addition & 0 deletions tests/test_pyfunction.rs
Expand Up @@ -182,6 +182,7 @@ fn test_from_py_with_defaults() {

// issue 2280 combination of from_py_with and Option<T> did not compile
#[pyfunction]
#[pyo3(signature = (int=None))]
fn from_py_with_option(#[pyo3(from_py_with = "optional_int")] int: Option<i32>) -> i32 {
int.unwrap_or(0)
}
Expand Down
18 changes: 18 additions & 0 deletions tests/ui/deprecations.rs
Expand Up @@ -39,6 +39,9 @@ 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 Expand Up @@ -103,6 +106,10 @@ fn extract_bound(obj: &Bound<'_, PyAny>) -> PyResult<i32> {
obj.extract()
}

fn extract_options(obj: &Bound<'_, PyAny>) -> PyResult<Option<i32>> {
obj.extract()
}

#[pyfunction]
fn pyfunction_from_py_with(
#[pyo3(from_py_with = "extract_gil_ref")] _gil_ref: i32,
Expand All @@ -124,6 +131,17 @@ fn pyfunction_option_1(_i: u32, _any: Option<i32>) {}
#[pyfunction]
fn pyfunction_option_2(_i: u32, _any: Option<i32>) {}

#[pyfunction]
fn pyfunction_option_3(_i: u32, _any: Option<i32>, _foo: Option<String>) {}

#[pyfunction]
fn pyfunction_option_4(
_i: u32,
#[pyo3(from_py_with = "extract_options")] _any: Option<i32>,
_foo: Option<String>,
) {
}

#[derive(Debug, FromPyObject)]
pub struct Zap {
#[pyo3(item)]
Expand Down

0 comments on commit bc2f105

Please sign in to comment.