Rewrite serializer

[Mingun]

This PR rewrites serializer and fixes the following issues:

• Fixes Help serializing empty attributes #252
• Fixes Vec<String> inside of struct serializes incorrectly with serde #280
• Fixes How to have tag on each item? #287
• Fixes Serialization of Hashmap duplicate outer tag in the inners elements. Deserialization has no such problem and fails on serialized xml. #343
• Fixes Serialization of enums adds extra tags #346
• Invalidates Deserializing $primitive= unit variants doesn't work correctly yet #354 (but it still should be checked that it works aligned with the serializer)
• Fixes How to pretty-print with serde? #361
• Partially addresses Handling of control characters #368
• Fixes Serializing $value enum variant doesn't work #429
• Fixes Use $value and $unflatten together #430

Notable changes:

• Removed $unflatten= prefix, as it not used anymore
• Removed $primitive= prefix, as it not used anymore
• $value special name split into two special names #text and #any #content. @dralley, please check especially, is the documentation clear about the differences?
• Change how attributes is defined. Now attribute should have a name started with @ character. Without that fields serialized as elements and can be deserialized only from elements. Deserialization from element or from attribute into one field is not possible anymore. It's a weird wish anyway

[codecov-commenter]

Codecov Report

Merging #490 (8511d6f) into master (8fda027) will increase coverage by 7.54%.
The diff coverage is 93.63%.

@@            Coverage Diff             @@
##           master     #490      +/-   ##
==========================================
+ Coverage   54.05%   61.59%   +7.54%     
==========================================
  Files          30       33       +3     
  Lines       12649    15358    +2709     
==========================================
+ Hits         6837     9460    +2623     
- Misses       5812     5898      +86     

Flag	Coverage Δ
unittests	61.59% <93.63%> (+7.54%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
src/de/seq.rs	93.10% <ø> (ø)
src/errors.rs	11.11% <ø> (ø)
src/se/element.rs	97.70% <ø> (ø)
src/de/mod.rs	66.64% <9.52%> (-2.42%)	⬇️
src/de/map.rs	67.60% <57.14%> (-4.47%)	⬇️
src/escapei.rs	12.35% <62.50%> (+0.25%)	⬆️
src/se/mod.rs	88.04% <83.87%> (+7.15%)	⬆️
src/writer.rs	89.41% <88.88%> (-0.30%)	⬇️
src/de/key.rs	96.75% <96.75%> (ø)
src/se/content.rs	97.20% <97.20%> (ø)
... and 14 more

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

[dralley]

+6,885 −1,917

I might need a few days for this

[rrichardson]

How does one achieve the previous behavior of enums where the enum value would be wrapped in the enum type tag?

enum Wat {
    Foo,
    Bar,
    Baz
} 
struct Test {
    wat: Wat, 
} 

results in

<Test>
    <Foo/> 
</Test> 

and if you add rename=#text to wat: Wat then you get

<Test>
    Foo
</Test> 

Is the intention that we wrap the enum in a wrapper struct if we want type tags for the enum?

<Test> 
    <Wat>Foo</Wat> 
</Test> 

I guess this makes it consistent with the way lists are handled, and also consistent with the philosophical standpoint that enum is simply one of N possible values, and not a container.

We should probably document this change, though.

[rrichardson]

It appears that the #text or #any attributes are required if you want to successfully deserialize an enum from a struct. I'm assuming that this is because the outer type tags of the enum are eaten, so we need to tell the deserializer exactly how to parse the contents.

mod try_enum {
    use quick_xml::{de::from_str, se::to_string};
    use serde::{Deserialize, Serialize};
    
    #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)]
    enum WatEnum {
        #[serde(rename="FOO")]
        Foo,
        #[serde(rename="BAR")]
        Bar,
        #[serde(rename="BAZ")]
        Baz
    }

    #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)]
    struct Wat {
        wat: WatEnum
    }

    #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)]
    struct Wat2 {
        #[serde(rename="#any")]
        wat: WatEnum
    }

    #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)]
    struct Wat3 {
        #[serde(rename="#text")]
        wat: WatEnum
    }

    #[test]  // This fails
    fn round_trip_enum_none() {
        let tst = Wat { wat: WatEnum::Bar };
        let data = to_string(&tst).unwrap();
        println!("{:?}", &data);
        let tst_again: Wat = from_str(data.as_str()).unwrap();
        assert_eq!(&tst, &tst_again);
    }

    #[test]  // This works
    fn round_trip_enum_any() {
        let tst = Wat2 { wat: WatEnum::Bar };
        let data = to_string(&tst).unwrap();
        println!("{:?}", &data);
        let tst_again: Wat2 = from_str(data.as_str()).unwrap();
        assert_eq!(&tst, &tst_again);
    }
 
    #[test]  // this works
    fn round_trip_enum_text() {
        let tst = Wat3 { wat: WatEnum::Bar };
        let data = to_string(&tst).unwrap();
        println!("{:?}", &data);
        let tst_again: Wat3 = from_str(data.as_str()).unwrap();
        assert_eq!(&tst, &tst_again);
    }
}

[dralley]

Have you run benchmarks before and after?

[Mingun]

@dralley, no. The goal is to make a correct and consistent implementation first. There a no reason to worry about performance while the old implementation is just incorrect and inconsistent

[Mingun]

It appears that the #text or #any attributes are required if you want to successfully deserialize an enum from a struct.

While in this PR this can be true, that is not desired behavior. This PR is just about serializer, I plan to continue work on deserializer part in next PRs. The final goal -- a consistent and composable implementation:

• consistent: the same types should be serialized and deserialized similarly independently of their location in the XML. The change of representation should depend only on explicitly added attributes in Rust code, not of a way, how types are composed
• composable: if type A includes in type B, then in XML representation markup for type A should be included in markup of type B following by predictable rules
• the most used patterns should be covered without (or with minimal) additional attributes. The such patterns are:
  • elements and attributes
  • XML xs:choice mapped to Rust enum
  • text content of nodes
  • deserialization with deserialize_any should give a result that close to result of converting XML to JSON (because internally serde uses JSON-like type model). This is important, because in many cases serde uses deserialize_any under the hood

[dralley]

There a no reason to worry about performance while the old implementation is just incorrect and inconsistent

I'm not worried about performance, correctness is of course more important, it would just be good to know if it deserves some attention afterwards

[Mingun]

I choose name #text because, as I remember, I saw that many HTML/XML DOM engines uses it as an internal name for a text node, and even visualize it with that name. But well, consistence is better that ideal namings. So I rename it to $text and merge

[seanpianka]

What is #[serde(rename="#text")] and why does it make enum deserialization work? Is there any pre-existing docs that explain what this is or why it's needed?

[Mingun]

What is #[serde(rename="#text")] and why does it make enum deserialization work?

I not exactly understand your question. Could you expand it?

Is there any pre-existing docs that explain what this is or why it's needed?

Because this change is a part of future release (which I plan to make this year) you can find docs about it in the local copy after generating:

$env:RUSTDOCFLAGS="--cfg docs_rs"; cargo +nightly doc --all-features --open

and go to the documentation of the de module.

You also can read documentation source here:

quick-xml/src/de/mod.rs

Line 3 in e877f4f

//! # Difference between `$text` and `$value` special names

If anything is incomprehensible, feel free to ask so that we can address ambiguities before release.