Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add better error message for Python in signature #2930

Merged
merged 1 commit into from
Feb 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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))]
| ^^^^^^^^^