From 47cf13239fb197875e54bd531cfab19df1550b71 Mon Sep 17 00:00:00 2001 From: Juniper Parsons Date: Tue, 23 Nov 2021 10:25:01 -0500 Subject: [PATCH] Disallow positional args after optional args --- CHANGELOG.md | 1 + examples/pyo3-pytests/src/datetime.rs | 2 +- examples/pyo3-pytests/tests/test_datetime.py | 2 +- pyo3-macros-backend/src/params.rs | 8 ++++++++ tests/ui/invalid_pyfunctions.rs | 3 +++ tests/ui/invalid_pyfunctions.stderr | 6 ++++++ tests/ui/invalid_pymethods.rs | 11 +++++++++++ tests/ui/invalid_pymethods.stderr | 12 ++++++++++++ 8 files changed, 43 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2e179725249..39b85f6b10d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `from_instance` -> `from_value` - `into_instance` -> `into_value` - Deprecate `PyType::is_instance`; it is inconsistent with other `is_instance` methods in PyO3. Instead of `typ.is_instance(obj)`, use `obj.is_instance(typ)`. [#2031](https://github.com/PyO3/pyo3/pull/2031) +- Optional parameters of `#[pymethods]` and `#[pyfunction]`s cannot be followed by required parameters, i.e. `fn opt_first(a: Option, b: i32) {}` is not allowed, while `fn opt_last(a:i32, b: Option) {}` is. [#2041](https://github.com/PyO3/pyo3/pull/2041) ### Removed diff --git a/examples/pyo3-pytests/src/datetime.rs b/examples/pyo3-pytests/src/datetime.rs index a3522b4845e..0a1e562ed16 100644 --- a/examples/pyo3-pytests/src/datetime.rs +++ b/examples/pyo3-pytests/src/datetime.rs @@ -49,8 +49,8 @@ fn time_with_fold<'p>( minute: u8, second: u8, microsecond: u32, - tzinfo: Option<&PyTzInfo>, fold: bool, + tzinfo: Option<&PyTzInfo>, ) -> PyResult<&'p PyTime> { PyTime::new_with_fold( py, diff --git a/examples/pyo3-pytests/tests/test_datetime.py b/examples/pyo3-pytests/tests/test_datetime.py index c8659835488..f9966f63fa6 100644 --- a/examples/pyo3-pytests/tests/test_datetime.py +++ b/examples/pyo3-pytests/tests/test_datetime.py @@ -139,7 +139,7 @@ def test_time_fold(t): @pytest.mark.xfail(PYPY, reason="Feature not available on PyPy") @pytest.mark.parametrize("fold", [False, True]) def test_time_fold(fold): - t = rdt.time_with_fold(0, 0, 0, 0, None, fold) + t = rdt.time_with_fold(0, 0, 0, 0, fold, None) assert t.fold == fold diff --git a/pyo3-macros-backend/src/params.rs b/pyo3-macros-backend/src/params.rs index e781a1d3ec9..147f70d084a 100644 --- a/pyo3-macros-backend/src/params.rs +++ b/pyo3-macros-backend/src/params.rs @@ -82,6 +82,8 @@ pub fn impl_arg_params( let mut required_positional_parameters = 0usize; let mut keyword_only_parameters = Vec::new(); + let mut all_positional_required = true; + for arg in spec.args.iter() { if arg.py || is_args(&spec.attrs, arg.name) || is_kwargs(&spec.attrs, arg.name) { continue; @@ -100,7 +102,13 @@ pub fn impl_arg_params( }); } else { if required { + ensure_spanned!( + all_positional_required, + arg.name.span() => "Required positional parameters cannot come after optional parameters" + ); required_positional_parameters += 1; + } else { + all_positional_required = false; } if posonly { positional_only_parameters += 1; diff --git a/tests/ui/invalid_pyfunctions.rs b/tests/ui/invalid_pyfunctions.rs index 680ca56e346..ab5abd2b66f 100644 --- a/tests/ui/invalid_pyfunctions.rs +++ b/tests/ui/invalid_pyfunctions.rs @@ -9,4 +9,7 @@ fn impl_trait_function(impl_trait: impl AsRef) {} #[pyfunction] async fn async_function() {} +#[pyfunction] +fn required_arg_after_optional(optional: Option, required: isize) {} + fn main() {} diff --git a/tests/ui/invalid_pyfunctions.stderr b/tests/ui/invalid_pyfunctions.stderr index 00bf963faed..fdec5afda3b 100644 --- a/tests/ui/invalid_pyfunctions.stderr +++ b/tests/ui/invalid_pyfunctions.stderr @@ -17,3 +17,9 @@ Additional crates such as `pyo3-asyncio` can be used to integrate async Rust and | 10 | async fn async_function() {} | ^^^^^ + +error: Required positional parameters cannot come after optional parameters + --> tests/ui/invalid_pyfunctions.rs:13:57 + | +13 | fn required_arg_after_optional(optional: Option, required: isize) {} + | ^^^^^^^^ diff --git a/tests/ui/invalid_pymethods.rs b/tests/ui/invalid_pymethods.rs index a0829635451..a46a190cade 100644 --- a/tests/ui/invalid_pymethods.rs +++ b/tests/ui/invalid_pymethods.rs @@ -113,4 +113,15 @@ impl MyClass { fn method_cannot_pass_module(&self, m: &PyModule) {} } +#[pymethods] +impl MyClass { + fn required_arg_after_optional(&self, optional: Option, required: isize) {} +} + +#[pymethods] +impl MyClass { + #[args(has_default = "1")] + fn default_arg_before_required(&self, has_default: isize, required: isize) {} +} + fn main() {} diff --git a/tests/ui/invalid_pymethods.stderr b/tests/ui/invalid_pymethods.stderr index 3556c2722eb..2a25508c24c 100644 --- a/tests/ui/invalid_pymethods.stderr +++ b/tests/ui/invalid_pymethods.stderr @@ -101,3 +101,15 @@ error: `pass_module` cannot be used on Python methods | 112 | #[pyo3(pass_module)] | ^^^^^^^^^^^ + +error: Required positional parameters cannot come after optional parameters + --> tests/ui/invalid_pymethods.rs:118:68 + | +118 | fn required_arg_after_optional(&self, optional: Option, required: isize) {} + | ^^^^^^^^ + +error: Required positional parameters cannot come after optional parameters + --> tests/ui/invalid_pymethods.rs:124:63 + | +124 | fn default_arg_before_required(&self, has_default: isize, required: isize) {} + | ^^^^^^^^