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

De/serialize enum as array #979

Closed
tailhook opened this issue Jul 6, 2017 · 11 comments
Closed

De/serialize enum as array #979

tailhook opened this issue Jul 6, 2017 · 11 comments
Labels

Comments

@tailhook
Copy link

tailhook commented Jul 6, 2017

Hi, I think one representation for enum is missing. E.g. if we have some enum like this:

enum State {
  Initializing,
  Reading(usize),
  Writing(usize, usize),
  Done,
}

The short and nice representation would be (showing json):

["Initializing"]
["Reading", 1234]
["Writing", 3, 1234]
["Done"]

Or even:

"Initializing"
["Reading", 1234]
["Writing", 3, 1234]
"Done"

This works well for unit variants and tuple variants. And is more compact than other representations. I think this is useful for representing state machines as in the example above.

What do you think?

@dtolnay
Copy link
Member

dtolnay commented Jul 9, 2017

Seems reasonable. Do you know of any existing serialization libraries in other languages that support this style, or JSON APIs that use this style?

@dtolnay dtolnay added the derive label Jul 9, 2017
@tailhook
Copy link
Author

tailhook commented Jul 9, 2017

From the top of my head WAMP protocol fits the "JSON APIs" category. Here are some messages from the spec:

[HELLO, Realm|uri, Details|dict]
[WELCOME, Session|id, Details|dict]
[ABORT, Details|dict, Reason|uri]
[GOODBYE, Details|dict, Reason|uri]

http://wamp-proto.org/static/rfc/draft-oberstet-hybi-crossbar-wamp.html#session-lifecycle

@tailhook
Copy link
Author

tailhook commented Jul 9, 2017

Technically WAMP also uses numeric IDs as the first item which might be a good option too.

@RReverser
Copy link

RReverser commented Jul 12, 2017

I had the same issue with parsing html5lib-tests (https://github.com/html5lib/html5lib-tests/blob/master/tokenizer/test1.test#L246), had to implement manually.

["DOCTYPE", name, public_id, system_id, correctness]
["StartTag", name, {attributes}*, true*]
["StartTag", name, {attributes}]
["EndTag", name]
["Comment", data]
["Character", data]

@tailhook
Copy link
Author

tailhook commented Dec 8, 2017

I'll try implementing this issue if no one is working on it. I'm not familiar with internals of serde though, so I'm not sure that I'll have enough time for the implementation.

Any preliminary hints? Objections?

@dtolnay
Copy link
Member

dtolnay commented Dec 8, 2017

This doesn't necessarily need to involve changing serde internals. Here is a small shim that you could drop into a crate to make this work.

@tailhook
Copy link
Author

tailhook commented Dec 8, 2017

@dtolnay, nice hack. I might use it somewhere. Still, it's a bit unfortunate that it has to be in serde(with) (i.e. can't be applied for the structure itself).

Just to be clear: are you opposed to get it into a serde? Or do you think this is a waste of time? Or PR is also okay?

@dtolnay
Copy link
Member

dtolnay commented Dec 9, 2017

I would prefer to design an approach that allows the logic here to live outside of Serde (like in that playground link) but still be used for type's Deserialize implementation. #1111 (comment) is a very similar situation so cc @Marwes. Essentially we want a concise way to derive the serialize/deserialize code for the type as usual, but then use some other implementation as the Serialize/Deserialize impl for the type.

@Marwes
Copy link
Contributor

Marwes commented Dec 9, 2017

So with #1111 (comment) it should be possible to remove the wrapper and do this instead (+ the serializer/deserializer wrappers)

#[derive(Serialize, Deserialize, Debug)]
#[serde(freestanding_serialize = "serialize_state")]
#[serde(freestanding_deserialize = "deserialize_state")]
enum State {
    Initializing,
    Reading(usize),
    Writing(usize, usize),
    Done,
}

impl Serialize for State {
    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error> {
        serialize_state(EnumAsArray(serializer))
    }
}

impl Deserialize for State {
    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error> {
        deserialize_state(EnumAsArray(deserializer))
    }
}

https://play.rust-lang.org/?gist=e51b0de2f0e798934c9d6cc0772f10b6&version=stable

@tailhook
Copy link
Author

tailhook commented Dec 9, 2017

@dtolnay, @Marwes take a look at #1118. That solution is better than freestanding_serialize because it allows encapsulating implementation into a module/crate. In your solution, you still need to write a one-line implementation for each structure.

@dtolnay
Copy link
Member

dtolnay commented Jan 6, 2019

Closing in favor of #1118.

@dtolnay dtolnay closed this as completed Jan 6, 2019
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

4 participants