Skip to content
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

Exhaustive internally tagged tests + support of internally tagged enums in non self-describing formats #2569

Draft
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

Mingun
Copy link
Contributor

@Mingun Mingun commented Aug 12, 2023

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 ContentDeserializer and ContentRefDeserializer do what they should not do, namely, determine the validity of certain data when calling deserialization methods. All Deserializer methods 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 any Visitor method it sees fit. It is Visitor responsibility to detect if called visit_* method appropriate or not. So I removed all decisions from ContentDeserializer and ContentRefDeserializer about validity of content in certain methods.

This change will allow to make the next step and eliminate all calls to deserialize_any from 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 replaced deserialize_any with deserialize_map because:

  • internally tagged enums serialized using serialize_map (for newtype variants) / serialize_struct (for unit and struct variants), that means that deserialization also should expect map
  • deserialization of structs looks the same -- calling Deserializer::deserialize_map or Deserializer::deserialize_struct and provide a visitor with visit_map and visit_seq. This is what both TaggedContentVisitor and InternallyTaggedUnitVisitor provides

The same considerations are used when replacing deserialize_any with deserialize_map for untagged variant; that code path is also called for adjacently tagged enums. Passed visitor supports only visit_map method.

This PR depends on the serde-deprecated/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

@Mingun Mingun changed the title Internally tagged tests Exhaustive internally tagged tests + support of internally tagged enums in non self-describing formats Aug 12, 2023
@Mingun Mingun force-pushed the internally-tagged-tests branch from 3ea3cc4 to dafede9 Compare August 12, 2023 11:11
@Mingun Mingun force-pushed the internally-tagged-tests branch from a08c600 to a20d574 Compare August 5, 2024 17:48
@Mingun Mingun force-pushed the internally-tagged-tests branch from a20d574 to 8fe93e5 Compare October 21, 2024 20:30
Mingun and others added 15 commits October 22, 2024 23:40
…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
@Mingun Mingun force-pushed the internally-tagged-tests branch from 8fe93e5 to 9702ce1 Compare October 22, 2024 18:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Support internally tagged enums with non-self-describing formats.
1 participant