SteveDunn / Vogen

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

Derived attribute causes exception #658

Open SteveDunn opened 2 months ago

SteveDunn commented 2 months ago

Describe the bug

This should work:

public class CustomValueObjectAttribute<T> : ValueObjectAttribute<T>
{
    public CustomValueObjectAttribute(
        Conversions conversions = Conversions.Default | Conversions.EfCoreValueConverter,
        CastOperator toPrimitiveCasting = CastOperator.Implicit,
        CastOperator fromPrimitiveCasting = CastOperator.Implicit,
        ComparisonGeneration comparisonGeneration = ComparisonGeneration.Omit)
    { }
}

... but it fails This is currently failing with InvalidCastException.

[CustomValueObject<Guid>]
public readonly partial record struct ConversationId
{
}

Generator 'ValueObjectGenerator' failed to generate source. It will not contribute to the output and compilation errors may occur as a result. Exception was of type 'InvalidCastException' with message 'Unable to cast object of type 'System.Int32' to type 'Microsoft.CodeAnalysis.INamedTypeSymbol'.'

Vogen v4.0.17

Steps to reproduce

Use the code above

Expected behaviour

No exceptions

SteveDunn commented 2 months ago

Unfortunately, derived attributes must include all of the parameter from the base attribute. So in your case, it's this immensely verbose declaration!

public class CustomValueObjectAttribute<T> : ValueObjectAttribute<T>
{
    public CustomValueObjectAttribute(
        Conversions conversions = Conversions.Default | Conversions.EfCoreValueConverter,
        Type? throws = null,
        Customizations customizations = Customizations.None,
        DeserializationStrictness deserializationStrictness = DeserializationStrictness.AllowValidAndKnownInstances,
        DebuggerAttributeGeneration debuggerAttributes = DebuggerAttributeGeneration.Default,
        ComparisonGeneration comparison = ComparisonGeneration.Omit,
        StringComparersGeneration stringComparers = StringComparersGeneration.Unspecified,
        CastOperator toPrimitiveCasting = CastOperator.Implicit,
        CastOperator fromPrimitiveCasting = CastOperator.Implicit,
        ParsableForStrings parsableForStrings = ParsableForStrings.Unspecified,
        ParsableForPrimitives parsableForPrimitives = ParsableForPrimitives.Unspecified,
        TryFromGeneration tryFromGeneration = TryFromGeneration.Unspecified,
        IsInitializedMethodGeneration isInitializedMethodGeneration = IsInitializedMethodGeneration.Unspecified,
        PrimitiveEqualityGeneration primitiveEqualityGeneration = PrimitiveEqualityGeneration.Unspecified)
        : base(
            conversions, throws, customizations, deserializationStrictness, debuggerAttributes, comparison, stringComparers,
            toPrimitiveCasting, fromPrimitiveCasting, parsableForStrings, parsableForPrimitives, tryFromGeneration,
            isInitializedMethodGeneration, primitiveEqualityGeneration)
    {
    }
}

It should be something like this, which is more natural, but Vogen doesn't currently support:

    public CustomValueObjectAttribute(
        Conversions conversions = Conversions.Default | Conversions.EfCoreValueConverter,
        ComparisonGeneration comparison = ComparisonGeneration.Omit,
        CastOperator toPrimitiveCasting = CastOperator.Implicit,
        CastOperator fromPrimitiveCasting = CastOperator.Implicit)
    : base(conversions: conversions, comparison: comparison, toPrimitiveCasting: toPrimitiveCasting, fromPrimitiveCasting: fromPrimitiveCasting)
    {
    }

This is because Vogen reads constructor parameters in order, rather than by name. So it expects the 14th parameter to be pf a certain type, the 13th to be of a certain type, etc. etc. In the case of this bug, the parameter at position 1 was expected to be a type (of the exception that is thrown during validation), but instead was an integer (representing the constant value of an enum).

I need to have a think about a better way to handle this, maybe:


[VogenConfig<Guid>]
public partial class CustomValueObject
{
        public readonly Conversions conversions = Conversions.Default | Conversions.EfCoreValueConverter;
        public readonly ComparisonGeneration comparison = ComparisonGeneration.Omit;
        public readonly CastOperator toPrimitiveCasting = CastOperator.Implicit;
        public readonly CastOperator fromPrimitiveCasting = CastOperator.Implicit;
}

That would then generate a `CustomValueObject<Guid>` attribute

I don't think this change is going to happen within the next couple of weeks though, so it looks like the ugly verbose way is the best way.

Mind you, if it's **assembly wide** config, then you can use the `[assembly: VogenDefaults(...)]`, but as I say, that's per-assembly.