Fixes #115 struct flattening

[juntyr]

Fixes #115 by allowing ron maps to be deserialised from structs. This is a hack. I am very unsure if this is something we want ron to support with serde's current model, or if we wanted to hold out for some extension to serde that would give Deserializer's more control about how to handle things like flatten and untagged enums etc.

• [ ] I've included my change in CHANGELOG.md

[juntyr]

@torkleyy thoughts?

[codecov-commenter]

Codecov Report

Merging #403 (512713c) into master (75ac75e) will increase coverage by 0.54%. The diff coverage is 96.20%.

:exclamation: Current head 512713c differs from pull request most recent head 8ee5179. Consider uploading reports for the commit 8ee5179 to get more accurate results

@@            Coverage Diff             @@
##           master     #403      +/-   ##
==========================================
+ Coverage   80.31%   80.85%   +0.54%     
==========================================
  Files          49       50       +1     
  Lines        6247     6393     +146     
==========================================
+ Hits         5017     5169     +152     
+ Misses       1230     1224       -6     

Impacted Files	Coverage Δ
src/de/mod.rs	76.11% <81.25%> (+1.65%)	:arrow_up:
tests/115_struct_flattening.rs	100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[torkleyy]

Yeah I'm also unsure what drawbacks this hack might have. I think there always will be some limitations when using RON + serde (which is the only option today), simply because serde was designed for JSON. IIRC there's also untagged enums that won't work with ron.

The question is is it worth having this feature? Could it prevent using someone from ron because a dependency crate uses struct flattening? I think probably not. The most common use of flatten I see is for all the ignored fields to store them in a map. I think we do support that, right?

[juntyr]

Yeah I'm also unsure what drawbacks this hack might have. I think there always will be some limitations when using RON + serde (which is the only option today), simply because serde was designed for JSON. IIRC there's also untagged enums that won't work with ron.

I once did a similar experiment for untagged enums, and they could sort of be supported by ron if ron::Value subscribes to serde‘s ideas (structs and enums become single-item maps). Both of these solutions are in no way pretty. Imo serde should allow Deserializer to supply its own Value type that can then get special support, and have special methods to handle all the enum encodings etc within the data format itself. In the absence of such support, we’ll have to decide and document which feature of serde we do guarantee support for.

The question is is it worth having this feature? Could it prevent using someone from ron because a dependency crate uses struct flattening? I think probably not. The most common use of flatten I see is for all the ignored fields to store them in a map. I think we do support that, right?

I thought so too, but unfortunately both use cases call deserialize_map, so it seems that we either support no flatten or all flatten.

[torkleyy]

I see. Well I don't think serde will implement any of that, looking at the issues it's doing bug fixes only.

[juntyr]

Re untagged enums: https://github.com/ron-rs/ron/compare/master...JuniperLangenstein:253-untagged-enums

[juntyr]

I also just thought of something else: in that prototype branch I made a while ago, struct-like things were deserialised as single-item maps into Value. This allows us to do the following round trip: Rust (-> ron) -> Value (-> ron -> Value) -> Rust. However, we cannot do Rust -> Value -> ron -> Rust, since the Value does a lot of heavy lifting to interpret those single maps.

We could, in theory, make single-item maps special in the Deserialize impl for Value and convert them into a ron-specific struct-like type immediately. This would allow us to do Rust -> Value -> ron -> Rust, and the Value would be much more like a ron AST than a JSON AST.

However, this would mysteriously break any single-item maps in user code that go through Value. We could, again, special case that (though this would be a bit ugly and would add even more edge cases), but as soon as such a Value would be written to ron, we'd be done. Unless we special-case the ron Deserializer impl as well. However, at this point we've got a paradox: flatten needs to interpret Struct(a: 2) as {"a": 2}, but our value case needs to interpret it as {"Struct": {"a": 2}}.

Hence, I think we have two options going forward with Values:

1. Stick with a JSON-AST-like Value but ensure it supports all enum shenanigans that serde offers, it can round trip through other formats, but Rust->Value->ron->Rust doesn't work

2. Have a ron-AST-like Value that supports all enum shenanigans that serde offers, it cannot round trip through other formats, and single-item maps don't round trip through Value

Even though a ron-AST-like Value would be nicer, I think if we want to have the best compatibility, we'd have to stick with the JSON-AST-like Value, but we could offer some convenience inspector methods to check if the Value looks e.g. like a self-describing enum

[juntyr]

Superseded by #455