lambda fixes

.gitattributes

@@ -0,0 +1,2 @@
+# Force LF for package-lock files - not all version of NPM detect line endings.
+package-lock.json text eol=lf

src/Microsoft.AspNetCore.Blazor.Razor.Extensions/BlazorDesignTimeNodeWriter.cs

@@ -425,30 +425,27 @@ public override void WriteComponentAttribute(CodeRenderingContext context, Compo
             }
             else if (node.BoundAttribute?.IsDelegateProperty() ?? false)
             {
-                // See the runtime version of this code for a thorough description of what we're doing here
+                // We always surround the expression with the delegate constructor. This makes type
+                // inference inside lambdas, and method group conversion do the right thing.
+                IntermediateToken token = null;
                 if ((cSharpNode = node.Children[0] as CSharpExpressionIntermediateNode) != null)
                 {
-                    // This is an escaped event handler
-                    context.CodeWriter.Write(DesignTimeVariable);
-                    context.CodeWriter.Write(" = ");
-                    context.CodeWriter.Write("new ");
-                    context.CodeWriter.Write(node.BoundAttribute.TypeName);
-                    context.CodeWriter.Write("(");
-                    context.CodeWriter.WriteLine();
-                    WriteCSharpToken(context, ((IntermediateToken)cSharpNode.Children[0]));
-                    context.CodeWriter.Write(");");
-                    context.CodeWriter.WriteLine();
+                    token = cSharpNode.Children[0] as IntermediateToken;
                 }
                 else
+                {
+                    token = node.Children[0] as IntermediateToken;
+                }
+
+                if (token != null)
                 {
                     context.CodeWriter.Write(DesignTimeVariable);
                     context.CodeWriter.Write(" = ");
                     context.CodeWriter.Write("new ");
                     context.CodeWriter.Write(node.BoundAttribute.TypeName);
                     context.CodeWriter.Write("(");
-                    context.CodeWriter.Write(node.BoundAttribute.GetDelegateSignature());
-                    context.CodeWriter.Write(" => ");
-                    WriteCSharpToken(context, ((IntermediateToken)node.Children[0]));
+                    context.CodeWriter.WriteLine();
+                    WriteCSharpToken(context, token);
                     context.CodeWriter.Write(");");
                     context.CodeWriter.WriteLine();
                 }

src/Microsoft.AspNetCore.Blazor.Razor.Extensions/BlazorExtensionInitializer.cs

@@ -66,6 +66,7 @@ public static void Register(RazorProjectEngineBuilder builder)
             builder.Features.Add(new ConfigureBlazorCodeGenerationOptions());
 
             builder.Features.Add(new ComponentDocumentClassifierPass());
+            builder.Features.Add(new ComplexAttributeContentPass());
             builder.Features.Add(new ComponentLoweringPass());
 
             builder.Features.Add(new ComponentTagHelperDescriptorProvider());

src/Microsoft.AspNetCore.Blazor.Razor.Extensions/BlazorRuntimeNodeWriter.cs

@@ -447,39 +447,24 @@ public override void WriteComponentAttribute(CodeRenderingContext context, Compo
             }
             else if (node.BoundAttribute?.IsDelegateProperty() ?? false)
             {
-                // This is a UIEventHandler property. We do some special code generation for this
-                // case so that it's easier to write for common cases.
-                //
-                // Example: 
-                //      <MyComponent OnClick="Foo()"/> 
-                //      --> builder.AddAttribute(X, "OnClick", new UIEventHandler((e) => Foo()));
-                //
-                // The constructor is important because we want to put type inference into a state where
-                // we know the delegate's type should be UIEventHandler. AddAttribute has an overload that
-                // accepts object, so without the 'new UIEventHandler' things will get ugly.
-                //
-                // The escape for this behavior is to prefix the expression with @. This is similar to
-                // how escaping works for ModelExpression in MVC.
-                // Example: 
-                //      <MyComponent OnClick="@Foo"/> 
-                //      --> builder.AddAttribute(X, "OnClick", new UIEventHandler(Foo));
+                // We always surround the expression with the delegate constructor. This makes type
+                // inference inside lambdas, and method group conversion do the right thing.
+                IntermediateToken token = null;
                 if ((cSharpNode = node.Children[0] as CSharpExpressionIntermediateNode) != null)
                 {
-                    // This is an escaped event handler;
-                    context.CodeWriter.Write("new ");
-                    context.CodeWriter.Write(node.BoundAttribute.TypeName);
-                    context.CodeWriter.Write("(");
-                    context.CodeWriter.Write(((IntermediateToken)cSharpNode.Children[0]).Content);
-                    context.CodeWriter.Write(")");
+                    token = cSharpNode.Children[0] as IntermediateToken;
                 }
                 else
+                {
+                    token = node.Children[0] as IntermediateToken;
+                }
+
+                if (token != null)
                 {
                     context.CodeWriter.Write("new ");
                     context.CodeWriter.Write(node.BoundAttribute.TypeName);
                     context.CodeWriter.Write("(");
-                    context.CodeWriter.Write(node.BoundAttribute.GetDelegateSignature());
-                    context.CodeWriter.Write(" => ");
-                    context.CodeWriter.Write(((IntermediateToken)node.Children[0]).Content);
+                    context.CodeWriter.Write(token.Content);
                     context.CodeWriter.Write(")");
                 }
             }

src/Microsoft.AspNetCore.Blazor.Razor.Extensions/ComplexAttributeContentPass.cs

@@ -0,0 +1,101 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using Microsoft.AspNetCore.Razor.Language;
+using Microsoft.AspNetCore.Razor.Language.Intermediate;
+
+namespace Microsoft.AspNetCore.Blazor.Razor
+{
+    // We don't support 'complex' content for components (mixed C# and markup) right now.
+    // It's not clear yet if Blazor will have a good scenario to use these constructs.
+    //
+    // This is where a lot of the complexity in the Razor/TagHelpers model creeps in and we
+    // might be able to avoid it if these features aren't needed.
+    internal class ComplexAttributeContentPass : IntermediateNodePassBase, IRazorOptimizationPass
+    {
+        // Run before other Blazor passes
+        public override int Order => -1000;
+
+        protected override void ExecuteCore(RazorCodeDocument codeDocument, DocumentIntermediateNode documentNode)
+        {
+            var nodes = documentNode.FindDescendantNodes<TagHelperIntermediateNode>();
+            for (var i = 0; i < nodes.Count; i++)
+            {
+                ProcessAttributes(nodes[i]);
+            }
+        }
+
+        private void ProcessAttributes(TagHelperIntermediateNode node)
+        {
+            for (var i = node.Children.Count - 1; i >= 0; i--)
+            {
+                if (node.Children[i] is TagHelperPropertyIntermediateNode propertyNode)
+                {
+                    if (HasComplexChildContent(propertyNode))
+                    {
+                        node.Diagnostics.Add(BlazorDiagnosticFactory.Create_UnsupportedComplexContent(
+                            propertyNode,
+                            propertyNode.AttributeName));
+                        node.Children.RemoveAt(i);
+                        continue;
+                    }
+
+                    node.Children[i] = new ComponentAttributeExtensionNode(propertyNode);
+                }
+                else if (node.Children[i] is TagHelperHtmlAttributeIntermediateNode htmlNode)
+                {
+                    if (HasComplexChildContent(htmlNode))
+                    {
+                        node.Diagnostics.Add(BlazorDiagnosticFactory.Create_UnsupportedComplexContent(
+                            htmlNode,
+                            htmlNode.AttributeName));
+                        node.Children.RemoveAt(i);
+                        continue;
+                    }
+                }
+            }
+        }
+
+        private static bool HasComplexChildContent(IntermediateNode node)
+        {
+            if (node.Children.Count == 1 &&
+                node.Children[0] is HtmlAttributeIntermediateNode htmlNode &&
+                htmlNode.Children.Count > 1)
+            {
+                // This case can be hit for a 'string' attribute
+                return true;
+            }
+            else if (node.Children.Count == 1 &&
+                node.Children[0] is CSharpExpressionIntermediateNode cSharpNode &&
+                cSharpNode.Children.Count > 1)
+            {
+                // This case can be hit when the attribute has an explicit @ inside, which
+                // 'escapes' any special sugar we provide for codegen.
+                //
+                // There's a special case here for explicit expressions. See https://github.com/aspnet/Razor/issues/2203
+                // handling this case as a tactical matter since it's important for lambdas.
+                if (cSharpNode.Children.Count == 3 &&
+                    cSharpNode.Children[0] is IntermediateToken token0 &&
+                    cSharpNode.Children[2] is IntermediateToken token2 &&
+                    token0.Content == "(" &&
+                    token2.Content == ")")
+                {
+                    cSharpNode.Children.RemoveAt(2);
+                    cSharpNode.Children.RemoveAt(0);
+
+                    // We were able to simplify it, all good.
+                    return false;
+                }
+
+                return true;
+            }
+            else if (node.Children.Count > 1)
+            {
+                // This is the common case for 'mixed' content
+                return true;
+            }
+
+            return false;
+        }
+    }
+}

src/Microsoft.AspNetCore.Blazor.Razor.Extensions/ComponentTagHelperDescriptorProvider.cs

@@ -114,11 +114,7 @@ private TagHelperDescriptor CreateDescriptor(INamedTypeSymbol type)
 
                     if (property.kind == PropertyKind.Delegate)
                     {
-                        var propertyType = (INamedTypeSymbol)property.property.Type;
-                        var parameters = propertyType.DelegateInvokeMethod.Parameters;
-
-                        var signature = "(" + string.Join(", ", parameters.Select(p => p.Name)) + ")";
-                        pb.Metadata.Add(DelegateSignatureMetadata, signature);
+                        pb.Metadata.Add(DelegateSignatureMetadata, bool.TrueString);
                     }
 
                     xml = property.property.GetDocumentationCommentXml();

src/Microsoft.AspNetCore.Blazor.Razor.Extensions/TagHelperBoundAttributeDescriptorExtensions.cs

@@ -16,19 +16,9 @@ public static bool IsDelegateProperty(this BoundAttributeDescriptor attribute)
             }
 
             var key = ComponentTagHelperDescriptorProvider.DelegateSignatureMetadata;
-            return attribute.Metadata.TryGetValue(key, out var value);
-        }
-
-        public static string GetDelegateSignature(this BoundAttributeDescriptor attribute)
-        {
-            if (attribute == null)
-            {
-                throw new ArgumentNullException(nameof(attribute));
-            }
-
-            var key = ComponentTagHelperDescriptorProvider.DelegateSignatureMetadata;
-            attribute.Metadata.TryGetValue(key, out var value);
-            return value;
+            return 
+                attribute.Metadata.TryGetValue(key, out var value) &&
+                string.Equals(value, bool.TrueString);
         }
     }
 }

test/Microsoft.AspNetCore.Blazor.Build.Test/ComponentRenderingRazorIntegrationTest.cs

@@ -153,8 +153,15 @@ void IComponent.SetParameters(ParameterCollection parameters)
         }
 
 
-        [Fact]
-        public void Render_ChildComponent_WithLambdaEventHandler()
+        [Theory]
+        [InlineData("e => Increment(e)")]
+        [InlineData("(e) => Increment(e)")]
+        [InlineData("@(e => Increment(e))")]
+        [InlineData("@(e => { Increment(e); })")]
+        [InlineData("Increment")]
+        [InlineData("@Increment")]
+        [InlineData("@(Increment)")]
+        public void Render_ChildComponent_WithEventHandler(string expression)
         {
             // Arrange
             AdditionalSyntaxTrees.Add(CSharpSyntaxTree.ParseText(@"
@@ -171,16 +178,17 @@ public class MyComponent : BlazorComponent
 }
 "));
 
-            var component = CompileToComponent(@"
+            var component = CompileToComponent($@"
 @addTagHelper *, TestAssembly
-<MyComponent OnClick=""Increment()""/>
+@using Microsoft.AspNetCore.Blazor
+<MyComponent OnClick=""{expression}""/>
 
-@functions {
+@functions {{
     private int counter;
-    private void Increment() {
+    private void Increment(UIEventArgs e) {{
         counter++;
-    }
-}");
+    }}
+}}");
 
             // Act
             var frames = GetRenderTree(component);

test/Microsoft.AspNetCore.Blazor.Build.Test/DesignTimeCodeGenerationRazorIntegrationTest.cs

@@ -268,7 +268,7 @@ public class MyComponent : BlazorComponent
             // Act
             var generated = CompileToCSharp(@"
 @addTagHelper *, TestAssembly
-<MyComponent OnClick=""Increment()""/>
+<MyComponent OnClick=""@((e) => { Increment(); })""/>
 
 @functions {
     private int counter;
@@ -308,9 +308,9 @@ protected override void BuildRenderTree(Microsoft.AspNetCore.Blazor.RenderTree.R
         {
             base.BuildRenderTree(builder);
 
-            __o = new Microsoft.AspNetCore.Blazor.UIEventHandler((eventArgs) => 
+            __o = new Microsoft.AspNetCore.Blazor.UIEventHandler(
 #line 2 ""x:\dir\subdir\Test\TestComponent.cshtml""
-                      Increment()
+                        (e) => { Increment(); }
 
 #line default
 #line hidden

test/Microsoft.AspNetCore.Blazor.Build.Test/RuntimeCodeGenerationRazorIntegrationTest.cs

@@ -248,7 +248,7 @@ public class MyComponent : BlazorComponent
             // Act
             var generated = CompileToCSharp(@"
 @addTagHelper *, TestAssembly
-<MyComponent OnClick=""Increment()""/>
+<MyComponent OnClick=""@(e => { Increment(); })""/>
 
 @functions {
     private int counter;
@@ -277,7 +277,7 @@ protected override void BuildRenderTree(Microsoft.AspNetCore.Blazor.RenderTree.R
         {
             base.BuildRenderTree(builder);
             builder.OpenComponent<Test.MyComponent>(0);
-            builder.AddAttribute(1, ""OnClick"", new Microsoft.AspNetCore.Blazor.UIEventHandler((eventArgs) => Increment()));
+            builder.AddAttribute(1, ""OnClick"", new Microsoft.AspNetCore.Blazor.UIEventHandler(e => { Increment(); }));
             builder.CloseComponent();
             builder.AddContent(2, ""\n\n"");
         }