Added repro for a bug with passing commands in markup controls

src/Framework/Framework/Compilation/Javascript/ParametrizedCode.cs

@@ -50,9 +50,14 @@ public ParametrizedCode(string code, OperatorPrecedence precedence = default)
 
         // TODO(exyi): add WriteTo(StringBuilder)
         /// <summary>
-        /// Converts this to string and assigns all parameters using `parameterAssignment`. If there is any missing, exception is thrown.
+        /// Converts this to string and assigns all parameters using `parameterAssignment`.
         /// </summary>
+        /// <exception cref="MissingAssignmentException">Thrown when some parameter is not assigned and has no default value.</exception>
         public string ToString(Func<CodeSymbolicParameter, CodeParameterAssignment> parameterAssignment) => ToString(parameterAssignment, out var _);
+        /// <summary>
+        /// Converts this to string and assigns all parameters using `parameterAssignment`.
+        /// </summary>
+        /// <exception cref="MissingAssignmentException">Thrown when some parameter is not assigned and has no default value.</exception>
         public string ToString(Func<CodeSymbolicParameter, CodeParameterAssignment> parameterAssignment, out bool allIsDefault)
         {
             allIsDefault = true;
@@ -205,7 +210,7 @@ private CodeParameterAssignment[] FindAssignment(Func<CodeSymbolicParameter, Cod
                     {
                         pp[i] = parameters[i].DefaultAssignment;
                         if (pp[i].Code == null)
-                            throw new InvalidOperationException($"Assignment of parameter '{parameters[i].Parameter}' was not found.");
+                            throw new MissingAssignmentException(parameters[i], this);
                     }
                 }
                 else
@@ -229,6 +234,11 @@ public IEnumerable<CodeSymbolicParameter> EnumerateAllParameters()
             }
         }
 
+        public record MissingAssignmentException(CodeParameterInfo Parameter, ParametrizedCode FullCode): RecordExceptions.RecordException
+        {
+            public override string Message => $"Assignment of parameter '{Parameter.Parameter}' was not found.";
+        }
+
         /// <summary>
         /// Builder class with reasonably fast Add operation. Use Build method to convert it to immutable ParametrizedCode
         /// </summary>

src/Framework/Framework/Controls/KnockoutHelper.cs

@@ -9,6 +9,7 @@
 using DotVVM.Framework.Compilation.Javascript.Ast;
 using DotVVM.Framework.Configuration;
 using DotVVM.Framework.Utils;
+using FastExpressionCompiler;
 using Newtonsoft.Json;
 
 namespace DotVVM.Framework.Controls
@@ -263,18 +264,26 @@ string getHandlerScript()
 
             string SubstituteArguments(ParametrizedCode parametrizedCode)
             {
-                return parametrizedCode.ToString(p =>
-                    p == JavascriptTranslator.CurrentElementParameter ? options.ElementAccessor :
-                    p == CommandBindingExpression.CurrentPathParameter ? CodeParameterAssignment.FromIdentifier(getContextPath(control)) :
-                    p == CommandBindingExpression.ControlUniqueIdParameter ? uniqueControlId?.GetParametrizedJsExpression(control) ?? CodeParameterAssignment.FromLiteral("") :
-                    p == JavascriptTranslator.KnockoutContextParameter ? knockoutContext :
-                    p == JavascriptTranslator.KnockoutViewModelParameter ? viewModel :
-                    p == CommandBindingExpression.OptionalKnockoutContextParameter ? optionalKnockoutContext :
-                    p == CommandBindingExpression.CommandArgumentsParameter ? options.CommandArgs ?? default :
-                    p == CommandBindingExpression.PostbackHandlersParameter ? CodeParameterAssignment.FromIdentifier(getHandlerScript()) :
-                    p == CommandBindingExpression.AbortSignalParameter ? abortSignal :
-                    default
-                );
+                try
+                {
+                    return parametrizedCode.ToString(p =>
+                        p == JavascriptTranslator.CurrentElementParameter ? options.ElementAccessor :
+                        p == CommandBindingExpression.CurrentPathParameter ? CodeParameterAssignment.FromIdentifier(getContextPath(control)) :
+                        p == CommandBindingExpression.ControlUniqueIdParameter ? uniqueControlId?.GetParametrizedJsExpression(control) ?? CodeParameterAssignment.FromLiteral("") :
+                        p == JavascriptTranslator.KnockoutContextParameter ? knockoutContext :
+                        p == JavascriptTranslator.KnockoutViewModelParameter ? viewModel :
+                        p == CommandBindingExpression.OptionalKnockoutContextParameter ? optionalKnockoutContext :
+                        p == CommandBindingExpression.CommandArgumentsParameter ? options.CommandArgs ?? default :
+                        p == CommandBindingExpression.PostbackHandlersParameter ? CodeParameterAssignment.FromIdentifier(getHandlerScript()) :
+                        p == CommandBindingExpression.AbortSignalParameter ? abortSignal :
+                        default
+                    );
+                }
+                catch (ParametrizedCode.MissingAssignmentException e) when (e.Parameter.Parameter == CommandBindingExpression.CommandArgumentsParameter)
+                {
+                    var returnType = expression.GetProperty<ResultTypeBindingProperty>(ErrorHandlingMode.ReturnNull)?.Type;
+                    throw new DotvvmControlException(control, $"The binding {expression} of type {returnType?.ToCode(stripNamespace: true) ?? "?"} requires arguments, but none were provided to the KnockoutHelper.GenerateClientPostback method.", innerException: e) { RelatedBinding = expression };
+                }
             }
         }
 

src/Samples/Common/DotVVM.Samples.Common.csproj

@@ -118,6 +118,9 @@
     <None Remove="Views\FeatureSamples\LambdaExpressions\ClientSideFiltering.dothtml" />
     <None Remove="Views\FeatureSamples\LambdaExpressions\LambdaExpressions.dothtml" />
     <None Remove="Views\FeatureSamples\Localization\Globalize.dothtml" />
+    <None Remove="Views\FeatureSamples\MarkupControl\CommandAsProperty.dotcontrol" />
+    <None Remove="Views\FeatureSamples\MarkupControl\CommandAsPropertyPage.dothtml" />
+    <None Remove="Views\FeatureSamples\MarkupControl\CommandAsPropertyWrapper.dotcontrol" />
     <None Remove="Views\FeatureSamples\MarkupControl\MarkupDefinedProperties.dothtml" />
     <None Remove="Views\FeatureSamples\MarkupControl\MarkupDefinedPropertiesControl.dotcontrol" />
     <None Remove="Views\FeatureSamples\MarkupControl\ServiceDependency\ServiceDependencyControl.dotcontrol" />

src/Samples/Common/DotvvmStartup.cs

@@ -267,6 +267,8 @@ private static void AddControls(DotvvmConfiguration config)
             config.Markup.AddCodeControls("cc", typeof(Loader));
             config.Markup.AddMarkupControl("sample", "EmbeddedResourceControls_Button", "embedded://EmbeddedResourceControls/Button.dotcontrol");
             config.Markup.AddMarkupControl("cc", "NodeControl", "Views/ControlSamples/HierarchyRepeater/NodeControl.dotcontrol");
+            config.Markup.AddMarkupControl("cc", "CommandAsProperty", "Views/FeatureSamples/MarkupControl/CommandAsProperty.dotcontrol");
+            config.Markup.AddMarkupControl("cc", "CommandAsPropertyWrapper", "Views/FeatureSamples/MarkupControl/CommandAsPropertyWrapper.dotcontrol");
 
             config.Markup.AutoDiscoverControls(new DefaultControlRegistrationStrategy(config, "sample", "Views/"));
 

src/Samples/Common/ViewModels/FeatureSamples/MarkupControl/CommandAsPropertyPageViewModel.cs

@@ -0,0 +1,37 @@
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Text;
+using System.Threading.Tasks;
+using DotVVM.Framework.ViewModel;
+using DotVVM.Framework.Hosting;
+
+namespace DotVVM.Samples.Common.ViewModels.FeatureSamples.MarkupControl
+{
+    public class CommandAsPropertyPageViewModel : DotvvmViewModelBase
+    {
+
+        public List<ItemModel> Items { get; set; } = new() {
+            new ItemModel() { Name = "One", IsTrue = true },
+            new ItemModel() { Name = "Two", IsTrue = false },
+            new ItemModel() { Name = "Three", IsTrue = true }
+        };
+
+        public ItemModel SelectedItem { get; set; }
+
+        public Task MyFunction(string name, bool isTrue)
+        {
+            SelectedItem = new ItemModel() { Name = name, IsTrue = isTrue };
+            return Task.CompletedTask;
+        }
+
+
+        public class ItemModel
+        {
+            public string Name { get; set; }
+            public bool IsTrue { get; set; }
+
+        }
+    }
+}
+

src/Samples/Common/Views/FeatureSamples/MarkupControl/CommandAsProperty.cs

@@ -0,0 +1,24 @@
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Text;
+using System.Threading.Tasks;
+using DotVVM.Framework.Binding;
+using DotVVM.Framework.Controls;
+
+namespace DotVVM.Samples.Common.Views.FeatureSamples.MarkupControl
+{
+    public class CommandAsProperty : DotvvmMarkupControl
+    {
+
+        public Func<Task> Click
+        {
+            get => (Func<Task>)GetValue(ClickProperty)!;
+            set => SetValue(ClickProperty, value);
+        }
+        public static readonly DotvvmProperty ClickProperty
+                = DotvvmProperty.Register<Func<Task>, CommandAsProperty>(c => c.Click, null);
+
+    }
+}
+

src/Samples/Common/Views/FeatureSamples/MarkupControl/CommandAsProperty.dotcontrol

@@ -0,0 +1,6 @@
+@viewModel System.String, mscorlib
+@baseType DotVVM.Samples.Common.Views.FeatureSamples.MarkupControl.CommandAsProperty, DotVVM.Samples.Common
+
+<%-- both options are legit --%>
+<dot:Button Text="{value: _this}" Click="{command: _control.Click}" />
+<dot:Button Text="{value: _this}" Click="{staticCommand: _control.Click}" />

src/Samples/Common/Views/FeatureSamples/MarkupControl/CommandAsPropertyPage.dothtml

@@ -0,0 +1,33 @@
+@viewModel DotVVM.Samples.Common.ViewModels.FeatureSamples.MarkupControl.CommandAsPropertyPageViewModel, DotVVM.Samples.Common
+
+<!DOCTYPE html>
+
+<html lang="en" xmlns="http://www.w3.org/1999/xhtml">
+<head>
+    <meta charset="utf-8" />
+    <title></title>
+</head>
+<body>
+
+    <h1>Pass as command</h1>
+    <cc:CommandAsPropertyWrapper Click="{command: (string name, bool isTrue) => _root.MyFunction(name, isTrue)}" />
+    <hr />
+
+    <h1>Pass as static command</h1>
+    <cc:CommandAsPropertyWrapper Click="{staticCommand: (string name, bool isTrue) => (_root.SelectedItem.Name = name; _root.SelectedItem.IsTrue = isTrue)}" />
+    <hr />
+
+    <%--<h1>Pass as value</h1>
+    <cc:CommandAsPropertyWrapper Click="{value: (string name, bool isTrue) => _root.MyFunction(name, isTrue)}" />
+    <hr />--%>
+
+    <%--<h1>Pass as resource</h1>
+    <cc:CommandAsPropertyWrapper Click="{resource: (string name, bool isTrue) => _root.MyFunction(name, isTrue)}" />
+    <hr />--%>
+
+    <p DataContext="{value: SelectedItem}" data-ui="result">Selected item: {{value: Name}}, {{value: IsTrue}}</p>
+
+</body>
+</html>
+
+

src/Samples/Common/Views/FeatureSamples/MarkupControl/CommandAsPropertyWrapper.cs

@@ -0,0 +1,24 @@
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Text;
+using System.Threading.Tasks;
+using DotVVM.Framework.Binding;
+using DotVVM.Framework.Controls;
+
+namespace DotVVM.Samples.Common.Views.FeatureSamples.MarkupControl
+{
+    public class CommandAsPropertyWrapper : DotvvmMarkupControl
+    {
+
+        public Func<string, bool, Task> Click
+        {
+            get { return (Func<string, bool, Task>)GetValue(ClickProperty); }
+            set { SetValue(ClickProperty, value); }
+        }
+        public static readonly DotvvmProperty ClickProperty
+            = DotvvmProperty.Register<Func<string, bool, Task>, CommandAsPropertyWrapper>(c => c.Click, null);
+
+    }
+}
+

src/Samples/Common/Views/FeatureSamples/MarkupControl/CommandAsPropertyWrapper.dotcontrol

@@ -0,0 +1,25 @@
+@viewModel DotVVM.Samples.Common.ViewModels.FeatureSamples.MarkupControl.CommandAsPropertyPageViewModel, DotVVM.Samples.Common
+@baseType DotVVM.Samples.Common.Views.FeatureSamples.MarkupControl.CommandAsPropertyWrapper, DotVVM.Samples.Common
+
+<dot:Repeater DataSource="{value: Items}">
+    <div style="display: flex; flex-direction: row; gap: 4em" data-ui="button-list">
+        <div>
+            <h2>Passed as command</h2>
+            <cc:CommandAsProperty Click="{command: _control.Click(_parent.Name, _parent.IsTrue)}" DataContext="{value: Name}" />
+        </div>
+        <div>
+            <h2>Passed as static command</h2>
+            <cc:CommandAsProperty Click="{staticCommand: _control.Click(_parent.Name, _parent.IsTrue)}" DataContext="{value: Name}" />
+        </div>
+
+        <%-- remove after we fix the lambda conversion --%>
+        <div>
+            <h2>Passed as value</h2>
+            <cc:CommandAsProperty Click="{value: _control.Click(_parent.Name, _parent.IsTrue)}" DataContext="{value: Name}" />
+        </div>
+        <div>
+            <h2>Passed as resource</h2>
+            <cc:CommandAsProperty Click="{resource: _control.Click(_parent.Name, _parent.IsTrue)}" DataContext="{value: Name}" />
+        </div>
+    </div>
+</dot:Repeater>

src/Samples/Tests/Tests/Feature/MarkupControlTests.cs

@@ -409,5 +409,29 @@ public void Feature_MarkupControl_MarkupDeclaredProperties()
                 browser.WaitFor(() => AssertUI.InnerTextEquals(browser.First("[data-ui=counter]"), "2"), 2000);
             });
         }
+
+        [Fact]
+        public void Feature_MarkupControl_CommandAsPropertyPage()
+        {
+            RunInAllBrowsers(browser => {
+                browser.NavigateToUrl(SamplesRouteUrls.FeatureSamples_MarkupControl_CommandAsPropertyPage);
+
+                var lists = browser.FindElements("div[data-ui=button-list]");
+                for (var j = 0; j < 4; j++)
+                {
+                    for (var i = 0; i < lists.Count; i++)
+                    {
+                        lists[i].ElementAt("input[type=button]", j).Click();
+                        AssertUI.InnerTextEquals(browser.Single("p[data-ui=result]"), (i % 3) switch {
+                            0 => "Selected item: One, true",
+                            1 => "Selected item: Two, false",
+                            _ => "Selected item: Three, true"
+                        });
+
+                        i++;
+                    }
+                }
+            });
+        }
     }
 }