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

Non-default enum tagging broken #77

Open
sp1ff opened this issue Oct 11, 2021 · 2 comments
Open

Non-default enum tagging broken #77

sp1ff opened this issue Oct 11, 2021 · 2 comments
Labels
serde Affects serde-lexpr

Comments

@sp1ff
Copy link

sp1ff commented Oct 11, 2021

When serializing, enumeration payloads can be tagged in four ways:

  1. externally (the default)
  2. internally (#[serde(tag = "type")])
  3. adjacently (#[serde(tag = "t", content = "c")])
  4. un-tagged (#[serde(untagged)])

Internal tagging is particularly useful when you want to version your structures; you can maintain different structs in your code, and in serialized format, each will appear to have a field named "version" (or whatever). E.g.

#[derive(Deserialize, Serialize)]
pub struct FirstVersion {
  // stuff
}
#[derive(Deserialize, Serialize)]
pub struct SecondVersion {
  // similar but not the same stuff
}
#[derive(Deserialize, Serialize)]
#[serde(tag = "version")]
enum Versions {
    #[serde(name = "v1")]
    V1(FirstVersion),
    #[serde(name = "v2")]
    V1(SecondVersion),
}

If I write out V1, it will be something like ((version . "V1") (x . 1) (y . "blah")), but deserialization will fail.

Full test case-- click to expand
use serde::{Deserialize, Serialize};

#[derive(Serialize, Deserialize, Debug, PartialEq)]
pub struct Thing {
    pub x: u32,
    pub y: f64,
}

#[derive(Serialize, Deserialize, Debug, PartialEq)]
pub struct NewThing {
    pub x: u32,
    pub y: f64,
    pub z: Vec<u8>,
}

#[derive(Debug, Deserialize, PartialEq, Serialize)]
enum ExternallyTaggedThings {
    V1(Thing),
    V2(NewThing),
}

#[derive(Debug, Deserialize, PartialEq, Serialize)]
#[serde(tag = "version")]
enum InternallyTaggedThings {
    V1(Thing),
    V2(NewThing),
}

#[derive(Debug, Deserialize, PartialEq, Serialize)]
#[serde(tag = "version", content = "c")]
enum AdjacentlyTaggedThings {
    V1(Thing),
    V2(NewThing),
}

#[derive(Debug, Deserialize, PartialEq, Serialize)]
#[serde(untagged)]
enum UnTaggedThings {
    V1(Thing),
    V2(NewThing),
}

#[cfg(test)]
mod test {

    use super::*;

    #[test]
    fn test_json() {
        use serde_json::{from_str, to_string};

        let x = ExternallyTaggedThings::V1(Thing { x: 1, y: 2.0 });
        assert_eq!(to_string(&x).unwrap(), r#"{"V1":{"x":1,"y":2.0}}"#);

        let y: ExternallyTaggedThings = from_str(r#"{"V1":{"x":1,"y":2.0}}"#).unwrap();
        assert_eq!(x, y);

        let x = InternallyTaggedThings::V1(Thing { x: 1, y: 2.0 });
        assert_eq!(to_string(&x).unwrap(), r#"{"version":"V1","x":1,"y":2.0}"#);

        let y: InternallyTaggedThings = from_str(r#"{"version":"V1","x":1,"y":2.0}"#).unwrap();
        assert_eq!(x, y);

        let x = AdjacentlyTaggedThings::V1(Thing { x: 1, y: 2.0 });
        assert_eq!(
            to_string(&x).unwrap(),
            r#"{"version":"V1","c":{"x":1,"y":2.0}}"#
        );

        let y: AdjacentlyTaggedThings =
            from_str(r#"{"version":"V1","c":{"x":1,"y":2.0}}"#).unwrap();
        assert_eq!(x, y);

        let x = UnTaggedThings::V1(Thing { x: 1, y: 2.0 });
        assert_eq!(to_string(&x).unwrap(), r#"{"x":1,"y":2.0}"#);

        let y: UnTaggedThings = from_str(r#"{"x":1,"y":2.0}"#).unwrap();
        assert_eq!(x, y);
    }

    #[test]
    fn test_sexp() {
        use serde_lexpr::{from_str, to_string};

        let x = ExternallyTaggedThings::V1(Thing { x: 1, y: 2.0 });
        assert_eq!(to_string(&x).unwrap(), r#"(V1 (x . 1) (y . 2.0))"#);

        let y: ExternallyTaggedThings = from_str(r#"(V1 (x . 1) (y . 2.0))"#).unwrap();
        assert_eq!(x, y);

        let x = InternallyTaggedThings::V1(Thing { x: 1, y: 2.0 });
        assert_eq!(
            to_string(&x).unwrap(),
            r#"((version . "V1") (x . 1) (y . 2.0))"#
        );

        let y: InternallyTaggedThings =
            from_str(r#"((version . "V1") (x . 1) (y . 2.0))"#).unwrap();
        assert_eq!(x, y); // <==== FAILS HERE

        let x = AdjacentlyTaggedThings::V1(Thing { x: 1, y: 2.0 });
        assert_eq!(
            to_string(&x).unwrap(),
            r#"((version . "V1") (c (x . 1) (y . 2.0)))"#
        );

        let y: AdjacentlyTaggedThings =
            from_str(r#"((version . "V1") (c (x . 1) (y . 2.0)))"#).unwrap();
        assert_eq!(x, y); // <==== FAILS HERE

        let x = UnTaggedThings::V1(Thing { x: 1, y: 2.0 });
        assert_eq!(to_string(&x).unwrap(), r#"((x . 1) (y . 2.0))"#);

        let y: UnTaggedThings = from_str(r#"((x . 1) (y . 2.0))"#).unwrap();
        assert_eq!(x, y); // <==== FAILS HERE
    }
}

fn main() {
    println!("Hello, world!");
}

I've debugged the internally tagged case down to the point where I can see Deserializer::deserialize_identifier being invoked (serde-lexpr/serc/value, line 259) on ("version" . "V1"), expect a symbol, and fail. That call is generated by the derive(Deserialize) code, so it seems like the place to change. But it's odd that there doesn't seem to be a natural place to check the "version" tag. Also, serde-json implements deserialize_identifier simply by reading a string-- seems like it, too would be broken here, so I'm still missing something.

@sp1ff sp1ff changed the title Internal tagging of enums not honored Non-default enum tagging broken Nov 11, 2021
@rotty rotty added the serde Affects serde-lexpr label Mar 10, 2023
@IFcoltransG
Copy link

IFcoltransG commented Dec 25, 2023

It may be to do with this line that doesn't visit symbols as identifiers.

@IFcoltransG
Copy link

Looking at the serde docs, it doesn't look like there is a method to visit identifiers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
serde Affects serde-lexpr
Projects
None yet
Development

No branches or pull requests

3 participants