Skip to content

Commit 822d797

Browse files
StefHStefH
authored
Fix logic for np(...) : always add source object and use configurable default value for non-nullable value-types (#522)
* Add more tests * fixes * NullPropagatingUseDefaultValueForNonNullableValueTypes * CollectExpressions * CollectExpressions
1 parent 2aee0be commit 822d797

File tree

7 files changed

+288
-52
lines changed

7 files changed

+288
-52
lines changed

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

+18-4
Original file line numberDiff line numberDiff line change
@@ -296,11 +296,20 @@ public bool ExpressionQualifiesForNullPropagation(Expression expression)
296296
{
297297
return
298298
expression is MemberExpression ||
299-
expression is ParameterExpression ||
299+
expression is ParameterExpression ||
300300
expression is MethodCallExpression ||
301301
expression is UnaryExpression;
302302
}
303303

304+
public Expression GenerateDefaultExpression(Type type)
305+
{
306+
#if NET35
307+
return Expression.Constant(Activator.CreateInstance(type));
308+
#else
309+
return Expression.Default(type);
310+
#endif
311+
}
312+
304313
private Expression GetMemberExpression(Expression expression)
305314
{
306315
if (ExpressionQualifiesForNullPropagation(expression))
@@ -330,11 +339,16 @@ private List<Expression> CollectExpressions(bool addSelf, Expression sourceExpre
330339

331340
var list = new List<Expression>();
332341

333-
if (addSelf && expression is MemberExpression memberExpressionFirst)
342+
if (addSelf)
334343
{
335-
if (TypeHelper.IsNullableType(memberExpressionFirst.Type) || !memberExpressionFirst.Type.GetTypeInfo().IsValueType)
344+
switch (expression)
336345
{
337-
list.Add(sourceExpression);
346+
case MemberExpression _:
347+
list.Add(sourceExpression);
348+
break;
349+
350+
default:
351+
break;
338352
}
339353
}
340354

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

+58-17
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,7 @@ Expression ParseConditionalOperator()
219219
_textParser.ValidateToken(TokenId.Colon, Res.ColonExpected);
220220
_textParser.NextToken();
221221
Expression expr2 = ParseConditionalOperator();
222-
expr = GenerateConditional(expr, expr1, expr2, errorPos);
222+
expr = GenerateConditional(expr, expr1, expr2, false, errorPos);
223223
}
224224
return expr;
225225
}
@@ -1145,7 +1145,7 @@ Expression ParseFunctionIif()
11451145
throw ParseError(errorPos, Res.IifRequiresThreeArgs);
11461146
}
11471147

1148-
return GenerateConditional(args[0], args[1], args[2], errorPos);
1148+
return GenerateConditional(args[0], args[1], args[2], false, errorPos);
11491149
}
11501150

11511151
// np(...) function
@@ -1166,9 +1166,9 @@ Expression ParseFunctionNullPropagation()
11661166
bool hasDefaultParameter = args.Length == 2;
11671167
Expression expressionIfFalse = hasDefaultParameter ? args[1] : Expression.Constant(null);
11681168

1169-
if (_expressionHelper.TryGenerateAndAlsoNotNullExpression(args[0], hasDefaultParameter, out Expression generatedExpression))
1169+
if (_expressionHelper.TryGenerateAndAlsoNotNullExpression(args[0], true, out Expression generatedExpression))
11701170
{
1171-
return GenerateConditional(generatedExpression, args[0], expressionIfFalse, errorPos);
1171+
return GenerateConditional(generatedExpression, args[0], expressionIfFalse, true, errorPos);
11721172
}
11731173

11741174
return args[0];
@@ -1234,7 +1234,7 @@ Expression ParseFunctionCast()
12341234
return Expression.ConvertChecked(_it, resolvedType);
12351235
}
12361236

1237-
Expression GenerateConditional(Expression test, Expression expressionIfTrue, Expression expressionIfFalse, int errorPos)
1237+
Expression GenerateConditional(Expression test, Expression expressionIfTrue, Expression expressionIfFalse, bool nullPropagating, int errorPos)
12381238
{
12391239
if (test.Type != typeof(bool))
12401240
{
@@ -1244,30 +1244,71 @@ Expression GenerateConditional(Expression test, Expression expressionIfTrue, Exp
12441244
if (expressionIfTrue.Type != expressionIfFalse.Type)
12451245
{
12461246
// If expressionIfTrue is a null constant and expressionIfFalse is ValueType:
1247-
// - create nullable constant from expressionIfTrue with type from expressionIfFalse
1248-
// - convert expressionIfFalse to nullable (unless it's already nullable)
12491247
if (Constants.IsNull(expressionIfTrue) && expressionIfFalse.Type.GetTypeInfo().IsValueType)
12501248
{
1251-
Type nullableType = TypeHelper.ToNullableType(expressionIfFalse.Type);
1252-
expressionIfTrue = Expression.Constant(null, nullableType);
1253-
if (!TypeHelper.IsNullableType(expressionIfFalse.Type))
1249+
if (nullPropagating && _parsingConfig.NullPropagatingUseDefaultValueForNonNullableValueTypes)
12541250
{
1255-
expressionIfFalse = Expression.Convert(expressionIfFalse, nullableType);
1251+
// If expressionIfFalse is a non-nullable type:
1252+
// generate default expression from the expressionIfFalse-type for expressionIfTrue
1253+
// Else
1254+
// create nullable constant from expressionIfTrue with type from expressionIfFalse
1255+
1256+
if (!TypeHelper.IsNullableType(expressionIfFalse.Type))
1257+
{
1258+
expressionIfTrue = _expressionHelper.GenerateDefaultExpression(expressionIfFalse.Type);
1259+
}
1260+
else
1261+
{
1262+
expressionIfTrue = Expression.Constant(null, expressionIfFalse.Type);
1263+
}
1264+
}
1265+
else
1266+
{
1267+
// - create nullable constant from expressionIfTrue with type from expressionIfFalse
1268+
// - convert expressionIfFalse to nullable (unless it's already nullable)
1269+
1270+
Type nullableType = TypeHelper.ToNullableType(expressionIfFalse.Type);
1271+
expressionIfTrue = Expression.Constant(null, nullableType);
1272+
1273+
if (!TypeHelper.IsNullableType(expressionIfFalse.Type))
1274+
{
1275+
expressionIfFalse = Expression.Convert(expressionIfFalse, nullableType);
1276+
}
12561277
}
12571278

12581279
return Expression.Condition(test, expressionIfTrue, expressionIfFalse);
12591280
}
12601281

12611282
// If expressionIfFalse is a null constant and expressionIfTrue is a ValueType:
1262-
// - create nullable constant from expressionIfFalse with type from expressionIfTrue
1263-
// - convert expressionIfTrue to nullable (unless it's already nullable)
12641283
if (Constants.IsNull(expressionIfFalse) && expressionIfTrue.Type.GetTypeInfo().IsValueType)
12651284
{
1266-
Type nullableType = TypeHelper.ToNullableType(expressionIfTrue.Type);
1267-
expressionIfFalse = Expression.Constant(null, nullableType);
1268-
if (!TypeHelper.IsNullableType(expressionIfTrue.Type))
1285+
if (nullPropagating && _parsingConfig.NullPropagatingUseDefaultValueForNonNullableValueTypes)
12691286
{
1270-
expressionIfTrue = Expression.Convert(expressionIfTrue, nullableType);
1287+
// If expressionIfTrue is a non-nullable type:
1288+
// generate default expression from the expressionIfFalse-type for expressionIfFalse
1289+
// Else
1290+
// create nullable constant from expressionIfFalse with type from expressionIfTrue
1291+
1292+
if (!TypeHelper.IsNullableType(expressionIfTrue.Type))
1293+
{
1294+
expressionIfFalse = _expressionHelper.GenerateDefaultExpression(expressionIfTrue.Type);
1295+
}
1296+
else
1297+
{
1298+
expressionIfFalse = Expression.Constant(null, expressionIfTrue.Type);
1299+
}
1300+
}
1301+
else
1302+
{
1303+
// - create nullable constant from expressionIfFalse with type from expressionIfTrue
1304+
// - convert expressionIfTrue to nullable (unless it's already nullable)
1305+
1306+
Type nullableType = TypeHelper.ToNullableType(expressionIfTrue.Type);
1307+
expressionIfFalse = Expression.Constant(null, nullableType);
1308+
if (!TypeHelper.IsNullableType(expressionIfTrue.Type))
1309+
{
1310+
expressionIfTrue = Expression.Convert(expressionIfTrue, nullableType);
1311+
}
12711312
}
12721313

12731314
return Expression.Condition(test, expressionIfTrue, expressionIfFalse);

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

+2
Original file line numberDiff line numberDiff line change
@@ -37,5 +37,7 @@ internal interface IExpressionHelper
3737
bool MemberExpressionIsDynamic(Expression expression);
3838

3939
Expression ConvertToExpandoObjectAndCreateDynamicExpression(Expression expression, Type type, string propertyName);
40+
41+
Expression GenerateDefaultExpression(Type type);
4042
}
4143
}

‎src/System.Linq.Dynamic.Core/ParsingConfig.cs

+7
Original file line numberDiff line numberDiff line change
@@ -191,5 +191,12 @@ public IQueryableAnalyzer QueryableAnalyzer
191191
/// Additional TypeConverters
192192
/// </summary>
193193
public IDictionary<Type, TypeConverter> TypeConverters { get; set; }
194+
195+
/// <summary>
196+
/// When using the NullPropagating function np(...), use a "default value" for non-nullable value types instead of "null value".
197+
///
198+
/// Default value is false.
199+
/// </summary>
200+
public bool NullPropagatingUseDefaultValueForNonNullableValueTypes { get; set; } = false;
194201
}
195202
}

‎test/System.Linq.Dynamic.Core.Tests/DynamicExpressionParserTests.cs

+7-4
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
using System.Collections.Generic;
2+
using System.Globalization;
23
using System.Linq.Dynamic.Core.CustomTypeProviders;
34
using System.Linq.Dynamic.Core.Exceptions;
45
using System.Linq.Dynamic.Core.Tests.Helpers.Models;
@@ -327,9 +328,11 @@ public void DynamicExpressionParser_ParseLambda_UseParameterizedNamesInDynamicQu
327328
public void DynamicExpressionParser_ParseLambda_UseParameterizedNamesInDynamicQuery_ForNullableProperty_true(string propName, string valueString)
328329
{
329330
// Assign
331+
var culture = CultureInfo.CreateSpecificCulture("en-US");
330332
var config = new ParsingConfig
331333
{
332-
UseParameterizedNamesInDynamicQuery = true
334+
UseParameterizedNamesInDynamicQuery = true,
335+
NumberParseCulture = culture
333336
};
334337

335338
// Act
@@ -348,7 +351,7 @@ public void DynamicExpressionParser_ParseLambda_UseParameterizedNamesInDynamicQu
348351
var propertyInfo = wrapperObj.GetType().GetProperty("Value", BindingFlags.Instance | BindingFlags.Public);
349352
object value = propertyInfo.GetValue(wrapperObj);
350353

351-
Check.That(value).IsEqualTo(Convert.ChangeType(valueString, Nullable.GetUnderlyingType(queriedPropType) ?? queriedPropType));
354+
value.Should().Be(Convert.ChangeType(valueString, Nullable.GetUnderlyingType(queriedPropType) ?? queriedPropType, culture));
352355
}
353356

354357
[Theory]
@@ -1070,7 +1073,7 @@ public void DynamicExpressionParser_ParseLambda_Operator_Less_Greater_With_Guids
10701073

10711074
// Assert
10721075
Assert.Equal(anotherId, result);
1073-
}
1076+
}
10741077

10751078
[Theory]
10761079
[InlineData("c => c.Age == 8", "c => (c.Age == 8)")]
@@ -1239,7 +1242,7 @@ public void DynamicExpressionParser_ParseLambda_String_TrimEnd_0_Parameters()
12391242

12401243
var @delegate = expression.Compile();
12411244

1242-
var result = (bool) @delegate.DynamicInvoke("This is a test ");
1245+
var result = (bool)@delegate.DynamicInvoke("This is a test ");
12431246

12441247
// Assert
12451248
result.Should().BeTrue();

0 commit comments

Comments
 (0)