Integer/boolean tags for internally/adjacently tagged enums

[AmaranthineCodices]

trafficstars

Closes #745.

Implements integer/boolean renames / aliases for internally and adjacently tagged enum variants:

#[derive(Serialize, Deserialize, Debug)]
#[serde(tag = "op")]
enum Something {
    #[serde(rename = 1)]
    A { a: i32 },
    #[serde(rename = 2)]
    B(u64),
    #[serde(rename = true)]
    C,
}

#[derive(Serialize, Deserialize, Debug)]
#[serde(tag = "op", content = "d")]
enum Adjacent {
    #[serde(rename = 1)]
    A { a: i32 },
    #[serde(rename = 2)]
    B(u64),
    #[serde(rename = true)]
    C,
}

UI

In attr.rs, I introduced a new Name variant, VariantName. To cut down on code duplication, I made Name generic over its internal name representation; Name is now Names<String>. I added a step to check.rs to ensure that non-string renames are only used on internally/adjacently tagged enums. It's not necessary to check this for structs or the container name; the types of those names are still String.

Deserialization

Deserialization was relatively straightforward to implement: rather than unconditionally generating constructors/fields for strings/bytes, we only do this if there are string-named variants. We do the same for ints/bools. One concern is that VARIANTS is still &'static [&'static str], but I think this is pretty low-impact, as it's used only for stringifying error messages. Changing it would be a breaking change as it would be visible in the signatures of Error::unknown_variant and Error::unknown_field.

Serialization

For serialization, I had to add a VariantName enum to serde itself, to be used in places where an integer/boolean can appear where previously only a String existed. We generate code in serde_derive for this enum wherever we invoke these methods.

The one part of this change that I'm not happy about is here: https://github.com/AmaranthineCodices/serde/blob/37b4c68c0a9aa7aba8c6ee148c9c5e64c6f3f28c/serde_derive/src/ser.rs#L700 I am not fond of this to_string call, but getting rid of it would be a breaking change as far as I can tell - the change would be visible in the public-facing signatures of Serializer::serialize_struct and Serializer::serialize_struct_variant. This would be a good change to include in a new major serde version, though.

TODO

• [x] Make sure we handle collect_other_fields in deserialization
• [x] Revisit deserialize_generated_identifier and see if I can remove the code duplication between it and deserialize_identifier
• [x] Add unit test for integer/boolean aliases
• [x] Revisit main_constructors in deserialize_identifier

[AmaranthineCodices]

@dtolnay I think this is ready for review now.

[s1341]

Is there an example of a workaround until this lands?

I need to deserialize something like:

enum Field {
    Null,
    Integer(u64),
    Float(f64),
    String((u32, u32)),
    Blob((u32, u32)),
    Object(MyObject),
}

Where the discriminators are 0, 1, 2, 5, 7, and 12, respectively. I cannot change the data format.

I believe that I need to use a custom Visitor, but cannot find a concise example of how to do so for this kind of enum. Can you point me in the right direction please?

[ByteAlex]

Hey!

Any news on this? Can one somehow hack this serde repo in as dependency? When I tried to use it from cargo via "git = ..., branch = ...", the Deserializers of the standard serde were not recognized.

I'd really need that for a project I'm working on.

[itohatweb]

Any update on this?

[ByteAlex]

I think @AmaranthineCodices needs to fix the CI / Test suite before @dtolnay is going to look at this.

[MForster]

I have rebased this PR to HEAD. With the exception of a few minor clippy issues it passes the CI, see https://github.com/MForster/serde/pull/1.

@dtolnay, can you take look at this PR? If it is basically good to go and just needs a few minor fixes, I can take care of that.

[Tamschi]

This being integrated would be helpful for me too; It's needed to describe the scripting commands in RPG Maker MV projects, which are saved in JSON with the following shape:

{
    "code": <integer>, // tag
    "indent": <integer>, // shared field, in an outer struct that the enum is flattened into
    "parameters": [<values>] // types vary by variant
}

Currently, the most effective way to proceed there for me is to deserialise the partial object of non-shared fields into a dynamically typed Value provided by serde_json and then unpack it further from there, matching on the "code" more or less manually, but that's far from ideal.

This particular enum likely has upwards of 100 variants, so being able to generate an efficient implementation there would be very welcome. For now I'll see how well I can bodge it with an unhygienic utility macro.

---

Update: I moved to a lazy typing system that doesn't use Serde (weird use case), so I currently don't need this anymore.

[raiguard]

EDIT: I realized after posting this that I was misinterpreting the documentation, and boolean tags aren't actually what I need here. Apologies for the useless noise.

I was very saddened when I found out that this wasn't already in serde. Huge +1 from me to get this reviewed and merged!

My usecase is for the Debug Adapter Protocol's response type. This can have different fields depending on the value of the success boolean. If I could rename the variants to booleans, it would work perfectly.

#[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)]
#[serde(tag = "command")]
#[serde(rename_all = "lowercase")]
pub enum Response {
    Initialize(ResponseStatus<Capabilities>),
}

#[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)]
#[serde(tag = "success")]
pub enum ResponseStatus<T> {
    #[serde(rename = true)] // Doesn't work :(
    Success(ResponsePayload<T>),
    #[serde(rename = false)] // Doesn't work :(
    Error(ErrorResponsePayload),
}

#[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)]
pub struct ResponsePayload<T> {
    pub seq: u32,
    pub request_seq: u32,
    pub body: Option<T>,
}

#[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)]
pub struct ErrorResponsePayload {
    pub seq: u32,
    pub request_seq: u32,
    pub message: Option<String>,
    pub body: Option<Message>,
}

[overlisted]

What did you end up using? That looks like a perfectly valid usecase to me.

[raiguard]

@overlisted What I mean was that I don't need to use this, because I misinterpreted the Debug Adapter Protocol documentation. However, having this feature would still make it more ergonomic, since I could do something similar to the snippet I posted to get specific success and error enums. But it's not necessary for my program to function.

(I thought that the entire body of the response would be replaced by a Message, but instead, it defines an extra field that is added to the body if it failed. That means I don't need to have a generic type and a type with Message - I can use one type for both.)

#[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)]
pub struct ResponsePayload<T> {
    pub seq: u32,
    pub request_seq: u32,
    pub success: bool,
    // An optional error message if `success` is false
    pub message: Option<String>,
    pub body: Option<T>,
}

[Milo123459]

Are there any updates on this?

[GnomedDev]

This would be amazing for Discord's GatewayEvent enum, specifically replacing the custom deserialization of https://github.com/serenity-rs/serenity/blob/f97322475eadf2dc78ea3e6d31b55811b3eb50ba/src/model/event.rs#L624-L711

[ByteAlex]

I'm also still hoping for this PR to be merged... We can just hope that @dtolnay will pick this up. 😓

[7sDream]

This is an awesome feature that will be helpful to many of my projects. I'm really looking forward to it being merged one day.

[zitsen]

This will be really helpful. +1 for waiting.

[ImUrX]

whats missing for this to merge? 1 year 😭

[ByteAlex]

@dtolnay Can we please get an information what is with this PR? There's lots of people waiting for this particular feature and the workaround to compile AmranthineCodices branch manually is getting really annoying, because we have to compile everything which uses serde with our custom branch as dependency.

[bobozaur]

Any updates regarding this? This seems to be the last piece preventing some of the private constructs of serde to be made public.

Really looking forward to reusing some of the components for custom deserializations.

[QuantumExplorer]

Would be lovely if this was merged one day.

[ysndr]

In the meantime I found https://github.com/serde-rs/serde/issues/745#issuecomment-1450072069 to be a reasonable workaround

[ByteAlex]

@ maintainers Could someone please react in some way to this PR? I've been using a completely messed up serde setup, which compiles from this branch for almost 2 years now.

It'd be great to get this into a release...

[AmaranthineCodices]

I am going to close this, as I don't have the personal bandwidth to push this through. Sorry for the delay, but I've lost all context for this and I can't drive it right now.