serde icon indicating copy to clipboard operation
serde copied to clipboard

Internally tagged enums do not correctly handle field collision with the discriminator

Open CSRessel opened this issue 4 months ago • 1 comments

Related to (open): https://github.com/serde-rs/serde/issues/2197

And related to (closed): https://github.com/serde-rs/serde/issues/1161

Models

#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
#[serde(tag = "type")]
pub enum Direction {
    #[serde(rename="left")]
    Left(Box<models::LeftDirection>),
    #[serde(rename="right")]
    Right(Box<models::RightDirection>),
}
#[derive(Clone, Default, Debug, PartialEq, Serialize, Deserialize)]
pub struct LeftDirection {
    #[serde(rename = "distance")]
    pub distance: f64,
    #[serde(rename = "type")]
    pub r#type: String,
}
#[derive(Clone, Default, Debug, PartialEq, Serialize, Deserialize)]
pub struct RightDirection {
    #[serde(rename = "distance")]
    pub distance: f64,
    #[serde(rename = "type")]
    pub r#type: String,
}

Repro

Initializing model...
[
    Left(
        LeftDirection {
            distance: 5.0,
            type: "left",
        },
    ),
    Right(
        RightDirection {
            distance: 1.0,
            type: "right",
        },
    ),
]

Trying serialization...
✗ Serialization failed!
expected: [{"type":"left","distance":5.0},{"type":"right","distance":1.0}]
actual:   [{"type":"left","distance":5.0,"type":"left"},{"type":"right","distance":1.0,"type":"right"}]

Trying deserialization...
✗ Deserialization failed: missing field `type` at line 7 column 10

Trying roundtrip...
✓ Roundtrip write succeeded...
✗ Roundtrip failed on read, after write: duplicate field `type` at line 1 column 37

Proposal

There are multiple ways that this could reasonably be handled

  1. Compile time error if internal member has a conflicting discriminator
  2. No runtime error at deserialization, and no runtime error at serialization IFF discriminator value matches expected enum discriminator
  3. No runtime error at deserialization or at serialization, and field is overwritten if the value is not the expected enum discriminator
  4. Runtime error at both serialization and deserialization

Currently there is a runtime error at deserialization, there is no runtime error at serialization, and then the JSON that is output has duplicate field names. In my opinion those first two things should be incompatible, and in my opinion that third thing is never desirable.

If it influences which of the above options makes the most sense:

  • (not sure if it's guaranteed in the serde project, but) I would consider it most intuitive if the ser -> deser behavior was the same as the deser -> ser
  • enums in the OpenAPI spec are by default tagged like this for performance, and the schemas that are used for code generation also explicitly state the discriminator field in the member struct

Rust playground: https://play.rust-lang.org/?version=stable&mode=debug&edition=2024&gist=c2f83fc0d3a269c71bd53cdfd20f0f50 Here is a full repro: https://github.com/CSRessel/serde-internal-tag-repro

CSRessel avatar Jul 31 '25 01:07 CSRessel

Obviously in my particular case (with the OpenAPI generated models) I can work around this with untagged enums! So I don't feel strongly about any of the suggested options. I wanted to open the issue just because I think the status quo here could be better, not because I need any particular one of these fixes. Let me know if any of this is unreasonable or doesn't make sense

CSRessel avatar Jul 31 '25 01:07 CSRessel