Serialize and deserialize a tagged newtype variant over unit () as if it was a unit variant

[tage64]

Let's say we have the following enum:

#[derive(Serialize, Deserialize, Debug)]
#[serde(tag = "result", rename_all = "snake_case")]
enum Response<T> {
    Success(T),
    Error {
        ...
    },
}

This PR makes it possible to serialize and deserialize Response::Success(()) into and from {"result": "success"}. Previously, this was not possible when the type parameter T was ().

Note that this is the same behaviour as if Response::Success would be a unit variant, but now it can be used in a more general context.

See This part of the docs for a background to internally tagged enums.

It's my first time contributing to Serde, so please appologise any mistakes regarding the contribution process.

References #2302

[dtolnay]

Thanks!

[dtolnay]

I think I see the Content::Map case corresponds to the serialize_map that has been added to serde/src/private/ser.rs in this PR. But what is Content::Seq handling?

[tage64]

When I first read your comment, I thought that The line with Content::Seq should not be there and I had made a mistake. I read the code for unit structs at the function below and somehow just thought that I should copy both lines, with Content::Map and Content::Seq. However, that would mean that my example enum above could be deserialized from ["success"], which may not be what we want. And I made a commit to fix this.

But after a bit more investigation, I see that the behaviour of unit structs seem to be just that. I.E. if we have a unit struct SomeStruct we could deserialize Response::Success(SomeStruct) from ["success"]. So for consistency this should perhaps also be allowed for units so that () and unit structs behave in the same way.

What do you think? Should we allow the sequence notation or not? If we decide to allow it, I will just revert my last commit.

[dtolnay]

I think we should land it like this and find out whether anyone needs the Seq case in practice.

Thanks!