Hierarchy implementation and fixes

[Xriuk]

Closes #74 and #213

I have added support for hierarchies where overwritten members are all added in the parent body with type casts, this is to match the runtime behaviour for generated expressions.

I have also allowed members tagged with [Projectable] to not have any body at all (abstract method/property) if they have at least one implemented member in derived classes.

I have added some tests, maybe not 100% of cases are covered, currently I'm not sure if any sub-hierarchies would be better flattened or not, eg:
A hierarchy like this:

public class A{
  [Projectable]
  public virtual int Id => 1;
}

public class B1 : A{
  [Projectable]
  public virtual int Id => 2;
}
public class B2 : B1{
  public virtual int Id => 3;
}

public class C : A{
  public virtual int Id => 4;
}

will generate a final Expression like this (the nested B1 expression gets replaced later):

@this => @this is B1 ? (@this is B2 ? ((B2)@this).Id : 2) : @this is C ? ((C)@this).Id : 1

And I'm not sure if it would be better to generate a flat expression like this instead (by associating the condition for B1 to the last else), or if this is something already handled by EF Core:

@this => @this is B2 ? ((B2)@this).Id : @this is B1 ? 2 : @this is C ? ((C)@this).Id : 1

I am open to any coding-style changes which might be required, also if everything seems good so far I could edit the docs to reflect the new changes.

[Xriuk]

I have some doubts about base.MyMethod() getting rewritten to @this.MyMethod(), the two invocations are not equivalent (one is the parent method, and one is the projected one), and would actually get to a loop of expressions getting replaced.

I would replace it with a cast like this ((BaseType)@this).MyMethod() and if the parent method is projected as well (like in my hierarchy implementation) I would replace it with just the method body instead of the full if/else if/else type chain.

So I would generate two expressions for each member in a hierarchy: one for the public invocations, and one for invocations from derived classes to avoid loops.

[Xriuk]

Also closes #213

I have rewritten base.MyMethod() to be translated to ((BaseType)@this).MyMethod() so that explicit invocations can be recognized and replaced later. Currently it should not change existing implementations, since a cast is usually ignored, but this could be replaced at the end if needed by removing the cast entirely after all the hierarchy replacements.

I am now generating 2 expressions for hierarchies: one "complete" which has all the typecasts and derived classes invocations, and one "base" which is used when replacing base.MyMethod() inside derived classes to avoid loops.

The "base" expressions currently do not have an assembly registry generated like regular expressions.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[koenbeuk]

thanks for this contribution!

I'm hesitant to merge it in its current form and reasons for why we have not yet implemented this thus far:

1. GetDerivedTypes only sees types that are available in the same project as the base assembly. Derived types in different assemblies are currently not detected
2. Stale incremental generations. If you add a new derived type, it wont currently get detected by the source generator and it wont re-emit generated code including that newly derived type.
3. The generator performance is hurt due to the need to include and scan the semantic model for derived types

Instead of doing this at compile time, we could do this at runtime instead which solves all 3 problems, at the cost of a slightly higher initial lookup cost when using this feature.

If we wanted to merge this then I recommend:

1. To make this an opt-in (users can set an attribute on the base types Projectable to declare that we should scan for derived types and inline them)
2. Fix stale incremental generations (can be done, but its tricky and has a cost associated with it. ExpressiveSharp already does this).

[koenbeuk]

Some feedback that Claude came up with (haven't verified):

Crashes / correctness bugs
Records crash the generator. Lines 209 & 220 hard-cast s.GetSyntax() to ClassDeclarationSyntax. A derived record is RecordDeclarationSyntax (a sibling, not a subclass) → InvalidCastException → generator failure. Record entities are common. Use TypeDeclarationSyntax.

Partial classes throw. Line 220 uses .Members.First(m => …isOverride…). For a partial class, each DeclaringSyntaxReference is one part; the part that doesn't declare the override makes .First throw InvalidOperationException. Should be .Where(...).Any() / FirstOrDefault.

Property base-detection is fragile and inconsistent with the method path. The method path uses UnwrapUnaryConvert (line 216), but the property path (lines 358–359) requires u.Type == u.Operand.Type.BaseType and p.Name == "@this" — i.e. exactly one cast level off a parameter literally named @this. After argument substitution the receiver may be a nested expression or a multi-level cast ((A)(B1)@this in a 3-level hierarchy), so base detection silently fails there and falls through to the dispatch version — risking wrong dispatch or recursion. The two paths should share one detection helper.

Derived overrides aren't required to be [Projectable]. The derivedTypes filter checks only IsOverride && Kind && Name, not for the [Projectable] attribute. A non-projectable override still gets emitted as ((Derived)@this).Member; at runtime the replacer can't expand it, leaving a raw CLR member access that EF may fail to translate (if it's a computed, non-mapped member). Either require [Projectable] on the override or emit a diagnostic.

Open generics explicitly unhandled — two // DEV: handle generic types TODOs in HierarchyMembersConverter. Generic derived types will produce wrong/invalid casts. Should be a guarded diagnostic until supported, not silent.

[Copilot]

Pull request overview

Copilot reviewed 34 out of 34 changed files in this pull request and generated 7 comments.

[Xriuk]

Not relevant because we are only interested in replacing base expressions inside other Projectable expressions, where the lambda parameter is normalized as "@this"

[Xriuk]

Thanks for the feedback! Yeah, from the beginning this seemed a bit "forced" to scan types from a source generator while it sounds more a runtime task.

So if it sounds good I would add a property to the Projectable attribute to allow opt-in (this would also allow empty body for the members as in this implementation).
This would generate only a single expression per member (as it already was) eliminating the need for my "_Base" expressions. And then in the expression replacer it would either replace the expression or scan and inline the derived types' expressions, and optionally add the parent expression at the end if not empty. And also replace "base.MyMethod()" invocations as in this implementation.

Would you prefer if I submit a different PR or keep working on this one?

[Xriuk]

I changed the implementation to be in the replacer rather than the generator. Waiting for feedbacks on this one.