-
-
Notifications
You must be signed in to change notification settings - Fork 777
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
Emit an error if a variant of an untagged enum is annoted with #[serde(rename)]
or #[serde(alias)]
#2797
base: master
Are you sure you want to change the base?
Emit an error if a variant of an untagged enum is annoted with #[serde(rename)]
or #[serde(alias)]
#2797
Conversation
…rde(rename)] or #[serde(alias)].
serde_derive/src/internals/ast.rs
Outdated
|
||
if container_is_untagged && attrs.name().renamed() { | ||
cx.error_spanned_by(&variant.ident, "renaming or adding a alias to a variant of an untagged enum does nothing"); | ||
} |
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.
Ideally, error should point to the each problematic attribute instead of a variant name. Also, compile tests should be added to test_suite/tests/ui
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 fixed those two things. Are there any other attributes that need to have error messages like this?
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, much better, but it is better to point to the alias = "..."
and rename = "..."
parts only, so when you have multiple attributes on a single line, errors pointed to the corresponding parts of that line.
Don't known about other attributes. You may check other if they need improvements.
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 made it point towards the alias
and rename
parts to be even more clear. I think #[serde(rename_all)]
may be another attribute that has this issue, but I couldn't get that error to work in 6d72564.
Can someone run crater-at-home on this to gauge the impact? I worry someone accidentally is using it in a crate that will impact many other crates. Or switch it to emitting a warning instead (can we do that from proc macros?) |
Unfortunately, I don't think this can be switched to a warning. Compile errors are handled by |
Fixes #2787.