Skip to content

Commit 62f42fa

Browse files
alexweavStefH
authored andcommitted
Fix certain cases where implicit conversions aren't correctly detected when parsing comparison operators (#285)
* Add failing test for one-way implicit type conversions * Search both types for implicit conversion methods, not just base type * Respect implicit conversions for non-value types * Make comparisons in HasImplicitConversion more explicit
1 parent dd4a659 commit 62f42fa

File tree

2 files changed

+59
-5
lines changed

2 files changed

+59
-5
lines changed

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

+12-3
Original file line numberDiff line numberDiff line change
@@ -436,11 +436,11 @@ Expression ParseComparisonOperator()
436436
// If left or right is NullLiteral, just continue. Else check if the types differ.
437437
if (!(Constants.IsNull(left) || Constants.IsNull(right)) && left.Type != right.Type)
438438
{
439-
if (left.Type.IsAssignableFrom(right.Type))
439+
if (left.Type.IsAssignableFrom(right.Type) || HasImplicitConversion(right.Type, left.Type))
440440
{
441441
right = Expression.Convert(right, left.Type);
442442
}
443-
else if (right.Type.IsAssignableFrom(left.Type))
443+
else if (right.Type.IsAssignableFrom(left.Type) || HasImplicitConversion(left.Type, right.Type))
444444
{
445445
left = Expression.Convert(left, right.Type);
446446
}
@@ -549,7 +549,16 @@ Expression ParseComparisonOperator()
549549

550550
private bool HasImplicitConversion(Type baseType, Type targetType)
551551
{
552-
return baseType.GetMethods(BindingFlags.Public | BindingFlags.Static)
552+
var baseTypeHasConversion = baseType.GetMethods(BindingFlags.Public | BindingFlags.Static)
553+
.Where(mi => mi.Name == "op_Implicit" && mi.ReturnType == targetType)
554+
.Any(mi => mi.GetParameters().FirstOrDefault()?.ParameterType == baseType);
555+
556+
if (baseTypeHasConversion)
557+
{
558+
return true;
559+
}
560+
561+
return targetType.GetMethods(BindingFlags.Public | BindingFlags.Static)
553562
.Where(mi => mi.Name == "op_Implicit" && mi.ReturnType == targetType)
554563
.Any(mi => mi.GetParameters().FirstOrDefault()?.ParameterType == baseType);
555564
}

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

+47-2
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
1-
using NFluent;
2-
using System.Collections.Generic;
1+
using System.Collections.Generic;
32
using System.Linq.Dynamic.Core.CustomTypeProviders;
43
using System.Linq.Dynamic.Core.Exceptions;
54
using System.Linq.Dynamic.Core.Tests.Helpers.Models;
65
using System.Linq.Expressions;
76
using System.Reflection;
7+
using NFluent;
88
using Xunit;
99
using User = System.Linq.Dynamic.Core.Tests.Helpers.Models.User;
1010

@@ -64,6 +64,36 @@ public override string ToString()
6464
}
6565
}
6666

67+
public class CustomClassWithOneWayImplicitConversion
68+
{
69+
public CustomClassWithOneWayImplicitConversion(string origin)
70+
{
71+
Origin = origin;
72+
}
73+
74+
public string Origin { get; }
75+
76+
public static implicit operator CustomClassWithOneWayImplicitConversion(string origin)
77+
{
78+
return new CustomClassWithOneWayImplicitConversion(origin);
79+
}
80+
81+
public override string ToString()
82+
{
83+
return Origin;
84+
}
85+
}
86+
87+
public class TestImplicitConversionContainer
88+
{
89+
public TestImplicitConversionContainer(CustomClassWithOneWayImplicitConversion oneWay)
90+
{
91+
OneWay = oneWay;
92+
}
93+
94+
public CustomClassWithOneWayImplicitConversion OneWay { get; }
95+
}
96+
6797
public class TextHolder
6898
{
6999
public TextHolder(string name, CustomTextClass note)
@@ -757,6 +787,21 @@ public void DynamicExpressionParser_ParseLambda_With_Concat_CustomType_String()
757787
Assert.Equal("note1 (name1)", result);
758788
}
759789

790+
[Fact]
791+
public void DynamicExpressionParser_ParseLambda_With_One_Way_Implicit_Conversions()
792+
{
793+
// Arrange
794+
var testValue = "test";
795+
var container = new TestImplicitConversionContainer(testValue);
796+
var expressionText = $"OneWay == \"{testValue}\"";
797+
798+
// Act
799+
var lambda = DynamicExpressionParser.ParseLambda<TestImplicitConversionContainer, bool>(ParsingConfig.Default, false, expressionText);
800+
801+
// Assert
802+
Assert.NotNull(lambda);
803+
}
804+
760805
[Fact]
761806
public void DynamicExpressionParser_ParseLambda_Operator_Less_Greater_With_Guids()
762807
{

0 commit comments

Comments
 (0)