From ba34ac9c37d055149f023f2b796d34a855602de6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Herceg?= Date: Tue, 28 Nov 2023 15:22:03 +0100 Subject: [PATCH 1/2] Added a failing test `BindingCompiler_ImplicitConversion_ConditionalNullableAndNonNullable` for ternary operator nullable conversions --- src/Tests/Binding/BindingCompilationTests.cs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/Tests/Binding/BindingCompilationTests.cs b/src/Tests/Binding/BindingCompilationTests.cs index 2e39037d69..400ed1b86a 100755 --- a/src/Tests/Binding/BindingCompilationTests.cs +++ b/src/Tests/Binding/BindingCompilationTests.cs @@ -835,6 +835,17 @@ public void BindingCompiler_ImplicitConversion_ConditionalStringAndNull() Assert.IsNull(resultNull); } + [TestMethod] + public void BindingCompiler_ImplicitConversion_ConditionalNullableAndNonNullable() + { + var resultNotNull = ExecuteBinding("_this.BoolProp ? _this.NullableDoubleProp : _this.DoubleProp", new[] { new TestViewModel { NullableDoubleProp = 11.1, DoubleProp = 22.2, BoolProp = true } }, expectedType: typeof(double?)); + Assert.AreEqual(11.1, resultNotNull); + resultNotNull = ExecuteBinding("!_this.BoolProp ? _this.NullableDoubleProp : _this.DoubleProp", new[] { new TestViewModel { NullableDoubleProp = 11.1, DoubleProp = 22.2, BoolProp = true } }, expectedType: typeof(double?)); + Assert.AreEqual(22.2, resultNotNull); + var resultNull = ExecuteBinding("_this.BoolProp ? null : _this.DoubleProp", new[] { new TestViewModel { NullableDoubleProp = 11.1, DoubleProp = 22.2, BoolProp = true } }, expectedType: typeof(double?)); + Assert.IsNull(resultNull); + } + [TestMethod] public void BindingCompiler_ImplicitConversion_ConditionalEnumAndLiteral() { From 046206c19c3f72be62f606c88e9441910da3798b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Standa=20Luke=C5=A1?= Date: Wed, 29 Nov 2023 17:16:03 +0100 Subject: [PATCH 2/2] Fix ternary expression with T : T? We allow non-standard T? -> T implicit conversion, which leads to ambiguity when T : T? are in conditional expression. Special case for this has been added. --- .../Binding/ExpressionBuildingVisitor.cs | 8 ++++++++ src/Tests/Binding/BindingCompilationTests.cs | 14 +++++++++++++- 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/Framework/Framework/Compilation/Binding/ExpressionBuildingVisitor.cs b/src/Framework/Framework/Compilation/Binding/ExpressionBuildingVisitor.cs index 5f2f55194e..3d7e5b7e90 100644 --- a/src/Framework/Framework/Compilation/Binding/ExpressionBuildingVisitor.cs +++ b/src/Framework/Framework/Compilation/Binding/ExpressionBuildingVisitor.cs @@ -325,6 +325,7 @@ protected override Expression VisitConditionalExpression(ConditionalExpressionBi Debug.Assert(trueConverted.Type != falseConverted.Type); // 1. We represent some "typeless expressions" as expression of type object // 2. We allow conversion to string and also have the implicit conversion from string literal to enum + // 3. For some reason, we allow T? -> T implicit conversion as well as T -> T?, this will prefer the nullable type // -> if we have an ambiguity, try to solve it by preferring the more specific type if (trueConverted.Type == typeof(object)) @@ -335,6 +336,13 @@ protected override Expression VisitConditionalExpression(ConditionalExpressionBi falseExpr = falseConverted; else if (falseConverted.Type == typeof(string)) trueExpr = trueConverted; + else if (trueConverted.Type.UnwrapNullableType() == falseConverted.Type.UnwrapNullableType()) + { + if (trueConverted.Type.IsNullable()) + falseExpr = falseConverted; + else + trueExpr = trueConverted; + } else throw new BindingCompilationException($"Type of conditional expression '{node.ToDisplayString()}' cannot be determined because because '{trueExpr.Type.ToCode()}' and '{falseExpr.Type.ToCode()}' implicitly convert to one another", node); } diff --git a/src/Tests/Binding/BindingCompilationTests.cs b/src/Tests/Binding/BindingCompilationTests.cs index 400ed1b86a..8ad41f60e9 100755 --- a/src/Tests/Binding/BindingCompilationTests.cs +++ b/src/Tests/Binding/BindingCompilationTests.cs @@ -842,8 +842,20 @@ public void BindingCompiler_ImplicitConversion_ConditionalNullableAndNonNullable Assert.AreEqual(11.1, resultNotNull); resultNotNull = ExecuteBinding("!_this.BoolProp ? _this.NullableDoubleProp : _this.DoubleProp", new[] { new TestViewModel { NullableDoubleProp = 11.1, DoubleProp = 22.2, BoolProp = true } }, expectedType: typeof(double?)); Assert.AreEqual(22.2, resultNotNull); - var resultNull = ExecuteBinding("_this.BoolProp ? null : _this.DoubleProp", new[] { new TestViewModel { NullableDoubleProp = 11.1, DoubleProp = 22.2, BoolProp = true } }, expectedType: typeof(double?)); + var resultNull = ExecuteBinding("_this.BoolProp ? null : _this.DoubleProp", new[] { new TestViewModel { NullableDoubleProp = 11.1, DoubleProp = 22.2, BoolProp = true } }, expectedType: typeof(object)); Assert.IsNull(resultNull); + + } + + [TestMethod] + public void BindingCompiler_ImplicitConversion_ConditionalNullableAndNonNullable_IntDouble() + { + var resultNotNull = ExecuteBinding("_this.BoolProp ? _this.NullableDoubleProp : _this.IntProp", new[] { new TestViewModel { NullableDoubleProp = 11.1, IntProp = 1, BoolProp = true } }); + Assert.AreEqual(11.1, resultNotNull); + resultNotNull = ExecuteBinding("!_this.BoolProp ? _this.NullableDoubleProp : _this.IntProp", new[] { new TestViewModel { NullableDoubleProp = 11.1, IntProp = 1, BoolProp = true } }, expectedType: typeof(double?)); + Assert.AreEqual(1.0, resultNotNull); + resultNotNull = ExecuteBinding("_this.BoolProp ? _this.NullableIntProp : _this.DoubleProp", new[] { new TestViewModel { DoubleProp = 11.1, NullableIntProp = 1, BoolProp = true } }, expectedType: typeof(double?)); + Assert.AreEqual(1.0, resultNotNull); } [TestMethod]