#[serde(flatten)] attribute support for enum variants

[srijs]

Hi!

I have the following use-case for a feature similar to #[serde(flatten)], but for enum variants rather than struct fields. Generally happy to dive into serde_derive myself in case you think this would be worth having, but it could also be that I'm trying to model this the wrong way.

I'm trying to parse JSON values in the estree format. The format specifies an ECMAScript syntax tree using an inhertance relationship, where generally only the nodes at the bottom of the hierarchy are tagged.

For example, one type of node is Expression, but there doesn't exist any node with the "Expression" tag, because the tag is assigned to the "concrete" expression nodes such as Identifier or Literal. The representation of the Expression node is basically just a union of all possible JSON representations of its descendant node types.

So far, I've been modelling it like this, which has been working fine for the most part:

#[derive(Deserialize)]
#[serde(tag = "type")]
pub enum Expression {
    Identifier { name: String },
    Literal { value: Value },
    ...
}

Now, the difficulty I've been having comes in when I try to mix types that are at the bottom of the node hierarchy with node types that have sub-types (such as Expression):

#[derive(Deserialize)]
#[serde(tag = "type")]
pub enum ForStatementInit {
    VariableDeclaration { kind: String, ... },
    Expression(Expression),
}

In this case, what I really want is for the ForStatementInit type to be the result of "adding" the node of type VariableDeclaration to the set of representations for Expression. What ends up happening though of course is that it expects a value tagged with either VariableDeclaration or Expression.

I can work around this by either nesting VariableDeclaration into a single-variant enum, and using #[serde(untagged)] on the ForStatementInit enum, or by manually "flattening" the expression types, like so:

#[derive(Deserialize)]
#[serde(tag = "type")]
pub enum ForStatementInit {
    VariableDeclaration { kind: String, ... },
    // Expression
    Identifier { name: String },
    Literal { value: Value },
}

However, what I think would be really useful is to be able to do something like this instead, which would apply the same "flattening" transformation in the example above:

#[derive(Deserialize)]
#[serde(tag = "type")]
pub enum ForStatementInit {
    VariableDeclaration { kind: String, ... },
    #[serde(flatten)]
    Expression(Expression),
}

Hope this makes somewhat sense, keen to hear if this is something that you think is worth solving in serde itself!

[srijs]

It looks like this is similar to #1350 where you suggest manually flattening, but that would be fairly cumbersome in this case (Expression might have a couple dozen variants) and I guess I'm still curious whether there's a "better" way to do this.

[dtolnay]

Seems reasonable! I would be interested in supporting this better. I have also occasionally wanted serde(untagged) to work as a variant attribute which would have a similar effect in this case.

#[derive(Deserialize)]
#[serde(tag = "type")]
pub enum ForStatementInit {
    VariableDeclaration { kind: String, ... },
    #[serde(untagged)]
    Expression(Expression),
}

[EliSnow]

@dtolnay, what would it take to implement untagged for variants? I have need for such a feature and may be interested in implementing it.

[dtolnay]

I haven't thought through how the feature would work. I would first want to see a handwritten Deserialize impl that behaves the way we would want the code generated for an untagged enum variant to behave. I would schedule 2-6 hours to put together some fully fleshed out handwritten impls to discover any edge cases, depending on your level of familiarity with the deserialization API.

[EliSnow]

a handwritten Deserialize impl that behaves the way we would want the code generated for an untagged enum variant to behave

Meaning a Deserialize impl that enables it without any variant macro? Would tweaking serde-yaml in a branch be sufficient?

[EliSnow]

@dtolnay, did you see my previous questions?

[dtolnay]

Correct, handwritten meaning without a macro involved. There shouldn't need to be changes to serde-yaml or any other format because you would be touching the enum's Deserialize impl only.

[EliSnow]

Ahh, my mistake. I was thinking Deserializer

[EliSnow]

@dtolnay, I threw together a few Deserialize impls. What edge cases am I lacking?

[dtolnay]

Trait bounds make this somewhat more complicated. For example:

trait Trait<T> {
    type Associated;
}

enum E<A, B, C> where A: Trait<C> {
    A(A::Associated),
    // #[serde(untagged)]
    B(B),
}

// not a legal type
#[allow(non_snake_case)]
struct AHelper<A, B, C> where A: Trait<C> {
    A: A::Associated,
}

Otherwise looks good to me.

[azriel91]

Hiya, is the last edge case necessary to be supported for this feature to be supported?
If it is, wondering if trying to do something like this is too crazy:

#[allow(non_snake_case)]
struct AHelper<A, B, C, A_Associated>
where
    A: Trait<Associated = A_Associated, C>,
{
    A: A_Associated,
}

[gengjiawen]

Which part should I look into to implement such a feature ?

[oli-obk]

Hi everyone, we have a PR for resolving this issue that I believe covers all the mentioned use cases: #2403 If one or two ppl could try this PR against their crates and see if their use cases are covered, that would be great!

[Iron-E]

Solves my use case. For anyone who didn't know, [patch] option was useful in testing the PR. Just put this in your Cargo.toml:

[patch.crates-io.serde]
git = "https://github.com/dewert99/serde"
branch = "partially_untagged"

[gavrilikhin-d]

Solved for my case. Really love how much code it saves!

[oli-obk]

Thanks!