Serde module macro

[pitdicker]

trafficstars

Every (de)serializer we have for use with serde_with comes with a lot of boilerplate. Each needs two modules (for a regular and optional variant), serialize and deserialize methods, a Visitor type and implementation, some documentation, and doc examples that serve as basic tests.

This is the perfect use case for a macro. The most important inputs for such a macro are:

• the name of the two modules
• the type that is to be serialized
• the types that are to be deserialized
• closures for the actual serialize function and deserialize visitors
• docstrings for errors and examples

Because we can't concatenate idents in macro's it unfortunately needs a few more inputs to have everything covered.

---

A disadvantage of macro's is that it becomes a kind of DSL. I went with the most straightforward solution: to make it to look like a struct with all the macro arguments.

The advantage is that in my opinion it becomes much easier to maintain the code and spot errors in documentation or implementation. And that is the part were our bugs used to be, not in the boilerplate. I think the line count speaks for itself; the implementations are ca. a quarter of before.

There is no change in functionality, but thanks to the macro the documentation is more consistent.

I did not replicate all doc examples. In my opinion the doc example for the regular modules is important, because that is what you point serde_with at. But I doubt anyone clicks on the individual serialize and deserialize functions. I did extend the module documentation a bit to highlight how to use them though.

Actually found another bug in our deserialization from timestamps while working on this.

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 80.82192% with 56 lines in your changes are missing coverage. Please review.

Project coverage is 92.17%. Comparing base (0aa46dd) to head (c945153).

Files	Patch %	Lines
src/serde.rs	54.16%	33 Missing :warning:
src/naive/datetime/serde.rs	86.11%	15 Missing :warning:
src/datetime/serde.rs	92.85%	8 Missing :warning:

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1396      +/-   ##
==========================================
+ Coverage   91.95%   92.17%   +0.22%     
==========================================
  Files          40       41       +1     
  Lines       18035    17013    -1022     
==========================================
- Hits        16584    15682     -902     
+ Misses       1451     1331     -120     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[djc]

So I generally don't love macros like this that have fairly intricate DSL nature to them. I don't think it's all that natural to use a struct-like syntax for something more complex like this, so IMO this is not an improvement in its current shape at least.

[pitdicker]

The problem is that I also don't particularly like it, but if 80% of all the code I have to scroll through is boilerplate I have a hard time catching issues. By now I think we are in pretty good shape, but we only support a few limited 64-bit timestamps. Once we add a couple thousand lines more it is even harder to maintain.

Do you see another way to organize things or get rid of the boilerplate, besides using a macro with suboptimal syntax?

[djc]

Do you see another way to organize things or get rid of the boilerplate, besides using a macro with suboptimal syntax?

Use an integration test to generate code, similar to the windows bindings stuff?

[pitdicker]

That could work. Not going to be easy; I'll look into it sometime.