fix (#342)

StefH · web-flow · commit ea887692d003 · 2020-01-15T19:58:57.000+01:00
diff --git a/src/Microsoft.EntityFrameworkCore.DynamicLinq/Microsoft.EntityFrameworkCore.DynamicLinq.csproj b/src/Microsoft.EntityFrameworkCore.DynamicLinq/Microsoft.EntityFrameworkCore.DynamicLinq.csproj
@@ -45,7 +45,7 @@
         <CopyLocalLockFileAssemblies>false</CopyLocalLockFileAssemblies>
         <NugetTargetMoniker>UAP,Version=v10.0</NugetTargetMoniker>
         <TargetPlatformIdentifier>UAP</TargetPlatformIdentifier>
-        <TargetPlatformVersion>10.0.14393.0</TargetPlatformVersion>
+        <TargetPlatformVersion>10.0.18362.0</TargetPlatformVersion>
         <TargetPlatformMinVersion>10.0.10240.0</TargetPlatformMinVersion>
         <TargetFrameworkIdentifier>.NETCore</TargetFrameworkIdentifier>
         <TargetFrameworkVersion>v5.0</TargetFrameworkVersion>
@@ -104,4 +104,4 @@
         <PackageReference Include="Microsoft.NETCore.UniversalWindowsPlatform" Version="6.0.10" />
         <PackageReference Include="Microsoft.EntityFrameworkCore" Version="1.1.0" />
     </ItemGroup>
-</Project>
+</Project>
diff --git a/src/System.Linq.Dynamic.Core/Parser/ExpressionHelper.cs b/src/System.Linq.Dynamic.Core/Parser/ExpressionHelper.cs
@@ -244,11 +244,9 @@ private void WrapConstantExpressions(ref Expression left, ref Expression right)
             }
         }
 
-        public bool TryGenerateAndAlsoNotNullExpression(Expression sourceExpression, out Expression generatedExpression)
+        public bool TryGenerateAndAlsoNotNullExpression(Expression sourceExpression, bool addSelf, out Expression generatedExpression)
         {
-            generatedExpression = sourceExpression;
-
-            var expressions = CollectExpressions(sourceExpression);
+            var expressions = CollectExpressions(addSelf, sourceExpression);
 
             if (expressions.Count == 1 && !(expressions[0] is MethodCallExpression))
             {
@@ -305,11 +303,20 @@ private static Expression GetMemberExpression(Expression expression)
             return null;
         }
 
-        private static List<Expression> CollectExpressions(Expression sourceExpression)
+        private static List<Expression> CollectExpressions(bool addSelf, Expression sourceExpression)
         {
-            var list = new List<Expression>();
             Expression expression = GetMemberExpression(sourceExpression);
 
+            var list = new List<Expression>();
+
+            if (addSelf && expression is MemberExpression memberExpressionFirst)
+            {
+                if (TypeHelper.IsNullableType(memberExpressionFirst.Type) || !memberExpressionFirst.Type.GetTypeInfo().IsValueType)
+                {
+                    list.Add(sourceExpression);
+                }
+            }
+
             while (expression is MemberExpression memberExpression)
             {
                 expression = GetMemberExpression(memberExpression.Expression);
diff --git a/src/System.Linq.Dynamic.Core/Parser/ExpressionParser.cs b/src/System.Linq.Dynamic.Core/Parser/ExpressionParser.cs
@@ -1138,26 +1138,11 @@ Expression ParseFunctionNullPropagation()
                 bool hasDefaultParameter = args.Length == 2;
                 Expression expressionIfFalse = hasDefaultParameter ? args[1] : Expression.Constant(null);
 
-                if (_expressionHelper.TryGenerateAndAlsoNotNullExpression(memberExpression, out Expression generatedExpression))
+                if (_expressionHelper.TryGenerateAndAlsoNotNullExpression(memberExpression, hasDefaultParameter, out Expression generatedExpression))
                 {
                     return GenerateConditional(generatedExpression, memberExpression, expressionIfFalse, errorPos);
                 }
 
-                if (!hasDefaultParameter)
-                {
-                    // If no default parameter has been supplied and the member expression is a single expression, just return it.
-                    return memberExpression;
-                }
-
-                bool canBeNullableType = TypeHelper.IsNullableType(memberExpression.Type) || !memberExpression.Type.GetTypeInfo().IsPrimitive;
-                if (canBeNullableType)
-                {
-                    // For nullable objects, generate 'x != null ? x : null'
-                    var notEqualToNull = _expressionHelper.GenerateNotEqual(memberExpression, Expression.Constant(null));
-                    return GenerateConditional(notEqualToNull, memberExpression, expressionIfFalse, errorPos);
-                }
-
-                // For non-nullable with default, just ignore default and return the single expression.
                 return memberExpression;
             }
 
diff --git a/src/System.Linq.Dynamic.Core/Parser/IExpressionHelper.cs b/src/System.Linq.Dynamic.Core/Parser/IExpressionHelper.cs
@@ -28,7 +28,7 @@ internal interface IExpressionHelper
 
         Expression OptimizeStringForEqualityIfPossible(string text, Type type);
 
-        bool TryGenerateAndAlsoNotNullExpression(Expression sourceExpression, out Expression generatedExpression);
+        bool TryGenerateAndAlsoNotNullExpression(Expression sourceExpression, bool addSelf, out Expression generatedExpression);
 
         void WrapConstantExpression(ref Expression argument);
     }
diff --git a/src/System.Linq.Dynamic.Core/System.Linq.Dynamic.Core.csproj b/src/System.Linq.Dynamic.Core/System.Linq.Dynamic.Core.csproj
@@ -42,7 +42,7 @@
         <CopyLocalLockFileAssemblies>false</CopyLocalLockFileAssemblies>
         <NugetTargetMoniker>UAP,Version=v10.0</NugetTargetMoniker>
         <TargetPlatformIdentifier>UAP</TargetPlatformIdentifier>
-        <TargetPlatformVersion>10.0.14393.0</TargetPlatformVersion>
+        <TargetPlatformVersion>10.0.18362.0</TargetPlatformVersion>
         <TargetPlatformMinVersion>10.0.10240.0</TargetPlatformMinVersion>
         <TargetFrameworkIdentifier>.NETCore</TargetFrameworkIdentifier>
         <TargetFrameworkVersion>v5.0</TargetFrameworkVersion>
diff --git a/test/System.Linq.Dynamic.Core.Tests/ExpressionTests.cs b/test/System.Linq.Dynamic.Core.Tests/ExpressionTests.cs
@@ -1307,6 +1307,9 @@ public void ExpressionTests_NullCoalescing()
         }
 
         [Theory]
+        [InlineData("np(str)", "Select(Param_0 => Param_0.str)")]
+        [InlineData("np(strNull)", "Select(Param_0 => Param_0.strNull)")]
+        [InlineData("np(str, \"x\")", "Select(Param_0 => IIF(((Param_0 != null) AndAlso (Param_0.str != null)), Param_0.str, \"x\"))")]
         [InlineData("np(g)", "Select(Param_0 => Param_0.g)")]
         [InlineData("np(gnullable)", "Select(Param_0 => Param_0.gnullable)")]
         [InlineData("np(dt)", "Select(Param_0 => Param_0.dt)")]
@@ -1332,9 +1335,12 @@ public void ExpressionTests_NullCoalescing()
         [InlineData("np(item.GuidNormal)", "Select(Param_0 => IIF(((Param_0 != null) AndAlso (Param_0.item != null)), Convert(Param_0.item.GuidNormal, Nullable`1), null))")]
 #endif
 
+        [InlineData("np(nested.strNull)", "Select(Param_0 => IIF(((Param_0 != null) AndAlso (Param_0.nested != null)), Param_0.nested.strNull, null))")]
+        [InlineData("np(nested.strNull, \"x\")", "Select(Param_0 => IIF((((Param_0 != null) AndAlso (Param_0.nested != null)) AndAlso (Param_0.nested.strNull != null)), Param_0.nested.strNull, \"x\"))")]
         [InlineData("np(nested.gnullable)", "Select(Param_0 => IIF(((Param_0 != null) AndAlso (Param_0.nested != null)), Param_0.nested.gnullable, null))")]
         [InlineData("np(nested.dtnullable)", "Select(Param_0 => IIF(((Param_0 != null) AndAlso (Param_0.nested != null)), Param_0.nested.dtnullable, null))")]
         [InlineData("np(nested.nullablenumber)", "Select(Param_0 => IIF(((Param_0 != null) AndAlso (Param_0.nested != null)), Param_0.nested.nullablenumber, null))")]
+        [InlineData("np(nested.nullablenumber, 42)", "Select(Param_0 => IIF((((Param_0 != null) AndAlso (Param_0.nested != null)) AndAlso (Param_0.nested.nullablenumber != null)), Param_0.nested.nullablenumber, 42))")]
         [InlineData("np(nested._enumnullable)", "Select(Param_0 => IIF(((Param_0 != null) AndAlso (Param_0.nested != null)), Param_0.nested._enumnullable, null))")]
         [InlineData("np(item.GuidNull)", "Select(Param_0 => IIF(((Param_0 != null) AndAlso (Param_0.item != null)), Param_0.item.GuidNull, null))")]
         public void ExpressionTests_NullPropagating(string test, string query)
@@ -1352,6 +1358,8 @@ public void ExpressionTests_NullPropagating(string test, string query)
                     _enumnullable = (TestEnum2?) TestEnum2.Var2,
                     number = 1,
                     nullablenumber = (int?) 2,
+                    str = "str",
+                    strNull = (string) null,
                     nested = new
                     {
                         g = Guid.NewGuid(),
@@ -1362,6 +1370,8 @@ public void ExpressionTests_NullPropagating(string test, string query)
                         _enumnullable = (TestEnum2?) TestEnum2.Var2,
                         number = 1,
                         nullablenumber = (int?) 2,
+                        str = "str",
+                        strNull = (string) null
                     },
                     item = new TestGuidNullClass
                     {
diff --git a/test/System.Linq.Dynamic.Core.Tests/Parser/ExpressionHelperTests.cs b/test/System.Linq.Dynamic.Core.Tests/Parser/ExpressionHelperTests.cs
@@ -85,31 +85,59 @@ public void ExpressionHelper_OptimizeStringForEqualityIfPossible_Guid_Invalid()
         }
 
         [Fact]
-        public void ExpressionHelper_TryGenerateAndAlsoNotNullExpression_ForMultiple()
+        public void ExpressionHelper_TryGenerateAndAlsoNotNullExpression_NestedNonNullable()
         {
             // Assign
-            Expression<Func<Item, int>> expression = (x) => x.Relation1.Relation2.Id;
+            Expression<Func<Item, int>> expression = x => x.Relation1.Relation2.Id;
 
             // Act
-            bool result = _expressionHelper.TryGenerateAndAlsoNotNullExpression(expression, out Expression generatedExpresion);
+            bool result = _expressionHelper.TryGenerateAndAlsoNotNullExpression(expression, true, out Expression generatedExpression);
 
             // Assert
             Check.That(result).IsTrue();
-            Check.That(generatedExpresion.ToString()).IsEqualTo("(((x != null) AndAlso (x.Relation1 != null)) AndAlso (x.Relation1.Relation2 != null))");
+            Check.That(generatedExpression.ToString()).IsEqualTo("(((x != null) AndAlso (x.Relation1 != null)) AndAlso (x.Relation1.Relation2 != null))");
         }
 
         [Fact]
-        public void ExpressionHelper_TryGenerateAndAlsoNotNullExpression_ForSingle()
+        public void ExpressionHelper_TryGenerateAndAlsoNotNullExpression_NestedNullable_AddSelfFalse()
         {
             // Assign
-            Expression<Func<Item, int>> expression = (x) => x.Id;
+            Expression<Func<Item, int?>> expression = x => x.Relation1.Relation2.IdNullable;
 
             // Act
-            bool result = _expressionHelper.TryGenerateAndAlsoNotNullExpression(expression, out Expression generatedExpresion);
+            bool result = _expressionHelper.TryGenerateAndAlsoNotNullExpression(expression, false, out Expression generatedExpression);
+
+            // Assert
+            Check.That(result).IsTrue();
+            Check.That(generatedExpression.ToString()).IsEqualTo("(((x != null) AndAlso (x.Relation1 != null)) AndAlso (x.Relation1.Relation2 != null))");
+        }
+
+        [Fact]
+        public void ExpressionHelper_TryGenerateAndAlsoNotNullExpression_NestedNullable_AddSelfTrue()
+        {
+            // Assign
+            Expression<Func<Item, int?>> expression = x => x.Relation1.Relation2.IdNullable;
+
+            // Act
+            bool result = _expressionHelper.TryGenerateAndAlsoNotNullExpression(expression, true, out Expression generatedExpression);
+
+            // Assert
+            Check.That(result).IsTrue();
+            Check.That(generatedExpression.ToString()).IsEqualTo("((((x != null) AndAlso (x.Relation1 != null)) AndAlso (x.Relation1.Relation2 != null)) AndAlso (x => x.Relation1.Relation2.IdNullable != null))");
+        }
+
+        [Fact]
+        public void ExpressionHelper_TryGenerateAndAlsoNotNullExpression_NonNullable()
+        {
+            // Assign
+            Expression<Func<Item, int>> expression = x => x.Id;
+
+            // Act
+            bool result = _expressionHelper.TryGenerateAndAlsoNotNullExpression(expression, true, out Expression generatedExpression);
 
             // Assert
             Check.That(result).IsFalse();
-            Check.That(generatedExpresion.ToString()).IsEqualTo("x => x.Id");
+            Check.That(generatedExpression.ToString()).IsEqualTo("x => x.Id");
         }
 
         class Item
@@ -127,6 +155,8 @@ class Relation1
         class Relation2
         {
             public int Id { get; set; }
+
+            public int? IdNullable { get; set; }
         }
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -244,11 +244,9 @@ private void WrapConstantExpressions(ref Expression left, ref Expression right)`
`244`	`244`	`}`
`245`	`245`	`}`
`246`	`246`
`247`		`- public bool TryGenerateAndAlsoNotNullExpression(Expression sourceExpression, out Expression generatedExpression)`
	`247`	`+ public bool TryGenerateAndAlsoNotNullExpression(Expression sourceExpression, bool addSelf, out Expression generatedExpression)`
`248`	`248`	`{`
`249`		`- generatedExpression = sourceExpression;`
`250`		`-`
`251`		`- var expressions = CollectExpressions(sourceExpression);`
	`249`	`+ var expressions = CollectExpressions(addSelf, sourceExpression);`
`252`	`250`
`253`	`251`	`if (expressions.Count == 1 && !(expressions[0] is MethodCallExpression))`
`254`	`252`	`{`
`@@ -305,11 +303,20 @@ private static Expression GetMemberExpression(Expression expression)`
`305`	`303`	`return null;`
`306`	`304`	`}`
`307`	`305`
`308`		`- private static List<Expression> CollectExpressions(Expression sourceExpression)`
	`306`	`+ private static List<Expression> CollectExpressions(bool addSelf, Expression sourceExpression)`
`309`	`307`	`{`
`310`		`- var list = new List<Expression>();`
`311`	`308`	`Expression expression = GetMemberExpression(sourceExpression);`
`312`	`309`
	`310`	`+ var list = new List<Expression>();`
	`311`	`+`
	`312`	`+ if (addSelf && expression is MemberExpression memberExpressionFirst)`
	`313`	`+ {`
	`314`	`+ if (TypeHelper.IsNullableType(memberExpressionFirst.Type) \|\| !memberExpressionFirst.Type.GetTypeInfo().IsValueType)`
	`315`	`+ {`
	`316`	`+ list.Add(sourceExpression);`
	`317`	`+ }`
	`318`	`+ }`
	`319`	`+`
`313`	`320`	`while (expression is MemberExpression memberExpression)`
`314`	`321`	`{`
`315`	`322`	`expression = GetMemberExpression(memberExpression.Expression);`
Original file line number	Diff line number	Diff line change
`@@ -28,7 +28,7 @@ internal interface IExpressionHelper`
`28`	`28`
`29`	`29`	`Expression OptimizeStringForEqualityIfPossible(string text, Type type);`
`30`	`30`
`31`		`- bool TryGenerateAndAlsoNotNullExpression(Expression sourceExpression, out Expression generatedExpression);`
	`31`	`+ bool TryGenerateAndAlsoNotNullExpression(Expression sourceExpression, bool addSelf, out Expression generatedExpression);`
`32`	`32`
`33`	`33`	`void WrapConstantExpression(ref Expression argument);`
`34`	`34`	`}`