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

Round Trip of Untagged Enum Struct Variant is Failing #2482

Closed
paxsonsa opened this issue Jun 23, 2023 · 3 comments
Closed

Round Trip of Untagged Enum Struct Variant is Failing #2482

paxsonsa opened this issue Jun 23, 2023 · 3 comments

Comments

@paxsonsa
Copy link

paxsonsa commented Jun 23, 2023

Hey there,

Using the latest version of serde_json I seem to have found an issue when round tripping an untagged enum struct variant with similarly named fields. Here is a playground you can try. What am I missing here?

https://www.rustexplorer.com/b/2w9j4u

Example code:

/*
[dependencies]
serde = { version = "1.0.164", features = ["derive", "rc"] }
serde_json = "1.0.97"
*/

use serde::{Deserialize, Serialize};

#[derive(Debug, Serialize, Deserialize, Clone)]
pub struct PropertyBinding {
    pub fieldA: String,
    pub fieldB: String,
}

#[derive(Debug, Serialize, Deserialize, Clone)]
#[serde(untagged)]
pub enum MyEnum {
    VariantA {
        value: (f64, f64, f64),
    },

    VariantB {
        value: (f64, f64, f64),
        inner: PropertyBinding,
    },
}

impl MyEnum {
    /// Create new property state bound to the given namespace and property.
    pub fn variantb(fieldA: String, fieldB: String, value: (f64, f64, f64)) -> Self {
        Self::VariantB {
            value,
            inner: PropertyBinding {
                fieldA,
                fieldB,
            },
        }
    }
}

fn main() {
    let state = MyEnum::variantb("namespace".to_string(), "property".to_string(), (5.0, 5.0, 5.0));
    let data = serde_json::to_string_pretty(&state).unwrap();
    println!("{}", data);

    let state: MyEnum = serde_json::from_str(&data).unwrap();
    println!("{:?}", state);
    assert!(matches!(state, MyEnum::VariantB { .. })); // << Paincs
}

Output:

{
  "value": [
    5.0,
    5.0,
    5.0
  ],
  "inner": {
    "fieldA": "namespace",
    "fieldB": "property"
  }
}
VariantA { value: (5.0, 5.0, 5.0) }
thread 'main' panicked at 'assertion failed: matches!(state, MyEnum :: VariantB { .. })', src/main.rs:48:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
@paxsonsa paxsonsa changed the title Round Trip of Enum Struct Variant is Failing Round Trip of Untagged Enum Struct Variant is Failing Jun 23, 2023
@paxsonsa
Copy link
Author

Closing issue, the solution here is to use #[serde(untagged, deny_unknown_fields)] on the MyEnum

@Mingun
Copy link
Contributor

Mingun commented Jun 23, 2023

Correct solution is to swap the order of the variants, because VariantA is a subset of VariantB. Untagged enums tries each variant in the declaration order until deserialization would be successful. But VariantA can be successfully deserialized from any VariantB representation

@paxsonsa
Copy link
Author

paxsonsa commented Jul 2, 2023

Thanks for taking the time to respond! I wasn't aware that the ordering of variants affected the deserialization! I must have missed that in the docs! Good to know!

That strikes me as something that could slip up some APIs over time as new variants are added to a enum as that behaviour is not explicitly clear reading the code which is why I opt'd for deny_unknown_fields. That is not to say your idea is wrong by any means just less ideal for my situation.

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

2 participants