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

Support internally tagged enums with non-self-describing formats. #2187

Open
kevincox opened this issue Mar 9, 2022 · 5 comments · May be fixed by #2569
Open

Support internally tagged enums with non-self-describing formats. #2187

kevincox opened this issue Mar 9, 2022 · 5 comments · May be fixed by #2569

Comments

@kevincox
Copy link

kevincox commented Mar 9, 2022

Right now deserialization of internally tagged enums rely on a call to serde::Deserializer::deserialize_any which often doesn't work well with non-self-describing formats. Ideally serde::Deserializer::deserialize_struct would be called instead so that the Deserializer knows what to do.

serde/serde_derive/src/de.rs

Lines 1340 to 1342 in 7e19ae8

let __tagged = try!(_serde::Deserializer::deserialize_any(
__deserializer,
_serde::__private::de::TaggedContentVisitor::<__Field>::new(#tag, #expecting)));

@Lucretiel
Copy link

I think the trouble you'll run into here is, what do you pass as fields? deserialize_struct looks like this:

   fn deserialize_struct<V>(
        self,
        name: &'static str,
        fields: &'static [&'static str],
        visitor: V,
    ) -> Result<V::Value, Self::Error>
    where
        V: Visitor<'de>;

Where the fields are the set of field names in the destination struct. For an internally tagged struct, unless all the variants have precisely the same set of field names, there's no valid list of string to pass to fields, since the set of fields varies depending on which variant it ends up being.

@kevincox
Copy link
Author

That is a good point. Maybe deserialize_map could be used? It is generally treated as similar to struct without knowing the fields upfront. However I think this is still problematic because if you don't get the tag field first as you won't know which type to deserialize the fields as.

I guess this is just a limitation of internal tagging? Maybe it should be called out in the docs.

@jonasbb
Copy link
Contributor

jonasbb commented Mar 10, 2022

You can deserialize internally tagged enums from sequences, so forcing deserialize_map is not an option. The enum below can be deserialized from these JSONs ["Bool", false] and ["Int", 123].

#[derive(Deserialize, Serialize, Debug)]
#[serde(tag = "tag")]
enum Internal {
    Int{i: i32},
    Bool{b: bool},
}

@kevincox
Copy link
Author

Oh, I didn't realize that was supported. IIUC though the serializer always produces structs/maps though? But that would prevent fixing this as it would break backwards compatibility. Maybe for serde 2 😅

@Mingun
Copy link
Contributor

Mingun commented Aug 11, 2023

You can deserialize internally tagged enums from sequences, so forcing deserialize_map is not an option.

Actually, calling specific deserialization method is just a hint what data is expected, deserializer is not obliged to follow it. It is free to call any Visitor method it sees fit

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
4 participants