Skip to content

Commit e58e1e0

Browse files
authored
Do not generate IIF(...) when np(...) is used for a single expression (#286)
* Do not generate IIF(...) when np(...) is used for a single expression * .
1 parent 5f8c65c commit e58e1e0

File tree

5 files changed

+154
-46
lines changed

5 files changed

+154
-46
lines changed

src/System.Linq.Dynamic.Core/Parser/ExpressionHelper.cs

+15-11
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
1-
using JetBrains.Annotations;
2-
using System.Collections.Generic;
1+
using System.Collections.Generic;
32
using System.Globalization;
43
using System.Linq.Dynamic.Core.Validation;
54
using System.Linq.Expressions;
65
using System.Reflection;
6+
using JetBrains.Annotations;
77

88
namespace System.Linq.Dynamic.Core.Parser
99
{
@@ -244,28 +244,32 @@ private void WrapConstantExpressions(ref Expression left, ref Expression right)
244244
}
245245
}
246246

247-
public Expression GenerateAndAlsoNotNullExpression(Expression sourceExpression)
247+
public bool TryGenerateAndAlsoNotNullExpression(Expression sourceExpression, out Expression generatedExpression)
248248
{
249-
var expresssions = CollectExpressions(sourceExpression);
250-
if (!expresssions.Any())
249+
generatedExpression = sourceExpression;
250+
251+
var expressions = CollectExpressions(sourceExpression);
252+
253+
if (expressions.Count == 1)
251254
{
252-
return null;
255+
generatedExpression = sourceExpression;
256+
return false;
253257
}
254258

255259
// Reverse the list
256-
expresssions.Reverse();
260+
expressions.Reverse();
257261

258262
// Convert all expressions into '!= null'
259-
var binaryExpressions = expresssions.Select(expression => Expression.NotEqual(expression, Expression.Constant(null))).ToArray();
263+
var binaryExpressions = expressions.Select(expression => Expression.NotEqual(expression, Expression.Constant(null))).ToArray();
260264

261265
// Convert all binary expressions into `AndAlso(...)`
262-
var andAlsoExpression = binaryExpressions[0];
266+
generatedExpression = binaryExpressions[0];
263267
for (int i = 1; i < binaryExpressions.Length; i++)
264268
{
265-
andAlsoExpression = Expression.AndAlso(andAlsoExpression, binaryExpressions[i]);
269+
generatedExpression = Expression.AndAlso(generatedExpression, binaryExpressions[i]);
266270
}
267271

268-
return andAlsoExpression;
272+
return true;
269273
}
270274

271275
private static Expression GetMemberExpression(Expression expression)

src/System.Linq.Dynamic.Core/Parser/ExpressionParser.cs

+27-17
Original file line numberDiff line numberDiff line change
@@ -1135,11 +1135,16 @@ Expression ParseFunctionNullPropagation()
11351135

11361136
if (args[0] is MemberExpression memberExpression)
11371137
{
1138-
var expressionTest = _expressionHelper.GenerateAndAlsoNotNullExpression(memberExpression);
1139-
var expressionIfTrue = memberExpression;
1140-
var expressionIfFalse = args.Length == 2 ? args[1] : Expression.Constant(null);
1138+
if (_expressionHelper.TryGenerateAndAlsoNotNullExpression(memberExpression, out Expression generatedExpression))
1139+
{
1140+
var expressionIfTrue = memberExpression;
1141+
var expressionIfFalse = args.Length == 2 ? args[1] : Expression.Constant(null);
1142+
1143+
return GenerateConditional(generatedExpression, expressionIfTrue, expressionIfFalse, errorPos);
1144+
}
11411145

1142-
return GenerateConditional(expressionTest, expressionIfTrue, expressionIfFalse, errorPos);
1146+
// The member expression is a single expression, just return it.
1147+
return memberExpression;
11431148
}
11441149

11451150
throw ParseError(errorPos, Res.NullPropagationRequiresMemberExpression);
@@ -1211,25 +1216,30 @@ Expression GenerateConditional(Expression test, Expression expr1, Expression exp
12111216

12121217
if (expr1.Type != expr2.Type)
12131218
{
1214-
if ((Constants.IsNull(expr1) && expr2.Type.GetTypeInfo().IsValueType) || (Constants.IsNull(expr2) && expr1.Type.GetTypeInfo().IsValueType))
1219+
// If expr1 is a null constant and expr2 is ValueType:
1220+
// - create nullable constant from expr1 with type from expr2
1221+
// - convert expr2 to nullable (unless it's already nullable)
1222+
if (Constants.IsNull(expr1) && expr2.Type.GetTypeInfo().IsValueType)
12151223
{
1216-
// If expr1 is a null constant and expr2 is a IsValueType:
1217-
// - create nullable constant from expr1 with type from expr2
1218-
// - convert expr2 to nullable
1219-
if (Constants.IsNull(expr1) && expr2.Type.GetTypeInfo().IsValueType)
1224+
Type nullableType = TypeHelper.ToNullableType(expr2.Type);
1225+
expr1 = Expression.Constant(null, nullableType);
1226+
if (!TypeHelper.IsNullableType(expr2.Type))
12201227
{
1221-
Type nullableType = TypeHelper.ToNullableType(expr2.Type);
1222-
expr1 = Expression.Constant(null, nullableType);
12231228
expr2 = Expression.Convert(expr2, nullableType);
12241229
}
12251230

1226-
// If expr2 is a null constant and expr1 is a IsValueType:
1227-
// - create nullable constant from expr2 with type from expr1
1228-
// - convert expr1 to nullable
1229-
if (Constants.IsNull(expr2) && expr1.Type.GetTypeInfo().IsValueType)
1231+
return Expression.Condition(test, expr1, expr2);
1232+
}
1233+
1234+
// If expr2 is a null constant and expr1 is a ValueType:
1235+
// - create nullable constant from expr2 with type from expr1
1236+
// - convert expr1 to nullable (unless it's already nullable)
1237+
if (Constants.IsNull(expr2) && expr1.Type.GetTypeInfo().IsValueType)
1238+
{
1239+
Type nullableType = TypeHelper.ToNullableType(expr1.Type);
1240+
expr2 = Expression.Constant(null, nullableType);
1241+
if (!TypeHelper.IsNullableType(expr1.Type))
12301242
{
1231-
Type nullableType = TypeHelper.ToNullableType(expr1.Type);
1232-
expr2 = Expression.Constant(null, nullableType);
12331243
expr1 = Expression.Convert(expr1, nullableType);
12341244
}
12351245

src/System.Linq.Dynamic.Core/Parser/IExpressionHelper.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ internal interface IExpressionHelper
2828

2929
Expression OptimizeStringForEqualityIfPossible(string text, Type type);
3030

31-
Expression GenerateAndAlsoNotNullExpression(Expression sourceExpression);
31+
bool TryGenerateAndAlsoNotNullExpression(Expression sourceExpression, out Expression generatedExpression);
3232

3333
void WrapConstantExpression(ref Expression argument);
3434
}

test/System.Linq.Dynamic.Core.Tests/ExpressionTests.cs

+92-13
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
1-
using Newtonsoft.Json.Linq;
2-
using NFluent;
3-
using System.Collections.Generic;
1+
using System.Collections.Generic;
42
using System.Dynamic;
53
using System.Globalization;
64
using System.Linq.Dynamic.Core.Exceptions;
75
using System.Linq.Dynamic.Core.Tests.Helpers;
86
using System.Linq.Dynamic.Core.Tests.Helpers.Models;
7+
using Newtonsoft.Json.Linq;
8+
using NFluent;
99
using Xunit;
1010

1111
namespace System.Linq.Dynamic.Core.Tests
@@ -37,6 +37,8 @@ public class TestGuidNullClass
3737
{
3838
public Guid? GuidNull { get; set; }
3939

40+
public Guid GuidNormal { get; set; }
41+
4042
public int Id { get; set; }
4143
}
4244

@@ -1304,11 +1306,87 @@ public void ExpressionTests_NullCoalescing()
13041306
Assert.Equal(expectedResult3, result3b.ToDynamicArray<int>());
13051307
}
13061308

1309+
[Theory]
1310+
[InlineData("np(g)", "Select(Param_0 => Param_0.g)")]
1311+
[InlineData("np(gnullable)", "Select(Param_0 => Param_0.gnullable)")]
1312+
[InlineData("np(dt)", "Select(Param_0 => Param_0.dt)")]
1313+
[InlineData("np(dtnullable)", "Select(Param_0 => Param_0.dtnullable)")]
1314+
[InlineData("np(number)", "Select(Param_0 => Param_0.number)")]
1315+
[InlineData("np(nullablenumber)", "Select(Param_0 => Param_0.nullablenumber)")]
1316+
[InlineData("np(_enum)", "Select(Param_0 => Param_0._enum)")]
1317+
[InlineData("np(_enumnullable)", "Select(Param_0 => Param_0._enumnullable)")]
1318+
1319+
#if NET452
1320+
[InlineData("np(nested.g)", "Select(Param_0 => IIF(((Param_0 != null) AndAlso (Param_0.nested != null)), Convert(Param_0.nested.g), null))")]
1321+
[InlineData("np(nested.dt)", "Select(Param_0 => IIF(((Param_0 != null) AndAlso (Param_0.nested != null)), Convert(Param_0.nested.dt), null))")]
1322+
[InlineData("np(nested.number)", "Select(Param_0 => IIF(((Param_0 != null) AndAlso (Param_0.nested != null)), Convert(Param_0.nested.number), null))")]
1323+
[InlineData("np(nested._enum)", "Select(Param_0 => IIF(((Param_0 != null) AndAlso (Param_0.nested != null)), Convert(Param_0.nested._enum), null))")]
1324+
[InlineData("np(item.Id)", "Select(Param_0 => IIF(((Param_0 != null) AndAlso (Param_0.item != null)), Convert(Param_0.item.Id), null))")]
1325+
[InlineData("np(item.GuidNormal)", "Select(Param_0 => IIF(((Param_0 != null) AndAlso (Param_0.item != null)), Convert(Param_0.item.GuidNormal), null))")]
1326+
#else
1327+
[InlineData("np(nested.g)", "Select(Param_0 => IIF(((Param_0 != null) AndAlso (Param_0.nested != null)), Convert(Param_0.nested.g, Nullable`1), null))")]
1328+
[InlineData("np(nested.dt)", "Select(Param_0 => IIF(((Param_0 != null) AndAlso (Param_0.nested != null)), Convert(Param_0.nested.dt, Nullable`1), null))")]
1329+
[InlineData("np(nested.number)", "Select(Param_0 => IIF(((Param_0 != null) AndAlso (Param_0.nested != null)), Convert(Param_0.nested.number, Nullable`1), null))")]
1330+
[InlineData("np(nested._enum)", "Select(Param_0 => IIF(((Param_0 != null) AndAlso (Param_0.nested != null)), Convert(Param_0.nested._enum, Nullable`1), null))")]
1331+
[InlineData("np(item.Id)", "Select(Param_0 => IIF(((Param_0 != null) AndAlso (Param_0.item != null)), Convert(Param_0.item.Id, Nullable`1), null))")]
1332+
[InlineData("np(item.GuidNormal)", "Select(Param_0 => IIF(((Param_0 != null) AndAlso (Param_0.item != null)), Convert(Param_0.item.GuidNormal, Nullable`1), null))")]
1333+
#endif
1334+
1335+
[InlineData("np(nested.gnullable)", "Select(Param_0 => IIF(((Param_0 != null) AndAlso (Param_0.nested != null)), Param_0.nested.gnullable, null))")]
1336+
[InlineData("np(nested.dtnullable)", "Select(Param_0 => IIF(((Param_0 != null) AndAlso (Param_0.nested != null)), Param_0.nested.dtnullable, null))")]
1337+
[InlineData("np(nested.nullablenumber)", "Select(Param_0 => IIF(((Param_0 != null) AndAlso (Param_0.nested != null)), Param_0.nested.nullablenumber, null))")]
1338+
[InlineData("np(nested._enumnullable)", "Select(Param_0 => IIF(((Param_0 != null) AndAlso (Param_0.nested != null)), Param_0.nested._enumnullable, null))")]
1339+
[InlineData("np(item.GuidNull)", "Select(Param_0 => IIF(((Param_0 != null) AndAlso (Param_0.item != null)), Param_0.item.GuidNull, null))")]
1340+
public void ExpressionTests_NullPropagating(string test, string query)
1341+
{
1342+
// Arrange
1343+
var q = new[]
1344+
{
1345+
new
1346+
{
1347+
g = Guid.NewGuid(),
1348+
gnullable = (Guid?) Guid.NewGuid(),
1349+
dt = DateTime.Now,
1350+
dtnullable = (DateTime?) DateTime.Now,
1351+
_enum = TestEnum2.Var1,
1352+
_enumnullable = (TestEnum2?) TestEnum2.Var2,
1353+
number = 1,
1354+
nullablenumber = (int?) 2,
1355+
nested = new
1356+
{
1357+
g = Guid.NewGuid(),
1358+
gnullable = (Guid?) Guid.NewGuid(),
1359+
dt = DateTime.Now,
1360+
dtnullable = (DateTime?) DateTime.Now,
1361+
_enum = TestEnum2.Var1,
1362+
_enumnullable = (TestEnum2?) TestEnum2.Var2,
1363+
number = 1,
1364+
nullablenumber = (int?) 2,
1365+
},
1366+
item = new TestGuidNullClass
1367+
{
1368+
Id = 100,
1369+
GuidNormal = Guid.NewGuid(),
1370+
GuidNull = Guid.NewGuid()
1371+
}
1372+
}
1373+
}.AsQueryable();
1374+
1375+
// Act
1376+
var resultDynamic = q.Select(test);
1377+
1378+
// Assert
1379+
string queryAsString = resultDynamic.ToString();
1380+
queryAsString = queryAsString.Substring(queryAsString.IndexOf(".Select") + 1).TrimEnd(']');
1381+
Check.That(queryAsString).Equals(query);
1382+
}
1383+
13071384
[Fact]
1308-
public void ExpressionTests_NullPropagating_DateTime_Value()
1385+
public void ExpressionTests_NullPropagating_DateTime()
13091386
{
13101387
// Arrange
1311-
var q = new[] {
1388+
var q = new[]
1389+
{
13121390
new { id = 1, date1 = (DateTime?) DateTime.Now, date2 = DateTime.Now.AddDays(-1)}
13131391
}.AsQueryable();
13141392

@@ -1324,7 +1402,8 @@ public void ExpressionTests_NullPropagating_DateTime_Value()
13241402
public void ExpressionTests_NullPropagating_NullableDateTime()
13251403
{
13261404
// Arrange
1327-
var q = new[] {
1405+
var q = new[]
1406+
{
13281407
new { id = 1, date1 = (DateTime?) DateTime.Now, date2 = DateTime.Now.AddDays(-1)}
13291408
}.AsQueryable();
13301409

@@ -1337,7 +1416,7 @@ public void ExpressionTests_NullPropagating_NullableDateTime()
13371416
}
13381417

13391418
[Fact]
1340-
public void ExpressionTests_NullPropagating_Integer_Null()
1419+
public void ExpressionTests_NullPropagating_WithoutDefaultValue()
13411420
{
13421421
// Arrange
13431422
var testModels = User.GenerateSampleModels(2, true).ToList();
@@ -1353,7 +1432,7 @@ public void ExpressionTests_NullPropagating_Integer_Null()
13531432
}
13541433

13551434
[Fact]
1356-
public void ExpressionTests_NullPropagating_Integer_Value()
1435+
public void ExpressionTests_NullPropagating_WithDefaultValue()
13571436
{
13581437
// Arrange
13591438
var testModels = User.GenerateSampleModels(2, true).ToList();
@@ -1756,24 +1835,24 @@ public void ExpressionTests_Type_StructWithIntegerEquality_BothVariablesInStruct
17561835
{
17571836
new
17581837
{
1759-
Id = new SnowflakeId(1L),
1838+
Id = new SnowflakeId(1L),
17601839
Var = 1
17611840
},
17621841
new
17631842
{
1764-
Id = new SnowflakeId(2L),
1843+
Id = new SnowflakeId(2L),
17651844
Var = 1
17661845
},
17671846
new
17681847
{
1769-
Id = new SnowflakeId(1L),
1848+
Id = new SnowflakeId(1L),
17701849
Var = 2
17711850
}
17721851
}.AsQueryable();
17731852

1774-
var resultValuesL = new[] {
1853+
var resultValuesL = new[] {
17751854
new {
1776-
Id = new SnowflakeId(1L),
1855+
Id = new SnowflakeId(1L),
17771856
Var = 1
17781857
}
17791858
}.AsQueryable().ToArray();

test/System.Linq.Dynamic.Core.Tests/Parser/ExpressionHelperTests.cs

+19-4
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ namespace System.Linq.Dynamic.Core.Tests.Parser
77
{
88
public class ExpressionHelperTests
99
{
10-
private readonly IExpressionHelper _expressionHelper;
10+
private readonly ExpressionHelper _expressionHelper;
1111

1212
public ExpressionHelperTests()
1313
{
@@ -85,16 +85,31 @@ public void ExpressionHelper_OptimizeStringForEqualityIfPossible_Guid_Invalid()
8585
}
8686

8787
[Fact]
88-
public void ExpressionHelper_GenerateAndAlsoNotNullExpression()
88+
public void ExpressionHelper_TryGenerateAndAlsoNotNullExpression_ForMultiple()
8989
{
9090
// Assign
9191
Expression<Func<Item, int>> expression = (x) => x.Relation1.Relation2.Id;
9292

9393
// Act
94-
Expression result = _expressionHelper.GenerateAndAlsoNotNullExpression(expression);
94+
bool result = _expressionHelper.TryGenerateAndAlsoNotNullExpression(expression, out Expression generatedExpresion);
9595

9696
// Assert
97-
Check.That(result.ToString()).IsEqualTo("(((x != null) AndAlso (x.Relation1 != null)) AndAlso (x.Relation1.Relation2 != null))");
97+
Check.That(result).IsTrue();
98+
Check.That(generatedExpresion.ToString()).IsEqualTo("(((x != null) AndAlso (x.Relation1 != null)) AndAlso (x.Relation1.Relation2 != null))");
99+
}
100+
101+
[Fact]
102+
public void ExpressionHelper_TryGenerateAndAlsoNotNullExpression_ForSingle()
103+
{
104+
// Assign
105+
Expression<Func<Item, int>> expression = (x) => x.Id;
106+
107+
// Act
108+
bool result = _expressionHelper.TryGenerateAndAlsoNotNullExpression(expression, out Expression generatedExpresion);
109+
110+
// Assert
111+
Check.That(result).IsFalse();
112+
Check.That(generatedExpresion.ToString()).IsEqualTo("x => x.Id");
98113
}
99114

100115
class Item

0 commit comments

Comments
 (0)