-
Notifications
You must be signed in to change notification settings - Fork 760
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
More ergonomic error messages for invalid #[pyclass] args #826
Conversation
5787128
to
ec2b66f
Compare
pyo3-derive-backend/src/pyclass.rs
Outdated
return Err(syn::Error::new_spanned( | ||
exp.path.clone(), | ||
"Unsupported parameter", | ||
)); | ||
format!("Expected one of gc/weakref/subclass/dict, but got {}", x), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could perhaps add a test for this case too?
tests/ui/invalid_pyclass_args.stderr
Outdated
error: Expected type name (e.g., Name), but got m :: MyClass | ||
--> $DIR/invalid_pyclass_args.rs:9:18 | ||
| | ||
9 | #[pyclass(name = m::MyClass)] | ||
| ^^^^^^^^^^ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm trying to decide if the but got x
portion of the error messages is necessary.
On the plus side, they make the error description slightly nicer to read.
However, the invalid "but got" input is already highlighted in the source code beneath the error, so we're kinda repeating the information twice.
Also, in this m::MyClass
case the formatting has been changed versus the original source code, which makes it a bit weird to read.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc @gilescope who reported the original issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, I chose a simplfied one.
Co-Authored-By: Georg Brandl <georg@python.org>
ec2b66f
to
3b17ab0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, looks great to me!
Cracking. Sent with GitHawk |
Resolves #823