We weren't handling a few cases that can occur during typing correctly.
Our passes that look at the content of attributes need to be prepared
for it to be empty in cases where the attribute has been partially
typed in the editor.

I added a smoke test for this that attempts to simulate typing and found
another issue to fix.

The end result of this is that the design for this kind of code is
simpler and takes a more 'brute-force' approach to understanding
attributes. I think this is a good change based on the problems with how
this code has been written today, there are too many possible cases to
try and have the code express and document them all.
This commit is contained in:
Ryan Nowak 2018-05-08 12:07:06 -07:00 committed by Ryan Nowak
parent 37788f3c9d
commit 3f5d25d314
13 changed files with 389 additions and 111 deletions

View File

@ -2,6 +2,7 @@
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
using System;
using System.Collections.Generic;
using System.Linq;
using Microsoft.AspNetCore.Blazor.Shared;
using Microsoft.AspNetCore.Razor.Language;
@ -368,73 +369,73 @@ namespace Microsoft.AspNetCore.Blazor.Razor
// to handle here, since there are a few different cases for how an attribute might be structured.
//
// This roughly follows the design of the runtime writer for simplicity.
HtmlContentIntermediateNode htmlNode;
CSharpExpressionIntermediateNode cSharpNode;
if (node.AttributeStructure == AttributeStructure.Minimized)
{
// Do nothing
}
else if (node.Children.Count != 1)
else if (node.Children.Count > 1)
{
// We don't expect this to happen, we just want to know if it can.
throw new InvalidOperationException("Attribute nodes should either be minimized or a single type of content." + node.Children[0].ToString());
throw new InvalidOperationException("Attribute nodes should either be minimized or a single type of content." + string.Join(", ", node.Children));
}
else if (node.BoundAttribute?.IsDelegateProperty() ?? false)
{
var tokens = node.Children;
if ((cSharpNode = node.Children[0] as CSharpExpressionIntermediateNode) != null)
{
tokens = node.Children[0].Children;
}
// We always surround the expression with the delegate constructor. This makes type
// inference inside lambdas, and method group conversion do the right thing.
context.CodeWriter.Write(DesignTimeVariable);
context.CodeWriter.Write(" = ");
context.CodeWriter.Write("new ");
context.CodeWriter.Write(node.BoundAttribute.TypeName);
context.CodeWriter.Write("(");
context.CodeWriter.WriteLine();
for (var i = 0; i < tokens.Count; i++)
{
WriteCSharpToken(context, (IntermediateToken)tokens[i]);
}
context.CodeWriter.Write(");");
context.CodeWriter.WriteLine();
}
else if ((cSharpNode = node.Children[0] as CSharpExpressionIntermediateNode) != null)
{
// This is the case when an attribute has an explicit C# transition like:
// <MyComponent Foo="@bar" />
context.CodeWriter.Write(DesignTimeVariable);
context.CodeWriter.Write(" = ");
for (var i = 0; i < cSharpNode.Children.Count; i++)
{
WriteCSharpToken(context, (IntermediateToken)cSharpNode.Children[i]);
}
context.CodeWriter.Write(";");
context.CodeWriter.WriteLine();
}
else if ((htmlNode = node.Children[0] as HtmlContentIntermediateNode) != null)
else if (node.Children.Count == 1 && node.Children[0] is HtmlContentIntermediateNode)
{
// Do nothing
}
else if (node.Children[0] is IntermediateToken token && token.IsCSharp)
else
{
context.CodeWriter.Write(DesignTimeVariable);
context.CodeWriter.Write(" = ");
// There are a few different forms that could be used to contain all of the tokens, but we don't really care
// exactly what it looks like - we just want all of the content.
//
// This can include an empty list in some cases like the following (sic):
// <MyComponent Value="
//
// Or a CSharpExpressionIntermediateNode when the attribute has an explicit transition like:
// <MyComponent Value="@value" />
//
// Of a list of tokens directly in the attribute.
var tokens = GetCSharpTokens(node);
for (var i = 0; i < node.Children.Count; i++)
if (node.BoundAttribute?.IsDelegateProperty() ?? false)
{
WriteCSharpToken(context, (IntermediateToken)node.Children[i]);
}
// We always surround the expression with the delegate constructor. This makes type
// inference inside lambdas, and method group conversion do the right thing.
context.CodeWriter.Write(DesignTimeVariable);
context.CodeWriter.Write(" = ");
context.CodeWriter.Write("new ");
context.CodeWriter.Write(node.BoundAttribute.TypeName);
context.CodeWriter.Write("(");
context.CodeWriter.WriteLine();
context.CodeWriter.Write(";");
context.CodeWriter.WriteLine();
for (var i = 0; i < tokens.Count; i++)
{
WriteCSharpToken(context, tokens[i]);
}
context.CodeWriter.Write(");");
context.CodeWriter.WriteLine();
}
else
{
// This is the case when an attribute has an explicit C# transition like:
// <MyComponent Foo="@bar" />
context.CodeWriter.Write(DesignTimeVariable);
context.CodeWriter.Write(" = ");
for (var i = 0; i < tokens.Count; i++)
{
WriteCSharpToken(context, tokens[i]);
}
context.CodeWriter.Write(";");
context.CodeWriter.WriteLine();
}
}
IReadOnlyList<IntermediateToken> GetCSharpTokens(ComponentAttributeExtensionNode attribute)
{
// We generally expect all children to be CSharp, this is here just in case.
return attribute.FindDescendantNodes<IntermediateToken>().Where(t => t.IsCSharp).ToArray();
}
}

View File

@ -347,63 +347,50 @@ namespace Microsoft.AspNetCore.Blazor.Razor
// Minimized attributes always map to 'true'
context.CodeWriter.Write("true");
}
else if (node.BoundAttribute?.IsDelegateProperty() ?? false)
else if (node.Children.Count > 1)
{
// 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 ((node.Children[0] as CSharpExpressionIntermediateNode) != null)
{
token = node.Children[0].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(token.Content);
context.CodeWriter.Write(")");
}
// We don't expect this to happen, we just want to know if it can.
throw new InvalidOperationException("Attribute nodes should either be minimized or a single type of content." + string.Join(", ", node.Children));
}
else if (node.Children[0] is CSharpExpressionIntermediateNode cSharpNode)
{
// We don't allow mixed content in component attributes. If this happens, then
// we should make sure that all of the tokens are the same kind. We report an
// error if user code tries to do this, so this check is to catch bugs in the
// compiler.
for (var i = 0; i < cSharpNode.Children.Count; i++)
{
var token = (IntermediateToken)cSharpNode.Children[i];
if (!token.IsCSharp)
{
throw new InvalidOperationException("Unexpected mixed content in a component.");
}
context.CodeWriter.Write(token.Content);
}
}
else if (node.Children[0] is HtmlContentIntermediateNode htmlNode)
else if (node.Children.Count == 1 && node.Children[0] is HtmlContentIntermediateNode htmlNode)
{
// This is how string attributes are lowered by default, a single HTML node with a single HTML token.
context.CodeWriter.WriteStringLiteral(((IntermediateToken)htmlNode.Children[0]).Content);
}
else if (node.Children[0] is IntermediateToken token)
{
// This is what we expect for non-string nodes.
context.CodeWriter.Write(((IntermediateToken)node.Children[0]).Content);
}
else
{
throw new InvalidOperationException("Unexpected node type " + node.Children[0].GetType().FullName);
// See comments in BlazorDesignTimeNodeWriter for a description of the cases that are possible.
var tokens = GetCSharpTokens(node);
if (node.BoundAttribute?.IsDelegateProperty() ?? false)
{
context.CodeWriter.Write("new ");
context.CodeWriter.Write(node.BoundAttribute.TypeName);
context.CodeWriter.Write("(");
for (var i = 0; i < tokens.Count; i++)
{
context.CodeWriter.Write(tokens[i].Content);
}
context.CodeWriter.Write(")");
}
else
{
for (var i = 0; i < tokens.Count; i++)
{
context.CodeWriter.Write(tokens[i].Content);
}
}
}
context.CodeWriter.Write(");");
context.CodeWriter.WriteLine();
IReadOnlyList<IntermediateToken> GetCSharpTokens(ComponentAttributeExtensionNode attribute)
{
// We generally expect all children to be CSharp, this is here just in case.
return attribute.FindDescendantNodes<IntermediateToken>().Where(t => t.IsCSharp).ToArray();
}
}
public override void WriteReferenceCapture(CodeRenderingContext context, RefExtensionNode node)

View File

@ -125,7 +125,6 @@ namespace Microsoft.AspNetCore.Blazor.Razor
Content = $"{BlazorApi.BindMethods.GetEventHandlerValue}<{eventArgsType}>(",
Kind = TokenKind.CSharp
},
original,
new IntermediateToken()
{
Content = $")",
@ -133,6 +132,11 @@ namespace Microsoft.AspNetCore.Blazor.Razor
}
};
for (var i = 0; i < original.Count; i++)
{
tokens.Insert(i + 1, original[i]);
}
if (parent is HtmlElementIntermediateNode)
{
var result = new HtmlAttributeIntermediateNode()
@ -172,25 +176,20 @@ namespace Microsoft.AspNetCore.Blazor.Razor
}
}
private static IntermediateToken GetAttributeContent(TagHelperPropertyIntermediateNode node)
private static IReadOnlyList<IntermediateToken> GetAttributeContent(TagHelperPropertyIntermediateNode node)
{
if (node.Children[0] is HtmlContentIntermediateNode htmlContentNode)
if (node.Children.Count == 1 && node.Children[0] is HtmlContentIntermediateNode htmlContentNode)
{
// This case can be hit for a 'string' attribute. We want to turn it into
// an expression.
var content = "\"" + ((IntermediateToken)htmlContentNode.Children.Single()).Content + "\"";
return new IntermediateToken() { Content = content, Kind = TokenKind.CSharp, };
}
else if (node.Children[0] is CSharpExpressionIntermediateNode cSharpNode)
{
// This case can be hit when the attribute has an explicit @ inside, which
// 'escapes' any special sugar we provide for codegen.
return ((IntermediateToken)cSharpNode.Children.Single());
var tokens = htmlContentNode.FindDescendantNodes<IntermediateToken>();
var content = "\"" + string.Join(string.Empty, tokens.Select(t => t.Content))+ "\"";
return new[] { new IntermediateToken() { Content = content, Kind = TokenKind.CSharp, } };
}
else
{
// This is the common case for 'mixed' content
return ((IntermediateToken)node.Children.Single());
return node.FindDescendantNodes<IntermediateToken>();
}
}
}

View File

@ -1,6 +1,7 @@
// 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 System.Linq;
using Xunit;
namespace Microsoft.AspNetCore.Blazor.Build.Test
@ -367,6 +368,45 @@ namespace Test
CompileToAssembly(generated);
}
[Fact] // https://github.com/aspnet/Blazor/issues/772
public void Regression_772()
{
// Arrange
AdditionalSyntaxTrees.Add(Parse(@"
using Microsoft.AspNetCore.Blazor.Components;
namespace Test
{
public class SurveyPrompt : BlazorComponent
{
[Parameter] private string Title { get; set; }
}
}
"));
// Act
var generated = CompileToCSharp(@"
@addTagHelper *, TestAssembly
@page ""/""
<h1>Hello, world!</h1>
Welcome to your new app.
<SurveyPrompt Title=""
");
// Assert
AssertDocumentNodeMatchesBaseline(generated.CodeDocument);
AssertCSharpDocumentMatchesBaseline(generated.CodeDocument);
// This has some errors
Assert.Collection(
generated.Diagnostics.OrderBy(d => d.Id),
d => Assert.Equal("RZ1034", d.Id),
d => Assert.Equal("RZ1035", d.Id));
}
[Fact]
public void BindToComponent_SpecifiesValue_WithMatchingProperties()
{

View File

@ -1,6 +1,7 @@
// 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 System.Linq;
using Xunit;
namespace Microsoft.AspNetCore.Blazor.Build.Test
@ -617,6 +618,45 @@ namespace Test
CompileToAssembly(generated);
}
[Fact] // https://github.com/aspnet/Blazor/issues/772
public void Regression_772()
{
// Arrange
AdditionalSyntaxTrees.Add(Parse(@"
using Microsoft.AspNetCore.Blazor.Components;
namespace Test
{
public class SurveyPrompt : BlazorComponent
{
[Parameter] private string Title { get; set; }
}
}
"));
// Act
var generated = CompileToCSharp(@"
@addTagHelper *, TestAssembly
@page ""/""
<h1>Hello, world!</h1>
Welcome to your new app.
<SurveyPrompt Title=""
");
// Assert
AssertDocumentNodeMatchesBaseline(generated.CodeDocument);
AssertCSharpDocumentMatchesBaseline(generated.CodeDocument);
// This has some errors
Assert.Collection(
generated.Diagnostics.OrderBy(d => d.Id),
d => Assert.Equal("RZ1034", d.Id),
d => Assert.Equal("RZ1035", d.Id));
}
[Fact]
public void BindToComponent_SpecifiesValue_WithMatchingProperties()
{

View File

@ -0,0 +1,42 @@
// <auto-generated/>
#pragma warning disable 1591
namespace Test
{
#line hidden
using System;
using System.Collections.Generic;
using System.Linq;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Blazor;
using Microsoft.AspNetCore.Blazor.Components;
[Microsoft.AspNetCore.Blazor.Components.RouteAttribute("/")]
public class TestComponent : Microsoft.AspNetCore.Blazor.Components.BlazorComponent
{
#pragma warning disable 219
private void __RazorDirectiveTokenHelpers__() {
((System.Action)(() => {
global::System.Object __typeHelper = "*, TestAssembly";
}
))();
((System.Action)(() => {
global::System.Object __typeHelper = "/";
}
))();
}
#pragma warning restore 219
#pragma warning disable 0414
private static System.Object __o = null;
#pragma warning restore 0414
#pragma warning disable 1998
protected override void BuildRenderTree(Microsoft.AspNetCore.Blazor.RenderTree.RenderTreeBuilder builder)
{
base.BuildRenderTree(builder);
__o = ;
builder.AddAttribute(-1, "ChildContent", (Microsoft.AspNetCore.Blazor.RenderFragment)((builder2) => {
}
));
}
#pragma warning restore 1998
}
}
#pragma warning restore 1591

View File

@ -0,0 +1,2 @@
x:\dir\subdir\Test\TestComponent.cshtml(8,2): Error RZ1035: Missing close angle for tag helper 'SurveyPrompt'.
x:\dir\subdir\Test\TestComponent.cshtml(8,2): Error RZ1034: Found a malformed 'SurveyPrompt' tag helper. Tag helpers must have a start and end tag or be self closing.

View File

@ -0,0 +1,35 @@
Document -
NamespaceDeclaration - - Test
UsingDirective - (3:1,1 [12] ) - System
UsingDirective - (18:2,1 [32] ) - System.Collections.Generic
UsingDirective - (53:3,1 [17] ) - System.Linq
UsingDirective - (73:4,1 [28] ) - System.Threading.Tasks
UsingDirective - (104:5,1 [33] ) - Microsoft.AspNetCore.Blazor
UsingDirective - (140:6,1 [44] ) - Microsoft.AspNetCore.Blazor.Components
RouteAttributeExtensionNode - - /
ClassDeclaration - - public - TestComponent - Microsoft.AspNetCore.Blazor.Components.BlazorComponent -
DesignTimeDirective -
DirectiveToken - (14:0,14 [32] ) - "*, Microsoft.AspNetCore.Blazor"
DirectiveToken - (14:0,14 [9] ) - "*, Test"
DirectiveToken - (14:0,14 [15] x:\dir\subdir\Test\TestComponent.cshtml) - *, TestAssembly
DirectiveToken - (37:1,6 [3] x:\dir\subdir\Test\TestComponent.cshtml) - "/"
CSharpCode -
IntermediateToken - - CSharp - #pragma warning disable 0414
CSharpCode -
IntermediateToken - - CSharp - private static System.Object __o = null;
CSharpCode -
IntermediateToken - - CSharp - #pragma warning restore 0414
MethodDeclaration - - protected override - void - BuildRenderTree
CSharpCode -
IntermediateToken - - CSharp - base.BuildRenderTree(builder);
HtmlContent - (29:0,29 [2] x:\dir\subdir\Test\TestComponent.cshtml)
IntermediateToken - (29:0,29 [2] x:\dir\subdir\Test\TestComponent.cshtml) - Html - \n
HtmlContent - (42:2,0 [2] x:\dir\subdir\Test\TestComponent.cshtml)
IntermediateToken - (42:2,0 [2] x:\dir\subdir\Test\TestComponent.cshtml) - Html - \n
HtmlElement - (44:3,0 [22] x:\dir\subdir\Test\TestComponent.cshtml) - h1
HtmlContent - (48:3,4 [13] x:\dir\subdir\Test\TestComponent.cshtml)
IntermediateToken - (48:3,4 [13] x:\dir\subdir\Test\TestComponent.cshtml) - Html - Hello, world!
HtmlContent - (66:3,22 [32] x:\dir\subdir\Test\TestComponent.cshtml)
IntermediateToken - (66:3,22 [32] x:\dir\subdir\Test\TestComponent.cshtml) - Html - \n\nWelcome to your new app.\n\n
ComponentExtensionNode - (98:7,0 [23] x:\dir\subdir\Test\TestComponent.cshtml) - SurveyPrompt - Test.SurveyPrompt
ComponentAttributeExtensionNode - (119:7,21 [0] x:\dir\subdir\Test\TestComponent.cshtml) - Title - Title

View File

@ -0,0 +1,10 @@
Source Location: (14:0,14 [15] x:\dir\subdir\Test\TestComponent.cshtml)
|*, TestAssembly|
Generated Location: (625:17,38 [15] )
|*, TestAssembly|
Source Location: (37:1,6 [3] x:\dir\subdir\Test\TestComponent.cshtml)
|"/"|
Generated Location: (741:21,37 [3] )
|"/"|

View File

@ -0,0 +1,30 @@
// <auto-generated/>
#pragma warning disable 1591
namespace Test
{
#line hidden
using System;
using System.Collections.Generic;
using System.Linq;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Blazor;
using Microsoft.AspNetCore.Blazor.Components;
[Microsoft.AspNetCore.Blazor.Components.RouteAttribute("/")]
public class TestComponent : Microsoft.AspNetCore.Blazor.Components.BlazorComponent
{
#pragma warning disable 1998
protected override void BuildRenderTree(Microsoft.AspNetCore.Blazor.RenderTree.RenderTreeBuilder builder)
{
base.BuildRenderTree(builder);
builder.OpenElement(0, "h1");
builder.AddContent(1, "Hello, world!");
builder.CloseElement();
builder.AddContent(2, "\n\nWelcome to your new app.\n\n");
builder.OpenComponent<Test.SurveyPrompt>(3);
builder.AddAttribute(4, "Title", );
builder.CloseComponent();
}
#pragma warning restore 1998
}
}
#pragma warning restore 1591

View File

@ -0,0 +1,2 @@
x:\dir\subdir\Test\TestComponent.cshtml(8,2): Error RZ1035: Missing close angle for tag helper 'SurveyPrompt'.
x:\dir\subdir\Test\TestComponent.cshtml(8,2): Error RZ1034: Found a malformed 'SurveyPrompt' tag helper. Tag helpers must have a start and end tag or be self closing.

View File

@ -0,0 +1,20 @@
Document -
NamespaceDeclaration - - Test
UsingDirective - (3:1,1 [14] ) - System
UsingDirective - (18:2,1 [34] ) - System.Collections.Generic
UsingDirective - (53:3,1 [19] ) - System.Linq
UsingDirective - (73:4,1 [30] ) - System.Threading.Tasks
UsingDirective - (104:5,1 [35] ) - Microsoft.AspNetCore.Blazor
UsingDirective - (140:6,1 [46] ) - Microsoft.AspNetCore.Blazor.Components
RouteAttributeExtensionNode - - /
ClassDeclaration - - public - TestComponent - Microsoft.AspNetCore.Blazor.Components.BlazorComponent -
MethodDeclaration - - protected override - void - BuildRenderTree
CSharpCode -
IntermediateToken - - CSharp - base.BuildRenderTree(builder);
HtmlElement - (44:3,0 [22] x:\dir\subdir\Test\TestComponent.cshtml) - h1
HtmlContent - (48:3,4 [13] x:\dir\subdir\Test\TestComponent.cshtml)
IntermediateToken - (48:3,4 [13] x:\dir\subdir\Test\TestComponent.cshtml) - Html - Hello, world!
HtmlContent - (66:3,22 [32] x:\dir\subdir\Test\TestComponent.cshtml)
IntermediateToken - (66:3,22 [32] x:\dir\subdir\Test\TestComponent.cshtml) - Html - \n\nWelcome to your new app.\n\n
ComponentExtensionNode - (98:7,0 [23] x:\dir\subdir\Test\TestComponent.cshtml) - SurveyPrompt - Test.SurveyPrompt
ComponentAttributeExtensionNode - (119:7,21 [0] x:\dir\subdir\Test\TestComponent.cshtml) - Title - Title

View File

@ -0,0 +1,70 @@
// 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 System;
using System.Linq;
using Xunit;
using Xunit.Sdk;
namespace Microsoft.AspNetCore.Blazor.Build.Test
{
// Similar to design time code generation tests, but goes a character at a time.
// Don't add many of these since they are slow - instead add features to existing
// tests here, and use these as smoke tests, not for detailed regression testing.
public class TypingTest : RazorIntegrationTestBase
{
internal override bool DesignTime => true;
internal override bool UseTwoPhaseCompilation => false;
[Fact]
public void DoSomeTyping()
{
// Arrange
AdditionalSyntaxTrees.Add(Parse(@"
using System;
using Microsoft.AspNetCore.Blazor.Components;
namespace Test
{
public class MyComponent : BlazorComponent
{
[Parameter] int Value { get; set; }
[Parameter] Action<int> ValueChanged { get; set; }
[Parameter] string AnotherValue { get; set; }
}
}
"));
var text = @"
@addTagHelper *, TestAssembly
<div>
<MyComponent bind-Value=""myValue"" AnotherValue=""hi""/>
<input type=""text"" bind=""@value"" />
<button ref=""_button"" onsubmit=""@FormSubmitted"">Click me</button>
</div>
<MyComponent
IntProperty=""123""
BoolProperty=""true""
StringProperty=""My string""
ObjectProperty=""new SomeType()""/>";
for (var i = 0; i <= text.Length; i++)
{
try
{
CompileToCSharp(text.Substring(0, i));
}
catch (Exception ex)
{
throw new XunitException($@"
Code generation failed on iteration {i} with source text:
{text.Substring(0, i)}
Exception:
{ex}
");
}
}
}
}
}