From cce018f60270e0ef401639c113499d74bc56b3b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andrej=20=C4=8Ci=C5=BEm=C3=A1rik?= Date: Wed, 14 Apr 2021 15:42:10 +0200 Subject: [PATCH 1/2] Fixed issue when compiling lambdas to Actions --- .../Binding/BindingExpressionBuilder.cs | 8 ++--- .../Binding/ExpressionBuildingVisitor.cs | 32 +++++++++++++++++-- .../Compilation/IBindingExpressionBuilder.cs | 3 +- 3 files changed, 36 insertions(+), 7 deletions(-) diff --git a/src/DotVVM.Framework/Compilation/Binding/BindingExpressionBuilder.cs b/src/DotVVM.Framework/Compilation/Binding/BindingExpressionBuilder.cs index bfb368a5bf..003702ed6d 100644 --- a/src/DotVVM.Framework/Compilation/Binding/BindingExpressionBuilder.cs +++ b/src/DotVVM.Framework/Compilation/Binding/BindingExpressionBuilder.cs @@ -26,7 +26,7 @@ public BindingExpressionBuilder(CompiledAssemblyCache compiledAssemblyCache, Ext this.extensionMethodsCache = extensionMethodsCache; } - public Expression Parse(string expression, DataContextStack dataContexts, BindingParserOptions options, params KeyValuePair[] additionalSymbols) + public Expression Parse(string expression, DataContextStack dataContexts, BindingParserOptions options, Type expectedType = null, params KeyValuePair[] additionalSymbols) { try { @@ -54,7 +54,7 @@ public Expression Parse(string expression, DataContextStack dataContexts, Bindin symbols = symbols.AddSymbols(options.ExtensionParameters.Select(p => CreateParameter(dataContexts, p.Identifier, p))); symbols = symbols.AddSymbols(additionalSymbols); - var visitor = new ExpressionBuildingVisitor(symbols, memberExpressionFactory); + var visitor = new ExpressionBuildingVisitor(symbols, memberExpressionFactory, expectedType); visitor.Scope = symbols.Resolve(options.ScopeParameter); return visitor.Visit(node); } @@ -143,10 +143,10 @@ public static Expression ParseWithLambdaConversion(this IBindingExpressionBuilde extensionParameter: new TypeConversion.MagicLambdaConversionExtensionParameter(index, p.Name, p.ParameterType))) )) .ToArray(); - return builder.Parse(expression, dataContexts, options, additionalSymbols.Concat(delegateSymbols).ToArray()); + return builder.Parse(expression, dataContexts, options, expectedType, additionalSymbols.Concat(delegateSymbols).ToArray()); } else - return builder.Parse(expression, dataContexts, options, additionalSymbols); + return builder.Parse(expression, dataContexts, options, expectedType, additionalSymbols); } } } diff --git a/src/DotVVM.Framework/Compilation/Binding/ExpressionBuildingVisitor.cs b/src/DotVVM.Framework/Compilation/Binding/ExpressionBuildingVisitor.cs index 4b18d7fb3b..b43d1fd7c1 100644 --- a/src/DotVVM.Framework/Compilation/Binding/ExpressionBuildingVisitor.cs +++ b/src/DotVVM.Framework/Compilation/Binding/ExpressionBuildingVisitor.cs @@ -9,6 +9,7 @@ using System.Threading.Tasks; using System.Collections.Immutable; using System.Diagnostics.CodeAnalysis; +using System.Reflection; namespace DotVVM.Framework.Compilation.Binding { @@ -17,15 +18,18 @@ public class ExpressionBuildingVisitor : BindingParserNodeVisitor public TypeRegistry Registry { get; set; } public Expression? Scope { get; set; } public bool ResolveOnlyTypeName { get; set; } + public Type? ExpectedType { get; set; } public ImmutableDictionary Variables { get; set; } = ImmutableDictionary.Empty; + private int expressionDepth; private List? currentErrors; private readonly MemberExpressionFactory memberExpressionFactory; - public ExpressionBuildingVisitor(TypeRegistry registry, MemberExpressionFactory memberExpressionFactory) + public ExpressionBuildingVisitor(TypeRegistry registry, MemberExpressionFactory memberExpressionFactory, Type? expectedType = null) { Registry = registry; + ExpectedType = expectedType; this.memberExpressionFactory = memberExpressionFactory; } @@ -90,6 +94,7 @@ public override Expression Visit(BindingParserNode node) var errors = currentErrors; try { + expressionDepth++; ThrowIfNotTypeNameRelevant(node); return base.Visit(node); } @@ -97,6 +102,7 @@ public override Expression Visit(BindingParserNode node) { currentErrors = errors; Registry = regBackup; + expressionDepth--; } } @@ -317,7 +323,7 @@ protected override Expression VisitLambda(LambdaBindingParserNode node) var body = Visit(node.BodyExpression); ThrowOnErrors(); - return Expression.Lambda(body, lambdaParameters); + return CreateLambdaExpression(body, lambdaParameters); } protected override Expression VisitLambdaParameter(LambdaParameterBindingParserNode node) @@ -329,6 +335,28 @@ protected override Expression VisitLambdaParameter(LambdaParameterBindingParserN return Expression.Parameter(parameterType, node.Name.ToDisplayString()); } + private Expression CreateLambdaExpression(Expression body, ParameterExpression[] parameters) + { + // Check if we have type information available + if (expressionDepth == 1 && ExpectedType != null && ReflectionUtils.IsDelegate(ExpectedType)) + { + if (ExpectedType.GetMethod("Invoke", BindingFlags.Public | BindingFlags.Instance).ReturnType == typeof(void)) + { + // This should be an Action<...> + // We must validate that lambda body contains a valid statement + if ((body.NodeType != ExpressionType.Default) && (body.NodeType != ExpressionType.Block) && (body.NodeType != ExpressionType.Call) && (body.NodeType != ExpressionType.Assign)) + throw new DotvvmCompilationException($"Only method invocations and assignments can be used as statements."); + + // Make sure the result type will be void by adding an empty expression + return Expression.Lambda(Expression.Block(body, Expression.Empty()), parameters); + } + } + + // This should be an Func<...> + // Also this is the default behaviour if no further type information was provided + return Expression.Lambda(body, parameters); + } + protected override Expression VisitBlock(BlockBindingParserNode node) { var left = HandleErrors(node.FirstExpression, Visit) ?? Expression.Default(typeof(void)); diff --git a/src/DotVVM.Framework/Compilation/IBindingExpressionBuilder.cs b/src/DotVVM.Framework/Compilation/IBindingExpressionBuilder.cs index f1bf606a0c..383ae88694 100644 --- a/src/DotVVM.Framework/Compilation/IBindingExpressionBuilder.cs +++ b/src/DotVVM.Framework/Compilation/IBindingExpressionBuilder.cs @@ -1,4 +1,5 @@ #nullable enable +using System; using System.Collections.Generic; using System.Linq.Expressions; using DotVVM.Framework.Compilation.ControlTree; @@ -8,6 +9,6 @@ namespace DotVVM.Framework.Compilation { public interface IBindingExpressionBuilder { - Expression Parse(string expression, DataContextStack dataContexts, BindingParserOptions options, params KeyValuePair[] additionalSymbols); + Expression Parse(string expression, DataContextStack dataContexts, BindingParserOptions options, Type? expectedType = null, params KeyValuePair[] additionalSymbols); } } From 1f0674e5185d9ca5ef87f4e364c1dca1b2ee2d0d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andrej=20=C4=8Ci=C5=BEm=C3=A1rik?= Date: Wed, 14 Apr 2021 15:43:12 +0200 Subject: [PATCH 2/2] Added lambda to Action tests --- .../Binding/BindingCompilationTests.cs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/DotVVM.Framework.Tests.Common/Binding/BindingCompilationTests.cs b/src/DotVVM.Framework.Tests.Common/Binding/BindingCompilationTests.cs index 2123635531..9825c88ba5 100755 --- a/src/DotVVM.Framework.Tests.Common/Binding/BindingCompilationTests.cs +++ b/src/DotVVM.Framework.Tests.Common/Binding/BindingCompilationTests.cs @@ -205,6 +205,19 @@ public void BindingCompiler_Invalid_LambdaParameters(string expr) Assert.ThrowsException(() => ExecuteBinding(expr, viewModel)); } + [TestMethod] + [DataRow("(TestViewModel vm) => vm.IntProp = 11")] + [DataRow("(TestViewModel vm) => vm.GetEnum()")] + [DataRow("(TestViewModel vm) => ()")] + [DataRow("(TestViewModel vm) => ;")] + public void BindingCompiler_Valid_LambdaToAction(string expr) + { + var viewModel = new TestViewModel(); + var binding = ExecuteBinding(expr, new[] { viewModel }, null, expectedType: typeof(Action)) as Action; + Assert.AreEqual(typeof(Action), binding.GetType()); + binding.Invoke(viewModel); + } + [TestMethod] public void BindingCompiler_Valid_ExtensionMethods() {