Skip to content

Commit a781ea0

Browse files
authored
Merge pull request #66 from EFNext/feat/virtual-expressive-members
Allow virtual expressive members but warn; added documentation on the restriction as well as a suggested workaround
2 parents b3d8861 + f07a9ce commit a781ea0

11 files changed

Lines changed: 306 additions & 15 deletions

File tree

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
{
2+
"features": {
3+
"ghcr.io/devcontainers/features/docker-in-docker:2": {
4+
"version": "2.17.0",
5+
"resolved": "ghcr.io/devcontainers/features/docker-in-docker@sha256:25b9f05705ffba7dbe503230ac76081419306f8c8bc88e0ce78c4ecd99a0c78c",
6+
"integrity": "sha256:25b9f05705ffba7dbe503230ac76081419306f8c8bc88e0ce78c4ecd99a0c78c"
7+
},
8+
"ghcr.io/devcontainers/features/dotnet:2": {
9+
"version": "2.5.0",
10+
"resolved": "ghcr.io/devcontainers/features/dotnet@sha256:0fc16547ed4db6d7ff2a9f5981d2b93eb314e568affb9958029ad794f1f9a093",
11+
"integrity": "sha256:0fc16547ed4db6d7ff2a9f5981d2b93eb314e568affb9958029ad794f1f9a093"
12+
},
13+
"ghcr.io/devcontainers/features/node:1": {
14+
"version": "1.7.1",
15+
"resolved": "ghcr.io/devcontainers/features/node@sha256:8c0de46939b61958041700ee89e3493f3b2e4131a06dc46b4d9423427d06e5f6",
16+
"integrity": "sha256:8c0de46939b61958041700ee89e3493f3b2e4131a06dc46b4d9423427d06e5f6"
17+
}
18+
}
19+
}

docs/advanced/limitations.md

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,67 @@ static bool IsNullOrWhiteSpace(string? s)
111111
```
112112
:::
113113

114+
## Virtual and Polymorphic Members {#virtual-polymorphic-members}
115+
116+
Expression-tree expansion happens at **compile time** and works purely from the **static (declared) type** of each receiver. It has no runtime instance to inspect, so it cannot honor C# virtual dispatch.
117+
118+
If you mark a `virtual`, `abstract`, or `override` member `[Expressive]` (a default interface member counts too -- it is implicitly virtual), the generator reports [EXP0038](../reference/diagnostics#exp0038). When the member is expanded for a query provider (EF Core, MongoDB), the call is resolved against the declared type and the **base** body is always inlined -- an overridden body in a derived type is never used:
119+
120+
```csharp
121+
public class Animal
122+
{
123+
public string Name { get; set; } = "";
124+
125+
[Expressive] // EXP0038
126+
public virtual string Describe() => $"Animal: {Name}";
127+
}
128+
129+
public class Dog : Animal
130+
{
131+
[Expressive] // EXP0038
132+
public override string Describe() => $"Dog: {Name}";
133+
}
134+
135+
// The static type is Animal, so expansion inlines the BASE body -- even for Dog rows:
136+
db.Animals.AsExpressive().Select(a => a.Describe()); // => "Animal: {Name}" in SQL
137+
```
138+
139+
This is by design: a query provider translates the expression to SQL/MQL and never materializes a CLR object, so there is no runtime type to dispatch on. (Contrast this with compiling the expression to a delegate and invoking it in memory, where the CLR *does* dispatch on the runtime type.)
140+
141+
### Recommended: test the runtime type explicitly
142+
143+
Branch on the concrete type so each arm has a statically-typed receiver. Every branch then expands to the correct body and the provider emits a `CASE`:
144+
145+
```csharp
146+
db.Animals.AsExpressive().Select(a => a switch
147+
{
148+
Dog d => d.Describe(), // expands Dog.Describe
149+
_ => a.Describe(), // expands Animal.Describe
150+
});
151+
```
152+
153+
### Recommended: use a non-virtual static/extension method
154+
155+
Move the logic into a single non-virtual `[Expressive]` method that performs the type test itself. This keeps the polymorphic shape in one place and produces no EXP0038:
156+
157+
```csharp
158+
public static class AnimalExpressions
159+
{
160+
[Expressive]
161+
public static string Describe(this Animal a) => a switch
162+
{
163+
Dog d => $"Dog: {d.Name}",
164+
_ => $"Animal: {a.Name}",
165+
};
166+
}
167+
168+
db.Animals.AsExpressive().Select(a => a.Describe());
169+
```
170+
171+
::: tip
172+
Declaring entity members `virtual` is common in EF Core because it enables lazy-loading proxies. That remains fine for plain navigation and scalar properties -- EXP0038 only concerns members you *also* mark `[Expressive]`.
173+
:::
174+
114175
## Performance: First-Execution Overhead
115176

116177
`ExpandExpressives()` walks the expression tree and substitutes `[Expressive]` member references on every query execution. This adds a small cost to the first execution of each unique query shape. EF Core caches the compiled query afterward, so subsequent executions of the same shape skip the expansion entirely.

docs/reference/diagnostics.md

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ See [Troubleshooting](./troubleshooting) for symptom-oriented guidance -- find t
3939
| [EXP0034](#exp0034) | Error | `[ExpressiveProperty]` requires an instance stub | -- |
4040
| [EXP0035](#exp0035) | Error | `[ExpressiveProperty]` target shadows inherited member | -- |
4141
| [EXP0036](#exp0036) | Info | `IExpressiveQueryable<T>` chain dropped to plain `IQueryable<T>` | -- |
42+
| [EXP0038](#exp0038) | Warning | `[Expressive]` member is virtual and will not dispatch polymorphically | -- |
4243
| [EXP1001](#exp1001) | Warning | Replace `[Projectable]` with `[Expressive]` | [Replace attribute](#exp1001-fix) |
4344
| [EXP1002](#exp1002) | Warning | Replace `UseProjectables()` with `UseExpressives()` | [Replace method call](#exp1002-fix) |
4445
| [EXP1003](#exp1003) | Warning | Replace Projectables namespace | [Replace namespace](#exp1003-fix) |
@@ -697,6 +698,53 @@ db.Orders.AsExpressiveDbSet()
697698

698699
---
699700

701+
## Virtual Member Diagnostic (EXP0038)
702+
703+
### EXP0038 -- Virtual member will not dispatch polymorphically {#exp0038}
704+
705+
**Severity:** Warning
706+
**Category:** Design
707+
708+
**Message:**
709+
```
710+
[Expressive] member '{0}' is virtual, abstract, or an override. When it is expanded into an
711+
expression tree (e.g. for EF Core or MongoDB), the call is resolved using the static (declared)
712+
type, so an overridden body in a derived type is never used. Test the runtime type explicitly
713+
(e.g. 'x switch { Derived d => d.Member, _ => x.Member }'), or move the logic into a non-virtual
714+
[Expressive] static/extension method.
715+
```
716+
717+
**Cause:** An `[Expressive]` member is declared `virtual`, `abstract`, or `override` (a default interface member counts -- it is implicitly virtual). Expression-tree expansion happens at compile time and only sees the **static (declared) type** of the receiver, so it cannot honor C# virtual dispatch. When the member is expanded for a query provider, the **base** body is always inlined; an overridden body in a derived type is never used.
718+
719+
This differs from compiling the expression to a delegate and invoking it in memory, where the CLR dispatches on the runtime type.
720+
721+
**Fix:** Branch on the runtime type so each arm has a statically-typed receiver, or move the logic into a single non-virtual `[Expressive]` static/extension method. See [Limitations: virtual and polymorphic members](../advanced/limitations#virtual-polymorphic-members) for full examples.
722+
723+
```csharp
724+
// Warning: virtual [Expressive] member
725+
[Expressive]
726+
public virtual string Describe() => $"Animal: {Name}";
727+
728+
// Fix 1: test the runtime type explicitly
729+
db.Animals.AsExpressive().Select(a => a switch
730+
{
731+
Dog d => d.Describe(),
732+
_ => a.Describe(),
733+
});
734+
735+
// Fix 2: non-virtual [Expressive] extension method that does the type test once
736+
[Expressive]
737+
public static string Describe(this Animal a) => a switch
738+
{
739+
Dog d => $"Dog: {d.Name}",
740+
_ => $"Animal: {a.Name}",
741+
};
742+
```
743+
744+
If a virtual member is intentional (you only ever compile it to an in-memory delegate, never translate it through a provider), suppress the warning with `#pragma warning disable EXP0038` or `<NoWarn>$(NoWarn);EXP0038</NoWarn>`.
745+
746+
---
747+
700748
## Migration Diagnostics (EXP1001--EXP1003)
701749

702750
These diagnostics are emitted by the `MigrationAnalyzer` in the `ExpressiveSharp.EntityFrameworkCore.CodeFixers` package. They detect usage of the legacy `EntityFrameworkCore.Projectables` library and offer automated code fixes to migrate to ExpressiveSharp.

src/ExpressiveSharp.Generator/ExpressiveGenerator.cs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,17 @@ private static void Execute(
229229
factoryCandidate.Identifier.Text));
230230
}
231231

232+
// EXP0038: virtual/abstract/override members are expanded using the static (declared)
233+
// type. Once the body is inlined into an expression tree (EF Core, MongoDB, ...), C#
234+
// virtual dispatch is lost, so an overridden body in a derived type is never used.
235+
if (memberSymbol.IsVirtual || memberSymbol.IsAbstract || memberSymbol.IsOverride)
236+
{
237+
context.ReportDiagnostic(Diagnostic.Create(
238+
Infrastructure.Diagnostics.VirtualMemberDispatchedStatically,
239+
memberSymbol.Locations.Length > 0 ? memberSymbol.Locations[0] : Location.None,
240+
memberSymbol.Name));
241+
}
242+
232243
var generatedClassName = ExpressionClassNameGenerator.GenerateClassName(expressive.ClassNamespace, expressive.NestedInClassNames);
233244
var methodSuffix = ExpressionClassNameGenerator.GenerateMethodSuffix(expressive.MemberName, expressive.ParameterTypeNames);
234245
var generatedFileName = expressive.ClassTypeParameterList is not null

src/ExpressiveSharp.Generator/Infrastructure/Diagnostics.cs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -198,4 +198,15 @@ static internal class Diagnostics
198198
category: "Design",
199199
DiagnosticSeverity.Error,
200200
isEnabledByDefault: true);
201+
202+
// EXP0036/EXP0037 live in WindowFunctionLiteralArgsAnalyzer (EntityFrameworkCore.CodeFixers).
203+
204+
public readonly static DiagnosticDescriptor VirtualMemberDispatchedStatically = new DiagnosticDescriptor(
205+
id: "EXP0038",
206+
title: "[Expressive] member is virtual and will not dispatch polymorphically",
207+
messageFormat: "[Expressive] member '{0}' is virtual, abstract, or an override. When it is expanded into an expression tree (e.g. for EF Core or MongoDB), the call is resolved using the static (declared) type, so an overridden body in a derived type is never used. Test the runtime type explicitly (e.g. 'x switch {{ Derived d => d.Member, _ => x.Member }}'), or move the logic into a non-virtual [Expressive] static/extension method.",
208+
category: "Design",
209+
DiagnosticSeverity.Warning,
210+
isEnabledByDefault: true,
211+
description: "Expression-tree expansion happens at compile time and only sees the static (declared) type of the receiver, so it cannot honor C# virtual dispatch. Declaring an [Expressive] member virtual/abstract/override therefore silently expands the base body for query providers. This differs from compiling the expression to a delegate and invoking it in memory, where the CLR dispatches on the runtime type.");
201212
}

src/ExpressiveSharp/Services/ExpressiveReplacer.cs

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -50,9 +50,6 @@ protected bool TryGetReflectedExpression(MemberInfo memberInfo, [NotNullWhen(tru
5050
[return: NotNullIfNotNull(nameof(node))]
5151
public virtual Expression? Replace(Expression? node) => Visit(node);
5252

53-
private static bool IsPolymorphicallyDispatched(MethodInfo? method)
54-
=> method is not null && method.IsVirtual && !method.IsFinal;
55-
5653
protected override Expression VisitMethodCall(MethodCallExpression node)
5754
{
5855
// Replace MethodGroup arguments with their reflected expressions.
@@ -78,10 +75,11 @@ protected override Expression VisitMethodCall(MethodCallExpression node)
7875

7976
VisitMethodCallCore(node);
8077

81-
var methodInfo = node.Object?.Type.GetConcreteMethod(node.Method) ?? node.Method;
78+
var methodInfo = node.Method.DeclaringType?.IsInterface == true
79+
? (node.Object?.Type.GetConcreteMethod(node.Method) ?? node.Method)
80+
: node.Method;
8281

83-
if (!IsPolymorphicallyDispatched(methodInfo) &&
84-
!_expandingMembers.Contains(methodInfo) &&
82+
if (!_expandingMembers.Contains(methodInfo) &&
8583
TryGetReflectedExpression(methodInfo, out var reflectedExpression))
8684
{
8785
_expandingMembers.Add(methodInfo);
@@ -162,15 +160,11 @@ when type.IsAssignableFrom(operand.Type)
162160
_ => node.Expression
163161
};
164162
var nodeMember = node.Member switch {
165-
PropertyInfo property when nodeExpression is not null
163+
PropertyInfo property when nodeExpression is not null && property.DeclaringType?.IsInterface == true
166164
=> nodeExpression.Type.GetConcreteProperty(property),
167165
_ => node.Member
168166
};
169167

170-
var virtualAccessor = nodeMember is PropertyInfo prop ? prop.GetMethod : null;
171-
if (IsPolymorphicallyDispatched(virtualAccessor))
172-
return base.VisitMember(node);
173-
174168
if (!_expandingMembers.Contains(nodeMember) &&
175169
TryGetReflectedExpression(nodeMember, out var reflectedExpression))
176170
{

tests/ExpressiveSharp.EntityFrameworkCore.IntegrationTests/Infrastructure/StoreQueryTestBase.cs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,4 +190,23 @@ public async Task ExpressiveFor_Inside_ExpressiveMember_ComposesCorrectly()
190190
// Totals: 240, 1500, 30, 250 → clamped to [0, 200]: 200, 200, 30, 200
191191
CollectionAssert.AreEqual(new[] { 200.0, 200.0, 30.0, 200.0 }, results);
192192
}
193+
194+
[TestMethod]
195+
public async Task VirtualExpressiveMember_FiltersInDatabase()
196+
{
197+
// Regression for the reverted "bad commit": a virtual [Expressive] member must expand
198+
// (using its static/declared body) and translate to SQL. The polymorphism gate used to
199+
// skip expansion, so LineItem.IsExpensive reached the provider untranslated and threw.
200+
// Compare against the database's own non-expressive predicate so the assertion is
201+
// independent of the seed details.
202+
Expression<Func<LineItem, bool>> expr = li => li.IsExpensive;
203+
var expanded = (Expression<Func<LineItem, bool>>)expr.ExpandExpressives();
204+
205+
var expected = await Context.Set<LineItem>().Where(li => li.UnitPrice > 40).CountAsync();
206+
Assert.IsTrue(expected > 0, "Seed should contain at least one expensive line item.");
207+
208+
var actual = await Context.Set<LineItem>().Where(expanded).CountAsync();
209+
210+
Assert.AreEqual(expected, actual);
211+
}
193212
}

tests/ExpressiveSharp.Generator.Tests/ExpressiveGenerator/DiagnosticTests.cs

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,74 @@ static class Mappings {
209209
"Expected EXP0014 for unresolvable target type in [ExpressiveFor]");
210210
}
211211

212+
[TestMethod]
213+
public void VirtualMethod_ReportsEXP0038()
214+
{
215+
var compilation = CreateCompilation(
216+
"""
217+
namespace Foo {
218+
class Animal {
219+
public string Name { get; set; }
220+
221+
[Expressive]
222+
public virtual string Describe() => "Animal: " + Name;
223+
}
224+
}
225+
""");
226+
var result = RunExpressiveGenerator(compilation);
227+
228+
var diag = result.Diagnostics.FirstOrDefault(d => d.Id == "EXP0038");
229+
Assert.IsNotNull(diag, "Expected EXP0038 for a virtual [Expressive] member");
230+
Assert.AreEqual(DiagnosticSeverity.Warning, diag.Severity);
231+
Assert.IsTrue(result.GeneratedTrees.Length > 0,
232+
"Generator should still produce output alongside the EXP0038 warning");
233+
}
234+
235+
[TestMethod]
236+
public void VirtualAndOverrideProperties_BothReportEXP0038()
237+
{
238+
var compilation = CreateCompilation(
239+
"""
240+
namespace Foo {
241+
class Animal {
242+
public string Name { get; set; }
243+
244+
[Expressive]
245+
public virtual string Label => Name;
246+
}
247+
248+
class Dog : Animal {
249+
[Expressive]
250+
public override string Label => "Dog: " + Name;
251+
}
252+
}
253+
""");
254+
var result = RunExpressiveGenerator(compilation);
255+
256+
Assert.AreEqual(2, result.Diagnostics.Count(d => d.Id == "EXP0038"),
257+
"Expected EXP0038 for both the virtual base property and its override");
258+
}
259+
260+
[TestMethod]
261+
public void NonVirtualMember_DoesNotReportEXP0038()
262+
{
263+
var compilation = CreateCompilation(
264+
"""
265+
namespace Foo {
266+
class Animal {
267+
public string Name { get; set; }
268+
269+
[Expressive]
270+
public string Describe() => "Animal: " + Name;
271+
}
272+
}
273+
""");
274+
var result = RunExpressiveGenerator(compilation);
275+
276+
Assert.IsFalse(result.Diagnostics.Any(d => d.Id == "EXP0038"),
277+
"A non-virtual [Expressive] member must not report EXP0038");
278+
}
279+
212280
// NOTE: EXP0010 (InterceptorEmissionFailed) is intentionally not tested.
213281
// It is a catch-all for unhandled exceptions during interceptor generation
214282
// in PolyfillInterceptorGenerator. No natural user input triggers it — it

tests/ExpressiveSharp.Generator.Tests/ExpressiveGenerator/InterfaceTests.cs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,11 @@ public interface IDefaultBase : IBase
5555

5656
var result = RunExpressiveGenerator(compilation);
5757

58-
Assert.AreEqual(0, result.Diagnostics.Length);
58+
// A default interface member is implicitly virtual, so EXP0038 fires: when expanded into
59+
// an expression tree the call resolves against the static (interface) type, not a runtime
60+
// override. The generator still emits the expression for the declared body.
61+
Assert.AreEqual(1, result.Diagnostics.Length);
62+
Assert.AreEqual("EXP0038", result.Diagnostics[0].Id);
5963
Assert.AreEqual(1, result.GeneratedTrees.Length);
6064

6165
return Verifier.Verify(result.GeneratedTrees[0].ToString());

tests/ExpressiveSharp.IntegrationTests/Scenarios/Store/Models/LineItem.cs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,4 +7,13 @@ public class LineItem
77
public string ProductName { get; set; } = "";
88
public double UnitPrice { get; set; }
99
public int Quantity { get; set; }
10+
11+
// Virtual [Expressive] member — regression coverage that static-type expansion still reaches
12+
// the query provider as translatable SQL. The reverted "bad commit" gate skipped expansion for
13+
// virtual members, so this would hit EF Core untranslated and throw. EXP0038 is expected here
14+
// by design and suppressed.
15+
#pragma warning disable EXP0038
16+
[Expressive]
17+
public virtual bool IsExpensive => UnitPrice > 40;
18+
#pragma warning restore EXP0038
1019
}

0 commit comments

Comments
 (0)