Skip to content

Commit

Permalink
Allow markup in local functions and @functions.
Browse files Browse the repository at this point in the history
- Allow markup to exist in all code blocks. Prior to this change whenever we'd see nested curly braces we would do dumb brace matching to skip over any potentially risky code; now we treat every curly brace as an opportunity to intermingle Markup.
- One fix I had to introduce was now that functions blocks are parsed like `@{}` blocks I ran into cases where certain reserved keywords would get improperly parsed. This exposed a bug in our parsing where we’d treat **class** and **namespace** as directives without a transition in a `@{}` block. For instance this:
```
@{
    class
}
```
would barf in the old parser by treating the `class` piece as a directive even though it did not have a transition. To account for this I changed our reserved directives to be parsed as directives instead of keywords (it's how they should have been parsed anyhow). This isn't a breaking change because the directive parsing logic is a subset of how keywords get parsed.

- One quirk this change introduces is a difference in behavior in regards to one error case. Before this change if you were to have `@if (foo)) { var bar = foo; }` the entire statement would be classified as C# and you'd get a C# error on the trailing `)`. With my changes we try to keep group statements together more closely and allow for HTML in unexpected or end of statement scenarios. So, with these new changes the above example would only have `@if (foo))` classified as C# and the rest as markup because the original was invalid.
- Added lots of tests,
- Modified the feature flag to maintain the old behavior when disabled.

dotnet/aspnetcore#5110
  • Loading branch information
NTaylorMullen committed Mar 20, 2019
1 parent cfee40a commit b1a640b
Show file tree
Hide file tree
Showing 58 changed files with 3,004 additions and 59 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -482,7 +482,14 @@ private CSharpStatementBodySyntax ParseStatementBody(Block block = null)
// Set up auto-complete and parse the code block
var editHandler = new AutoCompleteEditHandler(Language.TokenizeString);
SpanContext.EditHandler = editHandler;
ParseCodeBlock(builder, block, acceptTerminatingBrace: false);
ParseCodeBlock(builder, block);

if (EndOfFile)
{
Context.ErrorSink.OnError(
RazorDiagnosticFactory.CreateParsing_ExpectedEndOfBlockBeforeEOF(
new SourceSpan(block.Start, contentLength: 1 /* { OR } */), block.Name, "}", "{"));
}

EnsureCurrent();
SpanContext.ChunkGenerator = new StatementChunkGenerator();
Expand Down Expand Up @@ -521,7 +528,7 @@ private CSharpStatementBodySyntax ParseStatementBody(Block block = null)
return SyntaxFactory.CSharpStatementBody(leftBrace, codeBlock, rightBrace);
}

private void ParseCodeBlock(in SyntaxListBuilder<RazorSyntaxNode> builder, Block block, bool acceptTerminatingBrace = true)
private void ParseCodeBlock(in SyntaxListBuilder<RazorSyntaxNode> builder, Block block)
{
EnsureCurrent();
while (!EndOfFile && !At(SyntaxKind.RightBrace))
Expand All @@ -530,19 +537,6 @@ private void ParseCodeBlock(in SyntaxListBuilder<RazorSyntaxNode> builder, Block
ParseStatement(builder, block: block);
EnsureCurrent();
}

if (EndOfFile)
{
Context.ErrorSink.OnError(
RazorDiagnosticFactory.CreateParsing_ExpectedEndOfBlockBeforeEOF(
new SourceSpan(block.Start, contentLength: 1 /* { OR } */), block.Name, "}", "{"));
}
else if (acceptTerminatingBrace)
{
Assert(SyntaxKind.RightBrace);
SpanContext.EditHandler.AcceptedCharacters = AcceptedCharactersInternal.None;
AcceptAndMoveNext();
}
}

private void ParseStatement(in SyntaxListBuilder<RazorSyntaxNode> builder, Block block)
Expand Down Expand Up @@ -634,6 +628,24 @@ private void ParseStatement(in SyntaxListBuilder<RazorSyntaxNode> builder, Block
// Verbatim Block
AcceptAndMoveNext();
ParseCodeBlock(builder, block);

// ParseCodeBlock is responsible for parsing the insides of a code block (non-inclusive of braces).
// Therefore, there's one of two cases after parsing:
// 1. We've hit the End of File (incomplete parse block).
// 2. It's a complete parse block and we're at a right brace.

if (EndOfFile)
{
Context.ErrorSink.OnError(
RazorDiagnosticFactory.CreateParsing_ExpectedEndOfBlockBeforeEOF(
new SourceSpan(block.Start, contentLength: 1 /* { OR } */), block.Name, "}", "{"));
}
else
{
Assert(SyntaxKind.RightBrace);
SpanContext.EditHandler.AcceptedCharacters = AcceptedCharactersInternal.None;
AcceptAndMoveNext();
}
break;
case SyntaxKind.Keyword:
if (!TryParseKeyword(builder, whitespace: null, transition: null))
Expand Down Expand Up @@ -736,7 +748,7 @@ private void ParseStandardStatement(in SyntaxListBuilder<RazorSyntaxNode> builde
token.Kind != SyntaxKind.LeftBracket &&
token.Kind != SyntaxKind.RightBrace);

if (At(SyntaxKind.LeftBrace) ||
if ((!Context.FeatureFlags.AllowRazorInAllCodeBlocks && At(SyntaxKind.LeftBrace)) ||
At(SyntaxKind.LeftParenthesis) ||
At(SyntaxKind.LeftBracket))
{
Expand All @@ -752,6 +764,11 @@ private void ParseStandardStatement(in SyntaxListBuilder<RazorSyntaxNode> builde
return;
}
}
else if (Context.FeatureFlags.AllowRazorInAllCodeBlocks && At(SyntaxKind.LeftBrace))
{
Accept(read);
return;
}
else if (At(SyntaxKind.Transition) && (NextIs(SyntaxKind.LessThan, SyntaxKind.Colon)))
{
Accept(read);
Expand Down Expand Up @@ -847,6 +864,17 @@ private void SetupDirectiveParsers(IEnumerable<DirectiveDescriptor> directiveDes
MapDirectives(ParseTagHelperPrefixDirective, SyntaxConstants.CSharp.TagHelperPrefixKeyword);
MapDirectives(ParseAddTagHelperDirective, SyntaxConstants.CSharp.AddTagHelperKeyword);
MapDirectives(ParseRemoveTagHelperDirective, SyntaxConstants.CSharp.RemoveTagHelperKeyword);

// If there wasn't any extensible directives relating to the reserved directives then map them.
if (!_directiveParserMap.ContainsKey("class"))
{
MapDirectives(ParseReservedDirective, "class");
}

if (!_directiveParserMap.ContainsKey("namespace"))
{
MapDirectives(ParseReservedDirective, "namespace");
}
}

private void EnsureDirectiveIsAtStartOfLine()
Expand Down Expand Up @@ -883,17 +911,6 @@ protected void MapDirectives(Action<SyntaxListBuilder<RazorSyntaxNode>, CSharpTr
});

Keywords.Add(directive);

// These C# keywords are reserved for use in directives. It's an error to use them outside of
// a directive. This code removes the error generation if the directive *is* registered.
if (string.Equals(directive, "class", StringComparison.OrdinalIgnoreCase))
{
_keywordParserMap.Remove(CSharpKeyword.Class);
}
else if (string.Equals(directive, "namespace", StringComparison.OrdinalIgnoreCase))
{
_keywordParserMap.Remove(CSharpKeyword.Namespace);
}
}
}

Expand Down Expand Up @@ -1409,11 +1426,22 @@ private void ParseExtensibleDirective(in SyntaxListBuilder<RazorSyntaxNode> buil
ParseDirectiveBlock(directiveBuilder, descriptor, parseChildren: (childBuilder, startingBraceLocation) =>
{
NextToken();
Balance(childBuilder, BalancingModes.NoErrorOnFailure, SyntaxKind.LeftBrace, SyntaxKind.RightBrace, startingBraceLocation);
SpanContext.ChunkGenerator = new StatementChunkGenerator();
var existingEditHandler = SpanContext.EditHandler;
SpanContext.EditHandler = new CodeBlockEditHandler(Language.TokenizeString);
if (Context.FeatureFlags.AllowRazorInAllCodeBlocks)
{
var block = new Block(descriptor.Directive, directiveStart);
ParseCodeBlock(childBuilder, block);
}
else
{
Balance(childBuilder, BalancingModes.NoErrorOnFailure, SyntaxKind.LeftBrace, SyntaxKind.RightBrace, startingBraceLocation);
}
SpanContext.ChunkGenerator = new StatementChunkGenerator();
AcceptMarkerTokenIfNecessary();
childBuilder.Add(OutputTokensAsStatementLiteral());
Expand Down Expand Up @@ -1607,7 +1635,6 @@ private void SetupKeywordParsers()
MapKeywords(ParseTryStatement, CSharpKeyword.Try);
MapKeywords(ParseDoStatement, CSharpKeyword.Do);
MapKeywords(ParseUsingKeyword, CSharpKeyword.Using);
MapKeywords(ParseReservedDirective, CSharpKeyword.Class, CSharpKeyword.Namespace);
}

private void MapExpressionKeyword(Action<SyntaxListBuilder<RazorSyntaxNode>, CSharpTransitionSyntax> handler, CSharpKeyword keyword)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ internal static bool ModifiesInvalidContent(SyntaxNode target, SourceChange chan
{
var relativePosition = change.Span.AbsoluteIndex - target.Position;

if (target.GetContent().IndexOfAny(new[] { '{', '}' }, relativePosition, change.Span.Length) >= 0)
if (target.GetContent().IndexOfAny(new[] { '{', '}', '@', '<', '*', }, relativePosition, change.Span.Length) >= 0)
{
return true;
}
Expand All @@ -103,7 +103,7 @@ internal static bool IsAcceptableInsertion(SourceChange change)
// Internal for testing
internal static bool ContainsInvalidContent(SourceChange change)
{
if (change.NewText.IndexOfAny(new[] { '{', '}' }) >= 0)
if (change.NewText.IndexOfAny(new[] { '{', '}', '@', '<', '*', }) >= 0)
{
return true;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ public static RazorParserFeatureFlags Create(RazorLanguageVersion version)
var allowMinimizedBooleanTagHelperAttributes = false;
var allowHtmlCommentsInTagHelpers = false;
var allowComponentFileKind = false;
var allowRazorInAllCodeBlocks = false;
var experimental_AllowConditionalDataDashAttributes = false;

if (version.CompareTo(RazorLanguageVersion.Version_2_1) >= 0)
Expand All @@ -23,6 +24,7 @@ public static RazorParserFeatureFlags Create(RazorLanguageVersion version)
{
// Added in 3.0
allowComponentFileKind = true;
allowRazorInAllCodeBlocks = true;
}

if (version.CompareTo(RazorLanguageVersion.Experimental) >= 0)
Expand All @@ -34,6 +36,7 @@ public static RazorParserFeatureFlags Create(RazorLanguageVersion version)
allowMinimizedBooleanTagHelperAttributes,
allowHtmlCommentsInTagHelpers,
allowComponentFileKind,
allowRazorInAllCodeBlocks,
experimental_AllowConditionalDataDashAttributes);
}

Expand All @@ -43,6 +46,8 @@ public static RazorParserFeatureFlags Create(RazorLanguageVersion version)

public abstract bool AllowComponentFileKind { get; }

public abstract bool AllowRazorInAllCodeBlocks { get; }

public abstract bool EXPERIMENTAL_AllowConditionalDataDashAttributes { get; }

private class DefaultRazorParserFeatureFlags : RazorParserFeatureFlags
Expand All @@ -51,11 +56,13 @@ public DefaultRazorParserFeatureFlags(
bool allowMinimizedBooleanTagHelperAttributes,
bool allowHtmlCommentsInTagHelpers,
bool allowComponentFileKind,
bool allowRazorInAllCodeBlocks,
bool experimental_AllowConditionalDataDashAttributes)
{
AllowMinimizedBooleanTagHelperAttributes = allowMinimizedBooleanTagHelperAttributes;
AllowHtmlCommentsInTagHelpers = allowHtmlCommentsInTagHelpers;
AllowComponentFileKind = allowComponentFileKind;
AllowRazorInAllCodeBlocks = allowRazorInAllCodeBlocks;
EXPERIMENTAL_AllowConditionalDataDashAttributes = experimental_AllowConditionalDataDashAttributes;
}

Expand All @@ -65,6 +72,8 @@ public DefaultRazorParserFeatureFlags(

public override bool AllowComponentFileKind { get; }

public override bool AllowRazorInAllCodeBlocks { get; }

public override bool EXPERIMENTAL_AllowConditionalDataDashAttributes { get; }
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,18 @@ public void Templates_Runtime()
RunTimeTest();
}

[Fact]
public void Markup_InCodeBlocks_Runtime()
{
RunTimeTest();
}

[Fact]
public void Markup_InCodeBlocksWithTagHelper_Runtime()
{
RunRuntimeTagHelpersTest(TestTagHelperDescriptors.SimpleTagHelperDescriptors);
}

[Fact]
public void StringLiterals_Runtime()
{
Expand Down Expand Up @@ -503,6 +515,18 @@ public void Templates_DesignTime()
DesignTimeTest();
}

[Fact]
public void Markup_InCodeBlocks_DesignTime()
{
DesignTimeTest();
}

[Fact]
public void Markup_InCodeBlocksWithTagHelper_DesignTime()
{
RunDesignTimeTagHelpersTest(TestTagHelperDescriptors.SimpleTagHelperDescriptors);
}

[Fact]
public void StringLiterals_DesignTime()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,76 @@ protected ComponentCodeGenerationTestBase()

#region Basics

[Fact]
public void ChildComponent_InFunctionsDirective()
{
// Arrange
AdditionalSyntaxTrees.Add(Parse(@"
using Microsoft.AspNetCore.Components;
namespace Test
{
public class MyComponent : ComponentBase
{
}
}
"));

// Act
var generated = CompileToCSharp(@"
@addTagHelper *, TestAssembly
@using Microsoft.AspNetCore.Components.RenderTree;
@{ RenderChildComponent(builder); }
@functions {
void RenderChildComponent(RenderTreeBuilder builder)
{
<MyComponent />
}
}");

// Assert
AssertDocumentNodeMatchesBaseline(generated.CodeDocument);
AssertCSharpDocumentMatchesBaseline(generated.CodeDocument);
CompileToAssembly(generated);
}

[Fact]
public void ChildComponent_InLocalFunction()
{
// Arrange
AdditionalSyntaxTrees.Add(Parse(@"
using Microsoft.AspNetCore.Components;
namespace Test
{
public class MyComponent : ComponentBase
{
}
}
"));

// Act
var generated = CompileToCSharp(@"
@addTagHelper *, TestAssembly
@using Microsoft.AspNetCore.Components.RenderTree;
@{
void RenderChildComponent()
{
<MyComponent />
}
}
@{ RenderChildComponent(); }
");

// Assert
AssertDocumentNodeMatchesBaseline(generated.CodeDocument);
AssertCSharpDocumentMatchesBaseline(generated.CodeDocument);
CompileToAssembly(generated);
}

[Fact]
public void ChildComponent_Simple()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,36 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy
{
public class CSharpBlockTest : ParserTestBase
{
[Fact]
public void LocalFunctionsWithRazor()
{
ParseDocumentTest(
@"@{
void Foo()
{
var time = DateTime.Now
<strong>Hello the time is @time</strong>
}
}");
}

[Fact]
public void LocalFunctionsWithGenerics()
{
ParseDocumentTest(
@"@{
void Foo()
{
<strong>Hello the time is @{ DisplayCount(new List<string>()); }</strong>
}
void DisplayCount<T>(List<T> something)
{
<text>The count is something.Count</text>
}
}");
}

[Fact]
public void NestedCodeBlockWithCSharpAt()
{
Expand Down
Loading

0 comments on commit b1a640b

Please sign in to comment.