Exhaustive internally tagged tests + support of internally tagged enums in non self-describing formats #2569
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
Mingun
changed the title
Mingun
force-pushed
the
internally-tagged-tests
branch
from
3ea3cc4
to
dafede9
Compare
Mingun
force-pushed
the
internally-tagged-tests
branch
from
a08c600
to
a20d574
Compare
Mingun
force-pushed
the
internally-tagged-tests
branch
from
a20d574
to
8fe93e5
Compare
…dd comments about coverage
…internally_tagged_variant deserialize_untagged_variant in that place is called when deserialzie_with attribute is set. In that case it performs special actions that is better to use explicitly in deserialize_untagged_variant for readability
failures(1):
newtype_unit
…lize enums Helper methods visit_content_[seq|map] does the same as [Seq|Map]Deserializer::deserialize_any and used everywhere except here. Reuse them for consistency
…sibility of the Visitor
Examples of errors produced during deserialization of internally tagged enums in tests
if instead of a Seq/Map a Str("unexpected string") will be provided:
In tests/test_annotations.rs
flatten::enum_::internally_tagged::tuple:
before: `invalid type: string "unexpected string", expected tuple variant`
after : `invalid type: string "unexpected string", expected tuple variant Enum::Tuple`
flatten::enum_::internally_tagged::struct_from_map:
before: `invalid type: string "unexpected string", expected struct variant`
after : `invalid type: string "unexpected string", expected struct variant Enum::Struct`
…zer directly That is cheap, creating a Content[Ref]Deserializer is a no-op
Deserializer methods are only hints which deserializer is not obliged to follow. Both TaggedContentVisitor and InternallyTaggedUnitVisitor accepts only visit_map and visit_seq and that is what derived implementation of Deserialize does for structs. Therefore it is fine to call deserialize_map here, as that already did in derived deserialize implementation
…ms in non self-describing formats Visitor passed to the deserialize_any supports only visit_map method, so we can always request deserialize_map
…untagged and adjacently tagged enums
Mingun
force-pushed
the
internally-tagged-tests
branch
from
8fe93e5
to
9702ce1
Compare
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 PR starts as a refactor of internally tagged enums tests in preparation to updating #1922, but in the process of development, I found some interesting consequences.
The first is that
ContentDeserializerandContentRefDeserializerdo what they should not do, namely, determine the validity of certain data when calling deserialization methods. AllDeserializermethods are just a hints what data is expected from the deserializer, and the deserializer is not obliged to follow them. It is free to call anyVisitormethod it sees fit. It isVisitorresponsibility to detect if calledvisit_*method appropriate or not. So I removed all decisions fromContentDeserializerandContentRefDeserializerabout validity of content in certain methods.This change will allow to make the next step and eliminate all calls to
deserialize_anyfrom internally tagged enums. Because this method does not called anymore, this would allow to support internally tagged enums in non-self-describing formats. I've replaceddeserialize_anywithdeserialize_mapbecause:serialize_map(for newtype variants) /serialize_struct(for unit and struct variants), that means that deserialization also should expect mapDeserializer::deserialize_maporDeserializer::deserialize_structand provide a visitor withvisit_mapandvisit_seq. This is what bothTaggedContentVisitorandInternallyTaggedUnitVisitorprovidesThe same considerations are used when replacing
deserialize_anywithdeserialize_mapfor untagged variant; that code path is also called for adjacently tagged enums. Passed visitor supports onlyvisit_mapmethod.This PR depends on the serde-rs/test#31 and I temporary add a
[patch]section to use patched version to make tests passable. That is why this PR in a draft stage.Closes #2187
Replaces #2557