Add #[serde(rename_all_fields = "foo")] attribute #1695
Conversation
jplatte
changed the title
|
r? @dtolnay |
|
I'll try again since Triagebot Notifications are now a thing: r? @dtolnay I've opened this PR because you wrote over at #1061 (comment) that you'd accept a PR for this. If your opinion has changed, I won't be upset if this is closed without further reasoning. |
|
@dtolnay it looks like this was approved a few months back but not merged in. |
|
@kjeremy The person who left the review doesn't seem to be associated with serde in any way. |
jplatte
force-pushed
the
rename_all_fields
branch
from
3b7f72b
to
0dd37a7
Compare
|
Rebased to trigger new CI runs, the old ones had been stuck for months. |
|
CI fixed, no merge conflics. Ping @dtolnay |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Would love this! |
|
Any update on this? Would love to have this merged as well. |
jplatte
force-pushed
the
rename_all_fields
branch
from
0dd37a7
to
f41c21b
Compare
|
Rebased to fix conflicts with the syn 2.0 upgrade. |
serde_derive/src/internals/attr.rs
Outdated
| syn::Data::Union(syn::DataUnion { union_token, .. }) => { | ||
| let msg = "#[serde(rename_all_fields)] can only be used on enums"; | ||
| cx.error_spanned_by(union_token, msg); | ||
| } |
There was a problem hiding this comment.
I think using the struct/union token for the span of this error is not a good choice. The problem is not that the user wrote a struct. The problem is that the user wrote a rename_all_fields attribute on their struct. The fix is not for the user to turn their struct into an enum. The fix is to change the attribute. So the error should point to the attribute.
There was a problem hiding this comment.
Makes sense. When I just updated this, I noticed the (container-level) #[serde(default)] does the same thing of pointing to the enum / struct token. Should I send another PR to change that?
There was a problem hiding this comment.
Yep, let's fix that too. Good catch.
jplatte
force-pushed
the
rename_all_fields
branch
from
f41c21b
to
56be1c2
Compare
fixes #1061
If you don't like the style change in the second commit, I can remove it from this PR (or you can just cherry-pick the first commit).