deprecate implicit default for trailing optional arguments (#4078)

* deprecate "trailing optional arguments" implicit default behaviour

* add newsfragment

* generate individual deprecation messages per function

* add migration guide entry

Author: Icxolu

guide/src/async-await.md

@@ -12,6 +12,7 @@ use futures::channel::oneshot;
 use pyo3::prelude::*;
 
 #[pyfunction]
+#[pyo3(signature=(seconds, result=None))]
 async fn sleep(seconds: f64, result: Option<PyObject>) -> Option<PyObject> {
     let (tx, rx) = oneshot::channel();
     thread::spawn(move || {

guide/src/function/signature.md

@@ -121,9 +121,22 @@ num=-1
 
 ## Trailing optional arguments
 
+<div class="warning">
+
+⚠️ 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 = (...))]`.
+
+This is done to better align the Python and Rust definition of such functions and make it more intuitive to rewrite them from Python in Rust. Specifically `def some_fn(a: int, b: Optional[int]): ...` will not automatically default `b` to `none`, but requires an explicit default if desired, where as in current `pyo3` it is handled the other way around.
+
+During the migration window a `#[pyo3(signature = (...))]` will be required to silence the deprecation warning. After support for trailing optional arguments is fully removed, the signature attribute can be removed if all arguments should be required.
+</div>
+
+
 As a convenience, functions without a `#[pyo3(signature = (...))]` option will treat trailing `Option<T>` arguments as having a default of `None`. In the example below, PyO3 will create `increment` with a signature of `increment(x, amount=None)`.
 
 ```rust
+#![allow(deprecated)]
 use pyo3::prelude::*;
 
 /// Returns a copy of `x` increased by `amount`.

guide/src/migration.md

@@ -3,6 +3,41 @@
 This guide can help you upgrade code through breaking changes from one PyO3 version to the next.
 For a detailed list of all changes, see the [CHANGELOG](changelog.md).
 
+## from 0.21.* to 0.22
+
+### Deprecation of implicit default for trailing optional arguments
+<details open>
+<summary><small>Click to expand</small></summary>
+
+With `pyo3` 0.22 the implicit `None` default for trailing `Option<T>` type argument is deprecated. To migrate, place a `#[pyo3(signature = (...))]` attribute on affected functions or methods and specify the desired behavior.
+The migration warning specifies the corresponding signature to keep the current behavior. With 0.23 the signature will be required for any function containing `Option<T>` type parameters to prevent accidental
+and unnoticed changes in behavior. With 0.24 this restriction will be lifted again and `Option<T>` type arguments will be treated as any other argument _without_ special handling.
+
+Before:
+
+```rust
+# #![allow(deprecated, dead_code)]
+# use pyo3::prelude::*;
+#[pyfunction]
+fn increment(x: u64, amount: Option<u64>) -> u64 {
+    x + amount.unwrap_or(1)
+}
+```
+
+After:
+
+```rust
+# #![allow(dead_code)]
+# use pyo3::prelude::*;
+#[pyfunction]
+#[pyo3(signature = (x, amount=None))]
+fn increment(x: u64, amount: Option<u64>) -> u64 {
+    x + amount.unwrap_or(1)
+}
+```
+
+</details>
+
 ## from 0.20.* to 0.21
 <details open>
 <summary><small>Click to expand</small></summary>

newsfragments/4078.changed.md

@@ -0,0 +1 @@
+deprecate implicit default for trailing optional arguments

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<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)]
+            #[allow(dead_code)]
+            const SIGNATURE: () = ();
+            const _: () = SIGNATURE;
+        }
+    } else {
+        TokenStream::new()
+    }
+}

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

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;
@@ -637,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;
             }

pytests/src/datetime.rs

@@ -25,6 +25,7 @@ fn date_from_timestamp(py: Python<'_>, timestamp: i64) -> PyResult<Bound<'_, PyD
 }
 
 #[pyfunction]
+#[pyo3(signature=(hour, minute, second, microsecond, tzinfo=None))]
 fn make_time<'py>(
     py: Python<'py>,
     hour: u8,
@@ -101,6 +102,7 @@ fn get_delta_tuple<'py>(delta: &Bound<'py, PyDelta>) -> Bound<'py, PyTuple> {
 
 #[allow(clippy::too_many_arguments)]
 #[pyfunction]
+#[pyo3(signature=(year, month, day, hour, minute, second, microsecond, tzinfo=None))]
 fn make_datetime<'py>(
     py: Python<'py>,
     year: i32,
@@ -159,6 +161,7 @@ fn get_datetime_tuple_fold<'py>(dt: &Bound<'py, PyDateTime>) -> Bound<'py, PyTup
 }
 
 #[pyfunction]
+#[pyo3(signature=(ts, tz=None))]
 fn datetime_from_timestamp<'py>(
     py: Python<'py>,
     ts: f64,

src/tests/hygiene/pymethods.rs

@@ -309,6 +309,7 @@ impl Dummy {
         0
     }
 
+    #[pyo3(signature=(ndigits=::std::option::Option::None))]
     fn __round__(&self, ndigits: ::std::option::Option<u32>) -> u32 {
         0
     }

tests/test_arithmetics.rs

@@ -35,6 +35,7 @@ impl UnaryArithmetic {
         Self::new(self.inner.abs())
     }
 
+    #[pyo3(signature=(_ndigits=None))]
     fn __round__(&self, _ndigits: Option<u32>) -> Self {
         Self::new(self.inner.round())
     }

tests/test_mapping.rs

@@ -21,6 +21,7 @@ struct Mapping {
 #[pymethods]
 impl Mapping {
     #[new]
+    #[pyo3(signature=(elements=None))]
     fn new(elements: Option<&Bound<'_, PyList>>) -> PyResult<Self> {
         if let Some(pylist) = elements {
             let mut elems = HashMap::with_capacity(pylist.len());
@@ -59,6 +60,7 @@ impl Mapping {
         }
     }
 
+    #[pyo3(signature=(key, default=None))]
     fn get(&self, py: Python<'_>, key: &str, default: Option<PyObject>) -> Option<PyObject> {
         self.index
             .get(key)

tests/test_methods.rs

@@ -187,6 +187,7 @@ impl MethSignature {
     fn get_optional2(&self, test: Option<i32>) -> Option<i32> {
         test
     }
+    #[pyo3(signature=(_t1 = None, t2 = None, _t3 = None))]
     fn get_optional_positional(
         &self,
         _t1: Option<i32>,
@@ -745,11 +746,13 @@ impl MethodWithPyClassArg {
     fn inplace_add_pyref(&self, mut other: PyRefMut<'_, MethodWithPyClassArg>) {
         other.value += self.value;
     }
+    #[pyo3(signature=(other = None))]
     fn optional_add(&self, other: Option<&MethodWithPyClassArg>) -> MethodWithPyClassArg {
         MethodWithPyClassArg {
             value: self.value + other.map(|o| o.value).unwrap_or(10),
         }
     }
+    #[pyo3(signature=(other = None))]
     fn optional_inplace_add(&self, other: Option<&mut MethodWithPyClassArg>) {
         if let Some(other) = other {
             other.value += self.value;
@@ -851,6 +854,7 @@ struct FromSequence {
 #[pymethods]
 impl FromSequence {
     #[new]
+    #[pyo3(signature=(seq = None))]
     fn new(seq: Option<&Bound<'_, PySequence>>) -> PyResult<Self> {
         if let Some(seq) = seq {
             Ok(FromSequence {
@@ -1026,6 +1030,7 @@ macro_rules! issue_1506 {
 issue_1506!(
     #[pymethods]
     impl Issue1506 {
+        #[pyo3(signature = (_arg, _args, _kwargs=None))]
         fn issue_1506(
             &self,
             _py: Python<'_>,
@@ -1035,6 +1040,7 @@ issue_1506!(
         ) {
         }
 
+        #[pyo3(signature = (_arg, _args, _kwargs=None))]
         fn issue_1506_mut(
             &mut self,
             _py: Python<'_>,
@@ -1044,6 +1050,7 @@ issue_1506!(
         ) {
         }
 
+        #[pyo3(signature = (_arg, _args, _kwargs=None))]
         fn issue_1506_custom_receiver(
             _slf: Py<Self>,
             _py: Python<'_>,
@@ -1053,6 +1060,7 @@ issue_1506!(
         ) {
         }
 
+        #[pyo3(signature = (_arg, _args, _kwargs=None))]
         fn issue_1506_custom_receiver_explicit(
             _slf: Py<Issue1506>,
             _py: Python<'_>,
@@ -1063,6 +1071,7 @@ issue_1506!(
         }
 
         #[new]
+        #[pyo3(signature = (_arg, _args, _kwargs=None))]
         fn issue_1506_new(
             _py: Python<'_>,
             _arg: &Bound<'_, PyAny>,
@@ -1081,6 +1090,7 @@ issue_1506!(
         fn issue_1506_setter(&self, _py: Python<'_>, _value: i32) {}
 
         #[staticmethod]
+        #[pyo3(signature = (_arg, _args, _kwargs=None))]
         fn issue_1506_static(
             _py: Python<'_>,
             _arg: &Bound<'_, PyAny>,
@@ -1090,6 +1100,7 @@ issue_1506!(
         }
 
         #[classmethod]
+        #[pyo3(signature = (_arg, _args, _kwargs=None))]
         fn issue_1506_class(
             _cls: &Bound<'_, PyType>,
             _py: Python<'_>,

tests/test_pyfunction.rs

@@ -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)
     }
@@ -216,6 +217,7 @@ struct ValueClass {
 }
 
 #[pyfunction]
+#[pyo3(signature=(str_arg, int_arg, tuple_arg, option_arg = None, struct_arg = None))]
 fn conversion_error(
     str_arg: &str,
     int_arg: i64,
@@ -542,6 +544,7 @@ fn test_some_wrap_arguments() {
 #[test]
 fn test_reference_to_bound_arguments() {
     #[pyfunction]
+    #[pyo3(signature = (x, y = None))]
     fn reference_args<'py>(
         x: &Bound<'py, PyAny>,
         y: Option<&Bound<'py, PyAny>>,

tests/test_sequence.rs

@@ -17,6 +17,7 @@ struct ByteSequence {
 #[pymethods]
 impl ByteSequence {
     #[new]
+    #[pyo3(signature=(elements = None))]
     fn new(elements: Option<&Bound<'_, PyList>>) -> PyResult<Self> {
         if let Some(pylist) = elements {
             let mut elems = Vec::with_capacity(pylist.len());

tests/test_text_signature.rs

@@ -142,6 +142,7 @@ fn test_auto_test_signature_function() {
     }
 
     #[pyfunction]
+    #[pyo3(signature=(a, b=None, c=None))]
     fn my_function_6(a: i32, b: Option<i32>, c: Option<i32>) {
         let _ = (a, b, c);
     }