From df984ec8dfeda2a2175a0762be1605106b2e0750 Mon Sep 17 00:00:00 2001 From: dvermd <315743+dvermd@users.noreply.github.com> Date: Thu, 1 Oct 2020 06:34:54 +0200 Subject: [PATCH] Keyword only arguments (#1209) * allow keyword arguments without default value * allow keyword-only arguments --- pyo3-derive-backend/src/method.rs | 8 +-- pyo3-derive-backend/src/pyfunction.rs | 44 +++++++++++---- tests/test_methods.rs | 77 +++++++++++++++++++++++++++ tests/ui/invalid_macro_args.rs | 5 -- tests/ui/invalid_macro_args.stderr | 14 ++--- 5 files changed, 117 insertions(+), 31 deletions(-) diff --git a/pyo3-derive-backend/src/method.rs b/pyo3-derive-backend/src/method.rs index 5697df95..492ca08c 100644 --- a/pyo3-derive-backend/src/method.rs +++ b/pyo3-derive-backend/src/method.rs @@ -286,7 +286,7 @@ impl<'a> FnSpec<'a> { pub fn default_value(&self, name: &syn::Ident) -> Option { for s in self.attrs.iter() { match *s { - Argument::Arg(ref path, ref opt) => { + Argument::Arg(ref path, ref opt) | Argument::Kwarg(ref path, ref opt) => { if path.is_ident(name) { if let Some(ref val) = opt { let i: syn::Expr = syn::parse_str(&val).unwrap(); @@ -294,12 +294,6 @@ impl<'a> FnSpec<'a> { } } } - Argument::Kwarg(ref path, ref opt) => { - if path.is_ident(name) { - let i: syn::Expr = syn::parse_str(&opt).unwrap(); - return Some(quote!(#i)); - } - } _ => (), } } diff --git a/pyo3-derive-backend/src/pyfunction.rs b/pyo3-derive-backend/src/pyfunction.rs index 80ac1cf3..bfb943ff 100644 --- a/pyo3-derive-backend/src/pyfunction.rs +++ b/pyo3-derive-backend/src/pyfunction.rs @@ -14,7 +14,7 @@ pub enum Argument { VarArgs(syn::Path), KeywordArgs(syn::Path), Arg(syn::Path, Option), - Kwarg(syn::Path, String), + Kwarg(syn::Path, Option), } /// The attributes of the pyfunction macro @@ -90,12 +90,10 @@ impl PyFunctionAttr { )); } if self.has_varargs { - return Err(syn::Error::new_spanned( - item, - "Positional argument or varargs(*) is not allowed after *", - )); + self.arguments.push(Argument::Kwarg(path.clone(), None)); + } else { + self.arguments.push(Argument::Arg(path.clone(), None)); } - self.arguments.push(Argument::Arg(path.clone(), None)); Ok(()) } @@ -128,7 +126,8 @@ impl PyFunctionAttr { self.kw_arg_is_ok(item)?; if self.has_varargs { // kw only - self.arguments.push(Argument::Kwarg(name.clone(), value)); + self.arguments + .push(Argument::Kwarg(name.clone(), Some(value))); } else { self.has_kw = true; self.arguments @@ -251,7 +250,34 @@ mod test { Argument::Arg(parse_quote! {test1}, None), Argument::Arg(parse_quote! {test2}, Some("None".to_owned())), Argument::VarArgsSeparator, - Argument::Kwarg(parse_quote! {test3}, "None".to_owned()), + 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), ] ); } @@ -265,7 +291,7 @@ mod test { 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}, "None".to_owned()), + Argument::Kwarg(parse_quote! {test3}, Some("None".to_owned())), Argument::KeywordArgs(parse_quote! {kwargs}), ] ); diff --git a/tests/test_methods.rs b/tests/test_methods.rs index 40db57ae..b61ed09c 100644 --- a/tests/test_methods.rs +++ b/tests/test_methods.rs @@ -235,6 +235,21 @@ impl MethArgs { [a.to_object(py), args.into(), kwargs.to_object(py)].to_object(py) } + #[args("*", a = 2, b = 3)] + fn get_kwargs_only_with_defaults(&self, a: i32, b: i32) -> PyResult { + Ok(a + b) + } + + #[args("*", a, b)] + fn get_kwargs_only(&self, a: i32, b: i32) -> PyResult { + Ok(a + b) + } + + #[args("*", a = 1, b)] + fn get_kwargs_only_with_some_default(&self, a: i32, b: i32) -> PyResult { + Ok(a + b) + } + #[args(a, b = 2, "*", c = 3)] fn get_pos_arg_kw_sep1(&self, a: i32, b: i32, c: i32) -> PyResult { Ok(a + b + c) @@ -308,6 +323,53 @@ fn meth_args() { 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_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_pos_arg_kw_sep1(1) == 6"); py_run!(py, inst, "assert inst.get_pos_arg_kw_sep1(1, 2) == 6"); py_run!( @@ -315,6 +377,21 @@ fn meth_args() { 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"); diff --git a/tests/ui/invalid_macro_args.rs b/tests/ui/invalid_macro_args.rs index f99f5814..241d35c6 100644 --- a/tests/ui/invalid_macro_args.rs +++ b/tests/ui/invalid_macro_args.rs @@ -5,11 +5,6 @@ fn pos_after_kw(py: Python, a: i32, b: i32) -> PyObject { [a.to_object(py), vararg.into()].to_object(py) } -#[pyfunction(a, "*", b)] -fn pos_after_separator(py: Python, a: i32, b: i32) -> PyObject { - [a.to_object(py), vararg.into()].to_object(py) -} - #[pyfunction(kwargs = "**", a = 5)] fn kw_after_kwargs(py: Python, kwargs: &PyDict, a: i32) -> PyObject { [a.to_object(py), vararg.into()].to_object(py) diff --git a/tests/ui/invalid_macro_args.stderr b/tests/ui/invalid_macro_args.stderr index 1e9dab29..20e0dee0 100644 --- a/tests/ui/invalid_macro_args.stderr +++ b/tests/ui/invalid_macro_args.stderr @@ -4,14 +4,8 @@ error: Positional argument or varargs(*) is not allowed after keyword arguments 3 | #[pyfunction(a = 5, b)] | ^ -error: Positional argument or varargs(*) is not allowed after * - --> $DIR/invalid_macro_args.rs:8:22 - | -8 | #[pyfunction(a, "*", b)] - | ^ - error: Keyword argument or kwargs(**) is not allowed after kwargs(**) - --> $DIR/invalid_macro_args.rs:13:29 - | -13 | #[pyfunction(kwargs = "**", a = 5)] - | ^^^^^ + --> $DIR/invalid_macro_args.rs:8:29 + | +8 | #[pyfunction(kwargs = "**", a = 5)] + | ^^^^^