Handle imported @page directives.
- Prior to this imported `@page` directives would flow through the Razor system without error. This resulted in inconsistent behavior between MVC and Razor. Now, imported `@page` directives result in diagnostics on the page directive node. - Added two tests to verify that we still treat views with imported page directives as Razor pages BUT we also log a diagnostic on the page directive node. - Renamed the `ViewComponentDiagnosticFactory` class to `RazorExtensionsDiagnosticFactory` so it can be used for more than just view component diagnostics. This way we can ensure that our diagnostics don't end up overlapping. #1503
This commit is contained in:
parent
8a9bf9c71a
commit
d36838ed88
|
|
@ -94,6 +94,20 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Extensions
|
|||
internal static string FormatMvcRazorParser_InvalidPropertyType(object p0, object p1, object p2)
|
||||
=> string.Format(CultureInfo.CurrentCulture, GetString("MvcRazorParser_InvalidPropertyType"), p0, p1, p2);
|
||||
|
||||
/// <summary>
|
||||
/// The '@{0}' directive specified in {1} file will not be imported. The directive must appear at the top of each Razor cshtml file.
|
||||
/// </summary>
|
||||
internal static string PageDirectiveCannotBeImported
|
||||
{
|
||||
get => GetString("PageDirectiveCannotBeImported");
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// The '@{0}' directive specified in {1} file will not be imported. The directive must appear at the top of each Razor cshtml file.
|
||||
/// </summary>
|
||||
internal static string FormatPageDirectiveCannotBeImported(object p0, object p1)
|
||||
=> string.Format(CultureInfo.CurrentCulture, GetString("PageDirectiveCannotBeImported"), p0, p1);
|
||||
|
||||
private static string GetString(string name, params string[] formatterNames)
|
||||
{
|
||||
var value = _resourceManager.GetString(name);
|
||||
|
|
|
|||
|
|
@ -1,12 +1,13 @@
|
|||
// 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.IO;
|
||||
using System.Threading.Tasks;
|
||||
using Microsoft.AspNetCore.Razor.Language;
|
||||
|
||||
namespace Microsoft.AspNetCore.Mvc.Razor.Extensions
|
||||
{
|
||||
internal class ViewComponentDiagnosticFactory
|
||||
internal class RazorExtensionsDiagnosticFactory
|
||||
{
|
||||
private const string DiagnosticPrefix = "RZ";
|
||||
|
||||
|
|
@ -98,5 +99,19 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Extensions
|
|||
|
||||
return diagnostic;
|
||||
}
|
||||
|
||||
public static readonly RazorDiagnosticDescriptor PageDirective_CannotBeImported =
|
||||
new RazorDiagnosticDescriptor(
|
||||
$"{DiagnosticPrefix}3905",
|
||||
() => Resources.PageDirectiveCannotBeImported,
|
||||
RazorDiagnosticSeverity.Error);
|
||||
|
||||
public static RazorDiagnostic CreatePageDirective_CannotBeImported(SourceSpan source)
|
||||
{
|
||||
var fileName = Path.GetFileName(source.FilePath);
|
||||
var diagnostic = RazorDiagnostic.Create(PageDirective_CannotBeImported, source, PageDirective.Directive.Directive, fileName);
|
||||
|
||||
return diagnostic;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -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.Intermediate;
|
||||
|
||||
|
|
@ -41,6 +42,34 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Extensions
|
|||
method.Modifiers.Add("async");
|
||||
method.Modifiers.Add("override");
|
||||
method.ReturnType = $"global::{typeof(System.Threading.Tasks.Task).FullName}";
|
||||
|
||||
EnsureValidPageDirective(codeDocument);
|
||||
}
|
||||
|
||||
private void EnsureValidPageDirective(RazorCodeDocument codeDocument)
|
||||
{
|
||||
var document = codeDocument.GetDocumentIntermediateNode();
|
||||
var visitor = new Visitor();
|
||||
visitor.VisitDocument(document);
|
||||
|
||||
if (visitor.DirectiveNode.IsImported())
|
||||
{
|
||||
visitor.DirectiveNode.Diagnostics.Add(
|
||||
RazorExtensionsDiagnosticFactory.CreatePageDirective_CannotBeImported(visitor.DirectiveNode.Source.Value));
|
||||
}
|
||||
}
|
||||
|
||||
private class Visitor : IntermediateNodeWalker
|
||||
{
|
||||
public DirectiveIntermediateNode DirectiveNode { get; private set; }
|
||||
|
||||
public override void VisitDirective(DirectiveIntermediateNode node)
|
||||
{
|
||||
if (node.Descriptor == PageDirective.Directive)
|
||||
{
|
||||
DirectiveNode = node;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -135,4 +135,7 @@
|
|||
<data name="MvcRazorParser_InvalidPropertyType" xml:space="preserve">
|
||||
<value>Invalid tag helper property '{0}.{1}'. Dictionary values must not be of type '{2}'.</value>
|
||||
</data>
|
||||
<data name="PageDirectiveCannotBeImported" xml:space="preserve">
|
||||
<value>The '@{0}' directive specified in {1} file will not be imported. The directive must appear at the top of each Razor cshtml file.</value>
|
||||
</data>
|
||||
</root>
|
||||
|
|
@ -94,13 +94,13 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Extensions
|
|||
|
||||
if (methods.Length == 0)
|
||||
{
|
||||
diagnostic = ViewComponentDiagnosticFactory.CreateViewComponent_CannotFindMethod(type.ToDisplayString(FullNameTypeDisplayFormat));
|
||||
diagnostic = RazorExtensionsDiagnosticFactory.CreateViewComponent_CannotFindMethod(type.ToDisplayString(FullNameTypeDisplayFormat));
|
||||
method = null;
|
||||
return false;
|
||||
}
|
||||
else if (methods.Length > 1)
|
||||
{
|
||||
diagnostic = ViewComponentDiagnosticFactory.CreateViewComponent_AmbiguousMethods(type.ToDisplayString(FullNameTypeDisplayFormat));
|
||||
diagnostic = RazorExtensionsDiagnosticFactory.CreateViewComponent_AmbiguousMethods(type.ToDisplayString(FullNameTypeDisplayFormat));
|
||||
method = null;
|
||||
return false;
|
||||
}
|
||||
|
|
@ -120,7 +120,7 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Extensions
|
|||
}
|
||||
else
|
||||
{
|
||||
diagnostic = ViewComponentDiagnosticFactory.CreateViewComponent_AsyncMethod_ShouldReturnTask(type.ToDisplayString(FullNameTypeDisplayFormat));
|
||||
diagnostic = RazorExtensionsDiagnosticFactory.CreateViewComponent_AsyncMethod_ShouldReturnTask(type.ToDisplayString(FullNameTypeDisplayFormat));
|
||||
method = null;
|
||||
return false;
|
||||
}
|
||||
|
|
@ -130,19 +130,19 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Extensions
|
|||
// Will invoke synchronously. Method must not return void, Task or Task<T>.
|
||||
if (returnType.SpecialType == SpecialType.System_Void)
|
||||
{
|
||||
diagnostic = ViewComponentDiagnosticFactory.CreateViewComponent_SyncMethod_ShouldReturnValue(type.ToDisplayString(FullNameTypeDisplayFormat));
|
||||
diagnostic = RazorExtensionsDiagnosticFactory.CreateViewComponent_SyncMethod_ShouldReturnValue(type.ToDisplayString(FullNameTypeDisplayFormat));
|
||||
method = null;
|
||||
return false;
|
||||
}
|
||||
else if (returnType == _taskSymbol)
|
||||
{
|
||||
diagnostic = ViewComponentDiagnosticFactory.CreateViewComponent_SyncMethod_CannotReturnTask(type.ToDisplayString(FullNameTypeDisplayFormat));
|
||||
diagnostic = RazorExtensionsDiagnosticFactory.CreateViewComponent_SyncMethod_CannotReturnTask(type.ToDisplayString(FullNameTypeDisplayFormat));
|
||||
method = null;
|
||||
return false;
|
||||
}
|
||||
else if (returnType.IsGenericType && returnType.ConstructedFrom == _genericTaskSymbol)
|
||||
{
|
||||
diagnostic = ViewComponentDiagnosticFactory.CreateViewComponent_SyncMethod_CannotReturnTask(type.ToDisplayString(FullNameTypeDisplayFormat));
|
||||
diagnostic = RazorExtensionsDiagnosticFactory.CreateViewComponent_SyncMethod_CannotReturnTask(type.ToDisplayString(FullNameTypeDisplayFormat));
|
||||
method = null;
|
||||
return false;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -109,9 +109,8 @@ namespace Microsoft.AspNetCore.Razor.Language
|
|||
var directive = (DirectiveIntermediateNode)reference.Node;
|
||||
var descriptor = directive.Descriptor;
|
||||
var seenDirective = !seenDirectives.Add(descriptor);
|
||||
var imported = ReferenceEquals(directive.Annotations[CommonAnnotations.Imported], CommonAnnotations.Imported);
|
||||
|
||||
if (!imported)
|
||||
if (!directive.IsImported())
|
||||
{
|
||||
continue;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -11,6 +11,11 @@ namespace Microsoft.AspNetCore.Razor.Language.Intermediate
|
|||
{
|
||||
private static readonly IReadOnlyList<RazorDiagnostic> EmptyDiagnostics = Array.Empty<RazorDiagnostic>();
|
||||
|
||||
public static bool IsImported(this IntermediateNode node)
|
||||
{
|
||||
return ReferenceEquals(node.Annotations[CommonAnnotations.Imported], CommonAnnotations.Imported);
|
||||
}
|
||||
|
||||
public static IReadOnlyList<RazorDiagnostic> GetAllDiagnostics(this IntermediateNode node)
|
||||
{
|
||||
if (node == null)
|
||||
|
|
|
|||
|
|
@ -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.Intermediate;
|
||||
using Xunit;
|
||||
|
|
@ -9,6 +10,25 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Extensions
|
|||
{
|
||||
public class PageDirectiveTest
|
||||
{
|
||||
[Fact]
|
||||
public void TryGetPageDirective_ReturnsTrue_IfPageIsImported()
|
||||
{
|
||||
// Arrange
|
||||
var content = "Hello world";
|
||||
var sourceDocument = RazorSourceDocument.Create(content, "file");
|
||||
var importDocument = RazorSourceDocument.Create("@page", "imports.cshtml");
|
||||
var codeDocument = RazorCodeDocument.Create(sourceDocument, new[] { importDocument });
|
||||
var engine = CreateEngine();
|
||||
var irDocument = CreateIRDocument(engine, codeDocument);
|
||||
|
||||
// Act
|
||||
var result = PageDirective.TryGetPageDirective(irDocument, out var pageDirective);
|
||||
|
||||
// Assert
|
||||
Assert.True(result);
|
||||
Assert.Null(pageDirective.RouteTemplate);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void TryGetPageDirective_ReturnsFalse_IfPageDoesNotHaveDirective()
|
||||
{
|
||||
|
|
|
|||
|
|
@ -10,6 +10,32 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Extensions
|
|||
{
|
||||
public class RazorPageDocumentClassifierPassTest
|
||||
{
|
||||
[Fact]
|
||||
public void RazorPageDocumentClassifierPass_LogsErrorForImportedPageDirectives()
|
||||
{
|
||||
// Arrange
|
||||
var sourceSpan = new SourceSpan("import.cshtml", 0, 0, 0, 5);
|
||||
var expectedDiagnostic = RazorExtensionsDiagnosticFactory.CreatePageDirective_CannotBeImported(sourceSpan);
|
||||
var importDocument = RazorSourceDocument.Create("@page", "import.cshtml");
|
||||
var sourceDocument = RazorSourceDocument.Create("<p>Hello World</p>", "main.cshtml");
|
||||
var codeDocument = RazorCodeDocument.Create(sourceDocument, new[] { importDocument });
|
||||
var engine = CreateEngine();
|
||||
var irDocument = CreateIRDocument(engine, codeDocument);
|
||||
var pass = new RazorPageDocumentClassifierPass
|
||||
{
|
||||
Engine = engine
|
||||
};
|
||||
|
||||
// Act
|
||||
pass.Execute(codeDocument, 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_SetsDocumentKind()
|
||||
{
|
||||
|
|
|
|||
|
|
@ -159,7 +159,7 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Extensions
|
|||
|
||||
// Assert
|
||||
var diagnostic = Assert.Single(descriptor.GetAllDiagnostics());
|
||||
Assert.Equal(ViewComponentDiagnosticFactory.ViewComponent_CannotFindMethod.Id, diagnostic.Id);
|
||||
Assert.Equal(RazorExtensionsDiagnosticFactory.ViewComponent_CannotFindMethod.Id, diagnostic.Id);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
|
|
@ -208,7 +208,7 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Extensions
|
|||
|
||||
// Assert
|
||||
var diagnostic = Assert.Single(descriptor.GetAllDiagnostics());
|
||||
Assert.Equal(ViewComponentDiagnosticFactory.ViewComponent_AsyncMethod_ShouldReturnTask.Id, diagnostic.Id);
|
||||
Assert.Equal(RazorExtensionsDiagnosticFactory.ViewComponent_AsyncMethod_ShouldReturnTask.Id, diagnostic.Id);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
|
|
@ -225,7 +225,7 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Extensions
|
|||
|
||||
// Assert
|
||||
var diagnostic = Assert.Single(descriptor.GetAllDiagnostics());
|
||||
Assert.Equal(ViewComponentDiagnosticFactory.ViewComponent_AsyncMethod_ShouldReturnTask.Id, diagnostic.Id);
|
||||
Assert.Equal(RazorExtensionsDiagnosticFactory.ViewComponent_AsyncMethod_ShouldReturnTask.Id, diagnostic.Id);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
|
|
@ -242,7 +242,7 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Extensions
|
|||
|
||||
// Assert
|
||||
var diagnostic = Assert.Single(descriptor.GetAllDiagnostics());
|
||||
Assert.Equal(ViewComponentDiagnosticFactory.ViewComponent_SyncMethod_ShouldReturnValue.Id, diagnostic.Id);
|
||||
Assert.Equal(RazorExtensionsDiagnosticFactory.ViewComponent_SyncMethod_ShouldReturnValue.Id, diagnostic.Id);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
|
|
@ -259,7 +259,7 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Extensions
|
|||
|
||||
// Assert
|
||||
var diagnostic = Assert.Single(descriptor.GetAllDiagnostics());
|
||||
Assert.Equal(ViewComponentDiagnosticFactory.ViewComponent_SyncMethod_CannotReturnTask.Id, diagnostic.Id);
|
||||
Assert.Equal(RazorExtensionsDiagnosticFactory.ViewComponent_SyncMethod_CannotReturnTask.Id, diagnostic.Id);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
|
|
@ -276,7 +276,7 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Extensions
|
|||
|
||||
// Assert
|
||||
var diagnostic = Assert.Single(descriptor.GetAllDiagnostics());
|
||||
Assert.Equal(ViewComponentDiagnosticFactory.ViewComponent_SyncMethod_CannotReturnTask.Id, diagnostic.Id);
|
||||
Assert.Equal(RazorExtensionsDiagnosticFactory.ViewComponent_SyncMethod_CannotReturnTask.Id, diagnostic.Id);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue