Skip to content

Commit

Permalink
Fix equality operator resolution
Browse files Browse the repository at this point in the history
  • Loading branch information
@exyi
exyi committed Jul 1, 2023
1 parent 81c264b commit a98d13a
Show file tree
Hide file tree
Showing 5 changed files with 81 additions and 68 deletions.
77 changes: 27 additions & 50 deletions src/Framework/Framework/Compilation/Binding/OperatorResolution.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,6 @@ public static Expression GetBinaryOperator(
_ => null
};


// Try to find user defined operator
if (customOperator != null && (!leftType.IsPrimitive || !rightType.IsPrimitive))
{
Expand All @@ -107,18 +106,30 @@ public static Expression GetBinaryOperator(
throw new InvalidOperationException($"Cannot apply {operation} operator to two different enum types: {leftType.Name}, {rightType.Name}.");
}

// numeric operations
if (operation == ExpressionType.LeftShift)
if (operation is ExpressionType.Equal or ExpressionType.NotEqual && !leftType.IsValueType && !rightType.IsValueType)
{
return Expression.LeftShift(left, ConvertToMaybeNullable(right, typeof(int), true)!);
// https://github.com/dotnet/csharpstandard/blob/standard-v6/standard/expressions.md#11117-reference-type-equality-operators
// Every class type C implicitly provides the following predefined reference type equality operators:
// bool operator ==(C x, C y);
// bool operator !=(C x, C y);
return ReferenceEquality(left, right, operation == ExpressionType.NotEqual);
}
else if (operation == ExpressionType.RightShift)

if (operation is ExpressionType.LeftShift or ExpressionType.RightShift)
{
return Expression.RightShift(left, ConvertToMaybeNullable(right, typeof(int), true)!);
// https://github.com/dotnet/csharpstandard/blob/standard-v6/standard/expressions.md#1110-shift-operators
// * shift operators always take int32 as the second argument
var rightConverted = ConvertToMaybeNullable(right, typeof(int), true)!;
// * the first argument is int, uint, long, ulong (in this order)
var leftConverted = ConvertToMaybeNullable(left, typeof(int), false) ?? ConvertToMaybeNullable(left, typeof(uint), false) ?? ConvertToMaybeNullable(left, typeof(long), false) ?? ConvertToMaybeNullable(left, typeof(ulong), false)!;
if (leftConverted is null)
throw new InvalidOperationException($"Cannot apply {operation} operator to type {leftType.ToCode()}. The type must be convertible to an integer or have a custom operator defined.");
return operation == ExpressionType.LeftShift ? Expression.LeftShift(leftConverted, rightConverted) : Expression.RightShift(leftConverted, rightConverted);
}

// List of types in order of precendence
var enumType = leftType.IsEnum ? leftType : rightType.IsEnum ? rightType : null;
// all operators have defined "overloads" for two
var typeList = operation switch {
ExpressionType.Or or ExpressionType.And or ExpressionType.ExclusiveOr =>
new[] { typeof(bool), enumType, typeof(int), typeof(uint), typeof(long), typeof(ulong) },
Expand Down Expand Up @@ -152,56 +163,22 @@ public static Expression GetBinaryOperator(
// if (left.Type.IsNullable() || right.Type.IsNullable())
// return GetBinaryOperator(expressionFactory, left.UnwrapNullable(), right.UnwrapNullable(), operation);

// as a fallback, try finding overridden Equals method
if (operation == ExpressionType.Equal) return EqualsMethod(expressionFactory, left, right);
if (operation == ExpressionType.NotEqual) return Expression.Not(EqualsMethod(expressionFactory, left, right));

throw new InvalidOperationException($"Cannot apply {operation} operator to types {left.Type.Name} and {right.Type.Name}.");
}

public static Expression EqualsMethod(
MemberExpressionFactory expressionFactory,
Expression left,
Expression right
)
static Expression ReferenceEquality(Expression left, Expression right, bool not)
{
Expression? equatable = null;
Expression? theOther = null;
if (typeof(IEquatable<>).IsAssignableFrom(left.Type))
{
equatable = left;
theOther = right;
}
else if (typeof(IEquatable<>).IsAssignableFrom(right.Type))
{
equatable = right;
theOther = left;
}

if (equatable != null)
// * It is a binding-time error to use the predefined reference type equality operators to compare two references that are known to be different at binding-time. For example, if the binding-time types of the operands are two class types, and if neither derives from the other, then it would be impossible for the two operands to reference the same object. Thus, the operation is considered a binding-time error.
var leftT = left.Type;
var rightT = right.Type;
if (leftT != rightT && !(leftT.IsAssignableFrom(rightT) || rightT.IsAssignableFrom(leftT)))
{
var m = expressionFactory.CallMethod(equatable, BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.FlattenHierarchy, "Equals", null, new[] { theOther! });
if (m != null) return m;
if (!leftT.IsInterface && rightT.IsInterface)
throw new InvalidOperationException($"Cannot compare types {leftT.ToCode()} and {rightT.ToCode()}, because the classes are unrelated.");
if (leftT.IsSealed || rightT.IsSealed)
throw new InvalidOperationException($"Cannot compare types {leftT.ToCode()} and {rightT.ToCode()}, because {(leftT.IsSealed ? leftT : rightT).ToCode(stripNamespace: true)} is sealed and does not implement {rightT.ToCode(stripNamespace: true)}.");
}

if (left.Type.IsValueType)
{
equatable = left;
theOther = right;
}
else if (left.Type.IsValueType)
{
equatable = right;
theOther = left;
}

if (equatable != null)
{
theOther = TypeConversion.ImplicitConversion(theOther!, equatable.Type);
if (theOther != null) return Expression.Equal(equatable, theOther);
}

return expressionFactory.CallMethod(left, BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.FlattenHierarchy, "Equals", null, new[] { right });
return not ? Expression.ReferenceNotEqual(left, right) : Expression.ReferenceEqual(left, right);
}

static Expression? ConvertToMaybeNullable(
Expand Down
4 changes: 2 additions & 2 deletions src/Samples/Tests/Tests/DotVVM.Samples.Tests.csproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="16.9.4" />
<PackageReference Include="System.Text.Encoding.CodePages" Version="5.0.0" />
<PackageReference Include="System.ValueTuple" Version="4.4.0" />
<PackageReference Include="xunit" Version="2.4.1" />
<PackageReference Include="xunit.runner.visualstudio" Version="2.4.3">
<PackageReference Include="xunit" Version="2.4.2" />
<PackageReference Include="xunit.runner.visualstudio" Version="2.4.5">
<PrivateAssets>all</PrivateAssets>
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
</PackageReference>
Expand Down
36 changes: 33 additions & 3 deletions src/Tests/Binding/BindingCompilationTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1123,11 +1123,41 @@ public void Error_DifferentDataContext()
[DataRow("DoubleProp - 1", 0.5)]
[DataRow("DoubleProp + ShortProp", short.MaxValue + 1.5)]
[DataRow("NullableDoubleProp + ShortProp", null)]
public void BindingCompiler_OperatorType(string expr, object expectedResult)
{
var vm = new TestViewModel { IntProp = 100, DoubleProp = 1.5 };
[DataRow("ByteProp | ByteProp", (byte)255)]
[DataRow("DateTime == DateTime", true)]
[DataRow("NullableTimeOnly == NullableTimeOnly", true)]
[DataRow("NullableTimeOnly != NullableTimeOnly", false)]
[DataRow("NullableTimeOnly == TimeOnly", false)]
[DataRow("EnumList[0] > EnumList[1]", false)]
[DataRow("EnumList[0] < EnumList[1]", true)]
[DataRow("EnumList[0] == 'A'", true)]
[DataRow("EnumList[0] < 'C'", true)]
[DataRow("(EnumList[1] | 'C') == 'C'", false)]
[DataRow("(EnumList[2] & 1) != 0", false)]
public void BindingCompiler_BinaryOperator_ResultType(string expr, object expectedResult)
{
var vm = new TestViewModel { IntProp = 100, DoubleProp = 1.5, EnumList = new () { TestEnum.A, TestEnum.B, TestEnum.C, TestEnum.D } };
Assert.AreEqual(expectedResult, ExecuteBinding(expr, vm));
}

[TestMethod]
[ExpectedExceptionMessageSubstring(typeof(BindingPropertyException), "Reference equality is not defined for the types 'DotVVM.Framework.Tests.Binding.TestViewModel2' and 'DotVVM.Framework.Tests.Binding.TestViewModel'")]
public void BindingCompiler_InvalidReferenceComparison() =>
ExecuteBinding("TestViewModel2 == _this", new TestViewModel());

[TestMethod]
[ExpectedExceptionMessageSubstring(typeof(BindingPropertyException), "Cannot apply Equal operator to types DateTime and Object.")]
public void BindingCompiler_InvalidStructReferenceComparison() =>
ExecuteBinding("DateTime == Time", new TestViewModel());

[TestMethod]
[ExpectedExceptionMessageSubstring(typeof(BindingPropertyException), "Cannot apply Equal operator to types DateTime and Object.")]
public void BindingCompiler_InvalidStructComparison() =>
ExecuteBinding("DateTime == Time", new TestViewModel());
[TestMethod]
[ExpectedExceptionMessageSubstring(typeof(BindingPropertyException), "Cannot apply And operator to types TestEnum and Boolean")]
public void BindingCompiler_InvalidBitAndComparison() =>
ExecuteBinding("EnumProperty & 2 == 0", new TestViewModel());
}
class TestViewModel
{
Expand Down
30 changes: 18 additions & 12 deletions src/Tests/Binding/JavascriptCompilationTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -165,19 +165,25 @@ public void JavascriptCompilation_Parent()
}

[DataTestMethod]
// [DataRow("2+2", "2+2", DisplayName = "2+2")]
// [DataRow("2+2+2", "2+2+2", DisplayName = "2+2+2")]
// [DataRow("(2+2)+2", "2+2+2", DisplayName = "(2+2)+2")]
// [DataRow("2+(2+2)", "2+(2+2)", DisplayName = "2+(2+2)")]
// [DataRow("2+(2*2)", "2+2*2", DisplayName = "2+(2*2)")]
// [DataRow("2*(2+2)", "2*(2+2)", DisplayName = "2*(2+2)")]
// [DataRow("IntProp & (2+2)", "IntProp()&2+2", DisplayName = "IntProp & (2+2)")]
// [DataRow("IntProp & 2+2", "IntProp()&2+2", DisplayName = "IntProp & 2+2")]
// [DataRow("IntProp & -1", "IntProp()&-1", DisplayName = "IntProp & -1")]
// [DataRow("'a' + 'b'", "\"ab\"", DisplayName = "'a' + 'b'")]
// [DataRow("IntProp ^ 1", "IntProp()^1", DisplayName = "IntProp ^ 1")]
// [DataRow("'xx' + IntProp", "\"xx\"+IntProp()", DisplayName = "'xx' + IntProp")]
[DataRow("2+2", "2+2", DisplayName = "2+2")]
[DataRow("2+2+2", "2+2+2", DisplayName = "2+2+2")]
[DataRow("(2+2)+2", "2+2+2", DisplayName = "(2+2)+2")]
[DataRow("2+(2+2)", "2+(2+2)", DisplayName = "2+(2+2)")]
[DataRow("2+(2*2)", "2+2*2", DisplayName = "2+(2*2)")]
[DataRow("2*(2+2)", "2*(2+2)", DisplayName = "2*(2+2)")]
[DataRow("IntProp & (2+2)", "IntProp()&2+2", DisplayName = "IntProp & (2+2)")]
[DataRow("IntProp & 2+2", "IntProp()&2+2", DisplayName = "IntProp & 2+2")]
[DataRow("IntProp & -1", "IntProp()&-1", DisplayName = "IntProp & -1")]
[DataRow("'a' + 'b'", "\"ab\"", DisplayName = "'a' + 'b'")]
[DataRow("IntProp ^ 1", "IntProp()^1", DisplayName = "IntProp ^ 1")]
[DataRow("'xx' + IntProp", "\"xx\"+IntProp()", DisplayName = "'xx' + IntProp")]
[DataRow("true == (IntProp == 1)", "true==(IntProp()==1)", DisplayName = "true == (IntProp == 1)")]
[DataRow("TestViewModel2 == null", "TestViewModel2()==null", DisplayName = "TestViewModel2 == null")]
[DataRow("NullableDateOnly == null", "NullableDateOnly()==null", DisplayName = "NullableDateOnly == null")]
[DataRow("NullableDateOnly == NullableDateOnly", "NullableDateOnly()==NullableDateOnly()", DisplayName = "NullableDateOnly == NullableDateOnly")]
[DataRow("null != StringProp", "null!=StringProp()", DisplayName = "null != StringProp")]
[DataRow("(EnumProperty & 2) == 0", "dotvvm.translations.enums.fromInt(dotvvm.translations.enums.toInt(EnumProperty(),\"nEayAzHQ5xyCfSP6\")&2,\"nEayAzHQ5xyCfSP6\")==\"A\"", DisplayName = "(EnumProperty & 2) == 0")]
[DataRow("EnumProperty == 'B'", "EnumProperty()==\"B\"", DisplayName = "EnumProperty & 2 == 0")]
public void JavascriptCompilation_BinaryExpressions(string expr, string expectedJs)
{
var js = CompileBinding(expr, new [] { typeof(TestViewModel) });
Expand Down
2 changes: 1 addition & 1 deletion src/Tests/Parser/Binding/BindingParserTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -803,7 +803,7 @@ public void BindingParser_GenericExpression_MultipleInside()
var parser = bindingParserNodeFactory.SetupParser(originalString);
var node = parser.ReadExpression();

Assert.IsTrue(node is TypeOrFunctionReferenceBindingParserNode);
Assert.IsInstanceOfType(node, typeof(TypeOrFunctionReferenceBindingParserNode));
Assert.IsTrue(string.Equals(originalString, node.ToDisplayString()));
}

Expand Down

0 comments on commit a98d13a

Please sign in to comment.