From 72dffb2dd141f62c0d77cd04391ae0f821601604 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Standa=20Luke=C5=A1?= Date: Wed, 18 Oct 2023 20:58:41 +0200 Subject: [PATCH] Fix implicit conversion in expressions like `C ? P : null` should fix part of #1694 --- .../Binding/ExpressionBuildingVisitor.cs | 40 +++++++++++++++++-- src/Tests/Binding/BindingCompilationTests.cs | 20 ++++++++++ src/Tests/Binding/ImplicitConversionTests.cs | 7 ++++ 3 files changed, 64 insertions(+), 3 deletions(-) diff --git a/src/Framework/Framework/Compilation/Binding/ExpressionBuildingVisitor.cs b/src/Framework/Framework/Compilation/Binding/ExpressionBuildingVisitor.cs index 4d387a8742..5f2f55194e 100644 --- a/src/Framework/Framework/Compilation/Binding/ExpressionBuildingVisitor.cs +++ b/src/Framework/Framework/Compilation/Binding/ExpressionBuildingVisitor.cs @@ -13,6 +13,8 @@ using DotVVM.Framework.Compilation.Parser.Binding.Parser.Annotations; using DotVVM.Framework.Binding; using DotVVM.Framework.Compilation.ControlTree.Resolved; +using FastExpressionCompiler; +using System.Diagnostics; namespace DotVVM.Framework.Compilation.Binding { @@ -302,11 +304,43 @@ protected override Expression VisitConditionalExpression(ConditionalExpressionBi if (trueExpr.Type != falseExpr.Type) { - trueExpr = TypeConversion.ImplicitConversion(trueExpr, falseExpr.Type, allowToString: true) ?? trueExpr; - falseExpr = TypeConversion.ImplicitConversion(falseExpr, trueExpr.Type, allowToString: true) ?? falseExpr; + // implicit conversions are specified at https://github.com/ljw1004/csharpspec/blob/gh-pages/expressions.md#conditional-operator + // > If x has type X and y has type Y then + // > * If an implicit conversion (Implicit conversions) exists from X to Y, but not from Y to X, then Y is the type of the conditional expression. + // > * If an implicit conversion (Implicit conversions) exists from Y to X, but not from X to Y, then X is the type of the conditional expression. + // > * Otherwise, no expression type can be determined, and a compile-time error occurs. + // + + var trueConverted = TypeConversion.ImplicitConversion(trueExpr, falseExpr.Type, allowToString: true); + var falseConverted = TypeConversion.ImplicitConversion(falseExpr, trueExpr.Type, allowToString: true); + + if (trueConverted is null && falseConverted is null) + throw new BindingCompilationException($"Type of conditional expression '{node.ToDisplayString()}' cannot be determined because there is no implicit conversion between '{trueExpr.Type.ToCode()}' and '{falseExpr.Type.ToCode()}'", node); + else if (trueConverted is null) + falseExpr = falseConverted; + else if (falseConverted is null) + trueExpr = trueConverted; + else + { + 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 + // -> if we have an ambiguity, try to solve it by preferring the more specific type + + if (trueConverted.Type == typeof(object)) + falseExpr = falseConverted; + else if (falseConverted.Type == typeof(object)) + trueExpr = trueConverted; + else if (trueConverted.Type == typeof(string)) + falseExpr = falseConverted; + else if (falseConverted.Type == typeof(string)) + 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); + } } - return Expression.Condition(condition!, trueExpr, falseExpr); + return Expression.Condition(condition!, trueExpr.NotNull(), falseExpr.NotNull()); } protected override Expression VisitMemberAccess(MemberAccessBindingParserNode node) diff --git a/src/Tests/Binding/BindingCompilationTests.cs b/src/Tests/Binding/BindingCompilationTests.cs index 3deb4eaeea..2e39037d69 100755 --- a/src/Tests/Binding/BindingCompilationTests.cs +++ b/src/Tests/Binding/BindingCompilationTests.cs @@ -824,6 +824,26 @@ public void BindingCompiler_ImplicitConversion_ConditionalLongAndInt() Assert.AreEqual(5L, result); } + [TestMethod] + public void BindingCompiler_ImplicitConversion_ConditionalStringAndNull() + { + var resultNotNull = ExecuteBinding("_this.BoolProp ? _this.StringProp : null", new [] { new TestViewModel { StringProp = "test", BoolProp = true } }, expectedType: typeof(string)); + Assert.AreEqual("test", resultNotNull); + resultNotNull = ExecuteBinding("!_this.BoolProp ? null : _this.StringProp", new [] { new TestViewModel { StringProp = "test", BoolProp = true } }, expectedType: typeof(string)); + Assert.AreEqual("test", resultNotNull); + var resultNull = ExecuteBinding("_this.BoolProp ? _this.StringProp : null", new [] { new TestViewModel { StringProp = "test", BoolProp = false } }, expectedType: typeof(string)); + Assert.IsNull(resultNull); + } + + [TestMethod] + public void BindingCompiler_ImplicitConversion_ConditionalEnumAndLiteral() + { + var result = ExecuteBinding("_this.BoolProp ? _this.EnumProperty : 'A'", new [] { new TestViewModel { EnumProperty = TestEnum.B, BoolProp = false } }, expectedType: typeof(object)); + Assert.AreEqual(TestEnum.A, result); + result = ExecuteBinding("_this.BoolProp ? 'A' : _this.EnumProperty", new [] { new TestViewModel { EnumProperty = TestEnum.B, BoolProp = false } }, expectedType: typeof(object)); + Assert.AreEqual(TestEnum.B, result); + } + [TestMethod] public void BindingCompiler_SimpleBlockExpression() { diff --git a/src/Tests/Binding/ImplicitConversionTests.cs b/src/Tests/Binding/ImplicitConversionTests.cs index 8400dac2d3..07cd948f99 100644 --- a/src/Tests/Binding/ImplicitConversionTests.cs +++ b/src/Tests/Binding/ImplicitConversionTests.cs @@ -28,6 +28,13 @@ public void Conversion_IntToDouble() TypeConversion.EnsureImplicitConversion(Expression.Parameter(typeof(int)), typeof(double)); } + [TestMethod] + public void Conversion_NullConversion() + { + TypeConversion.EnsureImplicitConversion(Expression.Constant(null, typeof(object)), typeof(string)); + TypeConversion.EnsureImplicitConversion(Expression.Constant(null, typeof(object)), typeof(int?)); + } + [TestMethod] public void Conversion_ValidToString() {