Skip to content

Commit 630d826

Browse files
jjonesczjjonescz
authored
Keep complex attribute content (#9553)
* Add a test * Keep complex attribute content * Simplify form name codegen * Keep type check for mixed content * Add a test * Verify `@formname` mixed content C# diagnostics * Add integration test
1 parent 0e8b3ab commit 630d826

File tree

39 files changed

+677
-58
lines changed

39 files changed

+677
-58
lines changed

‎src/Compiler/Microsoft.AspNetCore.Razor.Language/src/Components/ComponentComplexAttributeContentPass.cs

Lines changed: 1 addition & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -55,13 +55,11 @@ private void ProcessAttributes(TagHelperIntermediateNode node)
5555

5656
private static void ProcessAttribute(IntermediateNode parent, IntermediateNode node, string attributeName)
5757
{
58-
var removeNode = false;
5958
var issueDiagnostic = false;
6059

6160
if (node.Children is [HtmlAttributeIntermediateNode { Children.Count: > 1 }])
6261
{
6362
// This case can be hit for a 'string' attribute
64-
removeNode = true;
6563
issueDiagnostic = true;
6664
}
6765
else if (node.Children is [CSharpExpressionIntermediateNode { Children.Count: > 1 } cSharpNode])
@@ -78,44 +76,26 @@ private static void ProcessAttribute(IntermediateNode parent, IntermediateNode n
7876
}
7977
else
8078
{
81-
removeNode = true;
8279
issueDiagnostic = true;
8380
}
8481
}
8582
else if (node.Children is [CSharpCodeIntermediateNode])
8683
{
8784
// This is the case when an attribute contains a code block @{ ... }
8885
// We don't support this.
89-
removeNode = true;
90-
issueDiagnostic = true;
91-
}
92-
else if (node.Children is [CSharpExpressionIntermediateNode, HtmlContentIntermediateNode { Children: [IntermediateToken { Content: "." }] }])
93-
{
94-
// This is the case when an attribute contains something like "@MyEnum."
95-
// We simplify this to remove the "." so that tooling can provide completion on "MyEnum"
96-
// in case the user is in the middle of typing
97-
node.Children.RemoveAt(1);
98-
99-
// We still want to issue a diagnostic, even though we simplified, because ultimately
100-
// we don't support this, so if the user isn't typing, we can't let this through
10186
issueDiagnostic = true;
10287
}
10388
else if (node.Children.Count > 1)
10489
{
10590
// This is the common case for 'mixed' content
106-
removeNode = true;
10791
issueDiagnostic = true;
10892
}
10993

11094
if (issueDiagnostic)
11195
{
112-
parent.Diagnostics.Add(ComponentDiagnosticFactory.Create_UnsupportedComplexContent(
96+
node.Diagnostics.Add(ComponentDiagnosticFactory.Create_UnsupportedComplexContent(
11397
node,
11498
attributeName));
11599
}
116-
if (removeNode)
117-
{
118-
parent.Children.Remove(node);
119-
}
120100
}
121101
}

‎src/Compiler/Microsoft.AspNetCore.Razor.Language/src/Components/ComponentDesignTimeNodeWriter.cs

Lines changed: 13 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -798,6 +798,12 @@ void writeTypeArgument(IntermediateNodeCollection typeArgumentComponents)
798798

799799
private void WriteComponentAttributeInnards(CodeRenderingContext context, ComponentAttributeIntermediateNode node, bool canTypeCheck)
800800
{
801+
if (node.Children.Count > 1)
802+
{
803+
Debug.Assert(node.HasDiagnostics, "We should have reported an error for mixed content.");
804+
// We render the children anyway, so tooling works.
805+
}
806+
801807
// We limit component attributes to simple cases. However there is still a lot of complexity
802808
// to handle here, since there are a few different cases for how an attribute might be structured.
803809
//
@@ -807,11 +813,6 @@ private void WriteComponentAttributeInnards(CodeRenderingContext context, Compon
807813
// Minimized attributes always map to 'true'
808814
context.CodeWriter.Write("true");
809815
}
810-
else if (node.Children.Count > 1)
811-
{
812-
// We don't expect this to happen, we just want to know if it can.
813-
throw new InvalidOperationException("Attribute nodes should either be minimized or a single type of content." + string.Join(", ", node.Children));
814-
}
815816
else if (node.Children.Count == 1 && node.Children[0] is HtmlContentIntermediateNode)
816817
{
817818
// We don't actually need the content at designtime, an empty string will do.
@@ -1170,27 +1171,20 @@ private void WriteSplatInnards(CodeRenderingContext context, SplatIntermediateNo
11701171

11711172
public sealed override void WriteFormName(CodeRenderingContext context, FormNameIntermediateNode node)
11721173
{
1173-
var tokens = node.FindDescendantNodes<IntermediateToken>();
1174-
if (tokens.Count == 0)
1174+
if (node.Children.Count > 1)
11751175
{
1176-
return;
1176+
Debug.Assert(node.HasDiagnostics, "We should have reported an error for mixed content.");
11771177
}
11781178

1179-
// Either all tokens should be C# or none of them.
1180-
if (tokens[0].IsCSharp)
1179+
foreach (var token in node.FindDescendantNodes<IntermediateToken>())
11811180
{
1182-
context.CodeWriter.Write(ComponentsApi.RuntimeHelpers.TypeCheck);
1183-
context.CodeWriter.Write("<string>(");
1184-
foreach (var token in tokens)
1181+
if (token.IsCSharp)
11851182
{
1186-
Debug.Assert(token.IsCSharp);
1183+
context.CodeWriter.Write(ComponentsApi.RuntimeHelpers.TypeCheck);
1184+
context.CodeWriter.Write("<string>(");
11871185
WriteCSharpToken(context, token);
1186+
context.CodeWriter.WriteLine(");");
11881187
}
1189-
context.CodeWriter.Write(");");
1190-
}
1191-
else
1192-
{
1193-
Debug.Assert(!tokens.Any(t => t.IsCSharp));
11941188
}
11951189
}
11961190

‎src/Compiler/Microsoft.AspNetCore.Razor.Language/src/Components/ComponentRenderModeLoweringPass.cs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4-
using System.Diagnostics;
54
using Microsoft.AspNetCore.Razor.Language.Intermediate;
65

76
namespace Microsoft.AspNetCore.Razor.Language.Components;
@@ -23,8 +22,6 @@ protected override void ExecuteCore(RazorCodeDocument codeDocument, DocumentInte
2322
{
2423
if (reference is { Node: TagHelperDirectiveAttributeIntermediateNode node, Parent: IntermediateNode parentNode } && node.TagHelper.IsRenderModeTagHelper())
2524
{
26-
Debug.Assert(node.Diagnostics.Count == 0);
27-
2825
if (parentNode is not ComponentIntermediateNode componentNode)
2926
{
3027
node.Diagnostics.Add(ComponentDiagnosticFactory.CreateAttribute_ValidOnlyOnComponent(node.Source, node.OriginalAttributeName));
@@ -38,6 +35,7 @@ protected override void ExecuteCore(RazorCodeDocument codeDocument, DocumentInte
3835
};
3936

4037
var renderModeNode = new RenderModeIntermediateNode() { Source = node.Source, Children = { expression } };
38+
renderModeNode.Diagnostics.AddRange(node.Diagnostics);
4139

4240
if (componentNode.Component.Metadata.ContainsKey(ComponentMetadata.Component.HasRenderModeDirectiveKey))
4341
{

‎src/Compiler/Microsoft.AspNetCore.Razor.Language/src/Components/ComponentRuntimeNodeWriter.cs

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -630,16 +630,17 @@ public override void WriteComponentAttribute(CodeRenderingContext context, Compo
630630

631631
private void WriteComponentAttributeInnards(CodeRenderingContext context, ComponentAttributeIntermediateNode node, bool canTypeCheck)
632632
{
633+
if (node.Children.Count > 1)
634+
{
635+
Debug.Assert(node.HasDiagnostics, "We should have reported an error for mixed content.");
636+
// We render the children anyway, so tooling works.
637+
}
638+
633639
if (node.AttributeStructure == AttributeStructure.Minimized)
634640
{
635641
// Minimized attributes always map to 'true'
636642
context.CodeWriter.Write("true");
637643
}
638-
else if (node.Children.Count > 1)
639-
{
640-
// We don't expect this to happen, we just want to know if it can.
641-
throw new InvalidOperationException("Attribute nodes should either be minimized or a single type of content." + string.Join(", ", node.Children));
642-
}
643644
else if (node.Children.Count == 1 && node.Children[0] is HtmlContentIntermediateNode htmlNode)
644645
{
645646
// This is how string attributes are lowered by default, a single HTML node with a single HTML token.
@@ -944,6 +945,11 @@ private void WriteSplatInnards(CodeRenderingContext context, SplatIntermediateNo
944945

945946
public sealed override void WriteFormName(CodeRenderingContext context, FormNameIntermediateNode node)
946947
{
948+
if (node.Children.Count > 1)
949+
{
950+
Debug.Assert(node.HasDiagnostics, "We should have reported an error for mixed content.");
951+
}
952+
947953
// string __formName = expression;
948954
context.CodeWriter.Write("string ");
949955
context.CodeWriter.Write(_scopeStack.FormNameVarName);

‎src/Compiler/Microsoft.AspNetCore.Razor.Language/test/IntegrationTests/ComponentCodeGenerationTestBase.cs

Lines changed: 112 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9897,7 +9897,96 @@ public enum MyEnum
98979897

98989898
// Assert
98999899
AssertDocumentNodeMatchesBaseline(generated.CodeDocument);
9900-
AssertCSharpDocumentMatchesBaseline(generated.CodeDocument, verifyLinePragmas: false);
9900+
AssertCSharpDocumentMatchesBaseline(generated.CodeDocument);
9901+
var result = CompileToAssembly(generated, throwOnFailure: false);
9902+
result.Diagnostics.Verify(
9903+
// x:\dir\subdir\Test\TestComponent.cshtml(1,31): error CS0119: 'TestComponent.MyEnum' is a type, which is not valid in the given context
9904+
// MyEnum
9905+
Diagnostic(ErrorCode.ERR_BadSKunknown, "MyEnum").WithArguments("Test.TestComponent.MyEnum", "type").WithLocation(1, 31));
9906+
Assert.NotEmpty(generated.Diagnostics);
9907+
}
9908+
9909+
[IntegrationTestFact, WorkItem("https://github.com/dotnet/razor/issues/9346")]
9910+
public void Component_ComplexContentInAttribute_02()
9911+
{
9912+
// Arrange
9913+
AdditionalSyntaxTrees.Add(Parse("""
9914+
using Microsoft.AspNetCore.Components;
9915+
9916+
namespace Test
9917+
{
9918+
public class MyComponent : ComponentBase
9919+
{
9920+
[Parameter] public string StringProperty { get; set; }
9921+
}
9922+
}
9923+
"""));
9924+
9925+
// Act
9926+
var generated = CompileToCSharp("""
9927+
<MyComponent StringProperty="@MyEnum+" />
9928+
9929+
@code {
9930+
public enum MyEnum
9931+
{
9932+
One,
9933+
Two
9934+
}
9935+
}
9936+
""");
9937+
9938+
// Assert
9939+
AssertDocumentNodeMatchesBaseline(generated.CodeDocument);
9940+
AssertCSharpDocumentMatchesBaseline(generated.CodeDocument);
9941+
var result = CompileToAssembly(generated, throwOnFailure: false);
9942+
result.Diagnostics.Verify(
9943+
// x:\dir\subdir\Test\TestComponent.cshtml(1,31): error CS0119: 'TestComponent.MyEnum' is a type, which is not valid in the given context
9944+
// MyEnum
9945+
Diagnostic(ErrorCode.ERR_BadSKunknown, "MyEnum").WithArguments("Test.TestComponent.MyEnum", "type").WithLocation(1, 31));
9946+
Assert.NotEmpty(generated.Diagnostics);
9947+
}
9948+
9949+
[IntegrationTestFact, WorkItem("https://github.com/dotnet/razor/issues/9346")]
9950+
public void Component_ComplexContentInAttribute_03()
9951+
{
9952+
// Arrange
9953+
AdditionalSyntaxTrees.Add(Parse("""
9954+
using Microsoft.AspNetCore.Components;
9955+
9956+
namespace Test
9957+
{
9958+
public class MyComponent : ComponentBase
9959+
{
9960+
[Parameter] public string StringProperty { get; set; }
9961+
}
9962+
}
9963+
"""));
9964+
9965+
// Act
9966+
var generated = CompileToCSharp("""
9967+
<MyComponent StringProperty="@x html @("string")" />
9968+
9969+
@code {
9970+
int x = 1;
9971+
}
9972+
""");
9973+
9974+
// Assert
9975+
AssertDocumentNodeMatchesBaseline(generated.CodeDocument);
9976+
AssertCSharpDocumentMatchesBaseline(generated.CodeDocument);
9977+
var result = CompileToAssembly(generated, throwOnFailure: false);
9978+
result.Diagnostics.Verify(
9979+
// x:\dir\subdir\Test\TestComponent.cshtml(1,32): error CS1003: Syntax error, ',' expected
9980+
// x
9981+
Diagnostic(ErrorCode.ERR_SyntaxError, "").WithArguments(",").WithLocation(1, 32),
9982+
DesignTime
9983+
// (23,91): error CS1501: No overload for method 'TypeCheck' takes 2 arguments
9984+
// __o = global::Microsoft.AspNetCore.Components.CompilerServices.RuntimeHelpers.TypeCheck<global::System.String>(
9985+
? Diagnostic(ErrorCode.ERR_BadArgCount, "TypeCheck<global::System.String>").WithArguments("TypeCheck", "2").WithLocation(23, 91)
9986+
// (17,138): error CS1501: No overload for method 'TypeCheck' takes 2 arguments
9987+
// __builder.AddComponentParameter(1, "StringProperty", global::Microsoft.AspNetCore.Components.CompilerServices.RuntimeHelpers.TypeCheck<global::System.String>(
9988+
: Diagnostic(ErrorCode.ERR_BadArgCount, "TypeCheck<global::System.String>").WithArguments("TypeCheck", "2").WithLocation(17, 138));
9989+
Assert.NotEmpty(generated.Diagnostics);
99019990
}
99029991

99039992
[IntegrationTestFact]
@@ -10502,7 +10591,20 @@ @using Microsoft.AspNetCore.Components.Web
1050210591

1050310592
// Assert
1050410593
AssertDocumentNodeMatchesBaseline(generated.CodeDocument);
10505-
AssertCSharpDocumentMatchesBaseline(generated.CodeDocument, verifyLinePragmas: false);
10594+
AssertCSharpDocumentMatchesBaseline(generated.CodeDocument);
10595+
var result = CompileToAssembly(generated, throwOnFailure: false);
10596+
if (DesignTime)
10597+
{
10598+
result.Diagnostics.Verify(
10599+
// x:\dir\subdir\Test\TestComponent.cshtml(2,74): error CS1503: Argument 1: cannot convert from 'int' to 'string'
10600+
// x
10601+
Diagnostic(ErrorCode.ERR_BadArgType, "x").WithArguments("1", "int", "string").WithLocation(2, 74));
10602+
}
10603+
else
10604+
{
10605+
result.Diagnostics.Verify();
10606+
}
10607+
Assert.NotEmpty(generated.Diagnostics);
1050610608
}
1050710609

1050810610
[IntegrationTestFact, WorkItem("https://github.com/dotnet/razor/issues/9077")]
@@ -10555,6 +10657,14 @@ @using Microsoft.AspNetCore.Components.Web
1055510657
// Assert
1055610658
AssertDocumentNodeMatchesBaseline(generated.CodeDocument);
1055710659
AssertCSharpDocumentMatchesBaseline(generated.CodeDocument);
10660+
var result = CompileToAssembly(generated, throwOnFailure: false);
10661+
result.Diagnostics.Verify(DesignTime
10662+
// (39,85): error CS7036: There is no argument given that corresponds to the required parameter 'value' of 'RuntimeHelpers.TypeCheck<T>(T)'
10663+
// global::Microsoft.AspNetCore.Components.CompilerServices.RuntimeHelpers.TypeCheck<string>();
10664+
? Diagnostic(ErrorCode.ERR_NoCorrespondingArgument, "TypeCheck<string>").WithArguments("value", "Microsoft.AspNetCore.Components.CompilerServices.RuntimeHelpers.TypeCheck<T>(T)").WithLocation(39, 85)
10665+
// (34,105): error CS7036: There is no argument given that corresponds to the required parameter 'value' of 'RuntimeHelpers.TypeCheck<T>(T)'
10666+
// string __formName = global::Microsoft.AspNetCore.Components.CompilerServices.RuntimeHelpers.TypeCheck<string>();
10667+
: Diagnostic(ErrorCode.ERR_NoCorrespondingArgument, "TypeCheck<string>").WithArguments("value", "Microsoft.AspNetCore.Components.CompilerServices.RuntimeHelpers.TypeCheck<T>(T)").WithLocation(34, 105));
1055810668
}
1055910669

1056010670
[IntegrationTestFact, WorkItem("https://github.com/dotnet/razor/issues/9077")]

‎src/Compiler/Microsoft.AspNetCore.Razor.Language/test/TestFiles/IntegrationTests/ComponentDesignTimeCodeGenerationTest/Component_ComplexContentInAttribute/TestComponent.diagnostics.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
x:\dir\subdir\Test\TestComponent.cshtml(1,30): Error RZ9986: Component attributes do not support complex content (mixed C# and markup). Attribute: 'StringProperty', text: 'MyEnum'
1+
x:\dir\subdir\Test\TestComponent.cshtml(1,30): Error RZ9986: Component attributes do not support complex content (mixed C# and markup). Attribute: 'StringProperty', text: 'MyEnum.'

‎src/Compiler/Microsoft.AspNetCore.Razor.Language/test/TestFiles/IntegrationTests/ComponentDesignTimeCodeGenerationTest/Component_ComplexContentInAttribute/TestComponent.ir.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@
1818
ComponentAttribute - (29:0,29 [8] x:\dir\subdir\Test\TestComponent.cshtml) - StringProperty - StringProperty - AttributeStructure.DoubleQuotes
1919
CSharpExpression - (30:0,30 [6] x:\dir\subdir\Test\TestComponent.cshtml)
2020
LazyIntermediateToken - (30:0,30 [6] x:\dir\subdir\Test\TestComponent.cshtml) - CSharp - MyEnum
21+
HtmlContent - (36:0,36 [1] x:\dir\subdir\Test\TestComponent.cshtml)
22+
LazyIntermediateToken - (36:0,36 [1] x:\dir\subdir\Test\TestComponent.cshtml) - Html - .
2123
HtmlContent - (41:0,41 [4] x:\dir\subdir\Test\TestComponent.cshtml)
2224
LazyIntermediateToken - (41:0,41 [4] x:\dir\subdir\Test\TestComponent.cshtml) - Html - \n\n
2325
CSharpCode - (52:2,7 [67] x:\dir\subdir\Test\TestComponent.cshtml)

‎src/Compiler/Microsoft.AspNetCore.Razor.Language/test/TestFiles/IntegrationTests/ComponentDesignTimeCodeGenerationTest/Component_ComplexContentInAttribute_02/TestComponent.codegen.cs

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
// <auto-generated/>
2+
#pragma warning disable 1591
3+
namespace Test
4+
{
5+
#line hidden
6+
using global::System;
7+
using global::System.Collections.Generic;
8+
using global::System.Linq;
9+
using global::System.Threading.Tasks;
10+
using global::Microsoft.AspNetCore.Components;
11+
public partial class TestComponent : global::Microsoft.AspNetCore.Components.ComponentBase
12+
{
13+
#pragma warning disable 219
14+
private void __RazorDirectiveTokenHelpers__() {
15+
}
16+
#pragma warning restore 219
17+
#pragma warning disable 0414
18+
private static object __o = null;
19+
#pragma warning restore 0414
20+
#pragma warning disable 1998
21+
protected override void BuildRenderTree(global::Microsoft.AspNetCore.Components.Rendering.RenderTreeBuilder __builder)
22+
{
23+
__o = global::Microsoft.AspNetCore.Components.CompilerServices.RuntimeHelpers.TypeCheck<global::System.String>(
24+
#nullable restore
25+
#line 1 "x:\dir\subdir\Test\TestComponent.cshtml"
26+
MyEnum
27+
28+
#line default
29+
#line hidden
30+
#nullable disable
31+
);
32+
__builder.AddAttribute(-1, "ChildContent", (global::Microsoft.AspNetCore.Components.RenderFragment)((__builder2) => {
33+
}
34+
));
35+
#pragma warning disable BL0005
36+
((global::Test.MyComponent)default).
37+
#nullable restore
38+
#line 1 "x:\dir\subdir\Test\TestComponent.cshtml"
39+
StringProperty
40+
41+
#line default
42+
#line hidden
43+
#nullable disable
44+
= default;
45+
#pragma warning restore BL0005
46+
#nullable restore
47+
#line 1 "x:\dir\subdir\Test\TestComponent.cshtml"
48+
__o = typeof(global::Test.MyComponent);
49+
50+
#line default
51+
#line hidden
52+
#nullable disable
53+
}
54+
#pragma warning restore 1998
55+
#nullable restore
56+
#line 3 "x:\dir\subdir\Test\TestComponent.cshtml"
57+
58+
public enum MyEnum
59+
{
60+
One,
61+
Two
62+
}
63+
64+
#line default
65+
#line hidden
66+
#nullable disable
67+
}
68+
}
69+
#pragma warning restore 1591

‎src/Compiler/Microsoft.AspNetCore.Razor.Language/test/TestFiles/IntegrationTests/ComponentDesignTimeCodeGenerationTest/Component_ComplexContentInAttribute_02/TestComponent.diagnostics.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
x:\dir\subdir\Test\TestComponent.cshtml(1,30): Error RZ9986: Component attributes do not support complex content (mixed C# and markup). Attribute: 'StringProperty', text: 'MyEnum+'

0 commit comments

Comments
 (0)