From a7178a66bd9b58e24ae0df36f80b71da44058a6f Mon Sep 17 00:00:00 2001 From: Ajay Bhargav Baaskaran Date: Fri, 16 Mar 2018 19:04:49 -0700 Subject: [PATCH] Show error if page directive is not at the top of file --- .../Properties/Resources.Designer.cs | 14 ++++ .../RazorExtensionsDiagnosticFactory.cs | 14 ++++ .../RazorPageDocumentClassifierPass.cs | 50 +++++++++++++- .../Resources.resx | 3 + .../RazorPageDocumentClassifierPassTest.cs | 67 +++++++++++++++++++ 5 files changed, 146 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.AspNetCore.Mvc.Razor.Extensions/Properties/Resources.Designer.cs b/src/Microsoft.AspNetCore.Mvc.Razor.Extensions/Properties/Resources.Designer.cs index 8c779001f4..dfa9a59ca9 100644 --- a/src/Microsoft.AspNetCore.Mvc.Razor.Extensions/Properties/Resources.Designer.cs +++ b/src/Microsoft.AspNetCore.Mvc.Razor.Extensions/Properties/Resources.Designer.cs @@ -262,6 +262,20 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Extensions internal static string FormatPageDirectiveCannotBeImported(object p0, object p1) => string.Format(CultureInfo.CurrentCulture, GetString("PageDirectiveCannotBeImported"), p0, p1); + /// + /// The '@{0}' directive must exist at the top of the file. Only comments and whitespace are allowed before the '@{0}' directive. + /// + internal static string PageDirectiveMustExistAtTheTopOfFile + { + get => GetString("PageDirectiveMustExistAtTheTopOfFile"); + } + + /// + /// The '@{0}' directive must exist at the top of the file. Only comments and whitespace are allowed before the '@{0}' directive. + /// + internal static string FormatPageDirectiveMustExistAtTheTopOfFile(object p0) + => string.Format(CultureInfo.CurrentCulture, GetString("PageDirectiveMustExistAtTheTopOfFile"), p0); + /// /// Mark the page as a Razor Page. /// diff --git a/src/Microsoft.AspNetCore.Mvc.Razor.Extensions/RazorExtensionsDiagnosticFactory.cs b/src/Microsoft.AspNetCore.Mvc.Razor.Extensions/RazorExtensionsDiagnosticFactory.cs index 1e2f6a1f7e..74adb4fe93 100644 --- a/src/Microsoft.AspNetCore.Mvc.Razor.Extensions/RazorExtensionsDiagnosticFactory.cs +++ b/src/Microsoft.AspNetCore.Mvc.Razor.Extensions/RazorExtensionsDiagnosticFactory.cs @@ -113,5 +113,19 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Extensions return diagnostic; } + + internal static readonly RazorDiagnosticDescriptor PageDirective_MustExistAtTheTopOfFile = + new RazorDiagnosticDescriptor( + $"{DiagnosticPrefix}3906", + () => Resources.PageDirectiveMustExistAtTheTopOfFile, + RazorDiagnosticSeverity.Error); + + public static RazorDiagnostic CreatePageDirective_MustExistAtTheTopOfFile(SourceSpan source) + { + var fileName = Path.GetFileName(source.FilePath); + var diagnostic = RazorDiagnostic.Create(PageDirective_MustExistAtTheTopOfFile, source, PageDirective.Directive.Directive); + + return diagnostic; + } } } diff --git a/src/Microsoft.AspNetCore.Mvc.Razor.Extensions/RazorPageDocumentClassifierPass.cs b/src/Microsoft.AspNetCore.Mvc.Razor.Extensions/RazorPageDocumentClassifierPass.cs index 1dfb460a47..99b3ba8d19 100644 --- a/src/Microsoft.AspNetCore.Mvc.Razor.Extensions/RazorPageDocumentClassifierPass.cs +++ b/src/Microsoft.AspNetCore.Mvc.Razor.Extensions/RazorPageDocumentClassifierPass.cs @@ -13,6 +13,25 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Extensions public static readonly string RazorPageDocumentKind = "mvc.1.0.razor-page"; public static readonly string RouteTemplateKey = "RouteTemplate"; + private static readonly RazorProjectEngine LeadingDirectiveParsingEngine = RazorProjectEngine.Create( + RazorConfiguration.Default, + RazorProjectFileSystem.Create("/"), + builder => + { + for (var i = builder.Phases.Count - 1; i >= 0; i--) + { + var phase = builder.Phases[i]; + builder.Phases.RemoveAt(i); + if (phase is IRazorDocumentClassifierPhase) + { + break; + } + } + + RazorExtensions.Register(builder); + builder.Features.Add(new LeadingDirectiveParserOptionsFeature()); + }); + protected override string DocumentKind => RazorPageDocumentKind; protected override bool IsMatch(RazorCodeDocument codeDocument, DocumentIntermediateNode documentNode) @@ -48,7 +67,7 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Extensions var document = codeDocument.GetDocumentIntermediateNode(); PageDirective.TryGetPageDirective(document, out var pageDirective); - EnsureValidPageDirective(pageDirective); + EnsureValidPageDirective(codeDocument, pageDirective); AddRouteTemplateMetadataAttribute(@namespace, @class, pageDirective); } @@ -75,7 +94,7 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Extensions @namespace.Children.Insert(classIndex, metadataAttributeNode); } - private void EnsureValidPageDirective(PageDirective pageDirective) + private void EnsureValidPageDirective(RazorCodeDocument codeDocument, PageDirective pageDirective) { Debug.Assert(pageDirective != null); @@ -84,6 +103,33 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Extensions pageDirective.DirectiveNode.Diagnostics.Add( RazorExtensionsDiagnosticFactory.CreatePageDirective_CannotBeImported(pageDirective.DirectiveNode.Source.Value)); } + else + { + // The document contains a page directive and it is not imported. + // We now want to make sure this page directive exists at the top of the file. + // We are going to do that by re-parsing the document until the very first line that is not Razor comment + // or whitespace. We then make sure the page directive still exists in the re-parsed IR tree. + var leadingDirectiveCodeDocument = RazorCodeDocument.Create(codeDocument.Source); + LeadingDirectiveParsingEngine.Engine.Process(leadingDirectiveCodeDocument); + + var documentIRNode = leadingDirectiveCodeDocument.GetDocumentIntermediateNode(); + if (!PageDirective.TryGetPageDirective(documentIRNode, out var _)) + { + // The page directive is not the leading directive. Add an error. + pageDirective.DirectiveNode.Diagnostics.Add( + RazorExtensionsDiagnosticFactory.CreatePageDirective_MustExistAtTheTopOfFile(pageDirective.DirectiveNode.Source.Value)); + } + } + } + + private class LeadingDirectiveParserOptionsFeature : RazorEngineFeatureBase, IConfigureRazorParserOptionsFeature + { + public int Order { get; } + + public void Configure(RazorParserOptionsBuilder options) + { + options.ParseLeadingDirectives = true; + } } } } diff --git a/src/Microsoft.AspNetCore.Mvc.Razor.Extensions/Resources.resx b/src/Microsoft.AspNetCore.Mvc.Razor.Extensions/Resources.resx index 41cf3a1b03..474537197e 100644 --- a/src/Microsoft.AspNetCore.Mvc.Razor.Extensions/Resources.resx +++ b/src/Microsoft.AspNetCore.Mvc.Razor.Extensions/Resources.resx @@ -171,6 +171,9 @@ The '@{0}' directive specified in {1} file will not be imported. The directive must appear at the top of each Razor cshtml file. + + The '@{0}' directive must precede all other elements defined in a Razor file. + Mark the page as a Razor Page. diff --git a/test/Microsoft.AspNetCore.Mvc.Razor.Extensions.Test/RazorPageDocumentClassifierPassTest.cs b/test/Microsoft.AspNetCore.Mvc.Razor.Extensions.Test/RazorPageDocumentClassifierPassTest.cs index e3ba1aa9d6..0ba9dd5178 100644 --- a/test/Microsoft.AspNetCore.Mvc.Razor.Extensions.Test/RazorPageDocumentClassifierPassTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Razor.Extensions.Test/RazorPageDocumentClassifierPassTest.cs @@ -1,6 +1,7 @@ // 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; using Microsoft.AspNetCore.Razor.Language; using Microsoft.AspNetCore.Razor.Language.Extensions; using Microsoft.AspNetCore.Razor.Language.Intermediate; @@ -36,6 +37,72 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Extensions Assert.Equal(expectedDiagnostic, diagnostic); } + [Fact] + public void RazorPageDocumentClassifierPass_LogsErrorIfDirectiveNotAtTopOfFile() + { + // Arrange + var sourceSpan = new SourceSpan( + "Test.cshtml", + absoluteIndex: 14 + Environment.NewLine.Length * 2, + lineIndex: 2, + characterIndex: 0, + length: 5 + Environment.NewLine.Length); + + var expectedDiagnostic = RazorExtensionsDiagnosticFactory.CreatePageDirective_MustExistAtTheTopOfFile(sourceSpan); + var content = @" +@somethingelse +@page +"; + var codeDocument = RazorCodeDocument.Create(RazorSourceDocument.Create(content, "Test.cshtml")); + + var engine = CreateEngine(); + var irDocument = CreateIRDocument(engine, codeDocument); + var pass = new RazorPageDocumentClassifierPass + { + Engine = engine + }; + + // Act + pass.Execute(codeDocument, irDocument); + var visitor = new Visitor(); + visitor.Visit(irDocument); + + // Assert + var pageDirectives = irDocument.FindDirectiveReferences(PageDirective.Directive); + var directive = Assert.Single(pageDirectives); + var diagnostic = Assert.Single(directive.Node.Diagnostics); + Assert.Equal(expectedDiagnostic, diagnostic); + } + + [Fact] + public void RazorPageDocumentClassifierPass_DoesNotLogErrorIfCommentAndWhitespaceBeforeDirective() + { + // Arrange + var content = @" +@* some comment *@ + +@page +"; + var codeDocument = RazorCodeDocument.Create(RazorSourceDocument.Create(content, "Test.cshtml")); + + var engine = CreateEngine(); + var irDocument = CreateIRDocument(engine, codeDocument); + var pass = new RazorPageDocumentClassifierPass + { + Engine = engine + }; + + // Act + pass.Execute(codeDocument, irDocument); + var visitor = new Visitor(); + visitor.Visit(irDocument); + + // Assert + var pageDirectives = irDocument.FindDirectiveReferences(PageDirective.Directive); + var directive = Assert.Single(pageDirectives); + Assert.Empty(directive.Node.Diagnostics); + } + [Fact] public void RazorPageDocumentClassifierPass_SetsDocumentKind() {