fix: source generated partials when using attributes derived from ValueObjectAttribute<T>

[jupjohn]

This PR aims to fix #319 where struct/class partials were not being generated when using custom attributes inheriting ValueObjectAttribute<T>. Root cause of this is explained in this comment: #319 (comment)

TODO:

• Add addition BaseType check on attribute's BaseType
• Snapshot test to ensure custom attributes derived from ValueObjectAttribute<T> generate the same source code as direct usage of ValueObjectAttribute<T>

[jupjohn]

@SteveDunn I see that the snapshot tests for generic attributes have been commented out. I'll add them back, potentially clean them up (if they're missing anything), and add my derived attribute test to it unless there's a reason for them to be commented out. Shouldn't be too out of scope of this PR.

[jupjohn]

Holding off on this PR until #322 is completed

[SteveDunn]

@SteveDunn I see that the snapshot tests for generic attributes have been commented out. I'll add them back, potentially clean them up (if they're missing anything), and add my derived attribute test to it unless there's a reason for them to be commented out. Shouldn't be too out of scope of this PR.

Yes, I had a lot of trouble testing these. I tested them in other way though, like in the consumer tests. I can't remember the exact reason why there were disabled, although it will likely become clear when trying to run them 🙈 !

[jupjohn]

Rebased onto main after #322 was merged, snapshot test added

[jupjohn]

Hey @SteveDunn, in case this got lost in your notifications (or you didn't get any) this is good to go 👌

[SteveDunn]

Great - thank you @jammehcow - sorry for the slow reply (it did get lost in a mountain of other notifications this week!)

Really appreciate your help - many thanks.

[SteveDunn]

LGTM!

[glen-84]

This doesn't seem to be working for me. Did you try with a non-int value?

I've tried:

public class IdAttribute : ValueObjectAttribute
{
    public IdAttribute() : base(typeof(long), Conversions.None) { }
}

and:

public class IdAttribute : ValueObjectAttribute<long>
{
    public IdAttribute() : base(Conversions.None) { }
}

And the underlying type remains as int.

[jupjohn]

@glen-84 Well you're right - that snapshot test fails if the type isn't int. I'll do some digging and get a fix through, thanks for reporting it!

[jupjohn]

I've got semi-working fix generating source for derived types but it requires you to copy the base constructor into your class which isn't ideal. Will try to figure out how to read parameter values of the bass class' constructor tomorrow, and give an explanation of why this feature didn't work.

[jupjohn]

In Vogen's current state, parameters passed to the attribute's constructor are extracted from the constructor as constants - not pulled from some post-construction field inside of ValueObjectAttribute. Due to this, passing values to the base class' constructor without specifying them in the derived attribute's constructor does nothing so to use derived attributes you must set your default parameter values in the class' constructor. I did find out that parameters passed to base constructors can't be inferred at compile time like Vogen does to top level attribute constructors.

There's a small fix required to extract generic type values & parameters from derived attributes which I'll add today, as well as a note in the README about requiring default parameter values in derived constructors.

I guess the real fix here would be to compile the attribute class if it's not one of Vogen's ones, then extract the values passed to base from backing fields. Not sure how viable this is with Roslyn or the potential performance degradation.

[houseofcat]

@jupjohn @SteveDunn Can I get confirmation this should be working?

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)
    { }
}

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

[SteveDunn]

@jupjohn @SteveDunn Can I get confirmation this should be working?

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)
    { }
}

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

Hi, yes, it should work. The whole deriving from attributes scenario is a bit up in the air. Ideally, people wouldn't derive, and that'd open up the possibility of using a faster Roslyn generator. It'd also make the Vogen code easier. But I can see the compelling reason to derive.
I'll take a look later on.
Thank you for the bug report!

[jupjohn]

Not to get too in the weeds of it (and keep in mind I don't know all of the limitations of source generators), but what if that derived attribute complexity was yanked out of the existing generator and moved to something that source generated partials with their corresponding ValueObjectAttribute attached. That way, one can focus purely on looking at the base attributes and the other acts as a "translator" of sorts.

Food for thought - I don't know if you can walk the syntax of another generator's output.

[SteveDunn]

Hi @jupjohn - unfortunately, it's not possible to combine or set dependencies on different source generators. There's a thread here: dotnet/roslyn#57239