Skip to content

Commit

Permalink
add better error message for Python in signature
Browse files Browse the repository at this point in the history
  • Loading branch information
davidhewitt committed Feb 3, 2023
1 parent 3149a80 commit 96efb0e
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 12 deletions.
32 changes: 22 additions & 10 deletions pyo3-macros-backend/src/pyfunction/signature.rs
Original file line number Diff line number Diff line change
Expand Up @@ -352,10 +352,21 @@ impl<'a> FunctionSignature<'a> {
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 args_iter = arguments.iter_mut();

let mut next_non_py_argument_checked = |name: &syn::Ident| {
for fn_arg in args_iter.by_ref() {
if fn_arg.py {
// If the user incorrectly tried to include py: Python in the
// signature, give a useful error as a hint.
ensure_spanned!(
name != fn_arg.name,
name.span() => "arguments of type `Python` must not be part of the signature"
);
// Otherwise try next argument.
continue;
}

let mut next_argument_checked = |name: &syn::Ident| match args_iter.next() {
Some(fn_arg) => {
ensure_spanned!(
name == fn_arg.name,
name.span() => format!(
Expand All @@ -364,17 +375,17 @@ impl<'a> FunctionSignature<'a> {
name.unraw(),
)
);
Ok(fn_arg)
return Ok(fn_arg);
}
None => bail_spanned!(
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)?;
let fn_arg = next_non_py_argument_checked(&arg.ident)?;
parse_state.add_argument(
&mut python_signature,
arg.ident.unraw().to_string(),
Expand All @@ -389,12 +400,12 @@ impl<'a> FunctionSignature<'a> {
parse_state.finish_pos_args(&python_signature, sep.span())?
}
SignatureItem::Varargs(varargs) => {
let fn_arg = next_argument_checked(&varargs.ident)?;
let fn_arg = next_non_py_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)?;
let fn_arg = next_non_py_argument_checked(&kwargs.ident)?;
fn_arg.is_kwargs = true;
parse_state.add_kwargs(&mut python_signature, &kwargs)?;
}
Expand All @@ -404,7 +415,8 @@ impl<'a> FunctionSignature<'a> {
};
}

if let Some(arg) = args_iter.next() {
// Ensure no non-py arguments remain
if let Some(arg) = args_iter.find(|arg| !arg.py) {
bail_spanned!(
attribute.kw.span() => format!("missing signature entry for argument `{}`", arg.name)
);
Expand Down
5 changes: 5 additions & 0 deletions tests/ui/invalid_pyfunction_signatures.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,11 @@ fn function_with_kwargs_after_kwargs(kwargs_a: Option<&PyDict>, kwargs_b: Option
let _ = kwargs_b;
}

#[pyfunction(signature = (py))]
fn signature_contains_py(py: Python<'_>) {
let _ = py;
}

#[pyclass]
struct MyClass;

Expand Down
10 changes: 8 additions & 2 deletions tests/ui/invalid_pyfunction_signatures.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,14 @@ error: `**kwargs_b` not allowed after `**kwargs_a`
41 | #[pyo3(signature = (**kwargs_a, **kwargs_b))]
| ^

error: arguments of type `Python` must not be part of the signature
--> tests/ui/invalid_pyfunction_signatures.rs:47:27
|
47 | #[pyfunction(signature = (py))]
| ^^

error: cannot define both function signature and legacy arguments
--> tests/ui/invalid_pyfunction_signatures.rs:53:12
--> tests/ui/invalid_pyfunction_signatures.rs:58:12
|
53 | #[pyo3(signature = (x))]
58 | #[pyo3(signature = (x))]
| ^^^^^^^^^

0 comments on commit 96efb0e

Please sign in to comment.