From 2e8c154fcb2952b9b42610258dd600883fda461f Mon Sep 17 00:00:00 2001 From: "N. Taylor Mullen" Date: Wed, 31 May 2017 12:17:37 -0700 Subject: [PATCH] Make namespace tokens tolerant to EOF and invalid states. - Updated our `QualifiedIdentifier` to properly parse invalid namespaces. This is more consistent with how the rest of the system works; we consume tokens until we determine if we're in an invalid or valid state. If invalid, we log an error and put back all invalid tokens for the parser to treat as non-directive tokens. - Added unit tests to validate our extensible directive system can handle malformed namespace tokens at EOF and inline. - Added a code gen test for Razor.Extensions to prove we've fixed the underlying issue for our `@namespace` directive that crashed VS. #1393 --- .../Legacy/CSharpCodeParser.cs | 48 ++++++++-- .../CodeGenerationIntegrationTest.cs | 21 +++++ .../InvalidNamespaceAtEOF.cshtml | 1 + ...nvalidNamespaceAtEOF_DesignTime.codegen.cs | 35 +++++++ .../InvalidNamespaceAtEOF_DesignTime.ir.txt | 36 ++++++++ ...alidNamespaceAtEOF_DesignTime.mappings.txt | 0 .../InvalidNamespaceAtEOF_Runtime.codegen.cs | 33 +++++++ .../InvalidNamespaceAtEOF_Runtime.ir.txt | 23 +++++ .../Legacy/CSharpDirectivesTest.cs | 91 +++++++++++++++++++ 9 files changed, 280 insertions(+), 8 deletions(-) create mode 100644 test/Microsoft.AspNetCore.Mvc.Razor.Extensions.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/InvalidNamespaceAtEOF.cshtml create mode 100644 test/Microsoft.AspNetCore.Mvc.Razor.Extensions.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/InvalidNamespaceAtEOF_DesignTime.codegen.cs create mode 100644 test/Microsoft.AspNetCore.Mvc.Razor.Extensions.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/InvalidNamespaceAtEOF_DesignTime.ir.txt create mode 100644 test/Microsoft.AspNetCore.Mvc.Razor.Extensions.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/InvalidNamespaceAtEOF_DesignTime.mappings.txt create mode 100644 test/Microsoft.AspNetCore.Mvc.Razor.Extensions.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/InvalidNamespaceAtEOF_Runtime.codegen.cs create mode 100644 test/Microsoft.AspNetCore.Mvc.Razor.Extensions.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/InvalidNamespaceAtEOF_Runtime.ir.txt diff --git a/src/Microsoft.AspNetCore.Razor.Language/Legacy/CSharpCodeParser.cs b/src/Microsoft.AspNetCore.Razor.Language/Legacy/CSharpCodeParser.cs index 7d46c3f7f6..b96181a9d1 100644 --- a/src/Microsoft.AspNetCore.Razor.Language/Legacy/CSharpCodeParser.cs +++ b/src/Microsoft.AspNetCore.Razor.Language/Legacy/CSharpCodeParser.cs @@ -909,21 +909,53 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy // qualified-identifier: // identifier // qualified-identifier . identifier - protected bool QualifiedIdentifier() + protected bool QualifiedIdentifier(out int identifierLength) { - if (At(CSharpSymbolType.Identifier)) + var currentIdentifierLength = 0; + var expectingDot = false; + var tokens = ReadWhile(token => { - AcceptAndMoveNext(); - - if (Optional(CSharpSymbolType.Dot)) + var type = token.Type; + if ((expectingDot && type == CSharpSymbolType.Dot) || + (!expectingDot && type == CSharpSymbolType.Identifier)) { - return QualifiedIdentifier(); + expectingDot = !expectingDot; + return true; + } + + if (type != CSharpSymbolType.WhiteSpace && + type != CSharpSymbolType.NewLine) + { + expectingDot = false; + currentIdentifierLength += token.Content.Length; + } + + return false; + }); + + identifierLength = currentIdentifierLength; + var validQualifiedIdentifier = expectingDot; + if (validQualifiedIdentifier) + { + foreach (var token in tokens) + { + identifierLength += token.Content.Length; + Accept(token); } return true; } else { + PutCurrentBack(); + + foreach (var token in tokens) + { + identifierLength += token.Content.Length; + PutBack(token); + } + + EnsureCurrent(); return false; } } @@ -1574,12 +1606,12 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy break; case DirectiveTokenKind.Namespace: - if (!QualifiedIdentifier()) + if (!QualifiedIdentifier(out var identifierLength)) { Context.ErrorSink.OnError( CurrentStart, LegacyResources.FormatDirectiveExpectsNamespace(descriptor.Name), - CurrentSymbol.Content.Length); + identifierLength); return; } diff --git a/test/Microsoft.AspNetCore.Mvc.Razor.Extensions.Test/IntegrationTests/CodeGenerationIntegrationTest.cs b/test/Microsoft.AspNetCore.Mvc.Razor.Extensions.Test/IntegrationTests/CodeGenerationIntegrationTest.cs index 8caacfaf94..9a60a1eb9e 100644 --- a/test/Microsoft.AspNetCore.Mvc.Razor.Extensions.Test/IntegrationTests/CodeGenerationIntegrationTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Razor.Extensions.Test/IntegrationTests/CodeGenerationIntegrationTest.cs @@ -27,6 +27,16 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Extensions.IntegrationTests private static readonly RazorSourceDocument DefaultImports = MvcRazorTemplateEngine.GetDefaultImports(); #region Runtime + [Fact] + public void InvalidNamespaceAtEOF_Runtime() + { + var references = CreateCompilationReferences(CurrentMvcShim); + RunRuntimeTest(references, expectedErrors: new[] + { + "Identifier expected" + }); + } + [Fact] public void IncompleteDirectives_Runtime() { @@ -235,6 +245,17 @@ public class DivTagHelper : {typeof(TagHelper).FullName} #endregion #region DesignTime + [Fact] + public void InvalidNamespaceAtEOF_DesignTime() + { + var references = CreateCompilationReferences(CurrentMvcShim); + RunDesignTimeTest(references, + expectedErrors: new[] + { + "Identifier expected" + }); + } + [Fact] public void IncompleteDirectives_DesignTime() { diff --git a/test/Microsoft.AspNetCore.Mvc.Razor.Extensions.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/InvalidNamespaceAtEOF.cshtml b/test/Microsoft.AspNetCore.Mvc.Razor.Extensions.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/InvalidNamespaceAtEOF.cshtml new file mode 100644 index 0000000000..6dfb72bc31 --- /dev/null +++ b/test/Microsoft.AspNetCore.Mvc.Razor.Extensions.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/InvalidNamespaceAtEOF.cshtml @@ -0,0 +1 @@ +@namespace Test. \ No newline at end of file diff --git a/test/Microsoft.AspNetCore.Mvc.Razor.Extensions.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/InvalidNamespaceAtEOF_DesignTime.codegen.cs b/test/Microsoft.AspNetCore.Mvc.Razor.Extensions.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/InvalidNamespaceAtEOF_DesignTime.codegen.cs new file mode 100644 index 0000000000..0dd729a78d --- /dev/null +++ b/test/Microsoft.AspNetCore.Mvc.Razor.Extensions.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/InvalidNamespaceAtEOF_DesignTime.codegen.cs @@ -0,0 +1,35 @@ +namespace +{ + #line hidden + using TModel = global::System.Object; + using System; + using System.Collections.Generic; + using System.Linq; + using System.Threading.Tasks; + using Microsoft.AspNetCore.Mvc; + using Microsoft.AspNetCore.Mvc.Rendering; + using Microsoft.AspNetCore.Mvc.ViewFeatures; + public class TestFiles_IntegrationTests_CodeGenerationIntegrationTest_InvalidNamespaceAtEOF_cshtml : global::Microsoft.AspNetCore.Mvc.Razor.RazorPage + { + #pragma warning disable 219 + private void __RazorDirectiveTokenHelpers__() { + } + #pragma warning restore 219 + private static System.Object __o = null; + #pragma warning disable 1998 + public async override global::System.Threading.Tasks.Task ExecuteAsync() + { + } + #pragma warning restore 1998 + [global::Microsoft.AspNetCore.Mvc.Razor.Internal.RazorInjectAttribute] + public global::Microsoft.AspNetCore.Mvc.ViewFeatures.IModelExpressionProvider ModelExpressionProvider { get; private set; } + [global::Microsoft.AspNetCore.Mvc.Razor.Internal.RazorInjectAttribute] + public global::Microsoft.AspNetCore.Mvc.IUrlHelper Url { get; private set; } + [global::Microsoft.AspNetCore.Mvc.Razor.Internal.RazorInjectAttribute] + public global::Microsoft.AspNetCore.Mvc.IViewComponentHelper Component { get; private set; } + [global::Microsoft.AspNetCore.Mvc.Razor.Internal.RazorInjectAttribute] + public global::Microsoft.AspNetCore.Mvc.Rendering.IJsonHelper Json { get; private set; } + [global::Microsoft.AspNetCore.Mvc.Razor.Internal.RazorInjectAttribute] + public global::Microsoft.AspNetCore.Mvc.Rendering.IHtmlHelper Html { get; private set; } + } +} diff --git a/test/Microsoft.AspNetCore.Mvc.Razor.Extensions.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/InvalidNamespaceAtEOF_DesignTime.ir.txt b/test/Microsoft.AspNetCore.Mvc.Razor.Extensions.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/InvalidNamespaceAtEOF_DesignTime.ir.txt new file mode 100644 index 0000000000..6cf3ca93c9 --- /dev/null +++ b/test/Microsoft.AspNetCore.Mvc.Razor.Extensions.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/InvalidNamespaceAtEOF_DesignTime.ir.txt @@ -0,0 +1,36 @@ +Document - + Checksum - + NamespaceDeclaration - - + UsingStatement - - TModel = global::System.Object + UsingStatement - (1:0,1 [12] ) - System + UsingStatement - (16:1,1 [32] ) - System.Collections.Generic + UsingStatement - (51:2,1 [17] ) - System.Linq + UsingStatement - (71:3,1 [28] ) - System.Threading.Tasks + UsingStatement - (102:4,1 [30] ) - Microsoft.AspNetCore.Mvc + UsingStatement - (135:5,1 [40] ) - Microsoft.AspNetCore.Mvc.Rendering + UsingStatement - (178:6,1 [43] ) - Microsoft.AspNetCore.Mvc.ViewFeatures + ClassDeclaration - - public - TestFiles_IntegrationTests_CodeGenerationIntegrationTest_InvalidNamespaceAtEOF_cshtml - global::Microsoft.AspNetCore.Mvc.Razor.RazorPage - + DesignTimeDirective - + DirectiveToken - (231:7,8 [62] ) - global::Microsoft.AspNetCore.Mvc.Rendering.IHtmlHelper + DirectiveToken - (294:7,71 [4] ) - Html + DirectiveToken - (308:8,8 [54] ) - global::Microsoft.AspNetCore.Mvc.Rendering.IJsonHelper + DirectiveToken - (363:8,63 [4] ) - Json + DirectiveToken - (377:9,8 [53] ) - global::Microsoft.AspNetCore.Mvc.IViewComponentHelper + DirectiveToken - (431:9,62 [9] ) - Component + DirectiveToken - (450:10,8 [43] ) - global::Microsoft.AspNetCore.Mvc.IUrlHelper + DirectiveToken - (494:10,52 [3] ) - Url + DirectiveToken - (507:11,8 [70] ) - global::Microsoft.AspNetCore.Mvc.ViewFeatures.IModelExpressionProvider + DirectiveToken - (578:11,79 [23] ) - ModelExpressionProvider + DirectiveToken - (617:12,14 [96] ) - Microsoft.AspNetCore.Mvc.Razor.TagHelpers.UrlResolutionTagHelper, Microsoft.AspNetCore.Mvc.Razor + DirectiveToken - (729:13,14 [87] ) - Microsoft.AspNetCore.Mvc.Razor.TagHelpers.HeadTagHelper, Microsoft.AspNetCore.Mvc.Razor + DirectiveToken - (832:14,14 [87] ) - Microsoft.AspNetCore.Mvc.Razor.TagHelpers.BodyTagHelper, Microsoft.AspNetCore.Mvc.Razor + CSharpStatement - + RazorIRToken - - CSharp - private static System.Object __o = null; + MethodDeclaration - - public - async, override - global::System.Threading.Tasks.Task - ExecuteAsync + HtmlContent - (11:0,11 [5] InvalidNamespaceAtEOF.cshtml) + RazorIRToken - (11:0,11 [5] InvalidNamespaceAtEOF.cshtml) - Html - Test. + InjectDirective - + InjectDirective - + InjectDirective - + InjectDirective - + InjectDirective - diff --git a/test/Microsoft.AspNetCore.Mvc.Razor.Extensions.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/InvalidNamespaceAtEOF_DesignTime.mappings.txt b/test/Microsoft.AspNetCore.Mvc.Razor.Extensions.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/InvalidNamespaceAtEOF_DesignTime.mappings.txt new file mode 100644 index 0000000000..e69de29bb2 diff --git a/test/Microsoft.AspNetCore.Mvc.Razor.Extensions.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/InvalidNamespaceAtEOF_Runtime.codegen.cs b/test/Microsoft.AspNetCore.Mvc.Razor.Extensions.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/InvalidNamespaceAtEOF_Runtime.codegen.cs new file mode 100644 index 0000000000..0e8e0f2afd --- /dev/null +++ b/test/Microsoft.AspNetCore.Mvc.Razor.Extensions.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/InvalidNamespaceAtEOF_Runtime.codegen.cs @@ -0,0 +1,33 @@ +#pragma checksum "TestFiles/IntegrationTests/CodeGenerationIntegrationTest/InvalidNamespaceAtEOF.cshtml" "{ff1816ec-aa5e-4d10-87f7-6f4963833460}" "de132bd3e2a46a0d2ec953a168427c01e5829cde" +namespace +{ + #line hidden + using System; + using System.Collections.Generic; + using System.Linq; + using System.Threading.Tasks; + using Microsoft.AspNetCore.Mvc; + using Microsoft.AspNetCore.Mvc.Rendering; + using Microsoft.AspNetCore.Mvc.ViewFeatures; + public class TestFiles_IntegrationTests_CodeGenerationIntegrationTest_InvalidNamespaceAtEOF_cshtml : global::Microsoft.AspNetCore.Mvc.Razor.RazorPage + { + #pragma warning disable 1998 + public async override global::System.Threading.Tasks.Task ExecuteAsync() + { + BeginContext(11, 5, true); + WriteLiteral("Test."); + EndContext(); + } + #pragma warning restore 1998 + [global::Microsoft.AspNetCore.Mvc.Razor.Internal.RazorInjectAttribute] + public global::Microsoft.AspNetCore.Mvc.ViewFeatures.IModelExpressionProvider ModelExpressionProvider { get; private set; } + [global::Microsoft.AspNetCore.Mvc.Razor.Internal.RazorInjectAttribute] + public global::Microsoft.AspNetCore.Mvc.IUrlHelper Url { get; private set; } + [global::Microsoft.AspNetCore.Mvc.Razor.Internal.RazorInjectAttribute] + public global::Microsoft.AspNetCore.Mvc.IViewComponentHelper Component { get; private set; } + [global::Microsoft.AspNetCore.Mvc.Razor.Internal.RazorInjectAttribute] + public global::Microsoft.AspNetCore.Mvc.Rendering.IJsonHelper Json { get; private set; } + [global::Microsoft.AspNetCore.Mvc.Razor.Internal.RazorInjectAttribute] + public global::Microsoft.AspNetCore.Mvc.Rendering.IHtmlHelper Html { get; private set; } + } +} diff --git a/test/Microsoft.AspNetCore.Mvc.Razor.Extensions.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/InvalidNamespaceAtEOF_Runtime.ir.txt b/test/Microsoft.AspNetCore.Mvc.Razor.Extensions.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/InvalidNamespaceAtEOF_Runtime.ir.txt new file mode 100644 index 0000000000..637caafd8d --- /dev/null +++ b/test/Microsoft.AspNetCore.Mvc.Razor.Extensions.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/InvalidNamespaceAtEOF_Runtime.ir.txt @@ -0,0 +1,23 @@ +Document - + Checksum - + NamespaceDeclaration - - + UsingStatement - (1:0,1 [14] ) - System + UsingStatement - (16:1,1 [34] ) - System.Collections.Generic + UsingStatement - (51:2,1 [19] ) - System.Linq + UsingStatement - (71:3,1 [30] ) - System.Threading.Tasks + UsingStatement - (102:4,1 [32] ) - Microsoft.AspNetCore.Mvc + UsingStatement - (135:5,1 [42] ) - Microsoft.AspNetCore.Mvc.Rendering + UsingStatement - (178:6,1 [45] ) - Microsoft.AspNetCore.Mvc.ViewFeatures + ClassDeclaration - - public - TestFiles_IntegrationTests_CodeGenerationIntegrationTest_InvalidNamespaceAtEOF_cshtml - global::Microsoft.AspNetCore.Mvc.Razor.RazorPage - + MethodDeclaration - - public - async, override - global::System.Threading.Tasks.Task - ExecuteAsync + CSharpStatement - + RazorIRToken - - CSharp - BeginContext(11, 5, true); + HtmlContent - (11:0,11 [5] InvalidNamespaceAtEOF.cshtml) + RazorIRToken - (11:0,11 [5] InvalidNamespaceAtEOF.cshtml) - Html - Test. + CSharpStatement - + RazorIRToken - - CSharp - EndContext(); + InjectDirective - + InjectDirective - + InjectDirective - + InjectDirective - + InjectDirective - diff --git a/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/CSharpDirectivesTest.cs b/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/CSharpDirectivesTest.cs index a11b339e4f..0daa079a41 100644 --- a/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/CSharpDirectivesTest.cs +++ b/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/CSharpDirectivesTest.cs @@ -10,6 +10,97 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy { public class CSharpDirectivesTest : CsHtmlCodeParserTestBase { + [Fact] + public void DirectiveDescriptor_CanHandleEOFIncompleteNamespaceTokens() + { + // Arrange + var descriptor = DirectiveDescriptor.CreateDirective( + "custom", + DirectiveKind.SingleLine, + b => b.AddNamespaceToken()); + + // Act & Assert + ParseCodeBlockTest( + "@custom System.", + new[] { descriptor }, + new DirectiveBlock( + new DirectiveChunkGenerator(descriptor), + Factory.CodeTransition(), + Factory.MetaCode("custom").Accepts(AcceptedCharacters.None), + Factory.Span(SpanKind.Code, " ", markup: false).Accepts(AcceptedCharacters.WhiteSpace)), + new RazorError( + LegacyResources.FormatDirectiveExpectsNamespace("custom"), + 8, 0, 8, 7)); + } + + [Fact] + public void DirectiveDescriptor_CanHandleEOFInvalidNamespaceTokens() + { + // Arrange + var descriptor = DirectiveDescriptor.CreateDirective( + "custom", + DirectiveKind.SingleLine, + b => b.AddNamespaceToken()); + + // Act & Assert + ParseCodeBlockTest( + "@custom System<", + new[] { descriptor }, + new DirectiveBlock( + new DirectiveChunkGenerator(descriptor), + Factory.CodeTransition(), + Factory.MetaCode("custom").Accepts(AcceptedCharacters.None), + Factory.Span(SpanKind.Code, " ", markup: false).Accepts(AcceptedCharacters.WhiteSpace)), + new RazorError( + LegacyResources.FormatDirectiveExpectsNamespace("custom"), + 8, 0, 8, 7)); + } + [Fact] + public void DirectiveDescriptor_CanHandleIncompleteNamespaceTokens() + { + // Arrange + var descriptor = DirectiveDescriptor.CreateDirective( + "custom", + DirectiveKind.SingleLine, + b => b.AddNamespaceToken()); + + // Act & Assert + ParseCodeBlockTest( + "@custom System." + Environment.NewLine, + new[] { descriptor }, + new DirectiveBlock( + new DirectiveChunkGenerator(descriptor), + Factory.CodeTransition(), + Factory.MetaCode("custom").Accepts(AcceptedCharacters.None), + Factory.Span(SpanKind.Code, " ", markup: false).Accepts(AcceptedCharacters.WhiteSpace)), + new RazorError( + LegacyResources.FormatDirectiveExpectsNamespace("custom"), + 8, 0, 8, 7)); + } + + [Fact] + public void DirectiveDescriptor_CanHandleInvalidNamespaceTokens() + { + // Arrange + var descriptor = DirectiveDescriptor.CreateDirective( + "custom", + DirectiveKind.SingleLine, + b => b.AddNamespaceToken()); + + // Act & Assert + ParseCodeBlockTest( + "@custom System<" + Environment.NewLine, + new[] { descriptor }, + new DirectiveBlock( + new DirectiveChunkGenerator(descriptor), + Factory.CodeTransition(), + Factory.MetaCode("custom").Accepts(AcceptedCharacters.None), + Factory.Span(SpanKind.Code, " ", markup: false).Accepts(AcceptedCharacters.WhiteSpace)), + new RazorError( + LegacyResources.FormatDirectiveExpectsNamespace("custom"), + 8, 0, 8, 7)); + } + [Fact] public void DirectiveDescriptor_UnderstandsTypeTokens() {