Skip to content

Commit 2393949

Browse files
alexweavStefH
authored andcommitted
Ensure that one-way implicit conversions also work for value types (#287)
* Try and convert both ways regardless of which side is a constant * Add test for value type implicit conversion * Fix incorrectness in test and add explicit assert for all four configurations of implicit conversions * Test every existing case but where the comparison operands are reversed, just to make sure we hit every case
1 parent 62f42fa commit 2393949

File tree

2 files changed

+150
-11
lines changed

2 files changed

+150
-11
lines changed

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

+18-4
Original file line numberDiff line numberDiff line change
@@ -504,13 +504,27 @@ Expression ParseComparisonOperator()
504504

505505
if (!typesAreSameAndImplementCorrectInterface)
506506
{
507-
if (left.Type.GetTypeInfo().IsClass && right is ConstantExpression && HasImplicitConversion(left.Type, right.Type))
507+
if (left.Type.GetTypeInfo().IsClass && right is ConstantExpression)
508508
{
509-
left = Expression.Convert(left, right.Type);
509+
if (HasImplicitConversion(left.Type, right.Type))
510+
{
511+
left = Expression.Convert(left, right.Type);
512+
}
513+
else if (HasImplicitConversion(right.Type, left.Type))
514+
{
515+
right = Expression.Convert(right, left.Type);
516+
}
510517
}
511-
else if (right.Type.GetTypeInfo().IsClass && left is ConstantExpression && HasImplicitConversion(right.Type, left.Type))
518+
else if (right.Type.GetTypeInfo().IsClass && left is ConstantExpression)
512519
{
513-
right = Expression.Convert(right, left.Type);
520+
if (HasImplicitConversion(right.Type, left.Type))
521+
{
522+
right = Expression.Convert(right, left.Type);
523+
}
524+
else if (HasImplicitConversion(left.Type, right.Type))
525+
{
526+
left = Expression.Convert(left, right.Type);
527+
}
514528
}
515529
else
516530
{

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

+132-7
Original file line numberDiff line numberDiff line change
@@ -84,14 +84,87 @@ public override string ToString()
8484
}
8585
}
8686

87+
public class CustomClassWithReversedImplicitConversion
88+
{
89+
public CustomClassWithReversedImplicitConversion(string origin)
90+
{
91+
Origin = origin;
92+
}
93+
94+
public string Origin { get; }
95+
96+
public static implicit operator string(CustomClassWithReversedImplicitConversion origin)
97+
{
98+
return origin.ToString();
99+
}
100+
101+
public override string ToString()
102+
{
103+
return Origin;
104+
}
105+
}
106+
107+
public class CustomClassWithValueTypeImplicitConversion
108+
{
109+
public CustomClassWithValueTypeImplicitConversion(int origin)
110+
{
111+
Origin = origin;
112+
}
113+
114+
public int Origin { get; }
115+
116+
public static implicit operator CustomClassWithValueTypeImplicitConversion(int origin)
117+
{
118+
return new CustomClassWithValueTypeImplicitConversion(origin);
119+
}
120+
121+
public override string ToString()
122+
{
123+
return Origin.ToString();
124+
}
125+
}
126+
127+
public class CustomClassWithReversedValueTypeImplicitConversion
128+
{
129+
public CustomClassWithReversedValueTypeImplicitConversion(int origin)
130+
{
131+
Origin = origin;
132+
}
133+
134+
public int Origin { get; }
135+
136+
public static implicit operator int(CustomClassWithReversedValueTypeImplicitConversion origin)
137+
{
138+
return origin.Origin;
139+
}
140+
141+
public override string ToString()
142+
{
143+
return Origin.ToString();
144+
}
145+
}
146+
87147
public class TestImplicitConversionContainer
88148
{
89-
public TestImplicitConversionContainer(CustomClassWithOneWayImplicitConversion oneWay)
149+
public TestImplicitConversionContainer(
150+
CustomClassWithOneWayImplicitConversion oneWay,
151+
CustomClassWithReversedImplicitConversion reversed,
152+
CustomClassWithValueTypeImplicitConversion valueType,
153+
CustomClassWithReversedValueTypeImplicitConversion reversedValueType)
90154
{
91155
OneWay = oneWay;
156+
Reversed = Reversed;
157+
ValueType = valueType;
158+
ReversedValueType = reversedValueType;
92159
}
93160

94161
public CustomClassWithOneWayImplicitConversion OneWay { get; }
162+
163+
public CustomClassWithReversedImplicitConversion Reversed { get; }
164+
165+
public CustomClassWithValueTypeImplicitConversion ValueType { get; }
166+
167+
public CustomClassWithReversedValueTypeImplicitConversion ReversedValueType { get; }
95168
}
96169

97170
public class TextHolder
@@ -791,14 +864,66 @@ public void DynamicExpressionParser_ParseLambda_With_Concat_CustomType_String()
791864
public void DynamicExpressionParser_ParseLambda_With_One_Way_Implicit_Conversions()
792865
{
793866
// Arrange
794-
var testValue = "test";
795-
var container = new TestImplicitConversionContainer(testValue);
796-
var expressionText = $"OneWay == \"{testValue}\"";
867+
var testString = "test";
868+
var testInt = 6;
869+
var container = new TestImplicitConversionContainer(testString, new CustomClassWithReversedImplicitConversion(testString), testInt, new CustomClassWithReversedValueTypeImplicitConversion(testInt));
797870

798-
// Act
799-
var lambda = DynamicExpressionParser.ParseLambda<TestImplicitConversionContainer, bool>(ParsingConfig.Default, false, expressionText);
871+
var expressionTextString = $"OneWay == \"{testString}\"";
872+
var expressionTextReversed = $"Reversed == \"{testString}\"";
873+
var expressionTextValueType = $"ValueType == {testInt}";
874+
var expressionTextReversedValueType = $"ReversedValueType == {testInt}";
800875

801-
// Assert
876+
var invertedExpressionTextString = $"\"{testString}\" == OneWay";
877+
var invertedExpressionTextReversed = $"\"{testString}\" == Reversed";
878+
var invertedExpressionTextValueType = $"{testInt} == ValueType";
879+
var invertedExpressionTextReversedValueType = $"{testInt} == ReversedValueType";
880+
881+
// Act 1
882+
var lambda = DynamicExpressionParser.ParseLambda<TestImplicitConversionContainer, bool>(ParsingConfig.Default, false, expressionTextString);
883+
884+
// Assert 1
885+
Assert.NotNull(lambda);
886+
887+
// Act 2
888+
lambda = DynamicExpressionParser.ParseLambda<TestImplicitConversionContainer, bool>(ParsingConfig.Default, false, expressionTextReversed);
889+
890+
// Assert 2
891+
Assert.NotNull(lambda);
892+
893+
// Act 3
894+
lambda = DynamicExpressionParser.ParseLambda<TestImplicitConversionContainer, bool>(ParsingConfig.Default, false, expressionTextValueType);
895+
896+
// Assert 3
897+
Assert.NotNull(lambda);
898+
899+
// Act 4
900+
lambda = DynamicExpressionParser.ParseLambda<TestImplicitConversionContainer, bool>(ParsingConfig.Default, false, expressionTextReversedValueType);
901+
902+
// Assert 4
903+
Assert.NotNull(lambda);
904+
905+
// Act 5
906+
lambda = DynamicExpressionParser.ParseLambda<TestImplicitConversionContainer, bool>(ParsingConfig.Default, false, invertedExpressionTextString);
907+
908+
// Assert 5
909+
Assert.NotNull(lambda);
910+
911+
// Act 6
912+
lambda = DynamicExpressionParser.ParseLambda<TestImplicitConversionContainer, bool>(ParsingConfig.Default, false, invertedExpressionTextReversed);
913+
914+
// Assert 6
915+
Assert.NotNull(lambda);
916+
917+
// Act 7
918+
lambda = DynamicExpressionParser.ParseLambda<TestImplicitConversionContainer, bool>(ParsingConfig.Default, false, invertedExpressionTextValueType);
919+
920+
// Assert 7
921+
Assert.NotNull(lambda);
922+
923+
// Act 8
924+
lambda = DynamicExpressionParser.ParseLambda<TestImplicitConversionContainer, bool>(ParsingConfig.Default, false, invertedExpressionTextReversedValueType);
925+
926+
// Assert 8
802927
Assert.NotNull(lambda);
803928
}
804929

0 commit comments

Comments
 (0)