SteveDunn / Vogen

A semi-opinionated library which is a source generator and a code analyser. It Source generates Value Objects
Apache License 2.0
782 stars 46 forks source link

Static Abstract Members Support #1 #511

Closed Aragas closed 3 months ago

Aragas commented 10 months ago

Add support for static abstract members. From what I've seen, we can add at least these members:

public interface IVogen<TSelf, TValueObject>
    where TSelf : IVogen<TSelf, TValueObject>
    where TValueObject : notnull
{
    static abstract explicit operator TSelf(TValueObject value);
    static abstract explicit operator TValueObject(TSelf value);

    static abstract bool operator ==(TSelf left, TSelf right);
    static abstract bool operator !=(TSelf left, TSelf right);

    static abstract bool operator ==(TSelf left, TValueObject right);
    static abstract bool operator !=(TSelf left, TValueObject right);

    static abstract bool operator ==(TValueObject left, TSelf right);
    static abstract bool operator !=(TValueObject left, TSelf right);

    static abstract TSelf From(TValueObject value);
}

This will require the language version check and the runtime check, as I understand

The main usecase I see is user-defined extensions that don't need direct support from Vogen. As an example, I'm currently using it to define a generic EF Core Comparer

// We need to expose the IsInitialized value unfortunately
public interface IHasIsInitialized<in TSelf>
{
    static abstract bool IsInitialized(TSelf instance);
}

[ValueObject<string>]
public readonly partial struct ApplicationRole : IVogen<ApplicationRole, string>, IHasIsInitialized<ApplicationRole>
{
    public static bool IsInitialized(ApplicationRole instance) => instance._isInitialized;
}

public class VogenValueComparer<TVogen, TValueObject> : ValueComparer<TVogen>
    where TVogen : struct, IVogen<TVogen, TValueObject>, IHasIsInitialized<TVogen>
    where TValueObject : notnull
{
    private static int GetHashCodeInternal(TVogen instance) => TVogen.IsInitialized(instance) ? instance.GetHashCode() : 0;
    private static bool EqualsInternal(TVogen left, TVogen right) => left == right;

    public VogenValueComparer() : base((left, right) => EqualsInternal(left, right), vogen => GetHashCodeInternal(vogen)) { }

    public override int GetHashCode(TVogen instance) => GetHashCodeInternal(instance);
    public override bool Equals(TVogen left, TVogen right) => EqualsInternal(left, right);
}
SteveDunn commented 10 months ago

An interesting idea - thanks Aragas! I'll look into this shortly.

SteveDunn commented 4 months ago

It is tricky to implement this, especially for the casting operators, because they are not guaranteed to be the same for every value object. e.g., one VO can specify to have them generated, and another one can say to omit them. We could assume, and maybe have an analyzer that enforces, that if someone wants static abstract interfaces generated, then they must at least have explicit casting selected.

The equality operators look to be no problem though, as they are universal.

SteveDunn commented 4 months ago

@Aragas , this is now implemented in a branch. I'm working on a sample, and thought your value comparer would be ideal.

On the subject of EF Core, Vogen generates this code:

        public class EfCoreValueComparer : global::Microsoft.EntityFrameworkCore.ChangeTracking.ValueComparer<SomeId>
        {
            public EfCoreValueComparer() : base(
                (left, right) => DoCompare(left, right), 
                instance => instance._isInitialized ? instance._value.GetHashCode() : 0) 
            { 
            }

            static bool DoCompare(SomeId left, SomeId right)
            {
                // if both null, then they're equal
                if (left is null) return right is null;

                // if only right is null, then they're not equal
                if (right is null) return false;

                // if they're both the same reference, then they're equal
                if (ReferenceEquals(left, right)) return true;

                // if neither are initialized, then they're equal
                if(!left._isInitialized && !right._isInitialized) return true;

                return left._isInitialized && right._isInitialized && left._value.Equals(right._value);            
            }                
        }

Also generated is this extension method:

            public static class __EfCoreDateTimeOffsetVoEfCoreExtensions 
            {
                public static global::Microsoft.EntityFrameworkCore.Metadata.Builders.PropertyBuilder<EfCoreDateTimeOffsetVo> HasVogenConversion(this global::Microsoft.EntityFrameworkCore.Metadata.Builders.PropertyBuilder<EfCoreDateTimeOffsetVo> propertyBuilder) =>
                    propertyBuilder.HasConversion<EfCoreDateTimeOffsetVo.EfCoreValueConverter, EfCoreDateTimeOffsetVo.EfCoreValueComparer>();
            }

So, in your code, you have this:

            modelBuilder
                .Entity<EfCoreTestEntity>(builder =>
                {
                    builder
                        .Property(x => x.Id)
                        .HasVogenConversion()

                        // use the above instead to register the converter and comparer
                        // .HasConversion(new EfCoreDateTimeOffsetVo.EfCoreValueConverter())
                        .ValueGeneratedNever();
                });

I was struggling to see how I could fit this example in, so instead I went with the scenario of a 'unique ID factory', where it just needs to know that it's a value object of type int.

What do you think? Might you be able to help by adding another scenario, perhaps another EF core scenario where the generated stuff isn't needed and just uses the code you have above?

Aragas commented 3 months ago

@SteveDunn I would like to extended the ValueComparer a bit with #598 If possible, the Copy() could be also part of the IVogen<,> interface, as the IsInitialized() already is

If speaking about EF Core scenarios, I'll try to take a look at it, especially with the snapshot functionality

Aragas commented 3 months ago

Another point with IsInitialized() - right now it's an optional feature.
The current SAM(Static Abstract Members) implementation will include it into the main IVogen<> interface if specified by a config, which enforces every Vogen type to have it implemented.
Since not every type can/might have it, maybe it would make sense to create a separate interface IVogenHasIsInitialized<> which is automatically inherited if the method is not omited

Aragas commented 3 months ago

And if Copy() will also be implemented as an optional feature, the same would apply to it

Aragas commented 3 months ago

I also believe that I might have been wrong with the notnull constraint, since theoretically null values are permitted in reference types, we should remove it to avoid confusion https://github.com/SteveDunn/Vogen/blob/1265a0e30dc22f66e45c6d7cadadacce4946345d/src/Vogen/WriteStaticAbstracts.cs#L41

SteveDunn commented 3 months ago

Released in 4.0.5