Implement correct handling of $text fields in enums and add roundtrip tests
#574
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is important for Untagged enum for deserialization. Change all definitions for consistency.
|
Looking at the |
|
Yeah, it is. Quite surprising for that widely used library. |
dralley
reviewed
Mar 7, 2023
| @@ -75,7 +70,23 @@ enum ExternallyTagged { | |||
| Empty {}, | |||
| } | |||
|
|
|||
| /// Having both `#[serde(flatten)]` and `'static` fields in one struct leads to | |||
| /// incorrect code generation when deriving `Deserialize`. | |||
There was a problem hiding this comment.
Can we have a consistent way we can mark code which depends on fixing external bugs, such that they can be easily searched? Maybe TODO(external) or something.
There was a problem hiding this comment.
Yes, I forgot to mark those places with a TODO, will do!
I used an usual TODO and NOTE markers -- there are not many of them and the todo-tree VSCode extension shows them very well.
dralley
approved these changes
Mar 7, 2023
failures (23):
with_root::char_space
with_root::enum_::externally_tagged::text::tuple
with_root::enum_::internally_tagged::nested_struct
with_root::enum_::internally_tagged::newtype
with_root::enum_::internally_tagged::struct_
with_root::enum_::internally_tagged::text
with_root::enum_::untagged::nested_struct
with_root::enum_::untagged::newtype
with_root::enum_::untagged::struct_
with_root::enum_::untagged::text
with_root::enum_::untagged::tuple_struct
with_root::enum_::untagged::unit
with_root::flatten_struct
with_root::str_escaped
with_root::tuple
without_root::enum_::externally_tagged::text::tuple
without_root::enum_::internally_tagged::nested_struct
without_root::enum_::internally_tagged::newtype
without_root::enum_::internally_tagged::struct_
without_root::enum_::internally_tagged::text
without_root::enum_::untagged::nested_struct
without_root::enum_::untagged::struct_
without_root::enum_::untagged::text
Internally tagged and untagged enums implemented via intermediate buffering
which extracts incorrect form from the XML deserializer (Map or Str).
Attempt to deserialize types from that representation failed, because only
the deserializer knows, how to convert types using the requested type hints
failures (5):
with_root::char_space
with_root::enum_::externally_tagged::text::tuple
with_root::str_escaped
with_root::tuple
without_root::enum_::externally_tagged::text::tuple
failures (2):
with_root::enum_::externally_tagged::text::tuple
without_root::enum_::externally_tagged::text::tuple
This was referenced Mar 8, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is follow-up PR for #541 in which I forgot to make a corresponding serialization for the deserialization. In order to not make this mistake again, the last 6 commits enables serialization-deserialization roundtrip in serialization tests -- we try to deserialize what was serialized and check that we get the same object.
During that I found that some roundrips does not work, mostly because of serde-rs/serde#1183. That tests does not check the roundtrip.
Some structs failed to derive
Deserializedue to serde-rs/serde#2371. Although it is possible to implement it manually or use my fork where that error is fixed specially for that, the same tests suffers from the serde-rs/serde#1183 too, so I ended up with the current approach in order to reduce dependencies.