Basic implementation of Serializer

[Michael-F-Bryan]

I've started a basic implementation of serde::Serializer to allow serializing to XML. So far you can only serialize simple structs and some basic data types.

[Michael-F-Bryan]

You can now serialize maps and most primitives. I'm not sure how to deal with sequences though because XML can only really deal with key-value pairs.

The only way I can think of serializing the array [1, 2, 3, 4] to XML would be something like this:

<value>1</value>
<value>2</value>
<value>3</value>
<value>4</value>

That's not exactly round-trippable though.

@RReverser, can you think of a nice way for serializing sequences?

[SamWhited]

The only way I can think of serializing the array [1, 2, 3, 4] to XML would be something like this:

That is the idiomatic way to serialize sequences in XML; I don't think you'll be able to do better.

[Michael-F-Bryan]

Also, there are a few tests for making sure you get out what is expected. You need to use cargo test --lib though because the tests migrated from serde-xml fail and add a heap of noise to the output.

I imagine some time down the track we'll want to do something similar to serde-json and use either a "Pretty Formatter" or a "Compact Formatter" to tune the output and add correct indenting.

[Michael-F-Bryan]

That is the idiomatic way to serialize sequences in XML; I don't think you'll be able to do
better.

I agree that'd work for primitive types (numbers, strings, etc), but then you've got the situation where composite types are treated completely differently and serde doesn't distinguish between primitives and composites.

For example, say I have the following Rust code:

#[derive(Serialize)]
struct Person {
  name: String,
  age: u32,
}

let people = vec![
  Person{name: "Bob", age: 42},
  Person{name: "Mary", age: 18},
  ...
];

I'd want this as my output:

<person>
  <name>Bob</name>
  <age>42</age>
</person>
<person>
  <name>Mary</name>
  <age>18</age>
</person>
...

i.e. you just serialize each composite object and concatenate them. Whereas when serializing a sequence of primitives you'd want to wrap each primitive in a value tag and then concatenate them.

[SamWhited]

I'm not sure you would want that; if you're just serializing people, then what you've just printed isn't a valid XML document because there's no root node. However,

<people>
<person>
  <name>Bob</name>
  <age>42</age>
</person>
<person>
  <name>Mary</name>
  <age>18</age>
</person>
</people>

(or something similar) is valid.

[RReverser]

That's not exactly round-trippable though.

It actually is - that's what deserializer currently parses when asked for a sequence. It works for complex types as well, not just primitives.

[Michael-F-Bryan]

that's what deserializer currently parses when asked for a sequence

Awesome. If that's the case I'll play around with the deserializer a bit and try to design the serializer to mirror that.

(or something similar) is valid.

Yeah, that's pretty much what I was trying to get at. I guess I was just being sloppy and sent the comment without checking my snippet was a valid xml document.

[RReverser]

I had this concern about round-trip of structure properties though. Maybe we should eventually add custom prefix to distinguish attributes from nested elements, so that content would be always serialized back exactly how it was deserialized... Not sure yet.

[SamWhited]

Yeah, that's pretty much what I was trying to get at. I guess I was just being sloppy and sent the comment without checking my snippet was a valid xml document.

Ah, sorry about that, I just misunderstood your point then. I think we're actually on the same page :)

[Michael-F-Bryan]

@RReverser, I'm not quite sure how much more I need to do to get the full serializing of xml to work but at the moment this deals with basic structs (this passes).

Should we merge in the initial serializing pull request with warnings saying it's still a work in progress, then make a tracking issue for the various bits and pieces which need to be done before it's complete and production ready (tuple structs, enums, pretty-printing, etc)?

[dtolnay]

@RReverser and @Michael-F-Bryan, what work is remaining before this implementation could be accepted?

[Michael-F-Bryan]

I just updated this to use serde 1.0 and fixed some of the bitrot.

There are a few Error::UnsupportedOperations, but this should be able to serialize most of the types you'll typically use in XML (i.e. structs which contain nested structs or primitives). Notably, I don't think this can serialize a list/sequence yet.

I was thinking we can merge in the initial serialization, then work on adding the missing operations when they're needed.

[oli-obk]

I think we should accept this as it is. While it has some minor issues like unrelated formatting and tests in the library code instead of in unit tests, it looks good implementation wise. The formatting issues can be solved in the future by providing a rustfmt.toml with the project specific rules. The test placement needs a decision in general for the project.

[Michael-F-Bryan]

tests in the library code instead of in unit tests

@oli-obk I'm not sure what you mean here. If you are talking about the tests at the bottom of serialize.rs, It's standard practice to put unit tests at the bottom of the module they're testing. Likewise the "round trip" tests in tests/round_trip.rs are integration tests which exercise the entire serialize/deserialize pipeline, so they belong in the tests directory alongside the other integration tests.

Regardless, it sounds like this should be merged and any extra work like running cargo fmt or moving tests around can be done in future PRs. I've added #[ignore]'d tests for serializing sequences (both the unit test which serializes a vec![1, 2, 3, 4], and the round trip integration test), so when sequences are implemented all you need to do is remove the #[ignore] line.

[Michael-F-Bryan]

Note that travis will most probably fail this PR because there are a bunch of broken tests in master which were migrated across from elsewhere. Everything related to this PR passes.

[Michael-F-Bryan]

@RReverser, any chance you can merge this?