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

Optional tag for internally tagged enum #2231

Open
adamspofford-dfinity opened this issue Jun 21, 2022 · 4 comments
Open

Optional tag for internally tagged enum #2231

adamspofford-dfinity opened this issue Jun 21, 2022 · 4 comments

Comments

@adamspofford-dfinity
Copy link

adamspofford-dfinity commented Jun 21, 2022

I have an internally tagged enum where one variant should be considered the 'default'. If the tag is not present, the default variant should be selected and deserialized from the other fields. Unfortunately serde_derive does not seem to support this case. The fields of the variants overlap, so serde(untagged) cannot be used; the tag should be validated and an error should be raised if it's an incorrect tag, so serde(other) cannot be used either as it accepts any tag. It would be great if the tag could be specified as optional with a variant to default to. Two ways I can think of doing that are a serde(default) variant attribute, or a serde(default_tag = "tag_name") container attribute.

adamspofford-dfinity added a commit to dfinity/sdk that referenced this issue Jul 5, 2022
@mmun
Copy link

mmun commented Oct 21, 2022

I have this use case at work as well.

The serde(default) variant attribute seems good and aligns with how serde(rename = ...) operates on the tag value / variant name when applied as a variant attribute.

A container-level attribute also seems useful. default_tag is probably fine but it is slightly confusing that tag referes to the value here but in serde(tag = ...) it refers to the tag field name. 🤷

@kangalio
Copy link

I think a #[serde(default)] variant attribute is the better choice. It aligns with Rust's built-in #[default] attribute on enum variants: https://doc.rust-lang.org/std/default/trait.Default.html#enums

@ayngling
Copy link

I also have this use case at work.

I agree with the previous poster about #[serde(default)] being the better choice.

@kskalski
Copy link

kskalski commented Jul 7, 2023

I also have this use-case and I was hoping that with the implementation of #1402 this would work, but unfortunately the case with internally tagged enum is still requiring the tag field to be present:

    #[test]
    fn test_partially_untagged_enum_with_tag_field() {
        #[derive(Serialize, Deserialize, PartialEq, Debug)]
        enum Test {
            A { a: u32, b: u32 },
            B(u32),
            #[serde(untagged)]
            C(u32),
            #[serde(untagged)]
            D { a: u32, b: u32 },
        }
        let text = r#"{ "A": { "a": 1, "b": 2 } }"#;
        assert_eq!(Test::A { a: 1, b: 2 }, serde_json::from_str(text).unwrap());
        let text = r#"{ "B": 3 }"#;
        assert_eq!(Test::B(3), serde_json::from_str(text).unwrap());
        let text = r#"4"#;
        assert_eq!(Test::C(4), serde_json::from_str(text).unwrap());
        let text = r#" { "a": 5, "b": 6 }"#;
        assert_eq!(Test::D { a: 5, b: 6 }, serde_json::from_str(text).unwrap());

        #[derive(Serialize, Deserialize, PartialEq, Debug)]
        #[serde(tag = "ty")]
        enum TestTy {
            A { a: u32, b: u32 },
            B { a: u32 }, // needs named field instead of int
            #[serde(untagged)]
            C(u32),
            #[serde(untagged)]
            D { a: u32, b: u32 },
        }

        let text = r#"{ "ty": "A", "a": 1, "b": 2 }"#;
        assert_eq!(TestTy::A { a: 1, b: 2 }, serde_json::from_str(text).unwrap());
        let text = r#"{ "ty": "B", "a": 3 }"#;
        assert_eq!(TestTy::B { a: 3 }, serde_json::from_str(text).unwrap());
        let text = r#"4"#;
        assert_eq!(TestTy::C(4), serde_json::from_str(text).unwrap()); // FAIL!
        let text = r#" { "a": 5, "b": 6 }"#;
        assert_eq!(TestTy::D { a: 5, b: 6 }, serde_json::from_str(text).unwrap()); // FAIL!
    }

gives errors like "missing field 'ty'"

@dewert99 any idea if this is expected / easy to fix?

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

No branches or pull requests

5 participants