RazorPages page directives missing quotes should alert user

Fixes #5868
aspnet · Jun 22, 2017 · 1d063f6 · 1d063f6
1 parent de64c84
commit 1d063f6
Show file tree

Hide file tree

Showing 5 changed files with 121 additions and 133 deletions.
diff --git a/src/Microsoft.AspNetCore.Mvc.RazorPages/Infrastructure/PageDirectiveFeature.cs b/src/Microsoft.AspNetCore.Mvc.RazorPages/Infrastructure/PageDirectiveFeature.cs
@@ -2,9 +2,11 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
-using System.IO;
 using Microsoft.AspNetCore.Mvc.Razor.Extensions;
+using Microsoft.AspNetCore.Mvc.RazorPages.Internal;
 using Microsoft.AspNetCore.Razor.Language;
+using Microsoft.AspNetCore.Razor.Language.Intermediate;
+using Microsoft.Extensions.Logging;
 
 namespace Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure
 {
@@ -26,43 +28,42 @@ public static class PageDirectiveFeature
             builder.Features.Add(new PageDirectiveParserOptionsFeature());
         });
 
-        public static bool TryGetPageDirective(RazorProjectItem projectItem, out string template)
+        public static bool TryGetPageDirective(ILogger logger, RazorProjectItem projectItem, out string template)
         {
             if (projectItem == null)
             {
                 throw new ArgumentNullException(nameof(projectItem));
             }
 
             var sourceDocument = RazorSourceDocument.ReadFrom(projectItem);
-            return TryGetPageDirective(sourceDocument, out template);
+            return TryGetPageDirective(logger, sourceDocument, out template);
         }
 
-        public static bool TryGetPageDirective(Func<Stream> streamFactory, out string template)
-        {
-            if (streamFactory == null)
-            {
-                throw new ArgumentNullException(nameof(streamFactory));
-            }
-
-            using (var stream = streamFactory())
-            {
-                var sourceDocument = RazorSourceDocument.ReadFrom(stream, fileName: "Parse.cshtml");
-                return TryGetPageDirective(sourceDocument, out template);
-            }
-        }
-
-        private static bool TryGetPageDirective(RazorSourceDocument sourceDocument, out string template)
+        static bool TryGetPageDirective(
+            ILogger logger,
+            RazorSourceDocument sourceDocument,
+            out string template)
         {
             var codeDocument = RazorCodeDocument.Create(sourceDocument);
             PageDirectiveEngine.Process(codeDocument);
 
-            if (PageDirective.TryGetPageDirective(codeDocument.GetDocumentIntermediateNode(), out var pageDirective))
+            var documentIRNode = codeDocument.GetDocumentIntermediateNode();
+            if (PageDirective.TryGetPageDirective(documentIRNode, out var pageDirective))
             {
                 template = pageDirective.RouteTemplate;
                 return true;
             }
 
             template = null;
+
+            var visitor = new Visitor();
+            visitor.Visit(documentIRNode);
+            if (visitor.MalformedPageDirective != null)
+            {
+                logger.MalformedPageDirective(sourceDocument.FilePath, visitor.MalformedPageDirective.Diagnostics);
+                return true;
+            }
+
             return false;
         }
 
@@ -75,5 +76,18 @@ public void Configure(RazorParserOptionsBuilder options)
                 options.ParseOnlyLeadingDirectives = true;
             }
         }
+
+        private class Visitor : IntermediateNodeWalker
+        {
+            public MalformedDirectiveIntermediateNode MalformedPageDirective { get; private set; }
+
+            public override void VisitMalformedDirective(MalformedDirectiveIntermediateNode node)
+            {
+                if (node.Descriptor == PageDirective.Directive)
+                {
+                    MalformedPageDirective = node;
+                }
+            }
+        }
     }
 }
diff --git a/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/PageLoggerExtensions.cs b/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/PageLoggerExtensions.cs
@@ -2,9 +2,11 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
+using System.Collections.Generic;
 using Microsoft.AspNetCore.Mvc.Filters;
 using Microsoft.AspNetCore.Mvc.ModelBinding;
 using Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure;
+using Microsoft.AspNetCore.Razor.Language;
 using Microsoft.Extensions.Logging;
 
 namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal
@@ -14,6 +16,7 @@ internal static class PageLoggerExtensions
         private static readonly Action<ILogger, string, string[], ModelValidationState, Exception> _handlerMethodExecuting;
         private static readonly Action<ILogger, string, string, Exception> _handlerMethodExecuted;
         private static readonly Action<ILogger, object, Exception> _pageFilterShortCircuit;
+        private static readonly Action<ILogger, string, string[], Exception> _malformedPageDirective;
 
         static PageLoggerExtensions()
         {
@@ -33,6 +36,11 @@ static PageLoggerExtensions()
                LogLevel.Debug,
                3,
                "Request was short circuited at page filter '{PageFilter}'.");
+
+            _malformedPageDirective = LoggerMessage.Define<string, string[]>(
+                LogLevel.Warning,
+                104,
+                "The page directive at '{FilePath}' is malformed. Please fix the following issues: {Diagnostics}");
         }
 
         public static void ExecutingHandlerMethod(this ILogger logger, PageContext context, HandlerMethodDescriptor handler, object[] arguments)
@@ -76,5 +84,19 @@ public static void PageFilterShortCircuited(
         {
             _pageFilterShortCircuit(logger, filter, null);
         }
+
+        public static void MalformedPageDirective(this ILogger logger, string filePath, IList<RazorDiagnostic> diagnostics)
+        {
+            if (logger.IsEnabled(LogLevel.Warning))
+            {
+                var messages = new string[diagnostics.Count];
+                for (var i = 0; i < diagnostics.Count; i++)
+                {
+                    messages[i] = diagnostics[i].GetMessage();
+                }
+
+                _malformedPageDirective(logger, filePath, messages, null);
+            }
+        }
     }
 }
diff --git a/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/RazorProjectPageApplicationModelProvider.cs b/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/RazorProjectPageApplicationModelProvider.cs
@@ -4,6 +4,7 @@
 using Microsoft.AspNetCore.Mvc.ApplicationModels;
 using Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure;
 using Microsoft.AspNetCore.Razor.Language;
+using Microsoft.Extensions.Logging;
 using Microsoft.Extensions.Options;
 
 namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal
@@ -12,13 +13,16 @@ public class RazorProjectPageApplicationModelProvider : IPageApplicationModelPro
     {
         private readonly RazorProject _project;
         private readonly RazorPagesOptions _pagesOptions;
+        private readonly ILogger _logger;
 
         public RazorProjectPageApplicationModelProvider(
             RazorProject razorProject,
-            IOptions<RazorPagesOptions> pagesOptionsAccessor)
+            IOptions<RazorPagesOptions> pagesOptionsAccessor,
+            ILoggerFactory loggerFactory)
         {
             _project = razorProject;
             _pagesOptions = pagesOptionsAccessor.Value;
+            _logger = loggerFactory.CreateLogger<RazorProjectPageApplicationModelProvider>();
         }
 
         public int Order => -1000;
@@ -37,7 +41,7 @@ public void OnProvidersExecuting(PageApplicationModelProviderContext context)
                     continue;
                 }
 
-                if (!PageDirectiveFeature.TryGetPageDirective(item, out var routeTemplate))
+                if (!PageDirectiveFeature.TryGetPageDirective(_logger, item, out var routeTemplate))
                 {
                     // .cshtml pages without @page are not RazorPages.
                     continue;