Skip to content

Commit

Permalink
Strip insignificant whitespace at compile time (#23385)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
SteveSandersonMS authored Jun 30, 2020
1 parent 126f14d commit eb76931
Show file tree
Hide file tree
Showing 106 changed files with 1,591 additions and 234 deletions.
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
@using BasicWebSite.Services
@inject WeatherForecastService ForecastService

@preservewhitespace true
<h1>Weather forecast</h1>

<p>This component demonstrates fetching data from the server.</p>
Expand Down Expand Up @@ -34,7 +34,6 @@ else
</tbody>
</table>
}

@code {
[Parameter] public DateTime StartDate { get; set; }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ protected override void BuildRenderTree(Microsoft.AspNetCore.Components.Renderin
#line hidden
#nullable disable
);
__builder.AddMarkupContent(4, "\r\n");
__builder.CloseElement();
}
#pragma warning restore 1998
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,5 @@ Document -
LazyIntermediateToken - (74:3,0 [4] BasicComponent.cshtml) - Html -
CSharpExpression - (79:3,5 [29] BasicComponent.cshtml)
LazyIntermediateToken - (79:3,5 [29] BasicComponent.cshtml) - CSharp - string.Format("{0}", "Hello")
HtmlContent - (108:3,34 [2] BasicComponent.cshtml)
LazyIntermediateToken - (108:3,34 [2] BasicComponent.cshtml) - Html - \n
CSharpCode - (132:6,12 [37] BasicComponent.cshtml)
LazyIntermediateToken - (132:6,12 [37] BasicComponent.cshtml) - CSharp - \n void IDisposable.Dispose(){ }\n
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,15 @@
<data name="PageDirective_RouteToken_Name" xml:space="preserve">
<value>route template</value>
</data>
<data name="PreserveWhitespaceDirective_BooleanToken_Description" xml:space="preserve">
<value>True if whitespace should be preserved, otherwise false.</value>
</data>
<data name="PreserveWhitespaceDirective_BooleanToken_Name" xml:space="preserve">
<value>Preserve</value>
</data>
<data name="PreserveWhitespaceDirective_Description" xml:space="preserve">
<value>Specifies whether or not whitespace should be preserved exactly. Defaults to false for better performance.</value>
</data>
<data name="RefTagHelper_Documentation" xml:space="preserve">
<value>Populates the specified field or property with a reference to the element or component.</value>
</data>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// 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;

namespace Microsoft.AspNetCore.Razor.Language.Components
{
internal static class ComponentPreserveWhitespaceDirective
{
public static readonly DirectiveDescriptor Directive = DirectiveDescriptor.CreateDirective(
"preservewhitespace",
DirectiveKind.SingleLine,
builder =>
{
builder.AddBooleanToken(ComponentResources.PreserveWhitespaceDirective_BooleanToken_Name, ComponentResources.PreserveWhitespaceDirective_BooleanToken_Description);
builder.Usage = DirectiveUsage.FileScopedMultipleOccurring;
builder.Description = ComponentResources.PreserveWhitespaceDirective_Description;
});

public static void Register(RazorProjectEngineBuilder builder)
{
if (builder == null)
{
throw new ArgumentNullException(nameof(builder));
}

builder.AddDirective(Directive, FileKinds.Component, FileKinds.ComponentImport);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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.Linq;
using Microsoft.AspNetCore.Razor.Language.Intermediate;

namespace Microsoft.AspNetCore.Razor.Language.Components
Expand Down Expand Up @@ -38,17 +39,45 @@ protected override void ExecuteCore(RazorCodeDocument codeDocument, DocumentInte
return;
}

// Respect @preservewhitespace directives
if (PreserveWhitespaceIsEnabled(documentNode))
{
return;
}

var method = documentNode.FindPrimaryMethod();
if (method != null)
{
RemoveContiguousWhitespace(method.Children, TraversalDirection.Forwards);
RemoveContiguousWhitespace(method.Children, TraversalDirection.Backwards);

var visitor = new Visitor();
visitor.Visit(method);
}
}

private static bool PreserveWhitespaceIsEnabled(DocumentIntermediateNode documentNode)
{
// If there's no @preservewhitespace attribute, the default is that we *don't* preserve whitespace
var shouldPreserveWhitespace = false;

foreach (var preserveWhitespaceDirective in documentNode.FindDirectiveReferences(ComponentPreserveWhitespaceDirective.Directive))
{
var token = ((DirectiveIntermediateNode)preserveWhitespaceDirective.Node).Tokens.FirstOrDefault();
var shouldPreserveWhitespaceContent = token?.Content;
if (shouldPreserveWhitespaceContent != null)
{
shouldPreserveWhitespace = string.Equals(shouldPreserveWhitespaceContent, "true", StringComparison.Ordinal);
}
}

return shouldPreserveWhitespace;
}

private static void RemoveContiguousWhitespace(IntermediateNodeCollection nodes, TraversalDirection direction)
private static int RemoveContiguousWhitespace(IntermediateNodeCollection nodes, TraversalDirection direction, int? startIndex = null)
{
var position = direction == TraversalDirection.Forwards ? 0 : nodes.Count - 1;
var position = startIndex.GetValueOrDefault(direction == TraversalDirection.Forwards ? 0 : nodes.Count - 1);
var countRemoved = 0;
while (position >= 0 && position < nodes.Count)
{
var node = nodes[position];
Expand Down Expand Up @@ -76,7 +105,7 @@ private static void RemoveContiguousWhitespace(IntermediateNodeCollection nodes,
shouldContinueIteration = false;
break;

case CSharpCodeIntermediateNode codeIntermediateNode:
case CSharpCodeIntermediateNode _:
shouldRemoveNode = false;
shouldContinueIteration = false;
break;
Expand All @@ -90,6 +119,7 @@ private static void RemoveContiguousWhitespace(IntermediateNodeCollection nodes,
if (shouldRemoveNode)
{
nodes.RemoveAt(position);
countRemoved++;
if (direction == TraversalDirection.Forwards)
{
position--;
Expand All @@ -103,12 +133,48 @@ private static void RemoveContiguousWhitespace(IntermediateNodeCollection nodes,
break;
}
}

return countRemoved;
}

enum TraversalDirection
{
Forwards,
Backwards
}

class Visitor : IntermediateNodeWalker
{
public override void VisitMarkupElement(MarkupElementIntermediateNode node)
{
RemoveContiguousWhitespace(node.Children, TraversalDirection.Forwards);
RemoveContiguousWhitespace(node.Children, TraversalDirection.Backwards);
VisitDefault(node);
}

public override void VisitTagHelperBody(TagHelperBodyIntermediateNode node)
{
// The goal here is to remove leading/trailing whitespace inside component child content. However,
// at the time this whitespace pass runs, ComponentChildContent is still TagHelperBody in the tree.
RemoveContiguousWhitespace(node.Children, TraversalDirection.Forwards);
RemoveContiguousWhitespace(node.Children, TraversalDirection.Backwards);
VisitDefault(node);
}

public override void VisitDefault(IntermediateNode node)
{
// For any CSharpCodeIntermediateNode children, remove their preceding and trailing whitespace
for (var childIndex = 0; childIndex < node.Children.Count; childIndex++)
{
if (node.Children[childIndex] is CSharpCodeIntermediateNode)
{
childIndex -= RemoveContiguousWhitespace(node.Children, TraversalDirection.Backwards, childIndex - 1);
RemoveContiguousWhitespace(node.Children, TraversalDirection.Forwards, childIndex + 1);
}
}

base.VisitDefault(node);
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,28 @@ public static IDirectiveDescriptorBuilder AddAttributeToken(this IDirectiveDescr
return builder;
}

public static IDirectiveDescriptorBuilder AddBooleanToken(this IDirectiveDescriptorBuilder builder)
{
return AddBooleanToken(builder, name: null, description: null);
}

public static IDirectiveDescriptorBuilder AddBooleanToken(this IDirectiveDescriptorBuilder builder, string name, string description)
{
if (builder == null)
{
throw new ArgumentNullException(nameof(builder));
}

builder.Tokens.Add(
DirectiveTokenDescriptor.CreateToken(
DirectiveTokenKind.Boolean,
optional: false,
name: name,
description: description));

return builder;
}

public static IDirectiveDescriptorBuilder AddOptionalMemberToken(this IDirectiveDescriptorBuilder builder)
{
return AddOptionalMemberToken(builder, name: null, description: null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,6 @@ public enum DirectiveTokenKind
Member,
String,
Attribute,
Boolean,
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1322,7 +1322,8 @@ private void ParseExtensibleDirective(in SyntaxListBuilder<RazorSyntaxNode> buil
if (tokenDescriptor.Kind == DirectiveTokenKind.Member ||
tokenDescriptor.Kind == DirectiveTokenKind.Namespace ||
tokenDescriptor.Kind == DirectiveTokenKind.Type ||
tokenDescriptor.Kind == DirectiveTokenKind.Attribute)
tokenDescriptor.Kind == DirectiveTokenKind.Attribute ||
tokenDescriptor.Kind == DirectiveTokenKind.Boolean)
{
SpanContext.ChunkGenerator = SpanChunkGenerator.Null;
SpanContext.EditHandler.AcceptedCharacters = AcceptedCharactersInternal.Whitespace;
Expand Down Expand Up @@ -1417,6 +1418,22 @@ private void ParseExtensibleDirective(in SyntaxListBuilder<RazorSyntaxNode> buil
return;
}
break;

case DirectiveTokenKind.Boolean:
if (AtBooleanLiteral() && !CurrentToken.ContainsDiagnostics)
{
AcceptAndMoveNext();
}
else
{
Context.ErrorSink.OnError(
RazorDiagnosticFactory.CreateParsing_DirectiveExpectsBooleanLiteral(
new SourceSpan(CurrentStart, CurrentToken.Content.Length), descriptor.Directive));
builder.Add(BuildDirective());
return;
}
break;

case DirectiveTokenKind.Attribute:
if (At(SyntaxKind.LeftBracket))
{
Expand Down Expand Up @@ -1699,6 +1716,12 @@ private bool TryParseKeyword(in SyntaxListBuilder<RazorSyntaxNode> builder, IRea
return false;
}

private bool AtBooleanLiteral()
{
var result = CSharpTokenizer.GetTokenKeyword(CurrentToken);
return result.HasValue && (result.Value == CSharpKeyword.True || result.Value == CSharpKeyword.False);
}

private void SetupExpressionParsers()
{
MapExpressionKeyword(ParseAwaitExpression, CSharpKeyword.Await);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -426,6 +426,16 @@ public static RazorDiagnostic CreateParsing_DirectiveExpectsCSharpAttribute(Sour
{
return RazorDiagnostic.Create(Parsing_DirectiveExpectsCSharpAttribute, location, directiveName);
}

internal static readonly RazorDiagnosticDescriptor Parsing_DirectiveExpectsBooleanLiteral =
new RazorDiagnosticDescriptor(
$"{DiagnosticPrefix}1038",
() => Resources.DirectiveExpectsBooleanLiteral,
RazorDiagnosticSeverity.Error);
public static RazorDiagnostic CreateParsing_DirectiveExpectsBooleanLiteral(SourceSpan location, string directiveName)
{
return RazorDiagnostic.Create(Parsing_DirectiveExpectsBooleanLiteral, location, directiveName);
}
#endregion

#region Semantic Errors
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ public static RazorProjectEngine Create(
NamespaceDirective.Register(builder);
AttributeDirective.Register(builder);

AddComponentFeatures(builder);
AddComponentFeatures(builder, configuration.LanguageVersion);
}

LoadExtensions(builder, configuration.Extensions);
Expand Down Expand Up @@ -206,7 +206,7 @@ private static void AddDefaultFeatures(ICollection<IRazorFeature> features)
});
}

private static void AddComponentFeatures(RazorProjectEngineBuilder builder)
private static void AddComponentFeatures(RazorProjectEngineBuilder builder, RazorLanguageVersion razorLanguageVersion)
{
// Project Engine Features
builder.Features.Add(new ComponentImportProjectFeature());
Expand All @@ -218,6 +218,11 @@ private static void AddComponentFeatures(RazorProjectEngineBuilder builder)
ComponentPageDirective.Register(builder);
ComponentTypeParamDirective.Register(builder);

if (razorLanguageVersion.CompareTo(RazorLanguageVersion.Version_5_0) >= 0)
{
ComponentPreserveWhitespaceDirective.Register(builder);
}

// Document Classifier
builder.Features.Add(new ComponentDocumentClassifierPass());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -559,4 +559,7 @@
<data name="TagHelper_InvalidRequiredDirectiveAttributeName" xml:space="preserve">
<value>Invalid tag helper required directive attribute '{0}'. The directive attribute '{1}' should start with a '@' character.</value>
</data>
<data name="DirectiveExpectsBooleanLiteral" xml:space="preserve">
<value>The '{0}' directive expects a boolean literal.</value>
</data>
</root>
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,9 @@ public void Execute_RewritesHtml_Basic()
</html>");

var expected = NormalizeContent(@"
<html>
<head cool=""beans"">
<html><head cool=""beans"">
Hello, World!
</head>
</html>");
</head></html>");

var documentNode = Lower(document);

Expand Down Expand Up @@ -163,7 +161,7 @@ public void Execute_RewritesHtml_CSharpInAttributes()
</head>
</html>");

var expected = NormalizeContent("<div>foo</div>\n ");
var expected = NormalizeContent("<div>foo</div>");

var documentNode = Lower(document);

Expand Down Expand Up @@ -349,11 +347,8 @@ public void Execute_CanRewriteHtml_OptionWithNoSelectAncestor()
</html>");

var expected = NormalizeContent(@"
<head cool=""beans"">
<option value=""1"">One</option>
<option selected value=""2"">Two</option>
</head>
");
<head cool=""beans""><option value=""1"">One</option>
<option selected value=""2"">Two</option></head>");

var documentNode = Lower(document);

Expand Down
Loading

0 comments on commit eb76931

Please sign in to comment.