Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert changes introduced in #296 #333

Merged
merged 5 commits into from
Nov 18, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 10 additions & 12 deletions src/DynamicExpresso.Core/Parsing/Parser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -1053,7 +1053,7 @@ private Expression ParseIdentifier()
return ParseMemberAccess(thisParameterExpression);
}
}
catch(ParseException)
catch (ParseException)
{
// ignore
}
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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))
Expand Down
24 changes: 8 additions & 16 deletions src/DynamicExpresso.Core/Resolution/ExpressionUtils.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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))
metoule marked this conversation as resolved.
Show resolved Hide resolved
{
if (type.IsValueType || exact)
{
return Expression.Convert(expr, type);
}
return expr;
return Expression.Convert(expr, type);
}

return null;
Expand Down
7 changes: 3 additions & 4 deletions src/DynamicExpresso.Core/Resolution/MethodResolution.cs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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);
Expand All @@ -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>();
Expand Down
29 changes: 23 additions & 6 deletions test/DynamicExpresso.UnitTest/GithubIssues.cs
Original file line number Diff line number Diff line change
@@ -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

Expand Down Expand Up @@ -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...
Expand All @@ -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"));
}

Expand Down Expand Up @@ -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
Expand Down
Loading