KDL serialization experiments

[TheLostLambda]

Opening up a draft PR to ask a couple of design questions before making some changes to facet-serialize!

[codspeed-hq]

CodSpeed Instrumentation Performance Report

Merging #522 will not alter performance

_{Comparing facet-kdl (5acba6e) with main (0d05600)}

Summary

✅ 28 untouched benchmarks

[TheLostLambda]

The key functionality currently missing from facet-serialize is the ability to pass custom attribute information into the Serializer trait methods.

In KDL, this is vital in order to have a nice interface for distinguishing between "arguments" and "properties" (note that XML has the same issue with "elements" and "attributes"). For example, these should serialize differently:

struct Plugin {
    #[knus(argument)]
    name: String,
    #[knus(property)]
    url: String,
}

// Becomes something like:
// plugin "http" url="https://example.org/http"

But

struct Plugin {
    #[knus(argument)]
    name: String,
    #[knus(argument)]
    url: String,
}

// Becomes something like:
// plugin "http" "https://example.org/http"

Or

struct Plugin {
    #[knus(property)]
    name: String,
    #[knus(property)]
    url: String,
}

// Becomes something like:
// plugin name="http" url="https://example.org/http"

If this plan sounds good to @fasterthanlime and others, I'll crack on with passing through:

1. https://docs.rs/facet-core/latest/facet_core/enum.FieldAttribute.html
2. https://docs.rs/facet-core/latest/facet_core/enum.ShapeAttribute.html (just the arbitrary variant here, unless people would rather it be all attributes?)
3. https://docs.rs/facet-core/latest/facet_core/enum.VariantAttribute.html

From there it shouldn't be too bad to implement Serializer and build up a kdl-rs::KdlDocument which can be used to write out the KDL string in compact, pretty-printed, v1, or v2!

[TheLostLambda]

A more actionable question for @fasterthanlime , would you like methods like

fn serialize_field_name(&mut self, name: &'static str) -> Result<(), Self::Error>;

to be changed to methods that always take a slice of the arbitrary attributes:

fn serialize_field_name(&mut self, name: &'static str, attributes: &'static [&'static str]) -> Result<(), Self::Error>;

or should I add new method and keep the old one like this:

fn serialize_field_name(&mut self, name: &'static str) -> Result<(), Self::Error>;
fn serialize_field_name_with_attributes(&mut self, name: &'static str, attributes: &'static [&'static str]) -> Result<(), Self::Error>;

Having default implementations like this could mean that you just need to implement one of the two methods, but it would also make it possible to implement neither of them and end up with code that crashes (presumably) from a stack overflow:

fn serialize_field_name(&mut self, name: &'static str) -> Result<(), Self::Error> {
    serialize_field_name_with_attributes(self, name, &[])
}

fn serialize_field_name_with_attributes(&mut self, name: &'static str, attributes: &'static [&'static str]) -> Result<(), Self::Error> {
    serialize_field_name(self, name)
}

I'm leaning towards just one version of the function that always takes the attributes, since start_object already takes an len: Option<usize> (setting a precedent for always providing that potentially unneeded information) and the two-version approach would be annoying without the default implementations, but a dangerous foot-gun with them.

[TheLostLambda]

Can confirm, the two-default-impls-approach leads to a stack overflow — it looks like the last time a potential solution to this was even discussed was pre-1.0: rust-lang/rfcs#628

[fasterthanlime]

to be changed to methods that always take a slice of the arbitrary attributes:

We can change it but I'd rather not use arbitrary attributes for core/blessed crates like facet-kdl — I consider attributes like attr / child official/blessed and we should use Flags (cheaper to look up when serializing/deserializing) for them!

[TheLostLambda]

Ah, I see! I missed those! So just to ensure we're on the same page, you'd like me to add to these?

https://docs.rs/facet/latest/facet/struct.FieldFlags.html

And then change the method to be something like:

fn serialize_field_name(&mut self, name: &'static str, flags: &FieldFlags) -> Result<(), Self::Error>;

I'll need a couple more than are there currently, I think, so I'd be editing FieldFlags a bit!

[fasterthanlime]

Ah, I see! I missed those! So just to ensure we're on the same page, you'd like me to add to these?

https://docs.rs/facet/latest/facet/struct.FieldFlags.html

And then change the method to be something like:

fn serialize_field_name(&mut self, name: &'static str, flags: &FieldFlags) -> Result<(), Self::Error>;

I'll need a couple more than are there currently, I think, so I'd be editing FieldFlags a bit!

Yes, that sounds good!

[tversteeg]

@fasterthanlime Should this be closed? Since there's not any actual code and as not to discourage other people from taking on this implementation.

[TheLostLambda]

You're welcome to! I've been busier than intended, unfortunately... I might have the chance to give it a go this weekend, but I can just reopen the PR if so!