Skip to content

Commit dc01522

Browse files
koenbeukkoenbeuk
committed
Enhance nullable enum handling in expression generation and tests
1 parent 3011815 commit dc01522

5 files changed

Lines changed: 88 additions & 50 deletions

File tree

‎src/ExpressiveSharp.Generator/Emitter/ExpressionTreeEmitter.cs‎

Lines changed: 39 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -755,6 +755,38 @@ private bool TryEmitEnumMethodExpansion(IInvocationOperation invocation, out str
755755

756756
var receiverTypeFqn = receiverType.ToDisplayString(_fqnFormat);
757757

758+
// Hoist non-receiver argument emission so per-arm calls and the null-branch call reuse the same vars.
759+
var staticArgOffset = method.IsExtensionMethod ? 1 : 0;
760+
var sharedExtraArgVars = new List<string>();
761+
var argStartIndex = originalMethod.IsStatic ? staticArgOffset : 0;
762+
for (var i = argStartIndex; i < invocation.Arguments.Length; i++)
763+
{
764+
sharedExtraArgVars.Add(EmitOperation(invocation.Arguments[i].Value));
765+
}
766+
767+
string BuildCall(string operandVar)
768+
{
769+
string callArgsExpr;
770+
if (originalMethod.IsStatic)
771+
{
772+
var allArgs = new List<string> { operandVar };
773+
allArgs.AddRange(sharedExtraArgVars);
774+
callArgsExpr = $"new global::System.Linq.Expressions.Expression[] {{ {string.Join(", ", allArgs)} }}";
775+
var callVar = NextVar();
776+
AppendLine($"var {callVar} = {Expr}.Call({methodField}, {callArgsExpr});");
777+
return callVar;
778+
}
779+
else
780+
{
781+
callArgsExpr = sharedExtraArgVars.Count > 0
782+
? $"new global::System.Linq.Expressions.Expression[] {{ {string.Join(", ", sharedExtraArgVars)} }}"
783+
: "global::System.Array.Empty<global::System.Linq.Expressions.Expression>()";
784+
var callVar = NextVar();
785+
AppendLine($"var {callVar} = {Expr}.Call({operandVar}, {methodField}, {callArgsExpr});");
786+
return callVar;
787+
}
788+
}
789+
758790
// Build the ternary chain in reverse so the first member ends up as the outermost (and first-tested) branch.
759791
var currentVar = defaultVar;
760792
foreach (var member in enumMembers.AsEnumerable().Reverse())
@@ -772,33 +804,7 @@ private bool TryEmitEnumMethodExpansion(IInvocationOperation invocation, out str
772804
enumValueVar = lifted;
773805
}
774806

775-
// Static path passes the enum value as the first arg; instance path uses it as the receiver.
776-
string callVar;
777-
if (originalMethod.IsStatic)
778-
{
779-
var callArgVars = new List<string> { enumValueVar };
780-
var argOffset = method.IsExtensionMethod ? 1 : 0;
781-
for (var i = argOffset; i < invocation.Arguments.Length; i++)
782-
{
783-
callArgVars.Add(EmitOperation(invocation.Arguments[i].Value));
784-
}
785-
var callArgsExpr = $"new global::System.Linq.Expressions.Expression[] {{ {string.Join(", ", callArgVars)} }}";
786-
callVar = NextVar();
787-
AppendLine($"var {callVar} = {Expr}.Call({methodField}, {callArgsExpr});");
788-
}
789-
else
790-
{
791-
var callArgVars = new List<string>();
792-
for (var i = 0; i < invocation.Arguments.Length; i++)
793-
{
794-
callArgVars.Add(EmitOperation(invocation.Arguments[i].Value));
795-
}
796-
var callArgsExpr = callArgVars.Count > 0
797-
? $"new global::System.Linq.Expressions.Expression[] {{ {string.Join(", ", callArgVars)} }}"
798-
: "global::System.Array.Empty<global::System.Linq.Expressions.Expression>()";
799-
callVar = NextVar();
800-
AppendLine($"var {callVar} = {Expr}.Call({enumValueVar}, {methodField}, {callArgsExpr});");
801-
}
807+
var callVar = BuildCall(enumValueVar);
802808

803809
var condVar = NextVar();
804810
AppendLine($"var {condVar} = {Expr}.Equal({receiverVar}, {enumValueVar});");
@@ -808,17 +814,21 @@ private bool TryEmitEnumMethodExpansion(IInvocationOperation invocation, out str
808814
currentVar = ternaryVar;
809815
}
810816

811-
// For Nullable<TEnum>, wrap in: receiver == null ? default : chain
817+
// For Nullable<TEnum>, wrap in: receiver == null ? null-branch-call : chain
818+
// Calling the method with a null Nullable<TEnum> preserves runtime semantics — Nullable<T>.ToString()
819+
// returns "" for null, and extensions on Nullable<TEnum> can define their own null behavior.
812820
if (isNullable)
813821
{
814822
var nullConst = NextVar();
815823
AppendLine($"var {nullConst} = {Expr}.Constant(null, typeof({receiverTypeFqn}));");
816824

825+
var nullCallVar = BuildCall(nullConst);
826+
817827
var nullCheck = NextVar();
818828
AppendLine($"var {nullCheck} = {Expr}.Equal({receiverVar}, {nullConst});");
819829

820830
var wrappedVar = NextVar();
821-
AppendLine($"var {wrappedVar} = {Expr}.Condition({nullCheck}, {defaultVar}, {currentVar}, typeof({returnTypeFqn}));");
831+
AppendLine($"var {wrappedVar} = {Expr}.Condition({nullCheck}, {nullCallVar}, {currentVar}, typeof({returnTypeFqn}));");
822832
currentVar = wrappedVar;
823833
}
824834

‎tests/ExpressiveSharp.Generator.Tests/ExpressiveGenerator/EnumTests.ExpandEnumMethodsWithParameter.verified.txt‎

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -14,22 +14,20 @@ namespace ExpressiveSharp.Generated
1414
var p__this = global::System.Linq.Expressions.Expression.Parameter(typeof(global::Foo.Entity), "@this");
1515
var expr_0 = global::System.Linq.Expressions.Expression.Property(p__this, typeof(global::Foo.Entity).GetProperty("Status", global::System.Reflection.BindingFlags.Public | global::System.Reflection.BindingFlags.NonPublic | global::System.Reflection.BindingFlags.Instance)); // Status
1616
var expr_1 = global::System.Linq.Expressions.Expression.Constant(null, typeof(string));
17-
var expr_2 = global::System.Linq.Expressions.Expression.Constant(global::Foo.Status.Rejected, typeof(global::Foo.Status));
18-
var expr_3 = global::System.Linq.Expressions.Expression.Constant("Status: ", typeof(string)); // "Status: "
19-
var expr_4 = global::System.Linq.Expressions.Expression.Call(typeof(global::Foo.EnumExtensions).GetMethod("GetDisplayNameWithPrefix", global::System.Reflection.BindingFlags.Public | global::System.Reflection.BindingFlags.NonPublic | global::System.Reflection.BindingFlags.Static, null, new global::System.Type[] { typeof(global::Foo.Status), typeof(string) }, null), new global::System.Linq.Expressions.Expression[] { expr_2, expr_3 });
20-
var expr_5 = global::System.Linq.Expressions.Expression.Equal(expr_0, expr_2);
17+
var expr_2 = global::System.Linq.Expressions.Expression.Constant("Status: ", typeof(string)); // "Status: "
18+
var expr_3 = global::System.Linq.Expressions.Expression.Constant(global::Foo.Status.Rejected, typeof(global::Foo.Status));
19+
var expr_4 = global::System.Linq.Expressions.Expression.Call(typeof(global::Foo.EnumExtensions).GetMethod("GetDisplayNameWithPrefix", global::System.Reflection.BindingFlags.Public | global::System.Reflection.BindingFlags.NonPublic | global::System.Reflection.BindingFlags.Static, null, new global::System.Type[] { typeof(global::Foo.Status), typeof(string) }, null), new global::System.Linq.Expressions.Expression[] { expr_3, expr_2 });
20+
var expr_5 = global::System.Linq.Expressions.Expression.Equal(expr_0, expr_3);
2121
var expr_6 = global::System.Linq.Expressions.Expression.Condition(expr_5, expr_4, expr_1, typeof(string));
2222
var expr_7 = global::System.Linq.Expressions.Expression.Constant(global::Foo.Status.Approved, typeof(global::Foo.Status));
23-
var expr_8 = global::System.Linq.Expressions.Expression.Constant("Status: ", typeof(string)); // "Status: "
24-
var expr_9 = global::System.Linq.Expressions.Expression.Call(typeof(global::Foo.EnumExtensions).GetMethod("GetDisplayNameWithPrefix", global::System.Reflection.BindingFlags.Public | global::System.Reflection.BindingFlags.NonPublic | global::System.Reflection.BindingFlags.Static, null, new global::System.Type[] { typeof(global::Foo.Status), typeof(string) }, null), new global::System.Linq.Expressions.Expression[] { expr_7, expr_8 });
25-
var expr_10 = global::System.Linq.Expressions.Expression.Equal(expr_0, expr_7);
26-
var expr_11 = global::System.Linq.Expressions.Expression.Condition(expr_10, expr_9, expr_6, typeof(string));
27-
var expr_12 = global::System.Linq.Expressions.Expression.Constant(global::Foo.Status.Pending, typeof(global::Foo.Status));
28-
var expr_13 = global::System.Linq.Expressions.Expression.Constant("Status: ", typeof(string)); // "Status: "
29-
var expr_14 = global::System.Linq.Expressions.Expression.Call(typeof(global::Foo.EnumExtensions).GetMethod("GetDisplayNameWithPrefix", global::System.Reflection.BindingFlags.Public | global::System.Reflection.BindingFlags.NonPublic | global::System.Reflection.BindingFlags.Static, null, new global::System.Type[] { typeof(global::Foo.Status), typeof(string) }, null), new global::System.Linq.Expressions.Expression[] { expr_12, expr_13 });
30-
var expr_15 = global::System.Linq.Expressions.Expression.Equal(expr_0, expr_12);
31-
var expr_16 = global::System.Linq.Expressions.Expression.Condition(expr_15, expr_14, expr_11, typeof(string));
32-
return global::System.Linq.Expressions.Expression.Lambda<global::System.Func<global::Foo.Entity, string>>(expr_16, p__this);
23+
var expr_8 = global::System.Linq.Expressions.Expression.Call(typeof(global::Foo.EnumExtensions).GetMethod("GetDisplayNameWithPrefix", global::System.Reflection.BindingFlags.Public | global::System.Reflection.BindingFlags.NonPublic | global::System.Reflection.BindingFlags.Static, null, new global::System.Type[] { typeof(global::Foo.Status), typeof(string) }, null), new global::System.Linq.Expressions.Expression[] { expr_7, expr_2 });
24+
var expr_9 = global::System.Linq.Expressions.Expression.Equal(expr_0, expr_7);
25+
var expr_10 = global::System.Linq.Expressions.Expression.Condition(expr_9, expr_8, expr_6, typeof(string));
26+
var expr_11 = global::System.Linq.Expressions.Expression.Constant(global::Foo.Status.Pending, typeof(global::Foo.Status));
27+
var expr_12 = global::System.Linq.Expressions.Expression.Call(typeof(global::Foo.EnumExtensions).GetMethod("GetDisplayNameWithPrefix", global::System.Reflection.BindingFlags.Public | global::System.Reflection.BindingFlags.NonPublic | global::System.Reflection.BindingFlags.Static, null, new global::System.Type[] { typeof(global::Foo.Status), typeof(string) }, null), new global::System.Linq.Expressions.Expression[] { expr_11, expr_2 });
28+
var expr_13 = global::System.Linq.Expressions.Expression.Equal(expr_0, expr_11);
29+
var expr_14 = global::System.Linq.Expressions.Expression.Condition(expr_13, expr_12, expr_10, typeof(string));
30+
return global::System.Linq.Expressions.Expression.Lambda<global::System.Func<global::Foo.Entity, string>>(expr_14, p__this);
3331
}
3432
}
3533
}

‎tests/ExpressiveSharp.Generator.Tests/ExpressiveGenerator/EnumTests.ExpandExtensionMethodOnNullableEnum.verified.txt‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,10 @@ namespace ExpressiveSharp.Generated
3030
var expr_15 = global::System.Linq.Expressions.Expression.Equal(expr_0, expr_13);
3131
var expr_16 = global::System.Linq.Expressions.Expression.Condition(expr_15, expr_14, expr_11, typeof(string));
3232
var expr_17 = global::System.Linq.Expressions.Expression.Constant(null, typeof(global::Foo.Trend?));
33-
var expr_18 = global::System.Linq.Expressions.Expression.Equal(expr_0, expr_17);
34-
var expr_19 = global::System.Linq.Expressions.Expression.Condition(expr_18, expr_1, expr_16, typeof(string));
35-
return global::System.Linq.Expressions.Expression.Lambda<global::System.Func<global::Foo.Entity, string>>(expr_19, p__this);
33+
var expr_18 = global::System.Linq.Expressions.Expression.Call(typeof(global::Foo.TrendExtensions).GetMethod("Describe", global::System.Reflection.BindingFlags.Public | global::System.Reflection.BindingFlags.NonPublic | global::System.Reflection.BindingFlags.Static, null, new global::System.Type[] { typeof(global::Foo.Trend?) }, null), new global::System.Linq.Expressions.Expression[] { expr_17 });
34+
var expr_19 = global::System.Linq.Expressions.Expression.Equal(expr_0, expr_17);
35+
var expr_20 = global::System.Linq.Expressions.Expression.Condition(expr_19, expr_18, expr_16, typeof(string));
36+
return global::System.Linq.Expressions.Expression.Lambda<global::System.Func<global::Foo.Entity, string>>(expr_20, p__this);
3637
}
3738
}
3839
}

‎tests/ExpressiveSharp.Generator.Tests/ExpressiveGenerator/EnumTests.ExpandToStringOnNullableEnum.verified.txt‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,11 @@ namespace ExpressiveSharp.Generated
3030
var expr_16 = global::System.Linq.Expressions.Expression.Equal(expr_1, expr_14);
3131
var expr_17 = global::System.Linq.Expressions.Expression.Condition(expr_16, expr_15, expr_12, typeof(string));
3232
var expr_18 = global::System.Linq.Expressions.Expression.Constant(null, typeof(global::Foo.Trend?));
33-
var expr_19 = global::System.Linq.Expressions.Expression.Equal(expr_1, expr_18);
34-
var expr_20 = global::System.Linq.Expressions.Expression.Condition(expr_19, expr_2, expr_17, typeof(string));
35-
var expr_21 = global::System.Linq.Expressions.Expression.Field(null, typeof(string).GetField("Empty", global::System.Reflection.BindingFlags.Public | global::System.Reflection.BindingFlags.NonPublic | global::System.Reflection.BindingFlags.Static)); // string.Empty
36-
var expr_0 = global::System.Linq.Expressions.Expression.Coalesce(expr_20, expr_21);
33+
var expr_19 = global::System.Linq.Expressions.Expression.Call(expr_18, typeof(global::Foo.Trend?).GetMethod("ToString", global::System.Reflection.BindingFlags.Public | global::System.Reflection.BindingFlags.NonPublic | global::System.Reflection.BindingFlags.Instance, null, new global::System.Type[] { }, null), global::System.Array.Empty<global::System.Linq.Expressions.Expression>());
34+
var expr_20 = global::System.Linq.Expressions.Expression.Equal(expr_1, expr_18);
35+
var expr_21 = global::System.Linq.Expressions.Expression.Condition(expr_20, expr_19, expr_17, typeof(string));
36+
var expr_22 = global::System.Linq.Expressions.Expression.Field(null, typeof(string).GetField("Empty", global::System.Reflection.BindingFlags.Public | global::System.Reflection.BindingFlags.NonPublic | global::System.Reflection.BindingFlags.Static)); // string.Empty
37+
var expr_0 = global::System.Linq.Expressions.Expression.Coalesce(expr_21, expr_22);
3738
return global::System.Linq.Expressions.Expression.Lambda<global::System.Func<global::Foo.Entity, string>>(expr_0, p__this);
3839
}
3940
}

‎tests/ExpressiveSharp.IntegrationTests/Tests/EnumComparisonTests.cs‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,8 +87,33 @@ public void ToStringOnNullableEnum_ExpandExpressives_MaterializesAndEvaluates()
8787

8888
var labels = source.Select(expanded.Compile()).ToList();
8989

90+
// The null branch yields Nullable<Bucket>.ToString() which the runtime defines as "" for null.
9091
CollectionAssert.AreEqual(new[] { "Low", "Mid", "High", "" }, labels);
9192
}
93+
94+
[TestMethod]
95+
public void ExtensionOnNullableEnum_NullBranch_PreservesUserDefinedFallback()
96+
{
97+
var source = new List<EnumComparisonEntity>
98+
{
99+
new() { NullableValue = Bucket.Low },
100+
new() { NullableValue = null },
101+
}.AsQueryable();
102+
103+
Expression<Func<EnumComparisonEntity, string>> expr = e => e.NullableDescription;
104+
var expanded = (Expression<Func<EnumComparisonEntity, string>>)expr.ExpandExpressives();
105+
106+
var labels = source.Select(expanded.Compile()).ToList();
107+
108+
// The null branch must invoke Describe(null) so the extension's own null handling wins.
109+
CollectionAssert.AreEqual(new[] { "Low", "n/a" }, labels);
110+
}
111+
}
112+
113+
public static class BucketExtensions
114+
{
115+
public static string Describe(this Bucket? value) =>
116+
value.HasValue ? value.Value.ToString() : "n/a";
92117
}
93118

94119
public class EnumComparisonEntity
@@ -110,6 +135,9 @@ public class EnumComparisonEntity
110135

111136
[Expressive]
112137
public string NullableLabel => NullableValue.ToString() ?? string.Empty;
138+
139+
[Expressive]
140+
public string NullableDescription => NullableValue.Describe();
113141
}
114142

115143
public enum Bucket { Low, Mid, High }

0 commit comments

Comments
 (0)