Skip to content

Commit

Permalink
Fix RazorDiagnostics from being hidden and unordered.
Browse files Browse the repository at this point in the history
- Diagnostics were not being raised to the `RazorSyntaxTree` and weren't being ordered on final output. This resulted in some of our tests missing the fact that certain cases were generating errors.
- Made all three phases of Razor parsing order their diagnostics.
- Added `GetAllDiagnostics` methods to `SyntaxNode`s to be more consistent with IR documents.
- Updated test files. In some cases new errors were found because we're now lifting them to the `SyntaxTree`, in most others the errors are re-ordered.

**Note: In end-to-end scenarios diagnostics were not hidden, only unordered. The IR phase would find nested/hidden documents and lift them to the IR/C# documents.**
  • Loading branch information
NTaylorMullen committed May 8, 2019
1 parent 11e4db2 commit ef31a96
Show file tree
Hide file tree
Showing 73 changed files with 156 additions and 32 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,12 @@ public override RazorCSharpDocument WriteDocument(RazorCodeDocument codeDocument
context.Visitor.VisitDocument(documentNode);

var cSharp = context.CodeWriter.GenerateCode();

var allOrderedDiagnostics = context.Diagnostics.OrderBy(diagnostic => diagnostic.Span.AbsoluteIndex);
return new DefaultRazorCSharpDocument(
cSharp,
_options,
context.Diagnostics.ToArray(),
allOrderedDiagnostics.ToArray(),
context.SourceMappings.ToArray(),
context.LinePragmas.ToArray());
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,18 @@
// 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 System.Collections.Generic;
using System.Linq;
using Microsoft.AspNetCore.Razor.Language.Legacy;
using Microsoft.AspNetCore.Razor.Language.Syntax;

namespace Microsoft.AspNetCore.Razor.Language
{
internal class DefaultRazorSyntaxTree : RazorSyntaxTree
{
private readonly IReadOnlyList<RazorDiagnostic> _diagnostics;
private IReadOnlyList<RazorDiagnostic> _allDiagnostics;

public DefaultRazorSyntaxTree(
SyntaxNode root,
RazorSourceDocument source,
Expand All @@ -18,11 +21,35 @@ public DefaultRazorSyntaxTree(
{
Root = root;
Source = source;
Diagnostics = diagnostics;
_diagnostics = diagnostics;
Options = options;
}

public override IReadOnlyList<RazorDiagnostic> Diagnostics { get; }
public override IReadOnlyList<RazorDiagnostic> Diagnostics
{
get
{
if (_allDiagnostics == null)
{
var allDiagnostics = new HashSet<RazorDiagnostic>();
for (var i = 0; i < _diagnostics.Count; i++)
{
allDiagnostics.Add(_diagnostics[i]);
}

var rootDiagnostics = Root.GetAllDiagnostics();
for (var i = 0; i < rootDiagnostics.Count; i++)
{
allDiagnostics.Add(rootDiagnostics[i]);
}

var allOrderedDiagnostics = allDiagnostics.OrderBy(diagnostic => diagnostic.Span.AbsoluteIndex);
_allDiagnostics = allOrderedDiagnostics.ToArray();
}

return _allDiagnostics;
}
}

public override RazorParserOptions Options { get; }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,9 @@ public static IReadOnlyList<RazorDiagnostic> GetAllDiagnostics(this Intermediate

AddAllDiagnostics(node);

return diagnostics?.ToList() ?? EmptyDiagnostics;
var allOrderedDiagnostics = diagnostics?.OrderBy(diagnostic => diagnostic.Span.AbsoluteIndex);

return allOrderedDiagnostics?.ToList() ?? EmptyDiagnostics;

void AddAllDiagnostics(IntermediateNode n)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

using System;
using System.Collections.Generic;
using System.Linq;

namespace Microsoft.AspNetCore.Razor.Language.Legacy
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public static RazorSyntaxTree Parse(RazorSourceDocument source, RazorParserOptio
{
throw new ArgumentNullException(nameof(source));
}

var parser = new RazorParser(options ?? RazorParserOptions.CreateDefault());
return parser.Parse(source);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,25 @@ public static TNode AppendDiagnostic<TNode>(this TNode node, params RazorDiagnos
return (TNode)node.WithDiagnostics(allDiagnostics);
}

/// <summary>
/// Gets top-level and nested diagnostics from the <paramref name="node"/>.
/// </summary>
/// <typeparam name="TNode">The type of syntax node.</typeparam>
/// <param name="node">The syntax node.</param>
/// <returns>The list of <see cref="RazorDiagnostic"/>s.</returns>
public static IReadOnlyList<RazorDiagnostic> GetAllDiagnostics<TNode>(this TNode node) where TNode : SyntaxNode
{
if (node == null)
{
throw new ArgumentNullException(nameof(node));
}

var walker = new DiagnosticSyntaxWalker();
walker.Visit(node);

return walker.Diagnostics;
}

public static SourceLocation GetSourceLocation(this SyntaxNode node, RazorSourceDocument source)
{
if (node == null)
Expand Down Expand Up @@ -220,5 +239,29 @@ public static string GetContent<TNode>(this TNode node) where TNode : SyntaxNode
var content = string.Concat(tokens.Select(t => t.Content));
return content;
}

private class DiagnosticSyntaxWalker : SyntaxWalker
{
private readonly List<RazorDiagnostic> _diagnostics;

public DiagnosticSyntaxWalker()
{
_diagnostics = new List<RazorDiagnostic>();
}

public IReadOnlyList<RazorDiagnostic> Diagnostics => _diagnostics;

public override void Visit(SyntaxNode node)
{
if (node?.ContainsDiagnostics == true)
{
var diagnostics = node.GetDiagnostics();

_diagnostics.AddRange(diagnostics);

base.Visit(node);
}
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -80,11 +80,13 @@ public void BindFallback_InvalidSyntax_TrailingDash()
[Fact]
public void Bind_InvalidUseOfDirective_DoesNotThrow()
{
// We're looking for VS crash issues. Meaning if the parser returns
// diagnostics we don't want to throw.
var generated = CompileToCSharp(@"
<input type=""text"" bind=""@page"" />
@functions {
public string page { get; set; } = ""text"";
}");
}", throwOnFailure: false);

// Assert
Assert.Collection(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
TestFiles/IntegrationTests/CodeGenerationIntegrationTest/ExplicitExpressionWithMarkup.cshtml(1,11): Error RZ1026: Encountered end tag "div" with no matching start tag. Are your start/end tags properly balanced?
TestFiles/IntegrationTests/CodeGenerationIntegrationTest/ExplicitExpressionWithMarkup.cshtml(1,7): Error RZ1006: The explicit expression block is missing a closing ")" character. Make sure you have a matching ")" character for all the "(" characters within this block, and that none of the ")" characters are being interpreted as markup.
TestFiles/IntegrationTests/CodeGenerationIntegrationTest/ExplicitExpressionWithMarkup.cshtml(1,11): Error RZ1026: Encountered end tag "div" with no matching start tag. Are your start/end tags properly balanced?
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
TestFiles/IntegrationTests/CodeGenerationIntegrationTest/ExplicitExpressionWithMarkup.cshtml(1,11): Error RZ1026: Encountered end tag "div" with no matching start tag. Are your start/end tags properly balanced?
TestFiles/IntegrationTests/CodeGenerationIntegrationTest/ExplicitExpressionWithMarkup.cshtml(1,7): Error RZ1006: The explicit expression block is missing a closing ")" character. Make sure you have a matching ")" character for all the "(" characters within this block, and that none of the ")" characters are being interpreted as markup.
TestFiles/IntegrationTests/CodeGenerationIntegrationTest/ExplicitExpressionWithMarkup.cshtml(1,11): Error RZ1026: Encountered end tag "div" with no matching start tag. Are your start/end tags properly balanced?
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,10 @@ TestFiles/IntegrationTests/CodeGenerationIntegrationTest/IncompleteDirectives.cs
TestFiles/IntegrationTests/CodeGenerationIntegrationTest/IncompleteDirectives.cshtml(9,18): Error RZ1000: Unterminated string literal. Strings that start with a quotation mark (") must be terminated before the end of the line. However, strings that start with @ and a quotation mark (@") can span multiple lines.
TestFiles/IntegrationTests/CodeGenerationIntegrationTest/IncompleteDirectives.cshtml(9,18): Error RZ1036: Invalid tag helper directive look up text '"'. The correct look up text format is: "name, assemblyName".
TestFiles/IntegrationTests/CodeGenerationIntegrationTest/IncompleteDirectives.cshtml(11,2): Error RZ1018: Directive 'tagHelperPrefix' must have a value.
TestFiles/IntegrationTests/CodeGenerationIntegrationTest/IncompleteDirectives.cshtml(12,2): Error RZ1018: Directive 'tagHelperPrefix' must have a value.
TestFiles/IntegrationTests/CodeGenerationIntegrationTest/IncompleteDirectives.cshtml(12,1): Error RZ2001: The 'tagHelperPrefix' directive may only occur once per document.
TestFiles/IntegrationTests/CodeGenerationIntegrationTest/IncompleteDirectives.cshtml(13,18): Error RZ1000: Unterminated string literal. Strings that start with a quotation mark (") must be terminated before the end of the line. However, strings that start with @ and a quotation mark (@") can span multiple lines.
TestFiles/IntegrationTests/CodeGenerationIntegrationTest/IncompleteDirectives.cshtml(12,2): Error RZ1018: Directive 'tagHelperPrefix' must have a value.
TestFiles/IntegrationTests/CodeGenerationIntegrationTest/IncompleteDirectives.cshtml(13,1): Error RZ2001: The 'tagHelperPrefix' directive may only occur once per document.
TestFiles/IntegrationTests/CodeGenerationIntegrationTest/IncompleteDirectives.cshtml(13,18): Error RZ1000: Unterminated string literal. Strings that start with a quotation mark (") must be terminated before the end of the line. However, strings that start with @ and a quotation mark (@") can span multiple lines.
TestFiles/IntegrationTests/CodeGenerationIntegrationTest/IncompleteDirectives.cshtml(13,18): Error RZ1020: Invalid tag helper directive 'tagHelperPrefix' value. '"' is not allowed in prefix '"'.
TestFiles/IntegrationTests/CodeGenerationIntegrationTest/IncompleteDirectives.cshtml(15,10): Error RZ1013: The 'inherits' directive expects a type name.
TestFiles/IntegrationTests/CodeGenerationIntegrationTest/IncompleteDirectives.cshtml(16,1): Error RZ2001: The 'inherits' directive may only occur once per document.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,10 @@ TestFiles/IntegrationTests/CodeGenerationIntegrationTest/IncompleteDirectives.cs
TestFiles/IntegrationTests/CodeGenerationIntegrationTest/IncompleteDirectives.cshtml(9,18): Error RZ1000: Unterminated string literal. Strings that start with a quotation mark (") must be terminated before the end of the line. However, strings that start with @ and a quotation mark (@") can span multiple lines.
TestFiles/IntegrationTests/CodeGenerationIntegrationTest/IncompleteDirectives.cshtml(9,18): Error RZ1036: Invalid tag helper directive look up text '"'. The correct look up text format is: "name, assemblyName".
TestFiles/IntegrationTests/CodeGenerationIntegrationTest/IncompleteDirectives.cshtml(11,2): Error RZ1018: Directive 'tagHelperPrefix' must have a value.
TestFiles/IntegrationTests/CodeGenerationIntegrationTest/IncompleteDirectives.cshtml(12,2): Error RZ1018: Directive 'tagHelperPrefix' must have a value.
TestFiles/IntegrationTests/CodeGenerationIntegrationTest/IncompleteDirectives.cshtml(12,1): Error RZ2001: The 'tagHelperPrefix' directive may only occur once per document.
TestFiles/IntegrationTests/CodeGenerationIntegrationTest/IncompleteDirectives.cshtml(13,18): Error RZ1000: Unterminated string literal. Strings that start with a quotation mark (") must be terminated before the end of the line. However, strings that start with @ and a quotation mark (@") can span multiple lines.
TestFiles/IntegrationTests/CodeGenerationIntegrationTest/IncompleteDirectives.cshtml(12,2): Error RZ1018: Directive 'tagHelperPrefix' must have a value.
TestFiles/IntegrationTests/CodeGenerationIntegrationTest/IncompleteDirectives.cshtml(13,1): Error RZ2001: The 'tagHelperPrefix' directive may only occur once per document.
TestFiles/IntegrationTests/CodeGenerationIntegrationTest/IncompleteDirectives.cshtml(13,18): Error RZ1000: Unterminated string literal. Strings that start with a quotation mark (") must be terminated before the end of the line. However, strings that start with @ and a quotation mark (@") can span multiple lines.
TestFiles/IntegrationTests/CodeGenerationIntegrationTest/IncompleteDirectives.cshtml(13,18): Error RZ1020: Invalid tag helper directive 'tagHelperPrefix' value. '"' is not allowed in prefix '"'.
TestFiles/IntegrationTests/CodeGenerationIntegrationTest/IncompleteDirectives.cshtml(15,10): Error RZ1013: The 'inherits' directive expects a type name.
TestFiles/IntegrationTests/CodeGenerationIntegrationTest/IncompleteDirectives.cshtml(16,1): Error RZ2001: The 'inherits' directive may only occur once per document.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
TestFiles/IntegrationTests/CodeGenerationIntegrationTest/OpenedIf.cshtml(3,2): Error RZ1006: The if block is missing a closing "}" character. Make sure you have a matching "}" character for all the "{" characters within this block, and that none of the "}" characters are being interpreted as markup.
TestFiles/IntegrationTests/CodeGenerationIntegrationTest/OpenedIf.cshtml(4,3): Error RZ1026: Encountered end tag "body" with no matching start tag. Are your start/end tags properly balanced?
TestFiles/IntegrationTests/CodeGenerationIntegrationTest/OpenedIf.cshtml(5,3): Error RZ1026: Encountered end tag "html" with no matching start tag. Are your start/end tags properly balanced?
TestFiles/IntegrationTests/CodeGenerationIntegrationTest/OpenedIf.cshtml(3,2): Error RZ1006: The if block is missing a closing "}" character. Make sure you have a matching "}" character for all the "{" characters within this block, and that none of the "}" characters are being interpreted as markup.
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
TestFiles/IntegrationTests/CodeGenerationIntegrationTest/OpenedIf.cshtml(3,2): Error RZ1006: The if block is missing a closing "}" character. Make sure you have a matching "}" character for all the "{" characters within this block, and that none of the "}" characters are being interpreted as markup.
TestFiles/IntegrationTests/CodeGenerationIntegrationTest/OpenedIf.cshtml(4,3): Error RZ1026: Encountered end tag "body" with no matching start tag. Are your start/end tags properly balanced?
TestFiles/IntegrationTests/CodeGenerationIntegrationTest/OpenedIf.cshtml(5,3): Error RZ1026: Encountered end tag "html" with no matching start tag. Are your start/end tags properly balanced?
TestFiles/IntegrationTests/CodeGenerationIntegrationTest/OpenedIf.cshtml(3,2): Error RZ1006: The if block is missing a closing "}" character. Make sure you have a matching "}" character for all the "{" characters within this block, and that none of the "}" characters are being interpreted as markup.
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
TestFiles/IntegrationTests/CodeGenerationIntegrationTest/ParserError.cshtml(2,1): Error RZ1001: End of file was reached before the end of the block comment. All comments started with "/*" sequence must be terminated with a matching "*/" sequence.
TestFiles/IntegrationTests/CodeGenerationIntegrationTest/ParserError.cshtml(1,2): Error RZ1006: The code block is missing a closing "}" character. Make sure you have a matching "}" character for all the "{" characters within this block, and that none of the "}" characters are being interpreted as markup.
TestFiles/IntegrationTests/CodeGenerationIntegrationTest/ParserError.cshtml(2,1): Error RZ1001: End of file was reached before the end of the block comment. All comments started with "/*" sequence must be terminated with a matching "*/" sequence.
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
TestFiles/IntegrationTests/CodeGenerationIntegrationTest/ParserError.cshtml(2,1): Error RZ1001: End of file was reached before the end of the block comment. All comments started with "/*" sequence must be terminated with a matching "*/" sequence.
TestFiles/IntegrationTests/CodeGenerationIntegrationTest/ParserError.cshtml(1,2): Error RZ1006: The code block is missing a closing "}" character. Make sure you have a matching "}" character for all the "{" characters within this block, and that none of the "}" characters are being interpreted as markup.
TestFiles/IntegrationTests/CodeGenerationIntegrationTest/ParserError.cshtml(2,1): Error RZ1001: End of file was reached before the end of the block comment. All comments started with "/*" sequence must be terminated with a matching "*/" sequence.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
(1,11): Error RZ1006: The functions block is missing a closing "}" character. Make sure you have a matching "}" character for all the "{" characters within this block, and that none of the "}" characters are being interpreted as markup.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
(1,11): Error RZ1006: The functions block is missing a closing "}" character. Make sure you have a matching "}" character for all the "{" characters within this block, and that none of the "}" characters are being interpreted as markup.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
(1,17): Error RZ1006: The section block is missing a closing "}" character. Make sure you have a matching "}" character for all the "{" characters within this block, and that none of the "}" characters are being interpreted as markup.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
(1,17): Error RZ1006: The section block is missing a closing "}" character. Make sure you have a matching "}" character for all the "{" characters within this block, and that none of the "}" characters are being interpreted as markup.
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
(1,26): Error RZ1001: End of file was reached before the end of the block comment. All comments started with "/*" sequence must be terminated with a matching "*/" sequence.
(1,2): Error RZ1006: The foreach block is missing a closing "}" character. Make sure you have a matching "}" character for all the "{" characters within this block, and that none of the "}" characters are being interpreted as markup.
(1,26): Error RZ1001: End of file was reached before the end of the block comment. All comments started with "/*" sequence must be terminated with a matching "*/" sequence.
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
(1,4): Error RZ1024: End of file or an unexpected character was reached before the "span" tag could be parsed. Elements inside markup blocks must be complete. They must either be self-closing ("<br />") or have matching end tags ("<p>Hello</p>"). If you intended to display a "<" character, use the "&lt;" HTML entity.
(1,2): Error RZ1006: The code block is missing a closing "}" character. Make sure you have a matching "}" character for all the "{" characters within this block, and that none of the "}" characters are being interpreted as markup.
(1,4): Error RZ1024: End of file or an unexpected character was reached before the "span" tag could be parsed. Elements inside markup blocks must be complete. They must either be self-closing ("<br />") or have matching end tags ("<p>Hello</p>"). If you intended to display a "<" character, use the "&lt;" HTML entity.
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
(2,6): Error RZ1004: End-of-file was found after the "@" character. "@" must be followed by a valid code block. If you want to output an "@", escape it using the sequence: "@@"
(1,2): Error RZ1006: The code block is missing a closing "}" character. Make sure you have a matching "}" character for all the "{" characters within this block, and that none of the "}" characters are being interpreted as markup.
(2,6): Error RZ1004: End-of-file was found after the "@" character. "@" must be followed by a valid code block. If you want to output an "@", escape it using the sequence: "@@"
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
(1,12): Error RZ1006: The functions block is missing a closing "}" character. Make sure you have a matching "}" character for all the "{" characters within this block, and that none of the "}" characters are being interpreted as markup.
Loading

0 comments on commit ef31a96

Please sign in to comment.