Improve support of nullable types (#209)

Promote all operands of binary operators. When applying the ?. operator to nullable types, emit the member access on the underlying type. Fix #205
dynamicexpresso · Dec 12, 2021 · dcc633f · dcc633f
1 parent 1792b39
commit dcc633f
Show file tree

Hide file tree

Showing 3 changed files with 77 additions and 14 deletions.
diff --git a/src/DynamicExpresso.Core/Parsing/Parser.cs b/src/DynamicExpresso.Core/Parsing/Parser.cs
@@ -403,12 +403,6 @@ private Expression ParseComparison()
 				//			op.text, ref left, ref right, op.pos);
 				//}
 
-				if ((IsNullableType(left.Type) || IsNullableType(right.Type)) && (GetNonNullableType(left.Type) == right.Type || GetNonNullableType(right.Type) == left.Type))
-				{
-					left = GenerateNullableTypeConversion(left);
-					right = GenerateNullableTypeConversion(right);
-				}
-
 				CheckAndPromoteOperands(
 					isEquality ? typeof(ParseSignatures.IEqualitySignatures) : typeof(ParseSignatures.IRelationalSignatures),
 					ref left,
@@ -642,7 +636,7 @@ private Expression ParsePrimary()
 				if (_token.id == TokenId.Dot)
 				{
 					NextToken();
-					expr = ParseMemberAccess(null, expr);
+					expr = ParseMemberAccess(expr);
 				}
 				// special case for ?. and ?[ operators
 				else if (_token.id == TokenId.Question && (_parseChar == '.' || _parseChar == '['))
@@ -654,15 +648,17 @@ private Expression ParsePrimary()
 						NextToken();
 
 						// ?. operator changes value types to nullable types
-						var memberAccess = GenerateNullableTypeConversion(ParseMemberAccess(null, expr));
+						// the member access should be resolved on the underlying type
+						var memberAccess = GenerateNullableTypeConversion(ParseMemberAccess(GenerateGetNullableValue(expr)));
 						var nullExpr = ParserConstants.NullLiteralExpression;
 						CheckAndPromoteOperands(typeof(ParseSignatures.IEqualitySignatures), ref expr, ref nullExpr);
 						expr = GenerateConditional(GenerateEqual(expr, nullExpr), ParserConstants.NullLiteralExpression, memberAccess, _token.pos);
 					}
 					else if (_token.id == TokenId.OpenBracket)
 					{
 						// ?[ operator changes value types to nullable types
-						var elementAccess = GenerateNullableTypeConversion(ParseElementAccess(expr));
+						// the member access should be resolved on the underlying type
+						var elementAccess = GenerateNullableTypeConversion(ParseElementAccess(GenerateGetNullableValue(expr)));
 						var nullExpr = ParserConstants.NullLiteralExpression;
 						CheckAndPromoteOperands(typeof(ParseSignatures.IEqualitySignatures), ref expr, ref nullExpr);
 						expr = GenerateConditional(GenerateEqual(expr, nullExpr), ParserConstants.NullLiteralExpression, elementAccess, _token.pos);
@@ -692,6 +688,16 @@ private Expression ParsePrimary()
 			return expr;
 		}
 
+		/// <summary>
+		/// Generate a call to the Value property of the Nullable type */
+		/// </summary>
+		private Expression GenerateGetNullableValue(Expression expr)
+		{
+			if (!IsNullableType(expr.Type))
+				return expr;
+			return GeneratePropertyOrFieldExpression(expr.Type, expr, _token.pos, "Value");
+		}
+
 		private Expression ParsePrimaryStart()
 		{
 			switch (_token.id)
@@ -1425,6 +1431,11 @@ private Expression GenerateConversion(Expression expr, Type type, int errorPos)
 			}
 		}
 
+		private Expression ParseMemberAccess(Expression instance)
+		{
+			return ParseMemberAccess(null, instance);
+		}
+
 		private Expression ParseMemberAccess(Type type, Expression instance)
 		{
 			if (instance != null) type = instance.Type;
@@ -1749,6 +1760,12 @@ private void CheckAndPromoteOperand(Type signatures, ref Expression expr)
 
 		private void CheckAndPromoteOperands(Type signatures, ref Expression left, ref Expression right)
 		{
+			if ((IsNullableType(left.Type) || IsNullableType(right.Type)) && (GetNonNullableType(left.Type) == right.Type || GetNonNullableType(right.Type) == left.Type))
+			{
+				left = GenerateNullableTypeConversion(left);
+				right = GenerateNullableTypeConversion(right);
+			}
+
 			var args = new[] { left, right };
 
 			args = PrepareOperandArguments(signatures, args);

diff --git a/test/DynamicExpresso.UnitTest/GithubIssues.cs b/test/DynamicExpresso.UnitTest/GithubIssues.cs
@@ -258,9 +258,9 @@ public void GitHub_Issue_164_bis()
 			result = lambda.Invoke(new Scope { ValueInt = 5 });
 			Assert.AreEqual(5, result);
 
-			interpreter.SetVariable("scope", new Scope { ValueInt = 5 });
-			var resultNullableBool = interpreter.Eval<bool>("scope?.ValueInt?.HasValue");
-			Assert.IsTrue(resultNullableBool);
+			var scope = new Scope { Value = 5 };
+			interpreter.SetVariable("scope", scope);
+			Assert.AreEqual(scope?.Value.HasValue, interpreter.Eval<bool>("scope?.Value.HasValue"));
 
 			// must throw, because scope.ValueInt is not a nullable type
 			Assert.Throws<ParseException>(() => interpreter.Eval<bool>("scope.ValueInt.HasValue"));
@@ -398,6 +398,42 @@ public void GitHub_Issue_191()
 			Assert.IsNotNull(result);
 		}
 
+		[Test]
+		public void GitHub_Issue_205_Property_on_nullable()
+		{
+			var interpreter = new Interpreter();
+
+			DateTime? date = DateTime.UtcNow;
+			interpreter.SetVariable("date", date);
+
+			Assert.AreEqual(date?.Day, interpreter.Eval("date?.Day"));
+			Assert.AreEqual(date?.IsDaylightSavingTime(), interpreter.Eval("date?.IsDaylightSavingTime()"));
+
+			date = null;
+			interpreter.SetVariable("date", date);
+
+			Assert.AreEqual(date?.Day, interpreter.Eval("date?.Day"));
+			Assert.AreEqual(date?.IsDaylightSavingTime(), interpreter.Eval("date?.IsDaylightSavingTime()"));
+		}
+
+		[Test]
+		public void GitHub_Issue_205()
+		{
+			var interpreter = new Interpreter();
+
+			var date1 = DateTimeOffset.UtcNow;
+			DateTimeOffset? date2 = null;
+
+			interpreter.SetVariable("date1", date1);
+			interpreter.SetVariable("date2", date2);
+
+			Assert.IsNull(interpreter.Eval("(date1 - date2)?.Days"));
+
+			date2 = date1.AddDays(1);
+			interpreter.SetVariable("date2", date2);
+			Assert.AreEqual(-1, interpreter.Eval("(date1 - date2)?.Days"));
+		}
+
 		public class Utils
 		{
 			public static List<T> Array<T>(IEnumerable<T> collection) => new List<T>(collection);

diff --git a/test/DynamicExpresso.UnitTest/NullableTest.cs b/test/DynamicExpresso.UnitTest/NullableTest.cs
@@ -1,4 +1,4 @@
-using System;
+using System;
 using NUnit.Framework;
 // ReSharper disable ConvertNullableToShortForm
 // ReSharper disable PossibleNullReferenceException
@@ -237,7 +237,7 @@ public void NullableDateTimeOffset_DatetimeOffset()
 
 			var interpreter = new Interpreter();
 			interpreter.SetVariable("a", a, typeof(DateTimeOffset));
-			interpreter.SetVariable("b", b, typeof(Nullable<DateTimeOffset>));
+			interpreter.SetVariable("b", b, typeof(DateTimeOffset?));
 			interpreter.SetVariable("c", c, typeof(DateTimeOffset));
 			var expectedReturnType = typeof(bool);
 
@@ -270,6 +270,16 @@ public void NullableDateTimeOffset_DatetimeOffset()
 			lambda = interpreter.Parse("b != c");
 			Assert.AreEqual(expected, lambda.Invoke());
 			Assert.AreEqual(expectedReturnType, lambda.ReturnType);
+
+			lambda = interpreter.Parse("a - b");
+			Assert.AreEqual(a - b, lambda.Invoke());
+			Assert.AreEqual(typeof(TimeSpan?), lambda.ReturnType);
+
+			b = null;
+			interpreter.SetVariable("b", b, typeof(DateTimeOffset?));
+			lambda = interpreter.Parse("a - b");
+			Assert.AreEqual(a - b, lambda.Invoke());
+			Assert.AreEqual(typeof(TimeSpan?), lambda.ReturnType);
 		}
 	}
 }