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

Warn when defining untagged enum with multiple unit variants #1740

Open
tpdickso opened this issue Feb 13, 2020 · 2 comments
Open

Warn when defining untagged enum with multiple unit variants #1740

tpdickso opened this issue Feb 13, 2020 · 2 comments
Labels

Comments

@tpdickso
Copy link

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
Copy link
Author

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
Copy link

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 ?

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

No branches or pull requests

3 participants