From 8e8b48416940f1ca5899b5c949fd1e24a9211dd8 Mon Sep 17 00:00:00 2001 From: David Hewitt <1939362+davidhewitt@users.noreply.github.com> Date: Tue, 25 Oct 2022 07:23:21 +0100 Subject: [PATCH] add `#[pyo3(signature = (...))]` attribute (#2702) --- examples/decorator/src/lib.rs | 2 +- guide/src/class.md | 19 +- guide/src/class/call.md | 6 +- guide/src/function.md | 5 + guide/src/function/signature.md | 144 ++++- newsfragments/2702.added.md | 1 + newsfragments/2702.changed.md | 1 + pyo3-macros-backend/src/attributes.rs | 3 +- pyo3-macros-backend/src/deprecations.rs | 6 + pyo3-macros-backend/src/method.rs | 103 ++-- pyo3-macros-backend/src/params.rs | 121 ++-- pyo3-macros-backend/src/pyfunction.rs | 264 +++------ .../src/pyfunction/signature.rs | 546 ++++++++++++++++++ pyo3-macros-backend/src/pymethod.rs | 10 +- pyo3-macros-backend/src/utils.rs | 2 +- pyo3-macros/src/lib.rs | 4 +- pytests/noxfile.py | 2 +- pytests/src/deprecated_pyfunctions.rs | 68 +++ pytests/src/lib.rs | 8 + pytests/src/pyfunctions.rs | 14 +- pytests/tests/test_deprecated_pyfunctions.py | 91 +++ src/impl_/deprecations.rs | 12 + src/test_hygiene/pymethods.rs | 8 +- tests/test_compile_error.rs | 1 + tests/test_macros.rs | 6 +- tests/test_methods.rs | 527 ++++++++++++++++- tests/test_module.rs | 16 +- tests/test_pyfunction.rs | 4 +- tests/test_text_signature.rs | 8 +- tests/test_variable_arguments.rs | 4 +- tests/test_variable_arguments_deprecated.rs | 49 ++ tests/ui/deprecations.rs | 14 +- tests/ui/deprecations.stderr | 20 +- tests/ui/invalid_pyfunction_signatures.rs | 59 ++ tests/ui/invalid_pyfunction_signatures.stderr | 53 ++ tests/ui/invalid_pymethods.rs | 6 - tests/ui/invalid_pymethods.stderr | 12 +- 37 files changed, 1776 insertions(+), 443 deletions(-) create mode 100644 newsfragments/2702.added.md create mode 100644 newsfragments/2702.changed.md create mode 100644 pyo3-macros-backend/src/pyfunction/signature.rs create mode 100644 pytests/src/deprecated_pyfunctions.rs create mode 100644 pytests/tests/test_deprecated_pyfunctions.py create mode 100644 tests/test_variable_arguments_deprecated.rs create mode 100644 tests/ui/invalid_pyfunction_signatures.rs create mode 100644 tests/ui/invalid_pyfunction_signatures.stderr diff --git a/examples/decorator/src/lib.rs b/examples/decorator/src/lib.rs index de3fe125..bc369c62 100644 --- a/examples/decorator/src/lib.rs +++ b/examples/decorator/src/lib.rs @@ -36,7 +36,7 @@ impl PyCounter { self.count.get() } - #[args(args = "*", kwargs = "**")] + #[pyo3(signature = (*args, **kwargs))] fn __call__( &self, py: Python<'_>, diff --git a/guide/src/class.md b/guide/src/class.md index 19f5eb88..5e976817 100644 --- a/guide/src/class.md +++ b/guide/src/class.md @@ -632,9 +632,9 @@ impl MyClass { ## Method arguments -Similar to `#[pyfunction]`, the `#[args]` attribute can be used to specify the way that `#[pymethods]` accept arguments. Consult the documentation for [`function signatures`](./function/signature.md) to see the parameters this attribute accepts. +Similar to `#[pyfunction]`, the `#[pyo3(signature = (...))]` attribute can be used to specify the way that `#[pymethods]` accept arguments. Consult the documentation for [`function signatures`](./function/signature.md) to see the parameters this attribute accepts. -The following example defines a class `MyClass` with a method `method`. This method has an `#[args]` attribute which sets default values for `num` and `name`, and indicates that `py_args` should collect all extra positional arguments and `py_kwargs` all extra keyword arguments: +The following example defines a class `MyClass` with a method `method`. This method has a signature which sets default values for `num` and `name`, and indicates that `py_args` should collect all extra positional arguments and `py_kwargs` all extra keyword arguments: ```rust # use pyo3::prelude::*; @@ -647,29 +647,24 @@ use pyo3::types::{PyDict, PyTuple}; #[pymethods] impl MyClass { #[new] - #[args(num = "-1")] + #[pyo3(signature = (num=-1))] fn new(num: i32) -> Self { MyClass { num } } - #[args( - num = "10", - py_args = "*", - name = "\"Hello\"", - py_kwargs = "**" - )] + #[pyo3(signature = (num=10, *py_args, name="Hello", **py_kwargs))] fn method( &mut self, num: i32, - name: &str, py_args: &PyTuple, + name: &str, py_kwargs: Option<&PyDict>, ) -> String { let num_before = self.num; self.num = num; format!( - "py_args={:?}, py_kwargs={:?}, name={}, num={} num_before={}", - py_args, py_kwargs, name, self.num, num_before, + "num={} (was previously={}), py_args={:?}, name={}, py_kwargs={:?} ", + num, num_before, py_args, name, py_kwargs, ) } } diff --git a/guide/src/class/call.md b/guide/src/class/call.md index 9d3a6188..8cf188c9 100644 --- a/guide/src/class/call.md +++ b/guide/src/class/call.md @@ -71,7 +71,7 @@ def Counter(wraps): A [previous implementation] used a normal `u64`, which meant it required a `&mut self` receiver to update the count: ```rust,ignore -#[args(args = "*", kwargs = "**")] +#[pyo3(signature = (*args, **kwargs))] fn __call__(&mut self, py: Python<'_>, args: &PyTuple, kwargs: Option<&PyDict>) -> PyResult> { self.count += 1; let name = self.wraps.getattr(py, "__name__")?; @@ -98,7 +98,7 @@ As a result, something innocent like this will raise an exception: def say_hello(): if say_hello.count < 2: print(f"hello from decorator") - + say_hello() # RuntimeError: Already borrowed ``` @@ -113,4 +113,4 @@ This shows the dangers of running arbitrary Python code - note that "running arb This is especially important if you are writing unsafe code; Python code must never be able to cause undefined behavior. You must ensure that your Rust code is in a consistent state before doing any of the above things. [previous implementation]: https://github.com/PyO3/pyo3/discussions/2598 "Thread Safe Decorator · Discussion #2598 · PyO3/pyo3" -[`Cell`]: https://doc.rust-lang.org/std/cell/struct.Cell.html "Cell in std::cell - Rust" \ No newline at end of file +[`Cell`]: https://doc.rust-lang.org/std/cell/struct.Cell.html "Cell in std::cell - Rust" diff --git a/guide/src/function.md b/guide/src/function.md index a370254d..1821b93d 100644 --- a/guide/src/function.md +++ b/guide/src/function.md @@ -23,6 +23,7 @@ This chapter of the guide explains full usage of the `#[pyfunction]` attribute. - [Function options](#function-options) - [`#[pyo3(name = "...")]`](#name) + - [`#[pyo3(signature = (...))]`](#signature) - [`#[pyo3(text_signature = "...")]`](#text_signature) - [`#[pyo3(pass_module)]`](#pass_module) - [Per-argument options](#per-argument-options) @@ -64,6 +65,10 @@ The `#[pyo3]` attribute can be used to modify properties of the generated Python # }); ``` + - `#[pyo3(signature = (...))]` + + Defines the function signature in Python. See [Function Signatures](./function/signature.md). + - `#[pyo3(text_signature = "...")]` Sets the function signature visible in Python tooling (such as via [`inspect.signature`]). diff --git a/guide/src/function/signature.md b/guide/src/function/signature.md index 259fda19..4275a856 100644 --- a/guide/src/function/signature.md +++ b/guide/src/function/signature.md @@ -2,12 +2,124 @@ The `#[pyfunction]` attribute also accepts parameters to control how the generated Python function accepts arguments. Just like in Python, arguments can be positional-only, keyword-only, or accept either. `*args` lists and `**kwargs` dicts can also be accepted. These parameters also work for `#[pymethods]` which will be introduced in the [Python Classes](../class.md) section of the guide. -Like Python, by default PyO3 accepts all arguments as either positional or keyword arguments. The extra arguments to `#[pyfunction]` modify this behaviour. For example, below is a function that accepts arbitrary keyword arguments (`**kwargs` in Python syntax) and returns the number that was passed: +Like Python, by default PyO3 accepts all arguments as either positional or keyword arguments. There are two ways to modify this behaviour: + - The `#[pyo3(signature = (...))]` option which allows writing a signature in Python syntax. + - Extra arguments directly to `#[pyfunction]`. (See deprecated form) + +## Using `#[pyo3(signature = (...))]` + +For example, below is a function that accepts arbitrary keyword arguments (`**kwargs` in Python syntax) and returns the number that was passed: ```rust use pyo3::prelude::*; use pyo3::types::PyDict; +#[pyfunction] +#[pyo3(signature = (**kwds))] +fn num_kwds(kwds: Option<&PyDict>) -> usize { + kwds.map_or(0, |dict| dict.len()) +} + +#[pymodule] +fn module_with_functions(py: Python<'_>, m: &PyModule) -> PyResult<()> { + m.add_function(wrap_pyfunction!(num_kwds, m)?).unwrap(); + Ok(()) +} +``` + +Just like in Python, the following constructs can be part of the signature:: + + * `/`: positional-only arguments separator, each parameter defined before `/` is a positional-only parameter. + * `*`: var arguments separator, each parameter defined after `*` is a keyword-only parameter. + * `*args`: "args" is var args. Type of the `args` parameter has to be `&PyTuple`. + * `**kwargs`: "kwargs" receives keyword arguments. The type of the `kwargs` parameter has to be `Option<&PyDict>`. + * `arg=Value`: arguments with default value. + If the `arg` argument is defined after var arguments, it is treated as a keyword-only argument. + Note that `Value` has to be valid rust code, PyO3 just inserts it into the generated + code unmodified. + +Example: +```rust +# use pyo3::prelude::*; +use pyo3::types::{PyDict, PyTuple}; +# +# #[pyclass] +# struct MyClass { +# num: i32, +# } +#[pymethods] +impl MyClass { + #[new] + #[pyo3(signature = (num=-1))] + fn new(num: i32) -> Self { + MyClass { num } + } + + #[pyo3(signature = (num=10, *py_args, name="Hello", **py_kwargs))] + fn method( + &mut self, + num: i32, + py_args: &PyTuple, + name: &str, + py_kwargs: Option<&PyDict>, + ) -> String { + let num_before = self.num; + self.num = num; + format!( + "num={} (was previously={}), py_args={:?}, name={}, py_kwargs={:?} ", + num, num_before, py_args, name, py_kwargs, + ) + } + + fn make_change(&mut self, num: i32) -> PyResult { + self.num = num; + Ok(format!("num={}", self.num)) + } +} +``` +N.B. the position of the `/` and `*` arguments (if included) control the system of handling positional and keyword arguments. In Python: +```python +import mymodule + +mc = mymodule.MyClass() +print(mc.method(44, False, "World", 666, x=44, y=55)) +print(mc.method(num=-1, name="World")) +print(mc.make_change(44, False)) +``` +Produces output: +```text +py_args=('World', 666), py_kwargs=Some({'x': 44, 'y': 55}), name=Hello, num=44 +py_args=(), py_kwargs=None, name=World, num=-1 +num=44 +num=-1 +``` + +> Note: for keywords like `struct`, to use it as a function argument, use "raw ident" syntax `r#struct` in both the signature and the function definition: +> +> ```rust +> # #![allow(dead_code)] +> # use pyo3::prelude::*; +> #[pyfunction(signature = (r#struct = "foo"))] +> fn function_with_keyword(r#struct: &str) { +> # let _ = r#struct; +> /* ... */ +> } +> ``` + +## Deprecated form + +The `#[pyfunction]` macro can take the argument specification directly, but this method is deprecated in PyO3 0.18 because the `#[pyo3(signature)]` option offers a simpler syntax and better validation. + +The `#[pymethods]` macro has an `#[args]` attribute which accepts the deprecated form. + +Below are the same examples as above which using the deprecated syntax: + +```rust +# #![allow(deprecated)] + +use pyo3::prelude::*; +use pyo3::types::PyDict; + #[pyfunction(kwds="**")] fn num_kwds(kwds: Option<&PyDict>) -> usize { kwds.map_or(0, |dict| dict.len()) @@ -38,6 +150,7 @@ The following parameters can be passed to the `#[pyfunction]` attribute: Example: ```rust +# #![allow(deprecated)] # use pyo3::prelude::*; use pyo3::types::{PyDict, PyTuple}; # @@ -62,15 +175,16 @@ impl MyClass { fn method( &mut self, num: i32, - name: &str, py_args: &PyTuple, + name: &str, py_kwargs: Option<&PyDict>, - ) -> PyResult { + ) -> String { + let num_before = self.num; self.num = num; - Ok(format!( - "py_args={:?}, py_kwargs={:?}, name={}, num={}", - py_args, py_kwargs, name, self.num - )) + format!( + "num={} (was previously={}), py_args={:?}, name={}, py_kwargs={:?} ", + num, num_before, py_args, name, py_kwargs, + ) } fn make_change(&mut self, num: i32) -> PyResult { @@ -79,19 +193,3 @@ impl MyClass { } } ``` -N.B. the position of the `"/"` and `"*"` arguments (if included) control the system of handling positional and keyword arguments. In Python: -```python -import mymodule - -mc = mymodule.MyClass() -print(mc.method(44, False, "World", 666, x=44, y=55)) -print(mc.method(num=-1, name="World")) -print(mc.make_change(44, False)) -``` -Produces output: -```text -py_args=('World', 666), py_kwargs=Some({'x': 44, 'y': 55}), name=Hello, num=44 -py_args=(), py_kwargs=None, name=World, num=-1 -num=44 -num=-1 -``` diff --git a/newsfragments/2702.added.md b/newsfragments/2702.added.md new file mode 100644 index 00000000..6fa67fe1 --- /dev/null +++ b/newsfragments/2702.added.md @@ -0,0 +1 @@ +Add `#[pyo3(signature = (...))]` option for `#[pyfunction]` and `#[pymethods]`. diff --git a/newsfragments/2702.changed.md b/newsfragments/2702.changed.md new file mode 100644 index 00000000..e39fd3a8 --- /dev/null +++ b/newsfragments/2702.changed.md @@ -0,0 +1 @@ +Deprecate `#[args]` attribute and passing "args" specification directly to `#[pyfunction]` in favour of the new `#[pyo3(signature = (...))]` option. diff --git a/pyo3-macros-backend/src/attributes.rs b/pyo3-macros-backend/src/attributes.rs index cf35f20a..6235c7e1 100644 --- a/pyo3-macros-backend/src/attributes.rs +++ b/pyo3-macros-backend/src/attributes.rs @@ -9,6 +9,7 @@ use syn::{ }; pub mod kw { + syn::custom_keyword!(args); syn::custom_keyword!(annotation); syn::custom_keyword!(attribute); syn::custom_keyword!(dict); @@ -42,7 +43,7 @@ pub struct KeywordAttribute { } /// A helper type which parses the inner type via a literal string -/// e.g. LitStrValue -> parses "some::path" in quotes. +/// e.g. `LitStrValue` -> parses "some::path" in quotes. #[derive(Clone, Debug, PartialEq, Eq)] pub struct LitStrValue(pub T); diff --git a/pyo3-macros-backend/src/deprecations.rs b/pyo3-macros-backend/src/deprecations.rs index 198b1635..07b2f590 100644 --- a/pyo3-macros-backend/src/deprecations.rs +++ b/pyo3-macros-backend/src/deprecations.rs @@ -1,14 +1,20 @@ use proc_macro2::{Span, TokenStream}; use quote::{quote_spanned, ToTokens}; +// Clippy complains all these variants have the same prefix "Py"... +#[allow(clippy::enum_variant_names)] pub enum Deprecation { PyClassGcOption, + PyFunctionArguments, + PyMethodArgsAttribute, } impl Deprecation { fn ident(&self, span: Span) -> syn::Ident { let string = match self { Deprecation::PyClassGcOption => "PYCLASS_GC_OPTION", + Deprecation::PyFunctionArguments => "PYFUNCTION_ARGUMENTS", + Deprecation::PyMethodArgsAttribute => "PYMETHODS_ARGS_ATTRIBUTE", }; syn::Ident::new(string, span) } diff --git a/pyo3-macros-backend/src/method.rs b/pyo3-macros-backend/src/method.rs index 3165c5c9..742f50d4 100644 --- a/pyo3-macros-backend/src/method.rs +++ b/pyo3-macros-backend/src/method.rs @@ -3,11 +3,11 @@ use std::borrow::Cow; use crate::attributes::TextSignatureAttribute; -use crate::params::{accept_args_kwargs, impl_arg_params}; +use crate::deprecations::{Deprecation, Deprecations}; +use crate::params::impl_arg_params; use crate::pyfunction::PyFunctionOptions; -use crate::pyfunction::{PyFunctionArgPyO3Attributes, PyFunctionSignature}; +use crate::pyfunction::{DeprecatedArgs, FunctionSignature, PyFunctionArgPyO3Attributes}; use crate::utils::{self, PythonDoc}; -use crate::{deprecations::Deprecations, pyfunction::Argument}; use proc_macro2::{Span, TokenStream}; use quote::ToTokens; use quote::{quote, quote_spanned}; @@ -22,8 +22,11 @@ pub struct FnArg<'a> { pub mutability: &'a Option, pub ty: &'a syn::Type, pub optional: Option<&'a syn::Type>, + pub default: Option, pub py: bool, pub attrs: PyFunctionArgPyO3Attributes, + pub is_varargs: bool, + pub is_kwargs: bool, } impl<'a> FnArg<'a> { @@ -55,8 +58,11 @@ impl<'a> FnArg<'a> { mutability, ty: &cap.ty, optional: utils::option_type_argument(&cap.ty), + default: None, py: utils::is_python(&cap.ty), attrs: arg_attrs, + is_varargs: false, + is_kwargs: false, }) } } @@ -193,11 +199,10 @@ impl CallingConvention { /// /// Different other slots (tp_call, tp_new) can have other requirements /// and are set manually (see `parse_fn_type` below). - pub fn from_args(args: &[FnArg<'_>], attrs: &[Argument]) -> Self { - let (_, accept_kwargs) = accept_args_kwargs(attrs); - if args.is_empty() { + pub fn from_signature(signature: &FunctionSignature<'_>) -> Self { + if signature.arguments.is_empty() { Self::Noargs - } else if accept_kwargs { + } else if signature.python_signature.accepts_kwargs { // for functions that accept **kwargs, always prefer varargs Self::Varargs } else if cfg!(not(feature = "abi3")) { @@ -216,8 +221,7 @@ pub struct FnSpec<'a> { // Wrapped python name. This should not have any leading r#. // r# can be removed by syn::ext::IdentExt::unraw() pub python_name: syn::Ident, - pub attrs: Vec, - pub args: Vec>, + pub signature: FunctionSignature<'a>, pub output: syn::Type, pub doc: PythonDoc, pub deprecations: Deprecations, @@ -265,14 +269,16 @@ impl<'a> FnSpec<'a> { let PyFunctionOptions { text_signature, name, + mut deprecations, + signature, .. } = options; let MethodAttributes { ty: fn_type_attr, - args: fn_attrs, + deprecated_args, mut python_name, - } = parse_method_attributes(meth_attrs, name.map(|name| name.value.0))?; + } = parse_method_attributes(meth_attrs, name.map(|name| name.value.0), &mut deprecations)?; let (fn_type, skip_first_arg, fixed_convention) = Self::parse_fn_type(sig, fn_type_attr, &mut python_name)?; @@ -302,19 +308,30 @@ impl<'a> FnSpec<'a> { .collect::>()? }; + let signature = if let Some(signature) = signature { + ensure_spanned!( + deprecated_args.is_none(), + signature.kw.span() => "cannot define both function signature and legacy arguments" + ); + FunctionSignature::from_arguments_and_attribute(arguments, signature)? + } else if let Some(deprecated_args) = deprecated_args { + FunctionSignature::from_arguments_and_deprecated_args(arguments, deprecated_args)? + } else { + FunctionSignature::from_arguments(arguments) + }; + let convention = - fixed_convention.unwrap_or_else(|| CallingConvention::from_args(&arguments, &fn_attrs)); + fixed_convention.unwrap_or_else(|| CallingConvention::from_signature(&signature)); Ok(FnSpec { tp: fn_type, name, convention, python_name, - attrs: fn_attrs, - args: arguments, + signature, output: ty, doc, - deprecations: Deprecations::new(), + deprecations, text_signature, unsafety: sig.unsafety, }) @@ -412,45 +429,6 @@ impl<'a> FnSpec<'a> { Ok((fn_type, skip_first_arg, fixed_convention)) } - pub fn default_value(&self, name: &syn::Ident) -> Option { - for s in &self.attrs { - match s { - Argument::Arg(path, opt) | Argument::Kwarg(path, opt) => { - if path.is_ident(name) { - if let Some(val) = opt { - let i: syn::Expr = syn::parse_str(val).unwrap(); - return Some(i.into_token_stream()); - } - } - } - _ => (), - } - } - None - } - - pub fn is_pos_only(&self, name: &syn::Ident) -> bool { - for s in &self.attrs { - if let Argument::PosOnlyArg(path, _) = s { - if path.is_ident(name) { - return true; - } - } - } - false - } - - pub fn is_kw_only(&self, name: &syn::Ident) -> bool { - for s in &self.attrs { - if let Argument::Kwarg(path, _) = s { - if path.is_ident(name) { - return true; - } - } - } - false - } - /// Return a C wrapper function for this signature. pub fn get_wrapper_function( &self, @@ -626,19 +604,20 @@ impl<'a> FnSpec<'a> { } } -#[derive(Clone, PartialEq, Debug)] +#[derive(Debug)] struct MethodAttributes { ty: Option, - args: Vec, + deprecated_args: Option, python_name: Option, } fn parse_method_attributes( attrs: &mut Vec, mut python_name: Option, + deprecations: &mut Deprecations, ) -> Result { let mut new_attrs = Vec::new(); - let mut args = Vec::new(); + let mut deprecated_args = None; let mut ty: Option = None; macro_rules! set_ty { @@ -736,8 +715,12 @@ fn parse_method_attributes( } }; } else if path.is_ident("args") { - let attrs = PyFunctionSignature::from_meta(&nested)?; - args.extend(attrs.arguments) + ensure_spanned!( + deprecated_args.is_none(), + nested.span() => "args may only be specified once" + ); + deprecations.push(Deprecation::PyMethodArgsAttribute, nested.span()); + deprecated_args = Some(DeprecatedArgs::from_meta(&nested)?); } else { new_attrs.push(attr) } @@ -750,7 +733,7 @@ fn parse_method_attributes( Ok(MethodAttributes { ty, - args, + deprecated_args, python_name, }) } diff --git a/pyo3-macros-backend/src/params.rs b/pyo3-macros-backend/src/params.rs index a1c20e1a..ab784591 100644 --- a/pyo3-macros-backend/src/params.rs +++ b/pyo3-macros-backend/src/params.rs @@ -2,50 +2,28 @@ use crate::{ method::{FnArg, FnSpec}, - pyfunction::Argument, + pyfunction::FunctionSignature, }; use proc_macro2::{Span, TokenStream}; use quote::{quote, quote_spanned}; -use syn::ext::IdentExt; use syn::spanned::Spanned; use syn::Result; -/// Determine if the function gets passed a *args tuple or **kwargs dict. -pub fn accept_args_kwargs(attrs: &[Argument]) -> (bool, bool) { - let (mut accept_args, mut accept_kwargs) = (false, false); - - for s in attrs { - match s { - Argument::VarArgs(_) => accept_args = true, - Argument::KeywordArgs(_) => accept_kwargs = true, - _ => continue, - } - } - - (accept_args, accept_kwargs) -} - /// Return true if the argument list is simply (*args, **kwds). -pub fn is_forwarded_args(args: &[FnArg<'_>], attrs: &[Argument]) -> bool { - args.len() == 2 && is_args(attrs, args[0].name) && is_kwargs(attrs, args[1].name) -} - -fn is_args(attrs: &[Argument], name: &syn::Ident) -> bool { - for s in attrs.iter() { - if let Argument::VarArgs(path) = s { - return path.is_ident(name); - } - } - false -} - -fn is_kwargs(attrs: &[Argument], name: &syn::Ident) -> bool { - for s in attrs.iter() { - if let Argument::KeywordArgs(path) = s { - return path.is_ident(name); - } - } - false +pub fn is_forwarded_args(signature: &FunctionSignature<'_>) -> bool { + matches!( + signature.arguments.as_slice(), + [ + FnArg { + is_varargs: true, + .. + }, + FnArg { + is_kwargs: true, + .. + }, + ] + ) } pub fn impl_arg_params( @@ -54,19 +32,20 @@ pub fn impl_arg_params( py: &syn::Ident, fastcall: bool, ) -> Result<(TokenStream, Vec)> { - if spec.args.is_empty() { + if spec.signature.arguments.is_empty() { return Ok((TokenStream::new(), vec![])); } let args_array = syn::Ident::new("output", Span::call_site()); - if !fastcall && is_forwarded_args(&spec.args, &spec.attrs) { + if !fastcall && is_forwarded_args(&spec.signature) { // In the varargs convention, we can just pass though if the signature // is (*args, **kwds). let arg_convert = spec - .args + .signature + .arguments .iter() - .map(|arg| impl_arg_param(arg, spec, &mut 0, py, &args_array)) + .map(|arg| impl_arg_param(arg, &mut 0, py, &args_array)) .collect::>()?; return Ok(( quote! { @@ -77,55 +56,42 @@ pub fn impl_arg_params( )); }; - let mut positional_parameter_names = Vec::new(); - let mut positional_only_parameters = 0usize; - let mut required_positional_parameters = 0usize; - let mut keyword_only_parameters = Vec::new(); - - for arg in &spec.args { - if arg.py || is_args(&spec.attrs, arg.name) || is_kwargs(&spec.attrs, arg.name) { - continue; - } - let name = arg.name.unraw().to_string(); - let posonly = spec.is_pos_only(arg.name); - let kwonly = spec.is_kw_only(arg.name); - let required = !(arg.optional.is_some() || spec.default_value(arg.name).is_some()); - - if kwonly { - keyword_only_parameters.push(quote! { + let positional_parameter_names = &spec.signature.python_signature.positional_parameters; + let positional_only_parameters = &spec.signature.python_signature.positional_only_parameters; + let required_positional_parameters = &spec + .signature + .python_signature + .required_positional_parameters; + let keyword_only_parameters = spec + .signature + .python_signature + .keyword_only_parameters + .iter() + .map(|(name, required)| { + quote! { _pyo3::impl_::extract_argument::KeywordOnlyParameterDescription { name: #name, required: #required, } - }); - } else { - positional_parameter_names.push(name); - - if required { - required_positional_parameters = positional_parameter_names.len(); } - if posonly { - positional_only_parameters += 1; - } - } - } + }); let num_params = positional_parameter_names.len() + keyword_only_parameters.len(); let mut option_pos = 0; let param_conversion = spec - .args + .signature + .arguments .iter() - .map(|arg| impl_arg_param(arg, spec, &mut option_pos, py, &args_array)) + .map(|arg| impl_arg_param(arg, &mut option_pos, py, &args_array)) .collect::>()?; - let (accept_args, accept_kwargs) = accept_args_kwargs(&spec.attrs); - let args_handler = if accept_args { + let args_handler = if spec.signature.python_signature.accepts_varargs { quote! { _pyo3::impl_::extract_argument::TupleVarargs } } else { quote! { _pyo3::impl_::extract_argument::NoVarargs } }; - let kwargs_handler = if accept_kwargs { + let kwargs_handler = if spec.signature.python_signature.accepts_kwargs { quote! { _pyo3::impl_::extract_argument::DictVarkeywords } } else { quote! { _pyo3::impl_::extract_argument::NoVarkeywords } @@ -182,7 +148,6 @@ pub fn impl_arg_params( /// index and the index in option diverge when using py: Python fn impl_arg_param( arg: &FnArg<'_>, - spec: &FnSpec<'_>, option_pos: &mut usize, py: &syn::Ident, args_array: &syn::Ident, @@ -200,7 +165,7 @@ fn impl_arg_param( let name = arg.name; let name_str = name.to_string(); - if is_args(&spec.attrs, name) { + if arg.is_varargs { ensure_spanned!( arg.optional.is_none(), arg.name.span() => "args cannot be optional" @@ -212,7 +177,7 @@ fn impl_arg_param( #name_str )? }); - } else if is_kwargs(&spec.attrs, name) { + } else if arg.is_kwargs { ensure_spanned!( arg.optional.is_some(), arg.name.span() => "kwargs must be Option<_>" @@ -222,7 +187,7 @@ fn impl_arg_param( _kwargs.map(::std::convert::AsRef::as_ref), &mut { _pyo3::impl_::extract_argument::FunctionArgumentHolder::INIT }, #name_str, - || None + || ::std::option::Option::None )? }); } @@ -230,7 +195,7 @@ fn impl_arg_param( let arg_value = quote_arg_span!(#args_array[#option_pos]); *option_pos += 1; - let mut default = spec.default_value(name); + let mut default = arg.default.as_ref().map(|expr| quote!(#expr)); // Option arguments have special treatment: the default should be specified _without_ the // Some() wrapper. Maybe this should be changed in future?! diff --git a/pyo3-macros-backend/src/pyfunction.rs b/pyo3-macros-backend/src/pyfunction.rs index c97495b1..5e401818 100644 --- a/pyo3-macros-backend/src/pyfunction.rs +++ b/pyo3-macros-backend/src/pyfunction.rs @@ -7,40 +7,23 @@ use crate::{ self, get_pyo3_options, take_attributes, take_pyo3_options, CrateAttribute, FromPyWithAttribute, NameAttribute, TextSignatureAttribute, }, - deprecations::Deprecations, + deprecations::{Deprecation, Deprecations}, method::{self, CallingConvention, FnArg}, pymethod::check_generic, utils::{self, ensure_not_async_fn, get_pyo3_crate}, }; -use proc_macro2::{Span, TokenStream}; +use proc_macro2::TokenStream; use quote::{format_ident, quote}; -use syn::punctuated::Punctuated; -use syn::{ext::IdentExt, spanned::Spanned, NestedMeta, Path, Result}; +use syn::{ext::IdentExt, spanned::Spanned, NestedMeta, Result}; use syn::{ parse::{Parse, ParseBuffer, ParseStream}, token::Comma, }; +use syn::{punctuated::Punctuated, Path}; -#[derive(Debug, Clone, PartialEq, Eq)] -pub enum Argument { - PosOnlyArgsSeparator, - VarArgsSeparator, - VarArgs(syn::Path), - KeywordArgs(syn::Path), - PosOnlyArg(syn::Path, Option), - Arg(syn::Path, Option), - Kwarg(syn::Path, Option), -} +mod signature; -/// The attributes of the pyfunction macro -#[derive(Default)] -pub struct PyFunctionSignature { - pub arguments: Vec, - has_kw: bool, - has_posonly_args: bool, - has_varargs: bool, - has_kwargs: bool, -} +pub use self::signature::{FunctionSignature, SignatureAttribute}; #[derive(Clone, Debug)] pub struct PyFunctionArgPyO3Attributes { @@ -88,16 +71,37 @@ impl PyFunctionArgPyO3Attributes { } } -impl syn::parse::Parse for PyFunctionSignature { +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum Argument { + PosOnlyArgsSeparator, + VarArgsSeparator, + VarArgs(syn::Path), + KeywordArgs(syn::Path), + PosOnlyArg(syn::Path, Option), + Arg(syn::Path, Option), + Kwarg(syn::Path, Option), +} + +#[derive(Debug, Default)] +pub struct DeprecatedArgs { + pub arguments: Vec, + has_kw: bool, + has_posonly_args: bool, + has_varargs: bool, + has_kwargs: bool, +} + +// Deprecated parsing mode for the signature +impl syn::parse::Parse for DeprecatedArgs { fn parse(input: &ParseBuffer<'_>) -> syn::Result { let attr = Punctuated::::parse_terminated(input)?; Self::from_meta(&attr) } } -impl PyFunctionSignature { +impl DeprecatedArgs { pub fn from_meta<'a>(iter: impl IntoIterator) -> syn::Result { - let mut slf = PyFunctionSignature::default(); + let mut slf = DeprecatedArgs::default(); for item in iter { slf.add_item(item)? @@ -238,7 +242,8 @@ impl PyFunctionSignature { pub struct PyFunctionOptions { pub pass_module: Option, pub name: Option, - pub signature: Option, + pub deprecated_args: Option, + pub signature: Option, pub text_signature: Option, pub deprecations: Deprecations, pub krate: Option, @@ -264,9 +269,10 @@ impl Parse for PyFunctionOptions { options.krate = Some(input.parse()?); } else { // If not recognised attribute, this is "legacy" pyfunction syntax #[pyfunction(a, b)] - // - // TODO deprecate in favour of #[pyfunction(signature = (a, b), name = "foo")] - options.signature = Some(input.parse()?); + options + .deprecations + .push(Deprecation::PyFunctionArguments, input.span()); + options.deprecated_args = Some(input.parse()?); break; } } @@ -278,7 +284,7 @@ impl Parse for PyFunctionOptions { pub enum PyFunctionOption { Name(NameAttribute), PassModule(attributes::kw::pass_module), - Signature(PyFunctionSignature), + Signature(SignatureAttribute), TextSignature(TextSignatureAttribute), Crate(CrateAttribute), } @@ -313,51 +319,28 @@ impl PyFunctionOptions { &mut self, attrs: impl IntoIterator, ) -> Result<()> { + macro_rules! set_option { + ($key:ident) => { + { + ensure_spanned!( + self.$key.is_none(), + $key.span() => concat!("`", stringify!($key), "` may only be specified once") + ); + self.$key = Some($key); + } + }; + } for attr in attrs { match attr { - PyFunctionOption::Name(name) => self.set_name(name)?, - PyFunctionOption::PassModule(kw) => { - ensure_spanned!( - self.pass_module.is_none(), - kw.span() => "`pass_module` may only be specified once" - ); - self.pass_module = Some(kw); - } - PyFunctionOption::Signature(signature) => { - ensure_spanned!( - self.signature.is_none(), - // FIXME: improve the span of this error message - Span::call_site() => "`signature` may only be specified once" - ); - self.signature = Some(signature); - } - PyFunctionOption::TextSignature(text_signature) => { - ensure_spanned!( - self.text_signature.is_none(), - text_signature.kw.span() => "`text_signature` may only be specified once" - ); - self.text_signature = Some(text_signature); - } - PyFunctionOption::Crate(path) => { - ensure_spanned!( - self.krate.is_none(), - path.span() => "`crate` may only be specified once" - ); - self.krate = Some(path); - } + PyFunctionOption::Name(name) => set_option!(name), + PyFunctionOption::PassModule(pass_module) => set_option!(pass_module), + PyFunctionOption::Signature(signature) => set_option!(signature), + PyFunctionOption::TextSignature(text_signature) => set_option!(text_signature), + PyFunctionOption::Crate(krate) => set_option!(krate), } } Ok(()) } - - pub fn set_name(&mut self, name: NameAttribute) -> Result<()> { - ensure_spanned!( - self.name.is_none(), - name.span() => "`name` may only be specified once" - ); - self.name = Some(name); - Ok(()) - } } pub fn build_py_function( @@ -377,11 +360,17 @@ pub fn impl_wrap_pyfunction( check_generic(&func.sig)?; ensure_not_async_fn(&func.sig)?; - let python_name = options - .name - .map_or_else(|| func.sig.ident.unraw(), |name| name.value.0); + let PyFunctionOptions { + pass_module, + name, + deprecated_args, + signature, + text_signature, + deprecations, + krate, + } = options; - let signature = options.signature.unwrap_or_default(); + let python_name = name.map_or_else(|| func.sig.ident.unraw(), |name| name.value.0); let mut arguments = func .sig @@ -390,7 +379,7 @@ pub fn impl_wrap_pyfunction( .map(FnArg::parse) .collect::>>()?; - if options.pass_module.is_some() { + let tp = if pass_module.is_some() { const PASS_MODULE_ERR: &str = "expected &PyModule as first argument with `pass_module`"; ensure_spanned!( !arguments.is_empty(), @@ -401,35 +390,44 @@ pub fn impl_wrap_pyfunction( type_is_pymodule(arg.ty), arg.ty.span() => PASS_MODULE_ERR ); - } + method::FnType::FnModule + } else { + method::FnType::FnStatic + }; + + let signature = if let Some(signature) = signature { + ensure_spanned!( + deprecated_args.is_none(), + signature.kw.span() => "cannot define both function signature and legacy arguments" + ); + FunctionSignature::from_arguments_and_attribute(arguments, signature)? + } else if let Some(deprecated_args) = deprecated_args { + FunctionSignature::from_arguments_and_deprecated_args(arguments, deprecated_args)? + } else { + FunctionSignature::from_arguments(arguments) + }; let ty = method::get_return_info(&func.sig.output); let doc = utils::get_doc( &func.attrs, - options - .text_signature + text_signature .as_ref() .map(|attr| (Cow::Borrowed(&python_name), attr)), ); - let krate = get_pyo3_crate(&options.krate); + let krate = get_pyo3_crate(&krate); let spec = method::FnSpec { - tp: if options.pass_module.is_some() { - method::FnType::FnModule - } else { - method::FnType::FnStatic - }, + tp, name: &func.sig.ident, - convention: CallingConvention::from_args(&arguments, &signature.arguments), + convention: CallingConvention::from_signature(&signature), python_name, - attrs: signature.arguments, - args: arguments, + signature, output: ty, doc, - deprecations: options.deprecations, - text_signature: options.text_signature, + deprecations, + text_signature, unsafety: func.sig.unsafety, }; @@ -482,91 +480,3 @@ fn type_is_pymodule(ty: &syn::Type) -> bool { } false } - -#[cfg(test)] -mod tests { - use super::{Argument, PyFunctionSignature}; - use proc_macro2::TokenStream; - use quote::quote; - use syn::parse_quote; - - fn items(input: TokenStream) -> syn::Result> { - let py_fn_attr: PyFunctionSignature = syn::parse2(input)?; - Ok(py_fn_attr.arguments) - } - - #[test] - fn test_errs() { - assert!(items(quote! {test="1", test2}).is_err()); - assert!(items(quote! {test, "*", args="*"}).is_err()); - assert!(items(quote! {test, kwargs="**", args="*"}).is_err()); - assert!(items(quote! {test, kwargs="**", args}).is_err()); - } - - #[test] - fn test_simple_args() { - let args = items(quote! {test1, test2, test3="None"}).unwrap(); - assert!( - args == vec![ - Argument::Arg(parse_quote! {test1}, None), - Argument::Arg(parse_quote! {test2}, None), - Argument::Arg(parse_quote! {test3}, Some("None".to_owned())), - ] - ); - } - - #[test] - fn test_varargs() { - let args = items(quote! {test1, test2="None", "*", test3="None"}).unwrap(); - assert!( - args == vec![ - Argument::Arg(parse_quote! {test1}, None), - Argument::Arg(parse_quote! {test2}, Some("None".to_owned())), - Argument::VarArgsSeparator, - Argument::Kwarg(parse_quote! {test3}, Some("None".to_owned())), - ] - ); - - let args = items(quote! {"*", test1, test2}).unwrap(); - assert!( - args == vec![ - Argument::VarArgsSeparator, - Argument::Kwarg(parse_quote! {test1}, None), - Argument::Kwarg(parse_quote! {test2}, None), - ] - ); - - let args = items(quote! {"*", test1, test2="None"}).unwrap(); - assert!( - args == vec![ - Argument::VarArgsSeparator, - Argument::Kwarg(parse_quote! {test1}, None), - Argument::Kwarg(parse_quote! {test2}, Some("None".to_owned())), - ] - ); - - let args = items(quote! {"*", test1="None", test2}).unwrap(); - assert!( - args == vec![ - Argument::VarArgsSeparator, - Argument::Kwarg(parse_quote! {test1}, Some("None".to_owned())), - Argument::Kwarg(parse_quote! {test2}, None), - ] - ); - } - - #[test] - fn test_all() { - let args = - items(quote! {test1, test2="None", args="*", test3="None", kwargs="**"}).unwrap(); - assert!( - args == vec![ - Argument::Arg(parse_quote! {test1}, None), - Argument::Arg(parse_quote! {test2}, Some("None".to_owned())), - Argument::VarArgs(parse_quote! {args}), - Argument::Kwarg(parse_quote! {test3}, Some("None".to_owned())), - Argument::KeywordArgs(parse_quote! {kwargs}), - ] - ); - } -} diff --git a/pyo3-macros-backend/src/pyfunction/signature.rs b/pyo3-macros-backend/src/pyfunction/signature.rs new file mode 100644 index 00000000..e66f14bc --- /dev/null +++ b/pyo3-macros-backend/src/pyfunction/signature.rs @@ -0,0 +1,546 @@ +use std::cmp::max; + +use proc_macro2::{Span, TokenStream}; +use quote::ToTokens; +use syn::{ + ext::IdentExt, + parse::{Parse, ParseStream}, + punctuated::Punctuated, + spanned::Spanned, + Token, +}; + +use crate::{ + attributes::{kw, KeywordAttribute}, + method::FnArg, + pyfunction::Argument, +}; + +use super::DeprecatedArgs; + +pub struct Signature { + paren_token: syn::token::Paren, + pub items: Punctuated, +} + +impl Parse for Signature { + fn parse(input: ParseStream<'_>) -> syn::Result { + let content; + Ok(Signature { + paren_token: syn::parenthesized!(content in input), + items: content.parse_terminated(SignatureItem::parse)?, + }) + } +} + +impl ToTokens for Signature { + fn to_tokens(&self, tokens: &mut TokenStream) { + self.paren_token + .surround(tokens, |tokens| self.items.to_tokens(tokens)) + } +} + +#[derive(Debug, PartialEq, Eq)] +pub struct SignatureItemArgument { + pub ident: syn::Ident, + pub eq_and_default: Option<(Token![=], syn::Expr)>, +} + +#[derive(Debug, PartialEq, Eq)] +pub struct SignatureItemPosargsSep { + pub slash: Token![/], +} + +#[derive(Debug, PartialEq, Eq)] +pub struct SignatureItemVarargsSep { + pub asterisk: Token![*], +} + +#[derive(Debug, PartialEq, Eq)] +pub struct SignatureItemVarargs { + pub sep: SignatureItemVarargsSep, + pub ident: syn::Ident, +} + +#[derive(Debug, PartialEq, Eq)] +pub struct SignatureItemKwargs { + pub asterisks: (Token![*], Token![*]), + pub ident: syn::Ident, +} + +#[derive(Debug, PartialEq, Eq)] +pub enum SignatureItem { + Argument(Box), + PosargsSep(SignatureItemPosargsSep), + VarargsSep(SignatureItemVarargsSep), + Varargs(SignatureItemVarargs), + Kwargs(SignatureItemKwargs), +} + +impl Parse for SignatureItem { + fn parse(input: ParseStream<'_>) -> syn::Result { + let lookahead = input.lookahead1(); + if lookahead.peek(Token![*]) { + if input.peek2(Token![*]) { + input.parse().map(SignatureItem::Kwargs) + } else { + let sep = input.parse()?; + if input.is_empty() || input.peek(Token![,]) { + Ok(SignatureItem::VarargsSep(sep)) + } else { + Ok(SignatureItem::Varargs(SignatureItemVarargs { + sep, + ident: input.parse()?, + })) + } + } + } else if lookahead.peek(Token![/]) { + input.parse().map(SignatureItem::PosargsSep) + } else { + input.parse().map(SignatureItem::Argument) + } + } +} + +impl ToTokens for SignatureItem { + fn to_tokens(&self, tokens: &mut TokenStream) { + match self { + SignatureItem::Argument(arg) => arg.to_tokens(tokens), + SignatureItem::Varargs(varargs) => varargs.to_tokens(tokens), + SignatureItem::VarargsSep(sep) => sep.to_tokens(tokens), + SignatureItem::Kwargs(kwargs) => kwargs.to_tokens(tokens), + SignatureItem::PosargsSep(sep) => sep.to_tokens(tokens), + } + } +} + +impl Parse for SignatureItemArgument { + fn parse(input: ParseStream<'_>) -> syn::Result { + Ok(Self { + ident: input.parse()?, + eq_and_default: if input.peek(Token![=]) { + Some((input.parse()?, input.parse()?)) + } else { + None + }, + }) + } +} + +impl ToTokens for SignatureItemArgument { + fn to_tokens(&self, tokens: &mut TokenStream) { + self.ident.to_tokens(tokens); + if let Some((eq, default)) = &self.eq_and_default { + eq.to_tokens(tokens); + default.to_tokens(tokens); + } + } +} + +impl Parse for SignatureItemVarargsSep { + fn parse(input: ParseStream<'_>) -> syn::Result { + Ok(Self { + asterisk: input.parse()?, + }) + } +} + +impl ToTokens for SignatureItemVarargsSep { + fn to_tokens(&self, tokens: &mut TokenStream) { + self.asterisk.to_tokens(tokens); + } +} + +impl Parse for SignatureItemVarargs { + fn parse(input: ParseStream<'_>) -> syn::Result { + Ok(Self { + sep: input.parse()?, + ident: input.parse()?, + }) + } +} + +impl ToTokens for SignatureItemVarargs { + fn to_tokens(&self, tokens: &mut TokenStream) { + self.sep.to_tokens(tokens); + self.ident.to_tokens(tokens); + } +} + +impl Parse for SignatureItemKwargs { + fn parse(input: ParseStream<'_>) -> syn::Result { + Ok(Self { + asterisks: (input.parse()?, input.parse()?), + ident: input.parse()?, + }) + } +} + +impl ToTokens for SignatureItemKwargs { + fn to_tokens(&self, tokens: &mut TokenStream) { + self.asterisks.0.to_tokens(tokens); + self.asterisks.1.to_tokens(tokens); + self.ident.to_tokens(tokens); + } +} + +impl Parse for SignatureItemPosargsSep { + fn parse(input: ParseStream<'_>) -> syn::Result { + Ok(Self { + slash: input.parse()?, + }) + } +} + +impl ToTokens for SignatureItemPosargsSep { + fn to_tokens(&self, tokens: &mut TokenStream) { + self.slash.to_tokens(tokens); + } +} + +pub type SignatureAttribute = KeywordAttribute; + +#[derive(Default)] +pub struct PythonSignature { + pub positional_parameters: Vec, + pub positional_only_parameters: usize, + pub required_positional_parameters: usize, + pub accepts_varargs: bool, + // Tuples of keyword name and whether it is required + pub keyword_only_parameters: Vec<(String, bool)>, + pub accepts_kwargs: bool, +} + +pub struct FunctionSignature<'a> { + pub arguments: Vec>, + pub python_signature: PythonSignature, +} + +pub enum ParseState { + /// Accepting positional parameters, which might be positional only + Positional, + /// Accepting positional parameters after '/' + PositionalAfterPosargs, + /// Accepting keyword-only parameters after '*' or '*args' + Keywords(Option), + /// After `**kwargs` nothing is allowed + Done(String), +} + +impl ParseState { + fn add_argument( + &mut self, + signature: &mut PythonSignature, + name: String, + required: bool, + span: Span, + ) -> syn::Result<()> { + match self { + ParseState::Positional | ParseState::PositionalAfterPosargs => { + signature.positional_parameters.push(name); + if required { + signature.required_positional_parameters += 1; + ensure_spanned!( + signature.required_positional_parameters == signature.positional_parameters.len(), + span => "cannot have required positional parameter after an optional parameter" + ); + } + Ok(()) + } + ParseState::Keywords(_) => { + signature.keyword_only_parameters.push((name, required)); + Ok(()) + } + ParseState::Done(s) => { + bail_spanned!(span => format!("no more arguments are allowed after `**{}`", s)) + } + } + } + + fn add_varargs( + &mut self, + signature: &mut PythonSignature, + varargs: &SignatureItemVarargs, + ) -> syn::Result<()> { + match self { + ParseState::Positional | ParseState::PositionalAfterPosargs => { + signature.accepts_varargs = true; + *self = ParseState::Keywords(Some(varargs.ident.to_string())); + Ok(()) + } + ParseState::Keywords(s) => { + bail_spanned!(varargs.span() => format!("`*{}` not allowed after `*{}`", varargs.ident, s.as_deref().unwrap_or(""))) + } + ParseState::Done(s) => { + bail_spanned!(varargs.span() => format!("`*{}` not allowed after `**{}`", varargs.ident, s)) + } + } + } + + fn add_kwargs( + &mut self, + signature: &mut PythonSignature, + kwargs: &SignatureItemKwargs, + ) -> syn::Result<()> { + match self { + ParseState::Positional + | ParseState::PositionalAfterPosargs + | ParseState::Keywords(_) => { + signature.accepts_kwargs = true; + *self = ParseState::Done(kwargs.ident.to_string()); + Ok(()) + } + ParseState::Done(s) => { + bail_spanned!(kwargs.span() => format!("`**{}` not allowed after `**{}`", kwargs.ident, s)) + } + } + } + + fn finish_pos_only_args( + &mut self, + signature: &mut PythonSignature, + span: Span, + ) -> syn::Result<()> { + match self { + ParseState::Positional => { + signature.positional_only_parameters = signature.positional_parameters.len(); + *self = ParseState::PositionalAfterPosargs; + Ok(()) + } + ParseState::PositionalAfterPosargs => { + bail_spanned!(span => "`/` not allowed after `/`") + } + ParseState::Keywords(s) => { + bail_spanned!(span => format!("`/` not allowed after `*{}`", s.as_deref().unwrap_or(""))) + } + ParseState::Done(s) => { + bail_spanned!(span => format!("`/` not allowed after `**{}`", s)) + } + } + } + + fn finish_pos_args(&mut self, span: Span) -> syn::Result<()> { + match self { + ParseState::Positional | ParseState::PositionalAfterPosargs => { + *self = ParseState::Keywords(None); + Ok(()) + } + ParseState::Keywords(s) => { + bail_spanned!(span => format!("`*` not allowed after `*{}`", s.as_deref().unwrap_or(""))) + } + ParseState::Done(s) => { + bail_spanned!(span => format!("`*` not allowed after `**{}`", s)) + } + } + } +} + +impl<'a> FunctionSignature<'a> { + pub fn from_arguments_and_attribute( + mut arguments: Vec>, + attribute: SignatureAttribute, + ) -> syn::Result { + let mut parse_state = ParseState::Positional; + let mut python_signature = PythonSignature::default(); + + let mut args_iter = arguments.iter_mut().filter(|arg| !arg.py); // Python<'_> arguments don't show on the Python side. + + let mut next_argument_checked = |name: &syn::Ident| match args_iter.next() { + Some(fn_arg) => { + ensure_spanned!( + name == fn_arg.name, + name.span() => format!( + "expected argument from function definition `{}` but got argument `{}`", + fn_arg.name.unraw(), + name.unraw(), + ) + ); + Ok(fn_arg) + } + None => bail_spanned!( + name.span() => "signature entry does not have a corresponding function argument" + ), + }; + + for item in attribute.value.items { + match item { + SignatureItem::Argument(arg) => { + let fn_arg = next_argument_checked(&arg.ident)?; + parse_state.add_argument( + &mut python_signature, + arg.ident.unraw().to_string(), + arg.eq_and_default.is_none(), + arg.span(), + )?; + if let Some((_, default)) = arg.eq_and_default { + fn_arg.default = Some(default); + } + } + SignatureItem::VarargsSep(sep) => parse_state.finish_pos_args(sep.span())?, + SignatureItem::Varargs(varargs) => { + let fn_arg = next_argument_checked(&varargs.ident)?; + fn_arg.is_varargs = true; + parse_state.add_varargs(&mut python_signature, &varargs)?; + } + SignatureItem::Kwargs(kwargs) => { + let fn_arg = next_argument_checked(&kwargs.ident)?; + fn_arg.is_kwargs = true; + parse_state.add_kwargs(&mut python_signature, &kwargs)?; + } + SignatureItem::PosargsSep(sep) => { + parse_state.finish_pos_only_args(&mut python_signature, sep.span())? + } + }; + } + + if let Some(arg) = args_iter.next() { + bail_spanned!( + attribute.kw.span() => format!("missing signature entry for argument `{}`", arg.name) + ); + } + + Ok(FunctionSignature { + arguments, + python_signature, + }) + } + + /// The difference to `from_arguments_and_signature` is that deprecated args allowed entries to be: + /// - missing + /// - out of order + pub fn from_arguments_and_deprecated_args( + mut arguments: Vec>, + deprecated_args: DeprecatedArgs, + ) -> syn::Result { + let mut accepts_varargs = false; + let mut accepts_kwargs = false; + let mut keyword_only_parameters = Vec::new(); + + fn first_n_argument_names(arguments: &[FnArg<'_>], count: usize) -> Vec { + arguments + .iter() + .filter_map(|fn_arg| { + if fn_arg.py { + None + } else { + Some(fn_arg.name.unraw().to_string()) + } + }) + .take(count) + .collect() + } + + // Record highest counts observed based off argument positions + let mut positional_only_arguments_count = None; + let mut positional_arguments_count = None; + let mut required_positional_parameters = 0; + + let args_iter = arguments.iter_mut().filter(|arg| !arg.py); // Python<'_> arguments don't show on the Python side. + + for (i, fn_arg) in args_iter.enumerate() { + if let Some(argument) = deprecated_args + .arguments + .iter() + .find(|argument| match argument { + Argument::PosOnlyArg(path, _) + | Argument::Arg(path, _) + | Argument::Kwarg(path, _) + | Argument::VarArgs(path) + | Argument::KeywordArgs(path) => path.get_ident() == Some(fn_arg.name), + _ => false, + }) + { + match argument { + Argument::PosOnlyArg(_, default) | Argument::Arg(_, default) => { + if let Some(default) = default { + fn_arg.default = Some(syn::parse_str(default)?); + } else if fn_arg.optional.is_none() { + // Option<_> arguments always have an implicit None default with the old + // `#[args]` + required_positional_parameters = i + 1; + } + if matches!(argument, Argument::PosOnlyArg(_, _)) { + positional_only_arguments_count = Some(i + 1); + } + positional_arguments_count = Some(i + 1); + } + Argument::Kwarg(_, default) => { + fn_arg.default = default.as_deref().map(syn::parse_str).transpose()?; + keyword_only_parameters.push((fn_arg.name.to_string(), default.is_none())); + } + Argument::PosOnlyArgsSeparator => {} + Argument::VarArgsSeparator => {} + Argument::VarArgs(_) => { + fn_arg.is_varargs = true; + accepts_varargs = true; + } + Argument::KeywordArgs(_) => { + fn_arg.is_kwargs = true; + accepts_kwargs = true; + } + } + } else { + // Assume this is a required positional parameter + required_positional_parameters = i + 1; + positional_arguments_count = Some(i + 1); + } + } + + // fix up state based on observations above + let positional_only_parameters = positional_only_arguments_count.unwrap_or(0); + let positional_parameters = first_n_argument_names( + &arguments, + max( + positional_arguments_count.unwrap_or(0), + positional_only_arguments_count.unwrap_or(0), + ), + ); + + Ok(FunctionSignature { + arguments, + python_signature: PythonSignature { + positional_parameters, + positional_only_parameters, + required_positional_parameters, + accepts_varargs, + keyword_only_parameters, + accepts_kwargs, + }, + }) + } + + /// Without `#[pyo3(signature)]` or `#[args]` - just take the Rust function arguments as positional. + pub fn from_arguments(mut arguments: Vec>) -> Self { + let mut python_signature = PythonSignature::default(); + for arg in &arguments { + // Python<'_> arguments don't show in Python signature + if arg.py { + continue; + } + + if arg.optional.is_none() { + // This argument is required + python_signature.required_positional_parameters = + python_signature.positional_parameters.len() + 1; + } + + python_signature + .positional_parameters + .push(arg.name.unraw().to_string()); + } + + // Fixup any `Option<_>` arguments that were made implicitly made required by the deprecated + // branch above + for arg in arguments + .iter_mut() + .take(python_signature.required_positional_parameters) + { + arg.optional = None; + } + + Self { + arguments, + python_signature, + } + } +} diff --git a/pyo3-macros-backend/src/pymethod.rs b/pyo3-macros-backend/src/pymethod.rs index 0e5b8f49..27249ec6 100644 --- a/pyo3-macros-backend/src/pymethod.rs +++ b/pyo3-macros-backend/src/pymethod.rs @@ -398,7 +398,7 @@ fn impl_traverse_slot(cls: &syn::Type, spec: FnSpec<'_>) -> MethodAndSlotDef { } fn impl_py_class_attribute(cls: &syn::Type, spec: &FnSpec<'_>) -> syn::Result { - let (py_arg, args) = split_off_python_arg(&spec.args); + let (py_arg, args) = split_off_python_arg(&spec.signature.arguments); ensure_spanned!( args.is_empty(), args[0].ty.span() => "#[classattr] can only have one argument (of type pyo3::Python)" @@ -440,7 +440,7 @@ fn impl_py_class_attribute(cls: &syn::Type, spec: &FnSpec<'_>) -> syn::Result) -> syn::Result { - let (py_arg, args) = split_off_python_arg(&spec.args); + let (py_arg, args) = split_off_python_arg(&spec.signature.arguments); if args.is_empty() { bail_spanned!(spec.name.span() => "setter function expected to have one argument"); @@ -564,7 +564,7 @@ pub fn impl_py_setter_def( } fn impl_call_getter(cls: &syn::Type, spec: &FnSpec<'_>) -> syn::Result { - let (py_arg, args) = split_off_python_arg(&spec.args); + let (py_arg, args) = split_off_python_arg(&spec.signature.arguments); ensure_spanned!( args.is_empty(), args[0].ty.span() => "getter function can only have one argument (of type pyo3::Python)" @@ -1289,10 +1289,10 @@ fn extract_proto_arguments( proto_args: &[Ty], extract_error_mode: ExtractErrorMode, ) -> Result> { - let mut args = Vec::with_capacity(spec.args.len()); + let mut args = Vec::with_capacity(spec.signature.arguments.len()); let mut non_python_args = 0; - for arg in &spec.args { + for arg in &spec.signature.arguments { if arg.py { args.push(quote! { #py }); } else { diff --git a/pyo3-macros-backend/src/utils.rs b/pyo3-macros-backend/src/utils.rs index 1542e0b9..cbbe2b31 100644 --- a/pyo3-macros-backend/src/utils.rs +++ b/pyo3-macros-backend/src/utils.rs @@ -44,7 +44,7 @@ pub fn is_python(ty: &syn::Type) -> bool { } } -/// If `ty` is Option, return `Some(T)`, else None. +/// If `ty` is `Option`, return `Some(T)`, else `None`. pub fn option_type_argument(ty: &syn::Type) -> Option<&syn::Type> { if let syn::Type::Path(syn::TypePath { path, .. }) = ty { let seg = path.segments.last().filter(|s| s.ident == "Option")?; diff --git a/pyo3-macros/src/lib.rs b/pyo3-macros/src/lib.rs index 873fe306..730aaf65 100644 --- a/pyo3-macros/src/lib.rs +++ b/pyo3-macros/src/lib.rs @@ -87,8 +87,8 @@ pub fn pyclass(attr: TokenStream, input: TokenStream) -> TokenStream { /// | [`#[staticmethod]`][6]| Defines the method as a staticmethod, like Python's `@staticmethod` decorator.| /// | [`#[classmethod]`][7] | Defines the method as a classmethod, like Python's `@classmethod` decorator.| /// | [`#[classattr]`][9] | Defines a class variable. | -/// | [`#[args]`][10] | Define a method's default arguments and allows the function to receive `*args` and `**kwargs`. | -/// | [`#[pyo3( | Any of the `#[pyo3]` options supported on [`macro@pyfunction`]. | +/// | [`#[args]`][10] | Deprecated way to define a method's default arguments and allows the function to receive `*args` and `**kwargs`. Use `#[pyo3(signature = (...))]` instead. | +/// | [`#[pyo3( | Any of the `#[pyo3]` options supported on [`macro@pyfunction`]. | /// /// For more on creating class methods, /// see the [class section of the guide][1]. diff --git a/pytests/noxfile.py b/pytests/noxfile.py index 1a44a8c4..2eff2793 100644 --- a/pytests/noxfile.py +++ b/pytests/noxfile.py @@ -11,7 +11,7 @@ def test(session): session.install("numpy>=1.16") session.install("maturin") session.run_always("maturin", "develop") - session.run("pytest") + session.run("pytest", *session.posargs) @nox.session diff --git a/pytests/src/deprecated_pyfunctions.rs b/pytests/src/deprecated_pyfunctions.rs new file mode 100644 index 00000000..260b8eb2 --- /dev/null +++ b/pytests/src/deprecated_pyfunctions.rs @@ -0,0 +1,68 @@ +#![allow(deprecated)] + +use pyo3::prelude::*; +use pyo3::types::{PyDict, PyTuple}; + +#[pyfunction] +fn none() {} + +#[pyfunction(b = "\"bar\"", "*", c = "None")] +fn simple<'a>(a: i32, b: &'a str, c: Option<&'a PyDict>) -> (i32, &'a str, Option<&'a PyDict>) { + (a, b, c) +} + +#[pyfunction(b = "\"bar\"", args = "*", c = "None")] +fn simple_args<'a>( + a: i32, + b: &'a str, + c: Option<&'a PyDict>, + args: &'a PyTuple, +) -> (i32, &'a str, &'a PyTuple, Option<&'a PyDict>) { + (a, b, args, c) +} + +#[pyfunction(b = "\"bar\"", c = "None", kwargs = "**")] +fn simple_kwargs<'a>( + a: i32, + b: &'a str, + c: Option<&'a PyDict>, + kwargs: Option<&'a PyDict>, +) -> (i32, &'a str, Option<&'a PyDict>, Option<&'a PyDict>) { + (a, b, c, kwargs) +} + +#[pyfunction(a, b = "\"bar\"", args = "*", c = "None", kwargs = "**")] +fn simple_args_kwargs<'a>( + a: i32, + b: &'a str, + args: &'a PyTuple, + c: Option<&'a PyDict>, + kwargs: Option<&'a PyDict>, +) -> ( + i32, + &'a str, + &'a PyTuple, + Option<&'a PyDict>, + Option<&'a PyDict>, +) { + (a, b, args, c, kwargs) +} + +#[pyfunction(args = "*", kwargs = "**")] +fn args_kwargs<'a>( + args: &'a PyTuple, + kwargs: Option<&'a PyDict>, +) -> (&'a PyTuple, Option<&'a PyDict>) { + (args, kwargs) +} + +#[pymodule] +pub fn deprecated_pyfunctions(_py: Python<'_>, m: &PyModule) -> PyResult<()> { + m.add_function(wrap_pyfunction!(none, m)?)?; + m.add_function(wrap_pyfunction!(simple, m)?)?; + m.add_function(wrap_pyfunction!(simple_args, m)?)?; + m.add_function(wrap_pyfunction!(simple_kwargs, m)?)?; + m.add_function(wrap_pyfunction!(simple_args_kwargs, m)?)?; + m.add_function(wrap_pyfunction!(args_kwargs, m)?)?; + Ok(()) +} diff --git a/pytests/src/lib.rs b/pytests/src/lib.rs index 7d0abd23..d9d6e3f9 100644 --- a/pytests/src/lib.rs +++ b/pytests/src/lib.rs @@ -4,6 +4,7 @@ use pyo3::wrap_pymodule; pub mod buf_and_str; pub mod datetime; +pub mod deprecated_pyfunctions; pub mod dict_iter; pub mod misc; pub mod objstore; @@ -20,6 +21,9 @@ fn pyo3_pytests(py: Python<'_>, m: &PyModule) -> PyResult<()> { m.add_wrapped(wrap_pymodule!(buf_and_str::buf_and_str))?; #[cfg(not(Py_LIMITED_API))] m.add_wrapped(wrap_pymodule!(datetime::datetime))?; + m.add_wrapped(wrap_pymodule!( + deprecated_pyfunctions::deprecated_pyfunctions + ))?; m.add_wrapped(wrap_pymodule!(dict_iter::dict_iter))?; m.add_wrapped(wrap_pymodule!(misc::misc))?; m.add_wrapped(wrap_pymodule!(objstore::objstore))?; @@ -37,6 +41,10 @@ fn pyo3_pytests(py: Python<'_>, m: &PyModule) -> PyResult<()> { let sys_modules: &PyDict = sys.getattr("modules")?.downcast()?; sys_modules.set_item("pyo3_pytests.buf_and_str", m.getattr("buf_and_str")?)?; sys_modules.set_item("pyo3_pytests.datetime", m.getattr("datetime")?)?; + sys_modules.set_item( + "pyo3_pytests.deprecated_pyfunctions", + m.getattr("deprecated_pyfunctions")?, + )?; sys_modules.set_item("pyo3_pytests.dict_iter", m.getattr("dict_iter")?)?; sys_modules.set_item("pyo3_pytests.misc", m.getattr("misc")?)?; sys_modules.set_item("pyo3_pytests.objstore", m.getattr("objstore")?)?; diff --git a/pytests/src/pyfunctions.rs b/pytests/src/pyfunctions.rs index a259aea0..855f23af 100644 --- a/pytests/src/pyfunctions.rs +++ b/pytests/src/pyfunctions.rs @@ -1,25 +1,25 @@ use pyo3::prelude::*; use pyo3::types::{PyDict, PyTuple}; -#[pyfunction] +#[pyfunction(signature = ())] fn none() {} -#[pyfunction(b = "\"bar\"", "*", c = "None")] +#[pyfunction(signature = (a, b = "bar", *, c = None))] fn simple<'a>(a: i32, b: &'a str, c: Option<&'a PyDict>) -> (i32, &'a str, Option<&'a PyDict>) { (a, b, c) } -#[pyfunction(b = "\"bar\"", args = "*", c = "None")] +#[pyfunction(signature = (a, b = "bar", *args, c = None))] fn simple_args<'a>( a: i32, b: &'a str, - c: Option<&'a PyDict>, args: &'a PyTuple, + c: Option<&'a PyDict>, ) -> (i32, &'a str, &'a PyTuple, Option<&'a PyDict>) { (a, b, args, c) } -#[pyfunction(b = "\"bar\"", c = "None", kwargs = "**")] +#[pyfunction(signature = (a, b = "bar", c = None, **kwargs))] fn simple_kwargs<'a>( a: i32, b: &'a str, @@ -29,7 +29,7 @@ fn simple_kwargs<'a>( (a, b, c, kwargs) } -#[pyfunction(a, b = "\"bar\"", args = "*", c = "None", kwargs = "**")] +#[pyfunction(signature = (a, b = "bar", *args, c = None, **kwargs))] fn simple_args_kwargs<'a>( a: i32, b: &'a str, @@ -46,7 +46,7 @@ fn simple_args_kwargs<'a>( (a, b, args, c, kwargs) } -#[pyfunction(args = "*", kwargs = "**")] +#[pyfunction(signature = (*args, **kwargs))] fn args_kwargs<'a>( args: &'a PyTuple, kwargs: Option<&'a PyDict>, diff --git a/pytests/tests/test_deprecated_pyfunctions.py b/pytests/tests/test_deprecated_pyfunctions.py new file mode 100644 index 00000000..ec44d620 --- /dev/null +++ b/pytests/tests/test_deprecated_pyfunctions.py @@ -0,0 +1,91 @@ +from pyo3_pytests import deprecated_pyfunctions as pyfunctions + + +def none_py(): + return None + + +def test_none_py(benchmark): + benchmark(none_py) + + +def test_none_rs(benchmark): + rust = pyfunctions.none() + py = none_py() + assert rust == py + benchmark(pyfunctions.none) + + +def simple_py(a, b="bar", *, c=None): + return a, b, c + + +def test_simple_py(benchmark): + benchmark(simple_py, 1, "foo", c={1: 2}) + + +def test_simple_rs(benchmark): + rust = pyfunctions.simple(1, "foo", c={1: 2}) + py = simple_py(1, "foo", c={1: 2}) + assert rust == py + benchmark(pyfunctions.simple, 1, "foo", c={1: 2}) + + +def simple_args_py(a, b="bar", *args, c=None): + return a, b, args, c + + +def test_simple_args_py(benchmark): + benchmark(simple_args_py, 1, "foo", 4, 5, 6, c={1: 2}) + + +def test_simple_args_rs(benchmark): + rust = pyfunctions.simple_args(1, "foo", 4, 5, 6, c={1: 2}) + py = simple_args_py(1, "foo", 4, 5, 6, c={1: 2}) + assert rust == py + benchmark(pyfunctions.simple_args, 1, "foo", 4, 5, 6, c={1: 2}) + + +def simple_kwargs_py(a, b="bar", c=None, **kwargs): + return a, b, c, kwargs + + +def test_simple_kwargs_py(benchmark): + benchmark(simple_kwargs_py, 1, "foo", c={1: 2}, bar=4, foo=10) + + +def test_simple_kwargs_rs(benchmark): + rust = pyfunctions.simple_kwargs(1, "foo", c={1: 2}, bar=4, foo=10) + py = simple_kwargs_py(1, "foo", c={1: 2}, bar=4, foo=10) + assert rust == py + benchmark(pyfunctions.simple_kwargs, 1, "foo", c={1: 2}, bar=4, foo=10) + + +def simple_args_kwargs_py(a, b="bar", *args, c=None, **kwargs): + return (a, b, args, c, kwargs) + + +def test_simple_args_kwargs_py(benchmark): + benchmark(simple_args_kwargs_py, 1, "foo", "baz", bar=4, foo=10) + + +def test_simple_args_kwargs_rs(benchmark): + rust = pyfunctions.simple_args_kwargs(1, "foo", "baz", bar=4, foo=10) + py = simple_args_kwargs_py(1, "foo", "baz", bar=4, foo=10) + assert rust == py + benchmark(pyfunctions.simple_args_kwargs, 1, "foo", "baz", bar=4, foo=10) + + +def args_kwargs_py(*args, **kwargs): + return (args, kwargs) + + +def test_args_kwargs_py(benchmark): + benchmark(args_kwargs_py, 1, "foo", {1: 2}, bar=4, foo=10) + + +def test_args_kwargs_rs(benchmark): + rust = pyfunctions.args_kwargs(1, "foo", {1: 2}, bar=4, foo=10) + py = args_kwargs_py(1, "foo", {1: 2}, bar=4, foo=10) + assert rust == py + benchmark(pyfunctions.args_kwargs, 1, "foo", {1: 2}, a=4, foo=10) diff --git a/src/impl_/deprecations.rs b/src/impl_/deprecations.rs index a80a6a96..6a24c4ce 100644 --- a/src/impl_/deprecations.rs +++ b/src/impl_/deprecations.rs @@ -5,3 +5,15 @@ note = "implement a `__traverse__` `#[pymethod]` instead of using `gc` option" )] pub const PYCLASS_GC_OPTION: () = (); + +#[deprecated( + since = "0.18.0", + note = "passing arbitrary arguments to `#[pyfunction()]` to specify the signature is being replaced by `#[pyo3(signature)]`" +)] +pub const PYFUNCTION_ARGUMENTS: () = (); + +#[deprecated( + since = "0.18.0", + note = "the `#[args]` attribute for `#[methods]` is being replaced by `#[pyo3(signature)]`" +)] +pub const PYMETHODS_ARGS_ATTRIBUTE: () = (); diff --git a/src/test_hygiene/pymethods.rs b/src/test_hygiene/pymethods.rs index 79fd5a3d..eb0b12f7 100644 --- a/src/test_hygiene/pymethods.rs +++ b/src/test_hygiene/pymethods.rs @@ -367,13 +367,13 @@ impl Dummy { // Things with attributes - #[args(x = "1", "*", _z = "2")] + #[pyo3(signature = (_y, *, _z=2))] fn test(&self, _y: &Dummy, _z: i32) {} #[staticmethod] fn staticmethod() {} #[classmethod] fn clsmethod(_: &crate::types::PyType) {} - #[args(args = "*", kwds = "**")] + #[pyo3(signature = (*_args, **_kwds))] fn __call__( &self, _args: &crate::types::PyTuple, @@ -766,13 +766,13 @@ impl Dummy { // Things with attributes - #[args(x = "1", "*", _z = "2")] + #[pyo3(signature = (_y, *, _z=2))] fn test(&self, _y: &Dummy, _z: i32) {} #[staticmethod] fn staticmethod() {} #[classmethod] fn clsmethod(_: &crate::types::PyType) {} - #[args(args = "*", kwds = "**")] + #[pyo3(signature = (*_args, **_kwds))] fn __call__( &self, _args: &crate::types::PyTuple, diff --git a/tests/test_compile_error.rs b/tests/test_compile_error.rs index 4d5efab7..510d98ef 100644 --- a/tests/test_compile_error.rs +++ b/tests/test_compile_error.rs @@ -38,6 +38,7 @@ fn _test_compile_errors() { t.compile_fail("tests/ui/invalid_pyclass_args.rs"); t.compile_fail("tests/ui/invalid_pyclass_enum.rs"); t.compile_fail("tests/ui/invalid_pyclass_item.rs"); + t.compile_fail("tests/ui/invalid_pyfunction_signatures.rs"); #[cfg(not(Py_LIMITED_API))] t.compile_fail("tests/ui/invalid_pymethods_buffer.rs"); t.compile_fail("tests/ui/invalid_pymethod_names.rs"); diff --git a/tests/test_macros.rs b/tests/test_macros.rs index 87dcc628..a7ef4567 100644 --- a/tests/test_macros.rs +++ b/tests/test_macros.rs @@ -34,8 +34,8 @@ set_extends_via_macro!(MyClass2, MyBaseClass); macro_rules! fn_macro { ($sig:literal, $a_exp:expr, $b_exp:expr, $c_exp: expr) => { - // Try and pass a variable into the extends parameter - #[pyfunction($a_exp, $b_exp, "*", $c_exp)] + // Try and pass a variable into the signature parameter + #[pyfunction(signature = ($a_exp, $b_exp, *, $c_exp))] #[pyo3(text_signature = $sig)] fn my_function_in_macro(a: i32, b: Option, c: i32) { let _ = (a, b, c); @@ -43,7 +43,7 @@ macro_rules! fn_macro { }; } -fn_macro!("(a, b=None, *, c=42)", a, b = "None", c = 42); +fn_macro!("(a, b=None, *, c=42)", a, b = None, c = 42); macro_rules! property_rename_via_macro { ($prop_name:ident) => { diff --git a/tests/test_methods.rs b/tests/test_methods.rs index 04d2d721..9dddbd35 100644 --- a/tests/test_methods.rs +++ b/tests/test_methods.rs @@ -162,22 +162,477 @@ fn static_method_with_args() { }); } +#[allow(deprecated)] +mod deprecated { + use super::*; + + #[pyclass] + struct MethArgs {} + + #[pymethods] + impl MethArgs { + #[args(test)] + fn get_optional(&self, test: Option) -> i32 { + test.unwrap_or(10) + } + fn get_optional2(&self, test: Option) -> Option { + test + } + #[args(test = "None")] + fn get_optional3(&self, test: Option) -> Option { + test + } + fn get_optional_positional( + &self, + _t1: Option, + t2: Option, + _t3: Option, + ) -> Option { + t2 + } + + #[args(test = "10")] + fn get_default(&self, test: i32) -> i32 { + test + } + #[args("*", test = 10)] + fn get_kwarg(&self, test: i32) -> i32 { + test + } + #[args(args = "*", kwargs = "**")] + fn get_kwargs(&self, py: Python<'_>, args: &PyTuple, kwargs: Option<&PyDict>) -> PyObject { + [args.into(), kwargs.to_object(py)].to_object(py) + } + + #[args(args = "*", kwargs = "**")] + fn get_pos_arg_kw( + &self, + py: Python<'_>, + a: i32, + args: &PyTuple, + kwargs: Option<&PyDict>, + ) -> PyObject { + [a.to_object(py), args.into(), kwargs.to_object(py)].to_object(py) + } + + #[args(a, b, "/")] + fn get_pos_only(&self, a: i32, b: i32) -> i32 { + a + b + } + + #[args(a, "/", b)] + fn get_pos_only_and_pos(&self, a: i32, b: i32) -> i32 { + a + b + } + + #[args(a, "/", b, c = 5)] + fn get_pos_only_and_pos_and_kw(&self, a: i32, b: i32, c: i32) -> i32 { + a + b + c + } + + #[args(a, "/", "*", b)] + fn get_pos_only_and_kw_only(&self, a: i32, b: i32) -> i32 { + a + b + } + + #[args(a, "/", "*", b = 3)] + fn get_pos_only_and_kw_only_with_default(&self, a: i32, b: i32) -> i32 { + a + b + } + + #[args(a, "/", b, "*", c, d = 5)] + fn get_all_arg_types_together(&self, a: i32, b: i32, c: i32, d: i32) -> i32 { + a + b + c + d + } + + #[args(a, "/", args = "*")] + fn get_pos_only_with_varargs(&self, a: i32, args: Vec) -> i32 { + a + args.iter().sum::() + } + + #[args(a, "/", kwargs = "**")] + fn get_pos_only_with_kwargs( + &self, + py: Python<'_>, + a: i32, + kwargs: Option<&PyDict>, + ) -> PyObject { + [a.to_object(py), kwargs.to_object(py)].to_object(py) + } + + #[args("*", a = 2, b = 3)] + fn get_kwargs_only_with_defaults(&self, a: i32, b: i32) -> i32 { + a + b + } + + #[args("*", a, b)] + fn get_kwargs_only(&self, a: i32, b: i32) -> i32 { + a + b + } + + #[args("*", a = 1, b)] + fn get_kwargs_only_with_some_default(&self, a: i32, b: i32) -> i32 { + a + b + } + + #[args(args = "*", a)] + fn get_args_and_required_keyword( + &self, + py: Python<'_>, + args: &PyTuple, + a: i32, + ) -> PyObject { + (args, a).to_object(py) + } + + #[args(a, b = 2, "*", c = 3)] + fn get_pos_arg_kw_sep1(&self, a: i32, b: i32, c: i32) -> i32 { + a + b + c + } + + #[args(a, "*", b = 2, c = 3)] + fn get_pos_arg_kw_sep2(&self, a: i32, b: i32, c: i32) -> i32 { + a + b + c + } + + #[args(kwargs = "**")] + fn get_pos_kw(&self, py: Python<'_>, a: i32, kwargs: Option<&PyDict>) -> PyObject { + [a.to_object(py), kwargs.to_object(py)].to_object(py) + } + // "args" can be anything that can be extracted from PyTuple + #[args(args = "*")] + fn args_as_vec(&self, args: Vec) -> i32 { + args.iter().sum() + } + } + + #[test] + fn meth_args() { + Python::with_gil(|py| { + let inst = Py::new(py, MethArgs {}).unwrap(); + + py_run!(py, inst, "assert inst.get_optional() == 10"); + py_run!(py, inst, "assert inst.get_optional(100) == 100"); + py_run!(py, inst, "assert inst.get_optional2() == None"); + py_run!(py, inst, "assert inst.get_optional2(100) == 100"); + py_run!(py, inst, "assert inst.get_optional3() == None"); + py_run!(py, inst, "assert inst.get_optional3(100) == 100"); + py_run!( + py, + inst, + "assert inst.get_optional_positional(1, 2, 3) == 2" + ); + py_run!(py, inst, "assert inst.get_optional_positional(1) == None"); + py_run!(py, inst, "assert inst.get_default() == 10"); + py_run!(py, inst, "assert inst.get_default(100) == 100"); + py_run!(py, inst, "assert inst.get_kwarg() == 10"); + py_expect_exception!(py, inst, "inst.get_kwarg(100)", PyTypeError); + py_run!(py, inst, "assert inst.get_kwarg(test=100) == 100"); + py_run!(py, inst, "assert inst.get_kwargs() == [(), None]"); + py_run!(py, inst, "assert inst.get_kwargs(1,2,3) == [(1,2,3), None]"); + py_run!( + py, + inst, + "assert inst.get_kwargs(t=1,n=2) == [(), {'t': 1, 'n': 2}]" + ); + py_run!( + py, + inst, + "assert inst.get_kwargs(1,2,3,t=1,n=2) == [(1,2,3), {'t': 1, 'n': 2}]" + ); + + py_run!(py, inst, "assert inst.get_pos_arg_kw(1) == [1, (), None]"); + py_run!( + py, + inst, + "assert inst.get_pos_arg_kw(1, 2, 3) == [1, (2, 3), None]" + ); + py_run!( + py, + inst, + "assert inst.get_pos_arg_kw(1, b=2) == [1, (), {'b': 2}]" + ); + py_run!(py, inst, "assert inst.get_pos_arg_kw(a=1) == [1, (), None]"); + py_expect_exception!(py, inst, "inst.get_pos_arg_kw()", PyTypeError); + py_expect_exception!(py, inst, "inst.get_pos_arg_kw(1, a=1)", PyTypeError); + py_expect_exception!(py, inst, "inst.get_pos_arg_kw(b=2)", PyTypeError); + + py_run!(py, inst, "assert inst.get_pos_only(10, 11) == 21"); + py_expect_exception!(py, inst, "inst.get_pos_only(10, b = 11)", PyTypeError); + py_expect_exception!(py, inst, "inst.get_pos_only(a = 10, b = 11)", PyTypeError); + + py_run!(py, inst, "assert inst.get_pos_only_and_pos(10, 11) == 21"); + py_run!( + py, + inst, + "assert inst.get_pos_only_and_pos(10, b = 11) == 21" + ); + py_expect_exception!( + py, + inst, + "inst.get_pos_only_and_pos(a = 10, b = 11)", + PyTypeError + ); + + py_run!( + py, + inst, + "assert inst.get_pos_only_and_pos_and_kw(10, 11) == 26" + ); + py_run!( + py, + inst, + "assert inst.get_pos_only_and_pos_and_kw(10, b = 11) == 26" + ); + py_run!( + py, + inst, + "assert inst.get_pos_only_and_pos_and_kw(10, 11, c = 0) == 21" + ); + py_run!( + py, + inst, + "assert inst.get_pos_only_and_pos_and_kw(10, b = 11, c = 0) == 21" + ); + py_expect_exception!( + py, + inst, + "inst.get_pos_only_and_pos_and_kw(a = 10, b = 11)", + PyTypeError + ); + + py_run!( + py, + inst, + "assert inst.get_pos_only_and_kw_only(10, b = 11) == 21" + ); + py_expect_exception!( + py, + inst, + "inst.get_pos_only_and_kw_only(10, 11)", + PyTypeError + ); + py_expect_exception!( + py, + inst, + "inst.get_pos_only_and_kw_only(a = 10, b = 11)", + PyTypeError + ); + + py_run!( + py, + inst, + "assert inst.get_pos_only_and_kw_only_with_default(10) == 13" + ); + py_run!( + py, + inst, + "assert inst.get_pos_only_and_kw_only_with_default(10, b = 11) == 21" + ); + py_expect_exception!( + py, + inst, + "inst.get_pos_only_and_kw_only_with_default(10, 11)", + PyTypeError + ); + py_expect_exception!( + py, + inst, + "inst.get_pos_only_and_kw_only_with_default(a = 10, b = 11)", + PyTypeError + ); + + py_run!( + py, + inst, + "assert inst.get_all_arg_types_together(10, 10, c = 10) == 35" + ); + py_run!( + py, + inst, + "assert inst.get_all_arg_types_together(10, 10, c = 10, d = 10) == 40" + ); + py_run!( + py, + inst, + "assert inst.get_all_arg_types_together(10, b = 10, c = 10, d = 10) == 40" + ); + py_expect_exception!( + py, + inst, + "inst.get_all_arg_types_together(10, 10, 10)", + PyTypeError + ); + py_expect_exception!( + py, + inst, + "inst.get_all_arg_types_together(a = 10, b = 10, c = 10)", + PyTypeError + ); + + py_run!(py, inst, "assert inst.get_pos_only_with_varargs(10) == 10"); + py_run!( + py, + inst, + "assert inst.get_pos_only_with_varargs(10, 10) == 20" + ); + py_run!( + py, + inst, + "assert inst.get_pos_only_with_varargs(10, 10, 10, 10, 10) == 50" + ); + py_expect_exception!( + py, + inst, + "inst.get_pos_only_with_varargs(a = 10)", + PyTypeError + ); + + py_run!( + py, + inst, + "assert inst.get_pos_only_with_kwargs(10) == [10, None]" + ); + py_run!( + py, + inst, + "assert inst.get_pos_only_with_kwargs(10, b = 10) == [10, {'b': 10}]" + ); + py_run!( + py, + inst, + "assert inst.get_pos_only_with_kwargs(10, b = 10, c = 10, d = 10, e = 10) == [10, {'b': 10, 'c': 10, 'd': 10, 'e': 10}]" + ); + py_expect_exception!( + py, + inst, + "inst.get_pos_only_with_kwargs(a = 10)", + PyTypeError + ); + py_expect_exception!( + py, + inst, + "inst.get_pos_only_with_kwargs(a = 10, b = 10)", + PyTypeError + ); + + py_run!(py, inst, "assert inst.get_kwargs_only_with_defaults() == 5"); + py_run!( + py, + inst, + "assert inst.get_kwargs_only_with_defaults(a = 8) == 11" + ); + py_run!( + py, + inst, + "assert inst.get_kwargs_only_with_defaults(b = 8) == 10" + ); + py_run!( + py, + inst, + "assert inst.get_kwargs_only_with_defaults(a = 1, b = 1) == 2" + ); + py_run!( + py, + inst, + "assert inst.get_kwargs_only_with_defaults(b = 1, a = 1) == 2" + ); + + py_run!(py, inst, "assert inst.get_kwargs_only(a = 1, b = 1) == 2"); + py_run!(py, inst, "assert inst.get_kwargs_only(b = 1, a = 1) == 2"); + + py_run!( + py, + inst, + "assert inst.get_kwargs_only_with_some_default(a = 2, b = 1) == 3" + ); + py_run!( + py, + inst, + "assert inst.get_kwargs_only_with_some_default(b = 1) == 2" + ); + py_run!( + py, + inst, + "assert inst.get_kwargs_only_with_some_default(b = 1, a = 2) == 3" + ); + py_expect_exception!( + py, + inst, + "inst.get_kwargs_only_with_some_default()", + PyTypeError + ); + + py_run!( + py, + inst, + "assert inst.get_args_and_required_keyword(1, 2, a=3) == ((1, 2), 3)" + ); + py_run!( + py, + inst, + "assert inst.get_args_and_required_keyword(a=1) == ((), 1)" + ); + py_expect_exception!( + py, + inst, + "inst.get_args_and_required_keyword()", + PyTypeError + ); + + py_run!(py, inst, "assert inst.get_pos_arg_kw_sep1(1) == 6"); + py_run!(py, inst, "assert inst.get_pos_arg_kw_sep1(1, 2) == 6"); + py_run!( + py, + inst, + "assert inst.get_pos_arg_kw_sep1(1, 2, c=13) == 16" + ); + py_run!( + py, + inst, + "assert inst.get_pos_arg_kw_sep1(a=1, b=2, c=13) == 16" + ); + py_run!( + py, + inst, + "assert inst.get_pos_arg_kw_sep1(b=2, c=13, a=1) == 16" + ); + py_run!( + py, + inst, + "assert inst.get_pos_arg_kw_sep1(c=13, b=2, a=1) == 16" + ); + py_expect_exception!(py, inst, "inst.get_pos_arg_kw_sep1(1, 2, 3)", PyTypeError); + + py_run!(py, inst, "assert inst.get_pos_arg_kw_sep2(1) == 6"); + py_run!( + py, + inst, + "assert inst.get_pos_arg_kw_sep2(1, b=12, c=13) == 26" + ); + py_expect_exception!(py, inst, "inst.get_pos_arg_kw_sep2(1, 2)", PyTypeError); + + py_run!(py, inst, "assert inst.get_pos_kw(1, b=2) == [1, {'b': 2}]"); + py_expect_exception!(py, inst, "inst.get_pos_kw(1,2)", PyTypeError); + + py_run!(py, inst, "assert inst.args_as_vec(1,2,3) == 6"); + }); + } +} + #[pyclass] -struct MethArgs {} +struct MethSignature {} #[pymethods] -impl MethArgs { - #[args(test)] +impl MethSignature { + #[pyo3(signature = (test = None))] fn get_optional(&self, test: Option) -> i32 { test.unwrap_or(10) } + #[pyo3(signature = (test = None))] fn get_optional2(&self, test: Option) -> Option { test } - #[args(test = "None")] - fn get_optional3(&self, test: Option) -> Option { - test - } fn get_optional_positional( &self, _t1: Option, @@ -187,20 +642,20 @@ impl MethArgs { t2 } - #[args(test = "10")] + #[pyo3(signature = (test = 10))] fn get_default(&self, test: i32) -> i32 { test } - #[args("*", test = 10)] + #[pyo3(signature = (*, test = 10))] fn get_kwarg(&self, test: i32) -> i32 { test } - #[args(args = "*", kwargs = "**")] + #[pyo3(signature = (*args, **kwargs))] fn get_kwargs(&self, py: Python<'_>, args: &PyTuple, kwargs: Option<&PyDict>) -> PyObject { [args.into(), kwargs.to_object(py)].to_object(py) } - #[args(args = "*", kwargs = "**")] + #[pyo3(signature = (a, *args, **kwargs))] fn get_pos_arg_kw( &self, py: Python<'_>, @@ -211,42 +666,42 @@ impl MethArgs { [a.to_object(py), args.into(), kwargs.to_object(py)].to_object(py) } - #[args(a, b, "/")] + #[pyo3(signature = (a, b, /))] fn get_pos_only(&self, a: i32, b: i32) -> i32 { a + b } - #[args(a, "/", b)] + #[pyo3(signature = (a, /, b))] fn get_pos_only_and_pos(&self, a: i32, b: i32) -> i32 { a + b } - #[args(a, "/", b, c = 5)] + #[pyo3(signature = (a, /, b, c = 5))] fn get_pos_only_and_pos_and_kw(&self, a: i32, b: i32, c: i32) -> i32 { a + b + c } - #[args(a, "/", "*", b)] + #[pyo3(signature = (a, /, *, b))] fn get_pos_only_and_kw_only(&self, a: i32, b: i32) -> i32 { a + b } - #[args(a, "/", "*", b = 3)] + #[pyo3(signature = (a, /, *, b = 3))] fn get_pos_only_and_kw_only_with_default(&self, a: i32, b: i32) -> i32 { a + b } - #[args(a, "/", b, "*", c, d = 5)] + #[pyo3(signature = (a, /, b, *, c, d = 5))] fn get_all_arg_types_together(&self, a: i32, b: i32, c: i32, d: i32) -> i32 { a + b + c + d } - #[args(a, "/", args = "*")] + #[pyo3(signature = (a, /, *args))] fn get_pos_only_with_varargs(&self, a: i32, args: Vec) -> i32 { a + args.iter().sum::() } - #[args(a, "/", kwargs = "**")] + #[pyo3(signature = (a, /, **kwargs))] fn get_pos_only_with_kwargs( &self, py: Python<'_>, @@ -256,58 +711,57 @@ impl MethArgs { [a.to_object(py), kwargs.to_object(py)].to_object(py) } - #[args("*", a = 2, b = 3)] + #[pyo3(signature = (*, a = 2, b = 3))] fn get_kwargs_only_with_defaults(&self, a: i32, b: i32) -> i32 { a + b } - #[args("*", a, b)] + #[pyo3(signature = (*, a, b))] fn get_kwargs_only(&self, a: i32, b: i32) -> i32 { a + b } - #[args("*", a = 1, b)] + #[pyo3(signature = (*, a = 1, b))] fn get_kwargs_only_with_some_default(&self, a: i32, b: i32) -> i32 { a + b } - #[args(args = "*", a)] + #[pyo3(signature = (*args, a))] fn get_args_and_required_keyword(&self, py: Python<'_>, args: &PyTuple, a: i32) -> PyObject { (args, a).to_object(py) } - #[args(a, b = 2, "*", c = 3)] + #[pyo3(signature = (a, b = 2, *, c = 3))] fn get_pos_arg_kw_sep1(&self, a: i32, b: i32, c: i32) -> i32 { a + b + c } - #[args(a, "*", b = 2, c = 3)] + #[pyo3(signature = (a, *, b = 2, c = 3))] fn get_pos_arg_kw_sep2(&self, a: i32, b: i32, c: i32) -> i32 { a + b + c } - #[args(kwargs = "**")] + #[pyo3(signature = (a, **kwargs))] fn get_pos_kw(&self, py: Python<'_>, a: i32, kwargs: Option<&PyDict>) -> PyObject { [a.to_object(py), kwargs.to_object(py)].to_object(py) } + // "args" can be anything that can be extracted from PyTuple - #[args(args = "*")] + #[pyo3(signature = (*args))] fn args_as_vec(&self, args: Vec) -> i32 { args.iter().sum() } } #[test] -fn meth_args() { +fn meth_signature() { Python::with_gil(|py| { - let inst = Py::new(py, MethArgs {}).unwrap(); + let inst = Py::new(py, MethSignature {}).unwrap(); py_run!(py, inst, "assert inst.get_optional() == 10"); py_run!(py, inst, "assert inst.get_optional(100) == 100"); py_run!(py, inst, "assert inst.get_optional2() == None"); py_run!(py, inst, "assert inst.get_optional2(100) == 100"); - py_run!(py, inst, "assert inst.get_optional3() == None"); - py_run!(py, inst, "assert inst.get_optional3(100) == 100"); py_run!( py, inst, @@ -890,6 +1344,11 @@ impl r#RawIdents { #[classattr] const r#CLASS_ATTR_CONST: i32 = 6; + + #[pyo3(signature = (r#struct = "foo"))] + fn method_with_keyword<'a>(&self, r#struct: &'a str) -> &'a str { + r#struct + } } #[test] @@ -923,6 +1382,10 @@ fn test_raw_idents() { assert raw_idents_type.class_attr_fn == 5 assert raw_idents_type.CLASS_ATTR_CONST == 6 + + assert instance.method_with_keyword() == "foo" + assert instance.method_with_keyword("bar") == "bar" + assert instance.method_with_keyword(struct="baz") == "baz" "# ); }) @@ -1038,7 +1501,7 @@ fn test_option_pyclass_arg() { #[pyclass] struct SomePyClass {} - #[pyfunction(arg = "None")] + #[pyfunction(signature = (arg=None))] fn option_class_arg(arg: Option<&SomePyClass>) -> Option { arg.map(|_| SomePyClass {}) } diff --git a/tests/test_module.rs b/tests/test_module.rs index 61f6b103..f3977299 100644 --- a/tests/test_module.rs +++ b/tests/test_module.rs @@ -291,16 +291,16 @@ fn test_module_nesting() { // Test that argument parsing specification works for pyfunctions -#[pyfunction(a = 5, vararg = "*")] -fn ext_vararg_fn(py: Python<'_>, a: i32, vararg: &PyTuple) -> PyObject { - [a.to_object(py), vararg.into()].to_object(py) +#[pyfunction(signature = (a=5, *args))] +fn ext_vararg_fn(py: Python<'_>, a: i32, args: &PyTuple) -> PyObject { + [a.to_object(py), args.into()].to_object(py) } #[pymodule] fn vararg_module(_py: Python<'_>, m: &PyModule) -> PyResult<()> { - #[pyfn(m, a = 5, vararg = "*")] - fn int_vararg_fn(py: Python<'_>, a: i32, vararg: &PyTuple) -> PyObject { - ext_vararg_fn(py, a, vararg) + #[pyfn(m, signature = (a=5, *args))] + fn int_vararg_fn(py: Python<'_>, a: i32, args: &PyTuple) -> PyObject { + ext_vararg_fn(py, a, args) } m.add_function(wrap_pyfunction!(ext_vararg_fn, m)?).unwrap(); @@ -361,7 +361,7 @@ fn pyfunction_with_module_and_arg(module: &PyModule, string: String) -> PyResult module.name().map(|s| (s, string)) } -#[pyfunction(string = "\"foo\"")] +#[pyfunction(signature = (string="foo"))] #[pyo3(pass_module)] fn pyfunction_with_module_and_default_arg<'a>( module: &'a PyModule, @@ -370,7 +370,7 @@ fn pyfunction_with_module_and_default_arg<'a>( module.name().map(|s| (s, string.into())) } -#[pyfunction(args = "*", kwargs = "**")] +#[pyfunction(signature = (*args, **kwargs))] #[pyo3(pass_module)] fn pyfunction_with_module_and_args_kwargs<'a>( module: &'a PyModule, diff --git a/tests/test_pyfunction.rs b/tests/test_pyfunction.rs index f42c6f7e..55bb5422 100644 --- a/tests/test_pyfunction.rs +++ b/tests/test_pyfunction.rs @@ -11,7 +11,7 @@ use pyo3::types::{self, PyCFunction}; mod common; -#[pyfunction(arg = "true")] +#[pyfunction(signature = (arg = true))] fn optional_bool(arg: Option) -> String { format!("{:?}", arg) } @@ -181,7 +181,7 @@ fn test_from_py_with_defaults() { int.unwrap_or(0) } - #[pyfunction(len = "0")] + #[pyfunction(signature = (len=0))] fn from_py_with_default(#[pyo3(from_py_with = "PyAny::len")] len: usize) -> usize { len } diff --git a/tests/test_text_signature.rs b/tests/test_text_signature.rs index 86f120de..51b69cce 100644 --- a/tests/test_text_signature.rs +++ b/tests/test_text_signature.rs @@ -46,7 +46,7 @@ fn class_with_docs_and_signature() { #[pymethods] impl MyClass { #[new] - #[args(a, b = "None", "*", c = 42)] + #[pyo3(signature = (a, b=None, *, c=42))] fn __new__(a: i32, b: Option, c: i32) -> Self { let _ = (a, b, c); Self {} @@ -79,7 +79,7 @@ fn class_with_signature() { #[pymethods] impl MyClass { #[new] - #[args(a, b = "None", "*", c = 42)] + #[pyo3(signature = (a, b=None, *, c=42))] fn __new__(a: i32, b: Option, c: i32) -> Self { let _ = (a, b, c); Self {} @@ -104,7 +104,7 @@ fn class_with_signature() { #[test] fn test_function() { - #[pyfunction(a, b = "None", "*", c = 42)] + #[pyfunction(signature = (a, b=None, *, c=42))] #[pyo3(text_signature = "(a, b=None, *, c=42)")] fn my_function(a: i32, b: Option, c: i32) { let _ = (a, b, c); @@ -121,7 +121,7 @@ fn test_function() { fn test_pyfn() { #[pymodule] fn my_module(_py: Python<'_>, m: &PyModule) -> PyResult<()> { - #[pyfn(m, a, b = "None", "*", c = 42)] + #[pyfn(m, signature = (a, b=None, *, c=42))] #[pyo3(text_signature = "(a, b=None, *, c=42)")] fn my_function(a: i32, b: Option, c: i32) { let _ = (a, b, c); diff --git a/tests/test_variable_arguments.rs b/tests/test_variable_arguments.rs index 8bc74aa9..9f72bbda 100644 --- a/tests/test_variable_arguments.rs +++ b/tests/test_variable_arguments.rs @@ -11,13 +11,13 @@ struct MyClass {} #[pymethods] impl MyClass { #[staticmethod] - #[args(args = "*")] + #[pyo3(signature = (*args))] fn test_args(args: &PyTuple) -> &PyTuple { args } #[staticmethod] - #[args(kwargs = "**")] + #[pyo3(signature = (**kwargs))] fn test_kwargs(kwargs: Option<&PyDict>) -> Option<&PyDict> { kwargs } diff --git a/tests/test_variable_arguments_deprecated.rs b/tests/test_variable_arguments_deprecated.rs new file mode 100644 index 00000000..1c588a94 --- /dev/null +++ b/tests/test_variable_arguments_deprecated.rs @@ -0,0 +1,49 @@ +#![cfg(feature = "macros")] +#![allow(deprecated)] + +use pyo3::prelude::*; +use pyo3::types::{PyDict, PyTuple}; + +mod common; + +#[pyclass] +struct MyClass {} + +#[pymethods] +impl MyClass { + #[staticmethod] + #[args(args = "*")] + fn test_args(args: &PyTuple) -> &PyTuple { + args + } + + #[staticmethod] + #[args(kwargs = "**")] + fn test_kwargs(kwargs: Option<&PyDict>) -> Option<&PyDict> { + kwargs + } +} + +#[test] +fn variable_args() { + Python::with_gil(|py| { + let my_obj = py.get_type::(); + py_assert!(py, my_obj, "my_obj.test_args() == ()"); + py_assert!(py, my_obj, "my_obj.test_args(1) == (1,)"); + py_assert!(py, my_obj, "my_obj.test_args(1, 2) == (1, 2)"); + }); +} + +#[test] +fn variable_kwargs() { + Python::with_gil(|py| { + let my_obj = py.get_type::(); + py_assert!(py, my_obj, "my_obj.test_kwargs() == None"); + py_assert!(py, my_obj, "my_obj.test_kwargs(test=1) == {'test': 1}"); + py_assert!( + py, + my_obj, + "my_obj.test_kwargs(test1=1, test2=2) == {'test1':1, 'test2':2}" + ); + }); +} diff --git a/tests/ui/deprecations.rs b/tests/ui/deprecations.rs index 74b3c630..87e97184 100644 --- a/tests/ui/deprecations.rs +++ b/tests/ui/deprecations.rs @@ -5,6 +5,18 @@ use pyo3::prelude::*; #[pyclass(gc)] struct DeprecatedGc; -fn main() { +#[pyfunction(_opt = "None", x = "5")] +fn function_with_args(_opt: Option, _x: i32) {} +#[pyclass] +struct MyClass; + +#[pymethods] +impl MyClass { + #[args(_opt = "None", x = "5")] + fn function_with_args(&self, _opt: Option, _x: i32) {} +} + +fn main() { + function_with_args(None, 0); } diff --git a/tests/ui/deprecations.stderr b/tests/ui/deprecations.stderr index 472e656e..5d6ae67a 100644 --- a/tests/ui/deprecations.stderr +++ b/tests/ui/deprecations.stderr @@ -1,11 +1,23 @@ -error: use of deprecated constant `pyo3::impl_::deprecations::PYCLASS_GC_OPTION`: implement a `__traverse__` `#[pymethod]` instead of using `gc` option - --> tests/ui/deprecations.rs:5:11 +error: use of deprecated constant `pyo3::impl_::deprecations::PYFUNCTION_ARGUMENTS`: passing arbitrary arguments to `#[pyfunction()]` to specify the signature is being replaced by `#[pyo3(signature)]` + --> tests/ui/deprecations.rs:8:14 | -5 | #[pyclass(gc)] - | ^^ +8 | #[pyfunction(_opt = "None", x = "5")] + | ^^^^ | note: the lint level is defined here --> tests/ui/deprecations.rs:1:9 | 1 | #![deny(deprecated)] | ^^^^^^^^^^ + +error: use of deprecated constant `pyo3::impl_::deprecations::PYCLASS_GC_OPTION`: implement a `__traverse__` `#[pymethod]` instead of using `gc` option + --> tests/ui/deprecations.rs:5:11 + | +5 | #[pyclass(gc)] + | ^^ + +error: use of deprecated constant `pyo3::impl_::deprecations::PYMETHODS_ARGS_ATTRIBUTE`: the `#[args]` attribute for `#[methods]` is being replaced by `#[pyo3(signature)]` + --> tests/ui/deprecations.rs:16:12 + | +16 | #[args(_opt = "None", x = "5")] + | ^^^^ diff --git a/tests/ui/invalid_pyfunction_signatures.rs b/tests/ui/invalid_pyfunction_signatures.rs new file mode 100644 index 00000000..2bf849c0 --- /dev/null +++ b/tests/ui/invalid_pyfunction_signatures.rs @@ -0,0 +1,59 @@ +use pyo3::prelude::*; +use pyo3::types::{PyDict, PyTuple}; + +#[pyfunction] +#[pyo3(signature = ())] +fn function_with_one_argument_empty_signature(_x: i32) {} + +#[pyfunction] +#[pyo3(signature = (x))] +fn function_with_one_entry_signature_no_args() {} + +#[pyfunction] +#[pyo3(signature = (x))] +fn function_with_incorrect_argument_names(y: i32) { + let _ = y; +} + +#[pyfunction(x)] +#[pyo3(signature = (x))] +fn function_with_both_args_and_signature(x: i32) { + let _ = x; +} + +#[pyfunction] +#[pyo3(signature = (*, *args))] +fn function_with_args_after_args_sep(args: &PyTuple) { + let _ = args; +} + +#[pyfunction] +#[pyo3(signature = (*, *))] +fn function_with_args_sep_after_args_sep() {} + +#[pyfunction] +#[pyo3(signature = (**kwargs, *args))] +fn function_with_args_after_kwargs(kwargs: Option<&PyDict>, args: &PyTuple) { + let _ = args; +} + +#[pyfunction] +#[pyo3(signature = (**kwargs_a, **kwargs_b))] +fn function_with_kwargs_after_kwargs(kwargs_a: Option<&PyDict>, kwargs_b: Option<&PyDict>) { + let _ = kwargs_a; + let _ = kwargs_b; +} + +#[pyclass] +struct MyClass; + +#[pymethods] +impl MyClass { + #[args(x)] + #[pyo3(signature = (x))] + fn method_with_both_args_and_signature(&self, x: i32) { + let _ = x; + } +} + +fn main() {} diff --git a/tests/ui/invalid_pyfunction_signatures.stderr b/tests/ui/invalid_pyfunction_signatures.stderr new file mode 100644 index 00000000..6720bb87 --- /dev/null +++ b/tests/ui/invalid_pyfunction_signatures.stderr @@ -0,0 +1,53 @@ +error: missing signature entry for argument `_x` + --> tests/ui/invalid_pyfunction_signatures.rs:5:8 + | +5 | #[pyo3(signature = ())] + | ^^^^^^^^^ + +error: signature entry does not have a corresponding function argument + --> tests/ui/invalid_pyfunction_signatures.rs:9:21 + | +9 | #[pyo3(signature = (x))] + | ^ + +error: expected argument from function definition `y` but got argument `x` + --> tests/ui/invalid_pyfunction_signatures.rs:13:21 + | +13 | #[pyo3(signature = (x))] + | ^ + +error: cannot define both function signature and legacy arguments + --> tests/ui/invalid_pyfunction_signatures.rs:19:8 + | +19 | #[pyo3(signature = (x))] + | ^^^^^^^^^ + +error: `*args` not allowed after `*` + --> tests/ui/invalid_pyfunction_signatures.rs:25:24 + | +25 | #[pyo3(signature = (*, *args))] + | ^ + +error: `*` not allowed after `*` + --> tests/ui/invalid_pyfunction_signatures.rs:31:24 + | +31 | #[pyo3(signature = (*, *))] + | ^ + +error: `*args` not allowed after `**kwargs` + --> tests/ui/invalid_pyfunction_signatures.rs:35:31 + | +35 | #[pyo3(signature = (**kwargs, *args))] + | ^ + +error: `**kwargs_b` not allowed after `**kwargs_a` + --> tests/ui/invalid_pyfunction_signatures.rs:41:33 + | +41 | #[pyo3(signature = (**kwargs_a, **kwargs_b))] + | ^ + +error: cannot define both function signature and legacy arguments + --> tests/ui/invalid_pyfunction_signatures.rs:53:12 + | +53 | #[pyo3(signature = (x))] + | ^^^^^^^^^ diff --git a/tests/ui/invalid_pymethods.rs b/tests/ui/invalid_pymethods.rs index de78a1af..ab6035d8 100644 --- a/tests/ui/invalid_pymethods.rs +++ b/tests/ui/invalid_pymethods.rs @@ -113,12 +113,6 @@ impl MyClass { fn method_cannot_pass_module(&self, m: &PyModule) {} } -#[pymethods] -impl MyClass { - #[args(has_default = "1")] - fn default_arg_before_required(&self, has_default: isize, required: isize) {} -} - #[pymethods] impl MyClass { fn method_self_by_value(self){} diff --git a/tests/ui/invalid_pymethods.stderr b/tests/ui/invalid_pymethods.stderr index f64bdcdc..0cff803a 100644 --- a/tests/ui/invalid_pymethods.stderr +++ b/tests/ui/invalid_pymethods.stderr @@ -104,15 +104,15 @@ error: `pass_module` cannot be used on Python methods error: Python objects are shared, so 'self' cannot be moved out of the Python interpreter. Try `&self`, `&mut self, `slf: PyRef<'_, Self>` or `slf: PyRefMut<'_, Self>`. - --> tests/ui/invalid_pymethods.rs:124:29 + --> tests/ui/invalid_pymethods.rs:118:29 | -124 | fn method_self_by_value(self){} +118 | fn method_self_by_value(self){} | ^^^^ error[E0201]: duplicate definitions with name `__pymethod___new____`: - --> tests/ui/invalid_pymethods.rs:129:1 + --> tests/ui/invalid_pymethods.rs:123:1 | -129 | #[pymethods] +123 | #[pymethods] | ^^^^^^^^^^^^ | | | previous definition of `__pymethod___new____` here @@ -121,9 +121,9 @@ error[E0201]: duplicate definitions with name `__pymethod___new____`: = note: this error originates in the attribute macro `pymethods` (in Nightly builds, run with -Z macro-backtrace for more info) error[E0201]: duplicate definitions with name `__pymethod_func__`: - --> tests/ui/invalid_pymethods.rs:140:1 + --> tests/ui/invalid_pymethods.rs:134:1 | -140 | #[pymethods] +134 | #[pymethods] | ^^^^^^^^^^^^ | | | previous definition of `__pymethod_func__` here