diff --git a/src/ExpressiveSharp/Services/ExpressiveReplacer.cs b/src/ExpressiveSharp/Services/ExpressiveReplacer.cs index 9b8b6fdb..0257bb13 100644 --- a/src/ExpressiveSharp/Services/ExpressiveReplacer.cs +++ b/src/ExpressiveSharp/Services/ExpressiveReplacer.cs @@ -359,7 +359,7 @@ private PolymorphicPlan GetOrBuildPolymorphicPlan(Type rootType, MemberInfo base private PolymorphicPlan BuildPolymorphicPlan(Type rootType, MemberInfo baseMember) { var rootMember = ResolveConcreteMember(rootType, baseMember) ?? baseMember; - var rootRegistered = TryGetReflectedExpressionSafe(rootMember, out _); + var rootRegistered = TryGetReflectedExpressionForDispatch(rootMember, baseMember, out _); var arms = new List(); foreach (var assembly in AppDomain.CurrentDomain.GetAssemblies()) @@ -400,7 +400,7 @@ private PolymorphicPlan BuildPolymorphicPlan(Type rootType, MemberInfo baseMembe // its declaring ancestor's `is` test. Skip plain overrides (no registered body) — // EXP0032 flags those. if (concrete is null || concrete.DeclaringType != candidate - || !TryGetReflectedExpressionSafe(concrete, out _)) + || !TryGetReflectedExpressionForDispatch(concrete, baseMember, out _)) { continue; } @@ -452,6 +452,35 @@ private bool TryGetReflectedExpressionSafe(MemberInfo memberInfo, [NotNullWhen(t return TryGetReflectedExpression(memberInfo, out reflectedExpression); } + // Discovery probe for polymorphic dispatch: a member carrying [Expressive] whose expression + // cannot be resolved is a broken setup, not a skippable candidate — using the base body + // instead would silently change query results per row. Fail with the remedies spelled out. + private bool TryGetReflectedExpressionForDispatch(MemberInfo memberInfo, MemberInfo baseMember, + [NotNullWhen(true)] out LambdaExpression? reflectedExpression) + { + if (IsAbstractMember(memberInfo)) + { + reflectedExpression = null; + return false; + } + + try + { + return TryGetReflectedExpression(memberInfo, out reflectedExpression); + } + catch (Exception ex) + { + throw new InvalidOperationException( + $"Polymorphic dispatch for '{baseMember.DeclaringType}.{baseMember.Name}' requires the generated " + + $"expression for [Expressive] member '{memberInfo.DeclaringType}.{memberInfo.Name}', which could not " + + $"be resolved. Compile assembly '{memberInfo.DeclaringType?.Assembly.GetName().Name}' with the " + + "ExpressiveSharp source generator, remove [Expressive] from the override (optionally marking it " + + "[NotExpressive]) to fall back to the base expression, or disable polymorphic dispatch via " + + "ExpressiveOptions.DisablePolymorphicDispatch() (EF Core: UseExpressives(o => o.DisablePolymorphicDispatch())).", + ex); + } + } + private static bool IsAbstractMember(MemberInfo memberInfo) => memberInfo switch { MethodInfo method => method.IsAbstract, diff --git a/tests/ExpressiveSharp.Tests/Services/ExpressiveReplacerTests.cs b/tests/ExpressiveSharp.Tests/Services/ExpressiveReplacerTests.cs index b14d7285..5dbe36b3 100644 --- a/tests/ExpressiveSharp.Tests/Services/ExpressiveReplacerTests.cs +++ b/tests/ExpressiveSharp.Tests/Services/ExpressiveReplacerTests.cs @@ -157,4 +157,99 @@ public LambdaExpression FindGeneratedExpression(MemberInfo expressiveMemberInfo, public LambdaExpression? FindExternalExpression(MemberInfo memberInfo) => _expressions.TryGetValue(memberInfo, out var expr) ? expr : null; } + + public class PlanCacheBase + { + [Expressive] + public virtual int Value => 1; + } + + public class PlanCacheDerived : PlanCacheBase + { + [Expressive] + public override int Value => 2; + } + + private sealed class PartialStubResolver(bool provideDerivedBody) : IExpressiveResolver + { + public LambdaExpression FindGeneratedExpression(MemberInfo expressiveMemberInfo, + ExpressiveAttribute? expressiveAttribute = null) + { + if (expressiveMemberInfo.DeclaringType == typeof(PlanCacheDerived) && !provideDerivedBody) + { + return null!; + } + + var parameter = Expression.Parameter(expressiveMemberInfo.DeclaringType!, "x"); + var value = expressiveMemberInfo.DeclaringType == typeof(PlanCacheDerived) ? 2 : 1; + return Expression.Lambda(Expression.Constant(value), parameter); + } + + public LambdaExpression? FindExternalExpression(MemberInfo memberInfo) => null; + } + + private sealed class DerivedTypeIsFinder : ExpressionVisitor + { + public bool FoundDerivedTypeTest { get; private set; } + + protected override Expression VisitTypeBinary(TypeBinaryExpression node) + { + if (node.TypeOperand == typeof(PlanCacheDerived)) + { + FoundDerivedTypeTest = true; + } + + return base.VisitTypeBinary(node); + } + } + + public class ResilientBase { [Expressive] public virtual int Value => 1; } + + public class ResilientDerived : ResilientBase { [Expressive] public override int Value => 2; } + + private sealed class ThrowingDerivedResolver : IExpressiveResolver + { + public LambdaExpression FindGeneratedExpression(MemberInfo member, ExpressiveAttribute? attribute = null) + => member.DeclaringType == typeof(ResilientDerived) + ? throw new InvalidOperationException($"Unable to resolve generated expression for {member.Name}.") + : Expression.Lambda(Expression.Constant(1), Expression.Parameter(member.DeclaringType!, "x")); + + public LambdaExpression? FindExternalExpression(MemberInfo member) => null; + } + + [TestMethod] + public void Replace_DerivedOverrideThatFailsToResolve_ThrowsActionableError() + { + Expression> query = b => b.Value; + var replacer = new ExpressiveReplacer(new ThrowingDerivedResolver()); + + var ex = Assert.ThrowsExactly(() => replacer.Replace(query)); + + StringAssert.Contains(ex.Message, nameof(ResilientDerived)); + StringAssert.Contains(ex.Message, "DisablePolymorphicDispatch"); + Assert.IsNotNull(ex.InnerException); + } + + [TestMethod] + public void PolymorphicPlanCache_DoesNotLeakAcrossResolvers() + { + Expression> query = b => b.Value; + + var replacer1 = new ExpressiveReplacer(new PartialStubResolver(provideDerivedBody: true)); + replacer1.Replace(query); + + var precondition = new DerivedTypeIsFinder(); + precondition.Visit(replacer1.Replace(query)); + Assert.IsTrue(precondition.FoundDerivedTypeTest); + + replacer1.Replace(query); + + var replacer2 = new ExpressiveReplacer(new PartialStubResolver(provideDerivedBody: false)); + var expanded = replacer2.Replace(query); + + var finder = new DerivedTypeIsFinder(); + finder.Visit(expanded); + + Assert.IsFalse(finder.FoundDerivedTypeTest); + } }