Revert changes introduced in #296 (#333)

metoule · web-flow · commit 546a0cb1cf8c · 2024-11-18T23:38:52.000+01:00
* Fix format * Simplify ExpressionUtils.PromoteExpression * Revert changes done in #296
diff --git a/src/DynamicExpresso.Core/Parsing/Parser.cs b/src/DynamicExpresso.Core/Parsing/Parser.cs
@@ -228,7 +228,7 @@ private Expression ParseAssignment()
 				NextToken();
 				var right = ParseAssignment();
 
-				var promoted = ExpressionUtils.PromoteExpression(right, left.Type, true);
+				var promoted = ExpressionUtils.PromoteExpression(right, left.Type);
 				if (promoted == null)
 					throw ParseException.Create(_token.pos, ErrorMessages.CannotConvertValue,
 						TypeUtils.GetTypeName(right.Type), TypeUtils.GetTypeName(left.Type));
@@ -336,8 +336,8 @@ private Expression ParseComparison()
 		{
 			var left = ParseTypeTesting();
 			while (_token.id == TokenId.DoubleEqual || _token.id == TokenId.ExclamationEqual ||
-			       _token.id == TokenId.GreaterThan || _token.id == TokenId.GreaterThanEqual ||
-			       _token.id == TokenId.LessThan || _token.id == TokenId.LessThanEqual)
+				   _token.id == TokenId.GreaterThan || _token.id == TokenId.GreaterThanEqual ||
+				   _token.id == TokenId.LessThan || _token.id == TokenId.LessThanEqual)
 			{
 				var op = _token;
 				NextToken();
@@ -424,7 +424,7 @@ private Expression ParseTypeTesting()
 		{
 			var left = ParseShift();
 			while (_token.text == ParserConstants.KeywordIs
-			       || _token.text == ParserConstants.KeywordAs)
+				|| _token.text == ParserConstants.KeywordAs)
 			{
 				var typeOperator = _token.text;
 
@@ -523,7 +523,7 @@ private Expression ParseMultiplicative()
 		{
 			var left = ParseUnary();
 			while (_token.id == TokenId.Asterisk || _token.id == TokenId.Slash ||
-			       _token.id == TokenId.Percent)
+				   _token.id == TokenId.Percent)
 			{
 				var op = _token;
 				NextToken();
@@ -1053,7 +1053,7 @@ private Expression ParseIdentifier()
 					return ParseMemberAccess(thisParameterExpression);
 				}
 			}
-			catch(ParseException)
+			catch (ParseException)
 			{
 				// ignore
 			}
@@ -1116,14 +1116,14 @@ private Expression ParseDefaultOperator()
 		private Expression GenerateConditional(Expression test, Expression expr1, Expression expr2, int errorPos)
 		{
 			if (IsDynamicExpression(test))
-				return GenerateConditionalDynamic(test, expr1, expr2,errorPos);
+				return GenerateConditionalDynamic(test, expr1, expr2, errorPos);
 
 			if (test.Type != typeof(bool))
 				throw ParseException.Create(errorPos, ErrorMessages.FirstExprMustBeBool);
 			if (expr1.Type != expr2.Type)
 			{
-				var expr1As2 = expr2 != ParserConstants.NullLiteralExpression ? ExpressionUtils.PromoteExpression(expr1, expr2.Type, true) : null;
-				var expr2As1 = expr1 != ParserConstants.NullLiteralExpression ? ExpressionUtils.PromoteExpression(expr2, expr1.Type, true) : null;
+				var expr1As2 = expr2 != ParserConstants.NullLiteralExpression ? ExpressionUtils.PromoteExpression(expr1, expr2.Type) : null;
+				var expr2As1 = expr1 != ParserConstants.NullLiteralExpression ? ExpressionUtils.PromoteExpression(expr2, expr1.Type) : null;
 				if (expr1As2 != null && expr2As1 == null)
 				{
 					expr1 = expr1As2;
@@ -1838,7 +1838,7 @@ private Expression ParseElementAccess(Expression expr)
 
 				for (int i = 0; i < args.Length; i++)
 				{
-					args[i] = ExpressionUtils.PromoteExpression(args[i], typeof(int), true);
+					args[i] = ExpressionUtils.PromoteExpression(args[i], typeof(int));
 					if (args[i] == null)
 						throw ParseException.Create(errorPos, ErrorMessages.InvalidIndex);
 				}
@@ -1959,8 +1959,6 @@ private Expression GenerateGreaterThan(Expression left, Expression right)
 			return GenerateBinary(ExpressionType.GreaterThan, left, right);
 		}
 
-
-
 		private Expression GenerateGreaterThanEqual(Expression left, Expression right)
 		{
 			if (left.Type == typeof(string))
diff --git a/src/DynamicExpresso.Core/Resolution/ExpressionUtils.cs b/src/DynamicExpresso.Core/Resolution/ExpressionUtils.cs
@@ -7,19 +7,15 @@ namespace DynamicExpresso.Resolution
 {
 	internal static class ExpressionUtils
 	{
-		public static Expression PromoteExpression(Expression expr, Type type, bool exact)
+		public static Expression PromoteExpression(Expression expr, Type type)
 		{
 			if (expr.Type == type) return expr;
-			if (expr is ConstantExpression)
+			if (expr is ConstantExpression ce && ce == ParserConstants.NullLiteralExpression)
 			{
-				var ce = (ConstantExpression)expr;
-				if (ce == ParserConstants.NullLiteralExpression)
-				{
-					if (type.ContainsGenericParameters)
-						return null;
-					if (!type.IsValueType || TypeUtils.IsNullableType(type))
-						return Expression.Constant(null, type);
-				}
+				if (type.ContainsGenericParameters)
+					return null;
+				if (!type.IsValueType || TypeUtils.IsNullableType(type))
+					return Expression.Constant(null, type);
 			}
 
 			if (expr is InterpreterExpression ie)
@@ -40,13 +36,9 @@ public static Expression PromoteExpression(Expression expr, Type type, bool exac
 					return Expression.Convert(expr, genericType);
 			}
 
-			if (TypeUtils.IsCompatibleWith(expr.Type, type) || expr is DynamicExpression)
+			if (TypeUtils.IsCompatibleWith(expr.Type, type))
 			{
-				if (type.IsValueType || exact)
-				{
-					return Expression.Convert(expr, type);
-				}
-				return expr;
+				return Expression.Convert(expr, type);
 			}
 
 			return null;
diff --git a/src/DynamicExpresso.Core/Resolution/MethodResolution.cs b/src/DynamicExpresso.Core/Resolution/MethodResolution.cs
@@ -1,10 +1,9 @@
 using System;
 using System.Collections.Generic;
-using System.Linq.Expressions;
 using System.Linq;
+using System.Linq.Expressions;
 using System.Reflection;
 using System.Security;
-using System.Text;
 using DynamicExpresso.Exceptions;
 using DynamicExpresso.Parsing;
 using DynamicExpresso.Reflection;
@@ -93,7 +92,7 @@ public static bool CheckIfMethodIsApplicableAndPrepareIt(MethodData method, Expr
 						continue;
 					}
 
-					var promoted = ExpressionUtils.PromoteExpression(currentArgument, parameterType, true);
+					var promoted = ExpressionUtils.PromoteExpression(currentArgument, parameterType);
 					if (promoted != null)
 					{
 						promotedArgs.Add(promoted);
@@ -111,7 +110,7 @@ public static bool CheckIfMethodIsApplicableAndPrepareIt(MethodData method, Expr
 						continue;
 					}
 
-					var promoted = ExpressionUtils.PromoteExpression(currentArgument, paramsArrayElementType, true);
+					var promoted = ExpressionUtils.PromoteExpression(currentArgument, paramsArrayElementType);
 					if (promoted != null)
 					{
 						paramsArrayPromotedArgument = paramsArrayPromotedArgument ?? new List<Expression>();
diff --git a/test/DynamicExpresso.UnitTest/GithubIssues.cs b/test/DynamicExpresso.UnitTest/GithubIssues.cs
@@ -1,12 +1,12 @@
-using DynamicExpresso.Exceptions;
-using NUnit.Framework;
 using System;
 using System.Collections;
 using System.Collections.Generic;
+using System.Dynamic;
 using System.Linq;
 using System.Reflection;
 using System.Text.RegularExpressions;
-using System.Dynamic;
+using DynamicExpresso.Exceptions;
+using NUnit.Framework;
 
 // ReSharper disable SpecifyACultureInStringConversionExplicitly
 
@@ -775,7 +775,9 @@ public void GitHub_Issue_292()
 		}
 
 		[Test]
-		public void GitHub_Issue_295() {
+		[Ignore("The fix suggested in #296 break other use cases, so let's ignore this test for now")]
+		public void GitHub_Issue_295()
+		{
 			var evaluator = new Interpreter();
 
 			// create path helper functions in expressions...
@@ -787,10 +789,10 @@ public void GitHub_Issue_295() {
 			globalSettings.MyTestPath = "C:\\delme\\";
 			evaluator.SetVariable("GlobalSettings", globalSettings);
 
-			var works = (string) evaluator.Eval("StringConcat((string)GlobalSettings.MyTestPath,\"test.txt\")");
+			var works = (string)evaluator.Eval("StringConcat((string)GlobalSettings.MyTestPath,\"test.txt\")");
 			Assert.That(works, Is.EqualTo("C:\\delme\\test.txt"));
 
-			var doesntWork = (string) evaluator.Eval("StringConcat(GlobalSettings.MyTestPath,\"test.txt\")");
+			var doesntWork = (string)evaluator.Eval("StringConcat(GlobalSettings.MyTestPath,\"test.txt\")");
 			Assert.That(doesntWork, Is.EqualTo("C:\\delme\\test.txt"));
 		}
 
@@ -848,6 +850,21 @@ public void GitHub_Issue_314()
 			var exception2 = Assert.Throws<UnknownIdentifierException>(() => interpreter.Eval("b > 1"));
 			Assert.AreEqual("b", exception2.Identifier);
 		}
+
+		[Test]
+		public void GitHub_Issue_325()
+		{
+			var options = InterpreterOptions.Default | InterpreterOptions.LateBindObject;
+			var interpreter = new Interpreter(options);
+
+			var input = new
+			{
+				Prop1 = 4,
+			};
+
+			var expressionDelegate = interpreter.ParseAsDelegate<Func<object, bool>>($"input.Prop1 == null", "input");
+			Assert.IsFalse(expressionDelegate(input));
+		}
 	}
 
 	internal static class GithubIssuesTestExtensionsMethods

Original file line number	Diff line number	Diff line change
`@@ -228,7 +228,7 @@ private Expression ParseAssignment()`
`228`	`228`	`NextToken();`
`229`	`229`	`var right = ParseAssignment();`
`230`	`230`
`231`		`- var promoted = ExpressionUtils.PromoteExpression(right, left.Type, true);`
	`231`	`+ var promoted = ExpressionUtils.PromoteExpression(right, left.Type);`
`232`	`232`	`if (promoted == null)`
`233`	`233`	`throw ParseException.Create(_token.pos, ErrorMessages.CannotConvertValue,`
`234`	`234`	`TypeUtils.GetTypeName(right.Type), TypeUtils.GetTypeName(left.Type));`
`@@ -336,8 +336,8 @@ private Expression ParseComparison()`
`336`	`336`	`{`
`337`	`337`	`var left = ParseTypeTesting();`
`338`	`338`	`while (_token.id == TokenId.DoubleEqual \|\| _token.id == TokenId.ExclamationEqual \|\|`
`339`		`- _token.id == TokenId.GreaterThan \|\| _token.id == TokenId.GreaterThanEqual \|\|`
`340`		`- _token.id == TokenId.LessThan \|\| _token.id == TokenId.LessThanEqual)`
	`339`	`+ _token.id == TokenId.GreaterThan \|\| _token.id == TokenId.GreaterThanEqual \|\|`
	`340`	`+ _token.id == TokenId.LessThan \|\| _token.id == TokenId.LessThanEqual)`
`341`	`341`	`{`
`342`	`342`	`var op = _token;`
`343`	`343`	`NextToken();`
`@@ -424,7 +424,7 @@ private Expression ParseTypeTesting()`
`424`	`424`	`{`
`425`	`425`	`var left = ParseShift();`
`426`	`426`	`while (_token.text == ParserConstants.KeywordIs`
`427`		`- \|\| _token.text == ParserConstants.KeywordAs)`
	`427`	`+ \|\| _token.text == ParserConstants.KeywordAs)`
`428`	`428`	`{`
`429`	`429`	`var typeOperator = _token.text;`
`430`	`430`
`@@ -523,7 +523,7 @@ private Expression ParseMultiplicative()`
`523`	`523`	`{`
`524`	`524`	`var left = ParseUnary();`
`525`	`525`	`while (_token.id == TokenId.Asterisk \|\| _token.id == TokenId.Slash \|\|`
`526`		`- _token.id == TokenId.Percent)`
	`526`	`+ _token.id == TokenId.Percent)`
`527`	`527`	`{`
`528`	`528`	`var op = _token;`
`529`	`529`	`NextToken();`
`@@ -1053,7 +1053,7 @@ private Expression ParseIdentifier()`
`1053`	`1053`	`return ParseMemberAccess(thisParameterExpression);`
`1054`	`1054`	`}`
`1055`	`1055`	`}`
`1056`		`- catch(ParseException)`
	`1056`	`+ catch (ParseException)`
`1057`	`1057`	`{`
`1058`	`1058`	`// ignore`
`1059`	`1059`	`}`
`@@ -1116,14 +1116,14 @@ private Expression ParseDefaultOperator()`
`1116`	`1116`	`private Expression GenerateConditional(Expression test, Expression expr1, Expression expr2, int errorPos)`
`1117`	`1117`	`{`
`1118`	`1118`	`if (IsDynamicExpression(test))`
`1119`		`- return GenerateConditionalDynamic(test, expr1, expr2,errorPos);`
	`1119`	`+ return GenerateConditionalDynamic(test, expr1, expr2, errorPos);`
`1120`	`1120`
`1121`	`1121`	`if (test.Type != typeof(bool))`
`1122`	`1122`	`throw ParseException.Create(errorPos, ErrorMessages.FirstExprMustBeBool);`
`1123`	`1123`	`if (expr1.Type != expr2.Type)`
`1124`	`1124`	`{`
`1125`		`- var expr1As2 = expr2 != ParserConstants.NullLiteralExpression ? ExpressionUtils.PromoteExpression(expr1, expr2.Type, true) : null;`
`1126`		`- var expr2As1 = expr1 != ParserConstants.NullLiteralExpression ? ExpressionUtils.PromoteExpression(expr2, expr1.Type, true) : null;`
	`1125`	`+ var expr1As2 = expr2 != ParserConstants.NullLiteralExpression ? ExpressionUtils.PromoteExpression(expr1, expr2.Type) : null;`
	`1126`	`+ var expr2As1 = expr1 != ParserConstants.NullLiteralExpression ? ExpressionUtils.PromoteExpression(expr2, expr1.Type) : null;`
`1127`	`1127`	`if (expr1As2 != null && expr2As1 == null)`
`1128`	`1128`	`{`
`1129`	`1129`	`expr1 = expr1As2;`
`@@ -1838,7 +1838,7 @@ private Expression ParseElementAccess(Expression expr)`
`1838`	`1838`
`1839`	`1839`	`for (int i = 0; i < args.Length; i++)`
`1840`	`1840`	`{`
`1841`		`- args[i] = ExpressionUtils.PromoteExpression(args[i], typeof(int), true);`
	`1841`	`+ args[i] = ExpressionUtils.PromoteExpression(args[i], typeof(int));`
`1842`	`1842`	`if (args[i] == null)`
`1843`	`1843`	`throw ParseException.Create(errorPos, ErrorMessages.InvalidIndex);`
`1844`	`1844`	`}`
`@@ -1959,8 +1959,6 @@ private Expression GenerateGreaterThan(Expression left, Expression right)`
`1959`	`1959`	`return GenerateBinary(ExpressionType.GreaterThan, left, right);`
`1960`	`1960`	`}`
`1961`	`1961`
`1962`		`-`
`1963`		`-`
`1964`	`1962`	`private Expression GenerateGreaterThanEqual(Expression left, Expression right)`
`1965`	`1963`	`{`
`1966`	`1964`	`if (left.Type == typeof(string))`
Original file line number	Diff line number	Diff line change
`@@ -7,19 +7,15 @@ namespace DynamicExpresso.Resolution`
`7`	`7`	`{`
`8`	`8`	`internal static class ExpressionUtils`
`9`	`9`	`{`
`10`		`- public static Expression PromoteExpression(Expression expr, Type type, bool exact)`
	`10`	`+ public static Expression PromoteExpression(Expression expr, Type type)`
`11`	`11`	`{`
`12`	`12`	`if (expr.Type == type) return expr;`
`13`		`- if (expr is ConstantExpression)`
	`13`	`+ if (expr is ConstantExpression ce && ce == ParserConstants.NullLiteralExpression)`
`14`	`14`	`{`
`15`		`- var ce = (ConstantExpression)expr;`
`16`		`- if (ce == ParserConstants.NullLiteralExpression)`
`17`		`- {`
`18`		`- if (type.ContainsGenericParameters)`
`19`		`- return null;`
`20`		`- if (!type.IsValueType \|\| TypeUtils.IsNullableType(type))`
`21`		`- return Expression.Constant(null, type);`
`22`		`- }`
	`15`	`+ if (type.ContainsGenericParameters)`
	`16`	`+ return null;`
	`17`	`+ if (!type.IsValueType \|\| TypeUtils.IsNullableType(type))`
	`18`	`+ return Expression.Constant(null, type);`
`23`	`19`	`}`
`24`	`20`
`25`	`21`	`if (expr is InterpreterExpression ie)`
`@@ -40,13 +36,9 @@ public static Expression PromoteExpression(Expression expr, Type type, bool exac`
`40`	`36`	`return Expression.Convert(expr, genericType);`
`41`	`37`	`}`
`42`	`38`
`43`		`- if (TypeUtils.IsCompatibleWith(expr.Type, type) \|\| expr is DynamicExpression)`
	`39`	`+ if (TypeUtils.IsCompatibleWith(expr.Type, type))`
`44`	`40`	`{`
`45`		`- if (type.IsValueType \|\| exact)`
`46`		`- {`
`47`		`- return Expression.Convert(expr, type);`
`48`		`- }`
`49`		`- return expr;`
	`41`	`+ return Expression.Convert(expr, type);`
`50`	`42`	`}`
`51`	`43`
`52`	`44`	`return null;`