Stream: rustdoc

Topic: Deserialization Fix


view this post on Zulip CraftSpider (Feb 27 2021 at 22:06):

For #82299, it was found that order matters for untagged enums. Fixing this could be as easy as moving the variants, but that's easy to break. The best alternative is adjacent tagging, which would match the current layout. The issue with this is that it means the kind would be consumed by the inner. There is a fix, but it is a little ugly. Here's my proposal:
We make ItemEnum adjacently tagged. We make Item::kind into #[serde(ignore)]. We derive serialization, and manually implement deserialization to fill in kind based on the result of deserializing inner.

view this post on Zulip CraftSpider (Feb 27 2021 at 22:08):

This doesn't change the JSON at all, just how we implement one of the deserializations to prevent inner value conflicts. I think this is better than just moving stuff around because it also prevents a future problem with, say, TypeA(String) and TypeB(String)

view this post on Zulip Nixon Enraght-Moony (Feb 27 2021 at 22:40):

I think you can do this with clever serde

#[derive(Serialize, Deserialize, Debug)]
struct Item {
    name: String,
    #[serde(flatten)]
    inner: Inner,
}

#[derive(Serialize, Deserialize, Debug)]
#[serde(tag = "kind", content = "inner")]
enum Inner {
    Struct { name: String, n_field: u8 },
    Function { name: String, n_field: u8 },
}

playground

This way, we remove kind from Item, and have ItemEnum provide both the tag and the kind

view this post on Zulip CraftSpider (Feb 27 2021 at 22:53):

That would work, but my goal was to keep this non-breaking, so not remove kind from Item

view this post on Zulip CraftSpider (Feb 27 2021 at 22:55):

Otherwise that would definitely solve the problem

view this post on Zulip Nixon Enraght-Moony (Feb 27 2021 at 22:56):

my goal was to keep this non-breaking,

Why, breaking to whom?

The json output is uneffected, and the rust types are not exported anywhere

view this post on Zulip CraftSpider (Feb 27 2021 at 22:59):

That's true. And overall I don't think there's a problem with removing the kind, as I think the common cases you want to match on inner anyways. I'd like @HeroicKatora's input on this, as they're the main consumer and I believe have started copying over the rustdoc types (The goal is once rustdoc-json stabilizes, the types become part of the public interface. That's why I was thinking of a change to them as 'breaking')

view this post on Zulip Nixon Enraght-Moony (Feb 27 2021 at 23:01):

Yeah, one day we should put rustdoc-json-types on crates.io, (posibly before the format stabilizes, and only release 1.0 when rustdoc-json hits stable), but untill then, we should absolutly make breaking changes to the types

view this post on Zulip CraftSpider (Feb 27 2021 at 23:02):

Yeah, I've definitely been making breaking changes already too. Now is the time to make them :P

view this post on Zulip CraftSpider (Feb 27 2021 at 23:02):

I just always make sure to bump version when I do. Anyways, if removing kind and just exposing inner as a tagged enum works, that's definitely the cleanest solution.

view this post on Zulip Joshua Nelson (Feb 27 2021 at 23:06):

Sounds like a plan to me :)

view this post on Zulip HeroicKatora (Feb 28 2021 at 00:47):

The #[serde(flatten)] inner solution looks neat, especially if it leaves the current json structure in place. I'm not dependent on the files in rustdoc-json as of yet, just the json structure.

view this post on Zulip HeroicKatora (Feb 28 2021 at 00:48):

So, making breaking changes in those types is fine. Even breaking the format is not usually a problem. Note that I've customized the types anyways since in my code the version field is replaced by one that asserts the expected version :)

view this post on Zulip CraftSpider (Feb 28 2021 at 00:51):

Opened #82613 with the proposed flatten fix


Last updated: Oct 11 2021 at 22:34 UTC