From 26afdbd88959c94c17755f1ae7ce4add4fbe8489 Mon Sep 17 00:00:00 2001 From: Doug Bunting Date: Tue, 13 Jan 2015 20:54:45 -0800 Subject: [PATCH] Plumb `ParserErrorSink` through to `CodeBuilderContext` - precursor for #129 - remove unused `GeneratorResults` ctor to avoid duplicating `ParserResults` code nit: make a few `ParserResults` properties immutable - also change `ParserErrors` type to `IEnumerable` --- .../Generator/CodeBuilderContext.cs | 18 +++++- .../GeneratorResults.cs | 35 ++++-------- .../Parser/ParserContext.cs | 2 +- .../Parser/RazorParser.cs | 2 +- src/Microsoft.AspNet.Razor/ParserResults.cs | 55 ++++++++++++------- .../RazorTemplateEngine.cs | 2 +- .../CSharpRazorCodeLanguageTest.cs | 3 +- .../Framework/ParserTestBase.cs | 14 +++-- .../CSharpTagHelperRenderingUnitTest.cs | 4 +- .../CodeTree/CSharpCodeBuilderTests.cs | 4 +- .../Generator/CodeTree/ChunkVisitorTests.cs | 4 +- .../Parser/Html/HtmlAttributeTest.cs | 4 +- .../Parser/ParserVisitorExtensionsTest.cs | 27 +++++---- .../RazorTemplateEngineTest.cs | 3 +- 14 files changed, 105 insertions(+), 72 deletions(-) diff --git a/src/Microsoft.AspNet.Razor/Generator/CodeBuilderContext.cs b/src/Microsoft.AspNet.Razor/Generator/CodeBuilderContext.cs index 5d7897389c..9e84a7da46 100644 --- a/src/Microsoft.AspNet.Razor/Generator/CodeBuilderContext.cs +++ b/src/Microsoft.AspNet.Razor/Generator/CodeBuilderContext.cs @@ -1,6 +1,8 @@ // Copyright (c) Microsoft Open Technologies, Inc. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using Microsoft.AspNet.Razor.Parser; + namespace Microsoft.AspNet.Razor.Generator { /// @@ -12,9 +14,14 @@ namespace Microsoft.AspNet.Razor.Generator /// Instantiates a new instance of the object. /// /// A to copy information from. - public CodeBuilderContext(CodeGeneratorContext generatorContext) + /// + /// The used to collect s encountered + /// when parsing the current Razor document. + /// + public CodeBuilderContext(CodeGeneratorContext generatorContext, ParserErrorSink errorSink) : base(generatorContext) { + ErrorSink = errorSink; ExpressionRenderingMode = ExpressionRenderingMode.WriteToOutput; } @@ -23,9 +30,11 @@ namespace Microsoft.AspNet.Razor.Generator string className, string rootNamespace, string sourceFile, - bool shouldGenerateLinePragmas) + bool shouldGenerateLinePragmas, + ParserErrorSink errorSink) : base(host, className, rootNamespace, sourceFile, shouldGenerateLinePragmas) { + ErrorSink = errorSink; ExpressionRenderingMode = ExpressionRenderingMode.WriteToOutput; } @@ -56,5 +65,10 @@ namespace Microsoft.AspNet.Razor.Generator /// . /// public string Checksum { get; set; } + + /// + /// Used to aggregate s. + /// + public ParserErrorSink ErrorSink { get; } } } diff --git a/src/Microsoft.AspNet.Razor/GeneratorResults.cs b/src/Microsoft.AspNet.Razor/GeneratorResults.cs index ec02a3f935..2698cc6abe 100644 --- a/src/Microsoft.AspNet.Razor/GeneratorResults.cs +++ b/src/Microsoft.AspNet.Razor/GeneratorResults.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using Microsoft.AspNet.Razor.Generator.Compiler; +using Microsoft.AspNet.Razor.Parser; using Microsoft.AspNet.Razor.Parser.SyntaxTree; using Microsoft.AspNet.Razor.TagHelpers; @@ -24,7 +25,7 @@ namespace Microsoft.AspNet.Razor [NotNull] CodeTree codeTree) : this(parserResults.Document, parserResults.TagHelperDescriptors, - parserResults.ParserErrors, + parserResults.ErrorSink, codeBuilderResult, codeTree) { @@ -34,35 +35,21 @@ namespace Microsoft.AspNet.Razor /// Instantiates a new instance. /// /// The for the syntax tree. - /// s for the document. - /// s encountered when parsing the document. + /// + /// The s that apply to the current Razor document. + /// + /// + /// The used to collect s encountered when parsing the + /// current Razor document. + /// /// The results of generating code for the document. /// A for the document. public GeneratorResults([NotNull] Block document, [NotNull] IEnumerable tagHelperDescriptors, - [NotNull] IList parserErrors, + [NotNull] ParserErrorSink errorSink, [NotNull] CodeBuilderResult codeBuilderResult, [NotNull] CodeTree codeTree) - : this(parserErrors.Count == 0, document, tagHelperDescriptors, parserErrors, codeBuilderResult, codeTree) - { - } - - /// - /// Instantiates a new instance. - /// - /// true if parsing was successful, false otherwise. - /// The for the syntax tree. - /// s for the document. - /// s encountered when parsing the document. - /// The results of generating code for the document. - /// A for the document. - protected GeneratorResults(bool success, - [NotNull] Block document, - [NotNull] IEnumerable tagHelperDescriptors, - [NotNull] IList parserErrors, - [NotNull] CodeBuilderResult codeBuilderResult, - [NotNull] CodeTree codeTree) - : base(success, document, tagHelperDescriptors, parserErrors) + : base(document, tagHelperDescriptors, errorSink) { GeneratedCode = codeBuilderResult.Code; DesignTimeLineMappings = codeBuilderResult.DesignTimeLineMappings; diff --git a/src/Microsoft.AspNet.Razor/Parser/ParserContext.cs b/src/Microsoft.AspNet.Razor/Parser/ParserContext.cs index fa0a9bbf4b..95d8a30325 100644 --- a/src/Microsoft.AspNet.Razor/Parser/ParserContext.cs +++ b/src/Microsoft.AspNet.Razor/Parser/ParserContext.cs @@ -230,7 +230,7 @@ namespace Microsoft.AspNet.Razor.Parser // TagHelperDescriptors are not found by default. The RazorParser is responsible // for identifying TagHelperDescriptors and rebuilding ParserResults. tagHelperDescriptors: Enumerable.Empty(), - parserErrors: _errorSink.Errors.ToList()); + errorSink: _errorSink); } [Conditional("DEBUG")] diff --git a/src/Microsoft.AspNet.Razor/Parser/RazorParser.cs b/src/Microsoft.AspNet.Razor/Parser/RazorParser.cs index 01a04ed8e3..2d538e347c 100644 --- a/src/Microsoft.AspNet.Razor/Parser/RazorParser.cs +++ b/src/Microsoft.AspNet.Razor/Parser/RazorParser.cs @@ -197,7 +197,7 @@ namespace Microsoft.AspNet.Razor.Parser } // Return the new result - return new ParserResults(syntaxTree, descriptors, errorSink.Errors.ToList()); + return new ParserResults(syntaxTree, descriptors, errorSink); } /// diff --git a/src/Microsoft.AspNet.Razor/ParserResults.cs b/src/Microsoft.AspNet.Razor/ParserResults.cs index 97df423630..9d4b9709b0 100644 --- a/src/Microsoft.AspNet.Razor/ParserResults.cs +++ b/src/Microsoft.AspNet.Razor/ParserResults.cs @@ -2,6 +2,8 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System.Collections.Generic; +using System.Linq; +using Microsoft.AspNet.Razor.Parser; using Microsoft.AspNet.Razor.Parser.SyntaxTree; using Microsoft.AspNet.Razor.TagHelpers; @@ -15,15 +17,18 @@ namespace Microsoft.AspNet.Razor /// /// Instantiates a new instance. /// - /// The Razor syntax tree. - /// s that apply to the current Razor - /// document. - /// s encountered when parsing the current Razor - /// document. + /// The for the syntax tree. + /// + /// The s that apply to the current Razor document. + /// + /// + /// The used to collect s encountered when parsing the + /// current Razor document. + /// public ParserResults([NotNull] Block document, [NotNull] IEnumerable tagHelperDescriptors, - [NotNull] IList parserErrors) - : this(parserErrors == null || parserErrors.Count == 0, document, tagHelperDescriptors, parserErrors) + [NotNull] ParserErrorSink errorSink) + : this(!errorSink.Errors.Any(), document, tagHelperDescriptors, errorSink) { } @@ -31,40 +36,50 @@ namespace Microsoft.AspNet.Razor /// Instantiates a new instance. /// /// true if parsing was successful, false otherwise. - /// The Razor syntax tree. - /// s that apply to the current Razor - /// document. - /// s encountered when parsing the current Razor - /// document. + /// The for the syntax tree. + /// + /// The s that apply to the current Razor document. + /// + /// + /// The used to collect s encountered when parsing the + /// current Razor document. + /// protected ParserResults(bool success, [NotNull] Block document, [NotNull] IEnumerable tagHelperDescriptors, - [NotNull] IList errors) + [NotNull] ParserErrorSink errorSink) { Success = success; Document = document; TagHelperDescriptors = tagHelperDescriptors; - ParserErrors = errors ?? new List(); + ErrorSink = errorSink; + ParserErrors = errorSink.Errors; } /// - /// Indicates if parsing was successful (no errors) + /// Indicates if parsing was successful (no errors). /// - public bool Success { get; private set; } + /// true if parsing was successful, false otherwise. + public bool Success { get; } /// - /// The root node in the document's syntax tree + /// The root node in the document's syntax tree. /// - public Block Document { get; private set; } + public Block Document { get; } + + /// + /// Used to aggregate s. + /// + public ParserErrorSink ErrorSink { get; } /// /// The list of errors which occurred during parsing. /// - public IList ParserErrors { get; private set; } + public IEnumerable ParserErrors { get; } /// /// The s found for the current Razor document. /// - public IEnumerable TagHelperDescriptors { get; private set; } + public IEnumerable TagHelperDescriptors { get; } } } diff --git a/src/Microsoft.AspNet.Razor/RazorTemplateEngine.cs b/src/Microsoft.AspNet.Razor/RazorTemplateEngine.cs index 40e10a0474..1e38b4eb9e 100644 --- a/src/Microsoft.AspNet.Razor/RazorTemplateEngine.cs +++ b/src/Microsoft.AspNet.Razor/RazorTemplateEngine.cs @@ -252,7 +252,7 @@ namespace Microsoft.AspNet.Razor generator.DesignTimeMode = Host.DesignTimeMode; generator.Visit(results); - var codeBuilderContext = new CodeBuilderContext(generator.Context); + var codeBuilderContext = new CodeBuilderContext(generator.Context, results.ErrorSink); codeBuilderContext.Checksum = checksum; var builder = CreateCodeBuilder(codeBuilderContext); var builderResult = builder.Build(); diff --git a/test/Microsoft.AspNet.Razor.Test/CSharpRazorCodeLanguageTest.cs b/test/Microsoft.AspNet.Razor.Test/CSharpRazorCodeLanguageTest.cs index 44a734d091..a7475f3108 100644 --- a/test/Microsoft.AspNet.Razor.Test/CSharpRazorCodeLanguageTest.cs +++ b/test/Microsoft.AspNet.Razor.Test/CSharpRazorCodeLanguageTest.cs @@ -54,7 +54,8 @@ namespace Microsoft.AspNet.Razor.Test "myclass", "myns", string.Empty, - shouldGenerateLinePragmas: false); + shouldGenerateLinePragmas: false, + errorSink: new ParserErrorSink()); // Act var generator = language.CreateCodeBuilder(codeBuilderContext); diff --git a/test/Microsoft.AspNet.Razor.Test/Framework/ParserTestBase.cs b/test/Microsoft.AspNet.Razor.Test/Framework/ParserTestBase.cs index b617f48724..33516b6058 100644 --- a/test/Microsoft.AspNet.Razor.Test/Framework/ParserTestBase.cs +++ b/test/Microsoft.AspNet.Razor.Test/Framework/ParserTestBase.cs @@ -434,28 +434,30 @@ namespace Microsoft.AspNet.Razor.Test.Framework collector.AddError("{0} - FAILED :: Actual: << Null >>", expected); } - public static void EvaluateRazorErrors(IList actualErrors, IList expectedErrors) + public static void EvaluateRazorErrors(IEnumerable actualErrors, IList expectedErrors) { + var realCount = actualErrors.Count(); + // Evaluate the errors if (expectedErrors == null || expectedErrors.Count == 0) { - Assert.True(actualErrors.Count == 0, + Assert.True(realCount == 0, String.Format("Expected that no errors would be raised, but the following errors were:\r\n{0}", FormatErrors(actualErrors))); } else { - Assert.True(expectedErrors.Count == actualErrors.Count, + Assert.True(expectedErrors.Count == realCount, String.Format("Expected that {0} errors would be raised, but {1} errors were.\r\nExpected Errors: \r\n{2}\r\nActual Errors: \r\n{3}", expectedErrors.Count, - actualErrors.Count, + realCount, FormatErrors(expectedErrors), FormatErrors(actualErrors))); - Assert.Equal(expectedErrors.ToArray(), actualErrors.ToArray()); + Assert.Equal(expectedErrors, actualErrors); } WriteTraceLine("Expected Errors were raised:\r\n{0}", FormatErrors(expectedErrors)); } - public static string FormatErrors(IList errors) + public static string FormatErrors(IEnumerable errors) { if (errors == null) { diff --git a/test/Microsoft.AspNet.Razor.Test/Generator/CSharpTagHelperRenderingUnitTest.cs b/test/Microsoft.AspNet.Razor.Test/Generator/CSharpTagHelperRenderingUnitTest.cs index 41e915fca8..2f58dbc5f4 100644 --- a/test/Microsoft.AspNet.Razor.Test/Generator/CSharpTagHelperRenderingUnitTest.cs +++ b/test/Microsoft.AspNet.Razor.Test/Generator/CSharpTagHelperRenderingUnitTest.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using Microsoft.AspNet.Razor.Generator; using Microsoft.AspNet.Razor.Generator.Compiler; using Microsoft.AspNet.Razor.Generator.Compiler.CSharp; +using Microsoft.AspNet.Razor.Parser; using Microsoft.AspNet.Razor.TagHelpers; using Xunit; @@ -161,7 +162,8 @@ namespace Microsoft.AspNet.Razor.Test.Generator "MyClass", "MyNamespace", string.Empty, - shouldGenerateLinePragmas: true)); + shouldGenerateLinePragmas: true), + new ParserErrorSink()); } private class TrackingUniqueIdsTagHelperCodeRenderer : CSharpTagHelperCodeRenderer diff --git a/test/Microsoft.AspNet.Razor.Test/Generator/CodeTree/CSharpCodeBuilderTests.cs b/test/Microsoft.AspNet.Razor.Test/Generator/CodeTree/CSharpCodeBuilderTests.cs index e5a04834c9..a47d1cb13f 100644 --- a/test/Microsoft.AspNet.Razor.Test/Generator/CodeTree/CSharpCodeBuilderTests.cs +++ b/test/Microsoft.AspNet.Razor.Test/Generator/CodeTree/CSharpCodeBuilderTests.cs @@ -3,6 +3,7 @@ #if !ASPNETCORE50 using Microsoft.AspNet.Razor.Generator; +using Microsoft.AspNet.Razor.Parser; using Microsoft.AspNet.Razor.Parser.SyntaxTree; using Microsoft.AspNet.Razor.Test.Utils; using Moq; @@ -23,7 +24,8 @@ namespace Microsoft.AspNet.Razor.Test.Generator.CodeTree "TestClass", "TestNamespace", "Foo.cs", - shouldGenerateLinePragmas: false); + shouldGenerateLinePragmas: false, + errorSink: new ParserErrorSink()); codeBuilderContext.CodeTreeBuilder.AddUsingChunk("FakeNamespace1", syntaxTreeNode.Object); codeBuilderContext.CodeTreeBuilder.AddUsingChunk("FakeNamespace2.SubNamespace", syntaxTreeNode.Object); var codeBuilder = language.CreateCodeBuilder(codeBuilderContext); diff --git a/test/Microsoft.AspNet.Razor.Test/Generator/CodeTree/ChunkVisitorTests.cs b/test/Microsoft.AspNet.Razor.Test/Generator/CodeTree/ChunkVisitorTests.cs index 020fdcb2e0..b7244cb208 100644 --- a/test/Microsoft.AspNet.Razor.Test/Generator/CodeTree/ChunkVisitorTests.cs +++ b/test/Microsoft.AspNet.Razor.Test/Generator/CodeTree/ChunkVisitorTests.cs @@ -4,6 +4,7 @@ #if !ASPNETCORE50 using Microsoft.AspNet.Razor.Generator; using Microsoft.AspNet.Razor.Generator.Compiler; +using Microsoft.AspNet.Razor.Parser; using Moq; using Moq.Protected; using Xunit; @@ -34,7 +35,8 @@ namespace Microsoft.AspNet.Razor "myclass", "myns", string.Empty, - shouldGenerateLinePragmas: false); + shouldGenerateLinePragmas: false, + errorSink: new ParserErrorSink()); var writer = Mock.Of(); return new Mock>(writer, codeBuilderContext); } diff --git a/test/Microsoft.AspNet.Razor.Test/Parser/Html/HtmlAttributeTest.cs b/test/Microsoft.AspNet.Razor.Test/Parser/Html/HtmlAttributeTest.cs index 604e6d97fe..b80f409894 100644 --- a/test/Microsoft.AspNet.Razor.Test/Parser/Html/HtmlAttributeTest.cs +++ b/test/Microsoft.AspNet.Razor.Test/Parser/Html/HtmlAttributeTest.cs @@ -190,7 +190,7 @@ namespace Microsoft.AspNet.Razor.Test.Parser.Html var rewritten = rewritingContext.SyntaxTree; // Assert - Assert.Equal(0, results.ParserErrors.Count); + Assert.Equal(0, results.ParserErrors.Count()); EvaluateParseTree(rewritten, new MarkupBlock( new MarkupTagBlock( @@ -277,7 +277,7 @@ namespace Microsoft.AspNet.Razor.Test.Parser.Html var rewritten = rewritingContext.SyntaxTree; // Assert - Assert.Equal(0, results.ParserErrors.Count); + Assert.Equal(0, results.ParserErrors.Count()); Assert.Equal(rewritten.Children.Count(), results.Document.Children.Count()); } diff --git a/test/Microsoft.AspNet.Razor.Test/Parser/ParserVisitorExtensionsTest.cs b/test/Microsoft.AspNet.Razor.Test/Parser/ParserVisitorExtensionsTest.cs index 264c8be456..9e6dcc44dd 100644 --- a/test/Microsoft.AspNet.Razor.Test/Parser/ParserVisitorExtensionsTest.cs +++ b/test/Microsoft.AspNet.Razor.Test/Parser/ParserVisitorExtensionsTest.cs @@ -19,9 +19,10 @@ namespace Microsoft.AspNet.Razor.Test.Parser public void VisitThrowsOnNullVisitor() { ParserVisitor target = null; + var errorSink = new ParserErrorSink(); var results = new ParserResults(new BlockBuilder() { Type = BlockType.Comment }.Build(), Enumerable.Empty(), - parserErrors: new List()); + errorSink); Assert.Throws("self", () => target.Visit(results)); } @@ -39,9 +40,10 @@ namespace Microsoft.AspNet.Razor.Test.Parser // Arrange Mock targetMock = new Mock(); var root = new BlockBuilder() { Type = BlockType.Comment }.Build(); + var errorSink = new ParserErrorSink(); var results = new ParserResults(root, Enumerable.Empty(), - parserErrors: new List()); + errorSink); // Act targetMock.Object.Visit(results); @@ -56,11 +58,17 @@ namespace Microsoft.AspNet.Razor.Test.Parser // Arrange Mock targetMock = new Mock(); var root = new BlockBuilder() { Type = BlockType.Comment }.Build(); - List errors = new List() { + var errorSink = new ParserErrorSink(); + List errors = new List + { new RazorError("Foo", 1, 0, 1), - new RazorError("Bar", 2, 0, 2) + new RazorError("Bar", 2, 0, 2), }; - var results = new ParserResults(root, Enumerable.Empty(), errors); + foreach (var error in errors) + { + errorSink.OnError(error); + } + var results = new ParserResults(root, Enumerable.Empty(), errorSink); // Act targetMock.Object.Visit(results); @@ -76,11 +84,10 @@ namespace Microsoft.AspNet.Razor.Test.Parser // Arrange Mock targetMock = new Mock(); var root = new BlockBuilder() { Type = BlockType.Comment }.Build(); - List errors = new List() { - new RazorError("Foo", 1, 0, 1), - new RazorError("Bar", 2, 0, 2) - }; - var results = new ParserResults(root, Enumerable.Empty(), errors); + var errorSink = new ParserErrorSink(); + errorSink.OnError(new RazorError("Foo", 1, 0, 1)); + errorSink.OnError(new RazorError("Bar", 2, 0, 2)); + var results = new ParserResults(root, Enumerable.Empty(), errorSink); // Act targetMock.Object.Visit(results); diff --git a/test/Microsoft.AspNet.Razor.Test/RazorTemplateEngineTest.cs b/test/Microsoft.AspNet.Razor.Test/RazorTemplateEngineTest.cs index 53770f1729..fdc88f124f 100644 --- a/test/Microsoft.AspNet.Razor.Test/RazorTemplateEngineTest.cs +++ b/test/Microsoft.AspNet.Razor.Test/RazorTemplateEngineTest.cs @@ -119,7 +119,8 @@ namespace Microsoft.AspNet.Razor.Test "different-class", "different-ns", string.Empty, - shouldGenerateLinePragmas: true); + shouldGenerateLinePragmas: true, + errorSink: new ParserErrorSink()); var expected = new CSharpCodeBuilder(codeBuilderContext);