From 592ca06e0a4d617610ac91d12ededf9ed378cf82 Mon Sep 17 00:00:00 2001 From: David Hewitt <1939362+davidhewitt@users.noreply.github.com> Date: Mon, 10 Jan 2022 22:45:32 +0000 Subject: [PATCH] pyfunction: allow required positional after option --- CHANGELOG.md | 2 +- pyo3-macros-backend/src/params.rs | 13 ++------ pytests/pyo3-pytests/src/datetime.rs | 2 +- pytests/pyo3-pytests/tests/test_datetime.py | 8 ++--- tests/test_pyfunction.rs | 36 +++++++++++++++++++++ 5 files changed, 45 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 209d369b4db..eba57ef4bb7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,7 +39,6 @@ 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) - `PyErr::new_type` now takes an optional docstring and now returns `PyResult>` rather than a `ffi::PyTypeObject` pointer. - The `create_exception!` macro can now take an optional docstring. This docstring, if supplied, is visible to users (with `.__doc__` and `help()`) and accompanies your error type in your crate's documentation. @@ -62,6 +61,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Fix clippy warning `needless-option-as-deref` in code generated by `#[pyfunction]` and `#[pymethods]`. [#2040](https://github.com/PyO3/pyo3/pull/2040) - Fix undefined behavior in `PySlice::indices`. [#2061](https://github.com/PyO3/pyo3/pull/2061) - Use the Rust function path for `wrap_pymodule!` of a `#[pymodule]` with a `#[pyo3(name = "..")]` attribute, not the Python name. [#2081](https://github.com/PyO3/pyo3/pull/2081) +- Fix panic in `#[pyfunction]` generated code when a required argument following an `Option` was not provided. [#2093](https://github.com/PyO3/pyo3/pull/2093) ## [0.15.1] - 2021-11-19 diff --git a/pyo3-macros-backend/src/params.rs b/pyo3-macros-backend/src/params.rs index 85ee4b48336..ef70c41d456 100644 --- a/pyo3-macros-backend/src/params.rs +++ b/pyo3-macros-backend/src/params.rs @@ -81,8 +81,6 @@ 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,19 +98,14 @@ pub fn impl_arg_params( } }); } else { + positional_parameter_names.push(name); + 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; + required_positional_parameters = positional_parameter_names.len(); } if posonly { positional_only_parameters += 1; } - positional_parameter_names.push(name); } } diff --git a/pytests/pyo3-pytests/src/datetime.rs b/pytests/pyo3-pytests/src/datetime.rs index 7106987e94f..e833e57a377 100644 --- a/pytests/pyo3-pytests/src/datetime.rs +++ b/pytests/pyo3-pytests/src/datetime.rs @@ -51,8 +51,8 @@ fn time_with_fold<'p>( minute: u8, second: u8, microsecond: u32, - fold: bool, tzinfo: Option<&PyTzInfo>, + fold: bool, ) -> PyResult<&'p PyTime> { PyTime::new_with_fold( py, diff --git a/pytests/pyo3-pytests/tests/test_datetime.py b/pytests/pyo3-pytests/tests/test_datetime.py index f9966f63fa6..67d2da16666 100644 --- a/pytests/pyo3-pytests/tests/test_datetime.py +++ b/pytests/pyo3-pytests/tests/test_datetime.py @@ -1,12 +1,12 @@ import datetime as pdt import platform -import struct import re +import struct import sys -import pytest import pyo3_pytests.datetime as rdt -from hypothesis import given, example +import pytest +from hypothesis import example, given from hypothesis import strategies as st @@ -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, fold, None) + t = rdt.time_with_fold(0, 0, 0, 0, None, fold) assert t.fold == fold diff --git a/tests/test_pyfunction.rs b/tests/test_pyfunction.rs index 85bde39b75b..930fe780a55 100644 --- a/tests/test_pyfunction.rs +++ b/tests/test_pyfunction.rs @@ -295,3 +295,39 @@ fn use_pyfunction() { assert_eq!(f2.call1((42,)).unwrap().extract::().unwrap(), 42); }) } + +#[test] +fn required_argument_after_option() { + #[pyfunction] + pub fn foo(x: Option, y: i32) -> i32 { + y + x.unwrap_or_default() + } + + Python::with_gil(|py| { + let f = wrap_pyfunction!(foo, py).unwrap(); + + // it is an error to call this function with no arguments + py_expect_exception!( + py, + f, + "f()", + PyTypeError, + "foo() missing 2 required positional arguments: 'x' and 'y'" + ); + + // it is an error to call this function with one argument + py_expect_exception!( + py, + f, + "f(None)", + PyTypeError, + "foo() missing 1 required positional argument: 'y'" + ); + + // ok to call with two arguments + py_assert!(py, f, "f(None, 5) == 5"); + + // ok to call with keyword arguments + py_assert!(py, f, "f(x=None, y=5) == 5"); + }) +}