Suggestions for reducing implementation code duplication

[ijackson]

Thanks for merging #15 and fixing up the rustfmt and the tests. (I'll see if I can manage to do the latter myself properly next time.)

While working on that I noticed that the implementation contains a lot of rather similar code which might profitably be refactored. I would like to propose concrete changesas MR(s). I'm filing this ticket to get your feedback and opinion about what you'd like to see.

I hope to get some steer from you and then make some code change(s) as MRs; then you can decide if you like them or want to stick with the existing approach, or do something different.

The first suggestion is specific to educe and relates to the comparison traits. The 2nd and 3rd suggestions are to adopt two patterns enabled by less-used Rust language features but commonly used in Rust derive macros to unify parts of their implementations. They have a natural ordering: if we're going to do all three, we should probably do them in this order.

Thanks for your crate, and for your consideration.

Similarity between Ord, PartialOrd and PartialEq code

These implementations are extremely similar. I think they could be combined into a single implementation, taking a small amount of target-trait-specific information.

To be more concrete:

pub trait ComparisonTraitHandler {
    /// Returns `cmp` or `partial_cmp` or `eq`
    fn method_name() -> proc_macro2::TokenStream;
    /// Returns `Some(::core::cmp::Ordering::Equal)` or `..::Equal` or `true`
    fn equal_value() -> proc_macro2::TokenStream;
    ....
}
impl<T: ComparisonTraitHandler> TraitHandler for T {
   ... most of the existing three impls goes here, combined into one ...
}

impl ComparisonTraitHandler for CloneEnumHandler { ... }

Always using match, unifying enum and struct code

Taking clone as an example, currently for enums we generate roughly this:

    fn clone(&self) -> Self {
        match self {
            Self::Variant1 { f: _s_f } => Self::Variant1 { ...

That code would be fine for structs too. We'd just need to leave out ::Variant1.

The result would be to abolish most of the *_struct.rs files.

Probably the easiest way to implement this would be to provide a helper function which takes a Data and returns an iterator of "variants" which might be either actual enum variants, or just the one struct variant:

struct VariantInfo<'d_> {
    field: &'d Fields,
    variant_name: Option<&'d Ident>,
}
fn variants_for_struct_or_enum(data: &'d Data) -> impl Iterator<Item = VariantInfo<'d>> {...

If you like this idea in principle, I think my next step would be to make an RFC MR demonstrating it for one of the trait impls. That would let you critique the details of the pattern before anyone does a lot of work on all the impls.

Always using braced syntax

In Rust, it's legal to match and construct unit and tuple structs and variants with the braced syntax:

struct Unit;
struct Tuple(i32);

let Unit { } = Unit { };
let Tuple { 0: x } = Tuple { 0: 42 };

This means that the code for Fields::Named can, more or less, be used for tuples and units too. The only wrinkle is the need to handle the field name being either an index or an identifier. A small amount of utility code for obtaining the "field name" and derived field names would allow us to remove the different handling for Fields::Unnamed and Fields::Unit.

Again, if you like this idea, the next thing to do would be try this out for one of the implementations.