Merge pull request #2041 from StarryInternet/parsons20/optional-parameters-must-be-last

Disallow positional arguments after optional arguments

Author: davidhewitt

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<i32>, b: i32) {}` is not allowed, while `fn opt_last(a:i32, b: Option<i32>) {}` is. [#2041](https://github.com/PyO3/pyo3/pull/2041)
 
 ### Removed
 

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,

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
 
 

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;

tests/ui/invalid_pyfunctions.rs

@@ -9,4 +9,7 @@ fn impl_trait_function(impl_trait: impl AsRef<PyAny>) {}
 #[pyfunction]
 async fn async_function() {}
 
+#[pyfunction]
+fn required_arg_after_optional(optional: Option<isize>, required: isize) {}
+
 fn main() {}

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<isize>, required: isize) {}
+   |                                                         ^^^^^^^^

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<isize>, required: isize) {}
+}
+
+#[pymethods]
+impl MyClass {
+    #[args(has_default = "1")]
+    fn default_arg_before_required(&self, has_default: isize, required: isize) {}
+}
+
 fn main() {}

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<isize>, 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) {}
+    |                                                               ^^^^^^^^