Skip to content

Commit eb76931

Browse files
Strip insignificant whitespace at compile time (#23385)
* Define @preservewhitespace directive attribute * Have ComponentWhitespacePass respect preservewhitespace option * Tests for overriding the preservewhitespace option (and fix implementation) * Begin adding test infrastucture for ComponentWhitespacePass * Remove leading/trailing whitespace from markup elements recursively * Update baselines * Add test showing we don't remove whitespace between sibling elements * Remove whitespace before/after C# code statements (not expressions) * Update baselines * Slight improvements to test * Remove leading/trailing whitespace inside component child content * Update baselines * Fix Razor tests * Fix MVC test * Better fix for MVC test * CR: Make ComponentPreserveWhitespaceDirective conditional on langversion >= 5.0
1 parent 126f14d commit eb76931

File tree

106 files changed

+1591
-234
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

106 files changed

+1591
-234
lines changed

src/Mvc/test/WebSites/BasicWebSite/RazorComponents/FetchData.razor

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
@using BasicWebSite.Services
22
@inject WeatherForecastService ForecastService
3-
3+
@preservewhitespace true
44
<h1>Weather forecast</h1>
55

66
<p>This component demonstrates fetching data from the server.</p>
@@ -34,7 +34,6 @@ else
3434
</tbody>
3535
</table>
3636
}
37-
3837
@code {
3938
[Parameter] public DateTime StartDate { get; set; }
4039

src/Razor/Microsoft.AspNetCore.Mvc.Razor.Extensions/test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/BasicComponent_Runtime.codegen.cs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@ protected override void BuildRenderTree(Microsoft.AspNetCore.Components.Renderin
3434
#line hidden
3535
#nullable disable
3636
);
37-
__builder.AddMarkupContent(4, "\r\n");
3837
__builder.CloseElement();
3938
}
4039
#pragma warning restore 1998

src/Razor/Microsoft.AspNetCore.Mvc.Razor.Extensions/test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/BasicComponent_Runtime.ir.txt

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,5 @@ Document -
1616
LazyIntermediateToken - (74:3,0 [4] BasicComponent.cshtml) - Html -
1717
CSharpExpression - (79:3,5 [29] BasicComponent.cshtml)
1818
LazyIntermediateToken - (79:3,5 [29] BasicComponent.cshtml) - CSharp - string.Format("{0}", "Hello")
19-
HtmlContent - (108:3,34 [2] BasicComponent.cshtml)
20-
LazyIntermediateToken - (108:3,34 [2] BasicComponent.cshtml) - Html - \n
2119
CSharpCode - (132:6,12 [37] BasicComponent.cshtml)
2220
LazyIntermediateToken - (132:6,12 [37] BasicComponent.cshtml) - CSharp - \n void IDisposable.Dispose(){ }\n

src/Razor/Microsoft.AspNetCore.Razor.Language/src/ComponentResources.resx

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,15 @@
201201
<data name="PageDirective_RouteToken_Name" xml:space="preserve">
202202
<value>route template</value>
203203
</data>
204+
<data name="PreserveWhitespaceDirective_BooleanToken_Description" xml:space="preserve">
205+
<value>True if whitespace should be preserved, otherwise false.</value>
206+
</data>
207+
<data name="PreserveWhitespaceDirective_BooleanToken_Name" xml:space="preserve">
208+
<value>Preserve</value>
209+
</data>
210+
<data name="PreserveWhitespaceDirective_Description" xml:space="preserve">
211+
<value>Specifies whether or not whitespace should be preserved exactly. Defaults to false for better performance.</value>
212+
</data>
204213
<data name="RefTagHelper_Documentation" xml:space="preserve">
205214
<value>Populates the specified field or property with a reference to the element or component.</value>
206215
</data>
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
// Copyright (c) .NET Foundation. All rights reserved.
2+
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
3+
4+
using System;
5+
6+
namespace Microsoft.AspNetCore.Razor.Language.Components
7+
{
8+
internal static class ComponentPreserveWhitespaceDirective
9+
{
10+
public static readonly DirectiveDescriptor Directive = DirectiveDescriptor.CreateDirective(
11+
"preservewhitespace",
12+
DirectiveKind.SingleLine,
13+
builder =>
14+
{
15+
builder.AddBooleanToken(ComponentResources.PreserveWhitespaceDirective_BooleanToken_Name, ComponentResources.PreserveWhitespaceDirective_BooleanToken_Description);
16+
builder.Usage = DirectiveUsage.FileScopedMultipleOccurring;
17+
builder.Description = ComponentResources.PreserveWhitespaceDirective_Description;
18+
});
19+
20+
public static void Register(RazorProjectEngineBuilder builder)
21+
{
22+
if (builder == null)
23+
{
24+
throw new ArgumentNullException(nameof(builder));
25+
}
26+
27+
builder.AddDirective(Directive, FileKinds.Component, FileKinds.ComponentImport);
28+
}
29+
}
30+
}

src/Razor/Microsoft.AspNetCore.Razor.Language/src/Components/ComponentWhitespacePass.cs

Lines changed: 69 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

44
using System;
5+
using System.Linq;
56
using Microsoft.AspNetCore.Razor.Language.Intermediate;
67

78
namespace Microsoft.AspNetCore.Razor.Language.Components
@@ -38,17 +39,45 @@ protected override void ExecuteCore(RazorCodeDocument codeDocument, DocumentInte
3839
return;
3940
}
4041

42+
// Respect @preservewhitespace directives
43+
if (PreserveWhitespaceIsEnabled(documentNode))
44+
{
45+
return;
46+
}
47+
4148
var method = documentNode.FindPrimaryMethod();
4249
if (method != null)
4350
{
4451
RemoveContiguousWhitespace(method.Children, TraversalDirection.Forwards);
4552
RemoveContiguousWhitespace(method.Children, TraversalDirection.Backwards);
53+
54+
var visitor = new Visitor();
55+
visitor.Visit(method);
56+
}
57+
}
58+
59+
private static bool PreserveWhitespaceIsEnabled(DocumentIntermediateNode documentNode)
60+
{
61+
// If there's no @preservewhitespace attribute, the default is that we *don't* preserve whitespace
62+
var shouldPreserveWhitespace = false;
63+
64+
foreach (var preserveWhitespaceDirective in documentNode.FindDirectiveReferences(ComponentPreserveWhitespaceDirective.Directive))
65+
{
66+
var token = ((DirectiveIntermediateNode)preserveWhitespaceDirective.Node).Tokens.FirstOrDefault();
67+
var shouldPreserveWhitespaceContent = token?.Content;
68+
if (shouldPreserveWhitespaceContent != null)
69+
{
70+
shouldPreserveWhitespace = string.Equals(shouldPreserveWhitespaceContent, "true", StringComparison.Ordinal);
71+
}
4672
}
73+
74+
return shouldPreserveWhitespace;
4775
}
4876

49-
private static void RemoveContiguousWhitespace(IntermediateNodeCollection nodes, TraversalDirection direction)
77+
private static int RemoveContiguousWhitespace(IntermediateNodeCollection nodes, TraversalDirection direction, int? startIndex = null)
5078
{
51-
var position = direction == TraversalDirection.Forwards ? 0 : nodes.Count - 1;
79+
var position = startIndex.GetValueOrDefault(direction == TraversalDirection.Forwards ? 0 : nodes.Count - 1);
80+
var countRemoved = 0;
5281
while (position >= 0 && position < nodes.Count)
5382
{
5483
var node = nodes[position];
@@ -76,7 +105,7 @@ private static void RemoveContiguousWhitespace(IntermediateNodeCollection nodes,
76105
shouldContinueIteration = false;
77106
break;
78107

79-
case CSharpCodeIntermediateNode codeIntermediateNode:
108+
case CSharpCodeIntermediateNode _:
80109
shouldRemoveNode = false;
81110
shouldContinueIteration = false;
82111
break;
@@ -90,6 +119,7 @@ private static void RemoveContiguousWhitespace(IntermediateNodeCollection nodes,
90119
if (shouldRemoveNode)
91120
{
92121
nodes.RemoveAt(position);
122+
countRemoved++;
93123
if (direction == TraversalDirection.Forwards)
94124
{
95125
position--;
@@ -103,12 +133,48 @@ private static void RemoveContiguousWhitespace(IntermediateNodeCollection nodes,
103133
break;
104134
}
105135
}
136+
137+
return countRemoved;
106138
}
107139

108140
enum TraversalDirection
109141
{
110142
Forwards,
111143
Backwards
112144
}
145+
146+
class Visitor : IntermediateNodeWalker
147+
{
148+
public override void VisitMarkupElement(MarkupElementIntermediateNode node)
149+
{
150+
RemoveContiguousWhitespace(node.Children, TraversalDirection.Forwards);
151+
RemoveContiguousWhitespace(node.Children, TraversalDirection.Backwards);
152+
VisitDefault(node);
153+
}
154+
155+
public override void VisitTagHelperBody(TagHelperBodyIntermediateNode node)
156+
{
157+
// The goal here is to remove leading/trailing whitespace inside component child content. However,
158+
// at the time this whitespace pass runs, ComponentChildContent is still TagHelperBody in the tree.
159+
RemoveContiguousWhitespace(node.Children, TraversalDirection.Forwards);
160+
RemoveContiguousWhitespace(node.Children, TraversalDirection.Backwards);
161+
VisitDefault(node);
162+
}
163+
164+
public override void VisitDefault(IntermediateNode node)
165+
{
166+
// For any CSharpCodeIntermediateNode children, remove their preceding and trailing whitespace
167+
for (var childIndex = 0; childIndex < node.Children.Count; childIndex++)
168+
{
169+
if (node.Children[childIndex] is CSharpCodeIntermediateNode)
170+
{
171+
childIndex -= RemoveContiguousWhitespace(node.Children, TraversalDirection.Backwards, childIndex - 1);
172+
RemoveContiguousWhitespace(node.Children, TraversalDirection.Forwards, childIndex + 1);
173+
}
174+
}
175+
176+
base.VisitDefault(node);
177+
}
178+
}
113179
}
114180
}

src/Razor/Microsoft.AspNetCore.Razor.Language/src/DirectiveDescriptorBuilderExtensions.cs

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,28 @@ public static IDirectiveDescriptorBuilder AddAttributeToken(this IDirectiveDescr
117117
return builder;
118118
}
119119

120+
public static IDirectiveDescriptorBuilder AddBooleanToken(this IDirectiveDescriptorBuilder builder)
121+
{
122+
return AddBooleanToken(builder, name: null, description: null);
123+
}
124+
125+
public static IDirectiveDescriptorBuilder AddBooleanToken(this IDirectiveDescriptorBuilder builder, string name, string description)
126+
{
127+
if (builder == null)
128+
{
129+
throw new ArgumentNullException(nameof(builder));
130+
}
131+
132+
builder.Tokens.Add(
133+
DirectiveTokenDescriptor.CreateToken(
134+
DirectiveTokenKind.Boolean,
135+
optional: false,
136+
name: name,
137+
description: description));
138+
139+
return builder;
140+
}
141+
120142
public static IDirectiveDescriptorBuilder AddOptionalMemberToken(this IDirectiveDescriptorBuilder builder)
121143
{
122144
return AddOptionalMemberToken(builder, name: null, description: null);

src/Razor/Microsoft.AspNetCore.Razor.Language/src/DirectiveTokenKind.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,5 +10,6 @@ public enum DirectiveTokenKind
1010
Member,
1111
String,
1212
Attribute,
13+
Boolean,
1314
}
1415
}

src/Razor/Microsoft.AspNetCore.Razor.Language/src/Legacy/CSharpCodeParser.cs

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1322,7 +1322,8 @@ private void ParseExtensibleDirective(in SyntaxListBuilder<RazorSyntaxNode> buil
13221322
if (tokenDescriptor.Kind == DirectiveTokenKind.Member ||
13231323
tokenDescriptor.Kind == DirectiveTokenKind.Namespace ||
13241324
tokenDescriptor.Kind == DirectiveTokenKind.Type ||
1325-
tokenDescriptor.Kind == DirectiveTokenKind.Attribute)
1325+
tokenDescriptor.Kind == DirectiveTokenKind.Attribute ||
1326+
tokenDescriptor.Kind == DirectiveTokenKind.Boolean)
13261327
{
13271328
SpanContext.ChunkGenerator = SpanChunkGenerator.Null;
13281329
SpanContext.EditHandler.AcceptedCharacters = AcceptedCharactersInternal.Whitespace;
@@ -1417,6 +1418,22 @@ private void ParseExtensibleDirective(in SyntaxListBuilder<RazorSyntaxNode> buil
14171418
return;
14181419
}
14191420
break;
1421+
1422+
case DirectiveTokenKind.Boolean:
1423+
if (AtBooleanLiteral() && !CurrentToken.ContainsDiagnostics)
1424+
{
1425+
AcceptAndMoveNext();
1426+
}
1427+
else
1428+
{
1429+
Context.ErrorSink.OnError(
1430+
RazorDiagnosticFactory.CreateParsing_DirectiveExpectsBooleanLiteral(
1431+
new SourceSpan(CurrentStart, CurrentToken.Content.Length), descriptor.Directive));
1432+
builder.Add(BuildDirective());
1433+
return;
1434+
}
1435+
break;
1436+
14201437
case DirectiveTokenKind.Attribute:
14211438
if (At(SyntaxKind.LeftBracket))
14221439
{
@@ -1699,6 +1716,12 @@ private bool TryParseKeyword(in SyntaxListBuilder<RazorSyntaxNode> builder, IRea
16991716
return false;
17001717
}
17011718

1719+
private bool AtBooleanLiteral()
1720+
{
1721+
var result = CSharpTokenizer.GetTokenKeyword(CurrentToken);
1722+
return result.HasValue && (result.Value == CSharpKeyword.True || result.Value == CSharpKeyword.False);
1723+
}
1724+
17021725
private void SetupExpressionParsers()
17031726
{
17041727
MapExpressionKeyword(ParseAwaitExpression, CSharpKeyword.Await);

src/Razor/Microsoft.AspNetCore.Razor.Language/src/RazorDiagnosticFactory.cs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -426,6 +426,16 @@ public static RazorDiagnostic CreateParsing_DirectiveExpectsCSharpAttribute(Sour
426426
{
427427
return RazorDiagnostic.Create(Parsing_DirectiveExpectsCSharpAttribute, location, directiveName);
428428
}
429+
430+
internal static readonly RazorDiagnosticDescriptor Parsing_DirectiveExpectsBooleanLiteral =
431+
new RazorDiagnosticDescriptor(
432+
$"{DiagnosticPrefix}1038",
433+
() => Resources.DirectiveExpectsBooleanLiteral,
434+
RazorDiagnosticSeverity.Error);
435+
public static RazorDiagnostic CreateParsing_DirectiveExpectsBooleanLiteral(SourceSpan location, string directiveName)
436+
{
437+
return RazorDiagnostic.Create(Parsing_DirectiveExpectsBooleanLiteral, location, directiveName);
438+
}
429439
#endregion
430440

431441
#region Semantic Errors

0 commit comments

Comments
 (0)