serde-rs / serde

Serialization framework for Rust
https://serde.rs/
Apache License 2.0
8.82k stars 749 forks source link

Warn when defining untagged enum with multiple unit variants #1740

Open tpdickso opened 4 years ago

tpdickso commented 4 years ago

Currently, an enum such as this will compile, but will silently result in one of the two variants not being deserializable:

#[derive(Serialize, Deserialize)]
#[serde(untagged)]
enum T {
    A,
    B,
    C(f32)
}

This behaviour is potentially unintuitive, as the intent of the programmer writing this enum is likely that T::A and T::B would be deserialized from the strings "A" and "B". A warning in this case could be helpful, particularly if a link could be provided to an entry on the "Examples" section of the documentation with a recipe for doing what the programmer intended. This warning could potentially have some sort of annotation to silence it, but the easiest solution would just be to mark all but one of the unit variants as #[serde(skip_deserializing)], which does not change the semantics of the enum and makes it clearer that only the unit variant that isn't marked as skipped will ever be deserialized.

In practice this warning could apply to any untagged variant whose valid representations are a subset of the union of previous variants, but that's fairly impractical to warn for, since deserialize impls can be arbitrary. But I think it would be sufficiently useful to have a warning for this particular case, since it's easy to detect and I feel like it could be a common first attempt at the pattern "One of several named constants, or a fully-described value", such as the use case where I discovered this:

#[derive(Deserialize)]
#[serde(untagged)]
enum Color {
    White,
    Black,
    Transparent,
    Custom(Vector4<f32>)
}
tpdickso commented 4 years ago

N.B. There is support for emitting warnings from procedural macros, but it's currently an unstable API. I think could be worth adding to nightly, though, in anticipation of stabilization. https://doc.rust-lang.org/stable/proc_macro/struct.Diagnostic.html#method.warning

Inspirateur commented 1 month ago

the intent of the programmer writing this enum is likely that T::A and T::B would be deserialized from the strings "A" and "B"

I'm having the exact same problem, any workaround for deserializing like that ?