From e35ee53ee5da52ac5154c3c96f48bc2b4e4f047d Mon Sep 17 00:00:00 2001 From: Ryan Nowak Date: Wed, 15 Feb 2017 18:03:50 -0800 Subject: [PATCH] DefaultRazorIRLoweringPhase is too agressive This change fixes a bug where DefaultRazorIRLoweringPhase is too aggressive in merging HTML spans. You can hit the bug by delimiting two html spans with a metacode character like: @{ } The lowering phase will combine these HTML nodes, which is invalid as they don't have contiguous spans. The change here is to merge spans only when they both have an invalid location or are contiguous. --- .../DefaultRazorIRLoweringPhase.cs | 60 +++++++++++++------ .../Await_Runtime.codegen.cs | 6 +- .../EscapedTagHelpers_Runtime.codegen.cs | 13 +++- .../Instrumented_Runtime.codegen.cs | 3 +- .../RazorComments_Runtime.codegen.cs | 3 +- .../Templates_Runtime.codegen.cs | 3 +- 6 files changed, 61 insertions(+), 27 deletions(-) diff --git a/src/Microsoft.AspNetCore.Razor.Evolution/DefaultRazorIRLoweringPhase.cs b/src/Microsoft.AspNetCore.Razor.Evolution/DefaultRazorIRLoweringPhase.cs index 284d705ca3..b7bc7875fa 100644 --- a/src/Microsoft.AspNetCore.Razor.Evolution/DefaultRazorIRLoweringPhase.cs +++ b/src/Microsoft.AspNetCore.Razor.Evolution/DefaultRazorIRLoweringPhase.cs @@ -6,6 +6,7 @@ using System.Collections.Generic; using System.Linq; using Microsoft.AspNetCore.Razor.Evolution.Intermediate; using Microsoft.AspNetCore.Razor.Evolution.Legacy; +using System.Diagnostics; namespace Microsoft.AspNetCore.Razor.Evolution { @@ -176,16 +177,21 @@ namespace Microsoft.AspNetCore.Razor.Evolution _builder.Pop(); } - protected SourceSpan BuildSourceSpanFromNode(SyntaxTreeNode node) + protected SourceSpan? BuildSourceSpanFromNode(SyntaxTreeNode node) { var location = node.Start; - var sourceRange = new SourceSpan( + if (location == SourceLocation.Undefined) + { + return null; + } + + var span = new SourceSpan( node.Start.FilePath ?? Filename, node.Start.AbsoluteIndex, node.Start.LineIndex, node.Start.CharacterIndex, node.Length); - return sourceRange; + return span; } } @@ -227,7 +233,7 @@ namespace Microsoft.AspNetCore.Razor.Evolution } } } - + private class MainSourceVisitor : LoweringVisitor { private DeclareTagHelperFieldsIRNode _tagHelperFields; @@ -399,27 +405,43 @@ namespace Microsoft.AspNetCore.Razor.Evolution if (currentChildren.Count > 0 && currentChildren[currentChildren.Count - 1] is HtmlContentIRNode) { var existingHtmlContent = (HtmlContentIRNode)currentChildren[currentChildren.Count - 1]; - existingHtmlContent.Content = string.Concat(existingHtmlContent.Content, span.Content); - if (existingHtmlContent.Source != null) + var source = BuildSourceSpanFromNode(span); + if (existingHtmlContent.Source == null && source == null) { - var contentLength = existingHtmlContent.Source.Value.Length + span.Content.Length; + Combine(existingHtmlContent, span); + return; + } - existingHtmlContent.Source = new SourceSpan( - existingHtmlContent.Source.Value.FilePath ?? Filename, - existingHtmlContent.Source.Value.AbsoluteIndex, - existingHtmlContent.Source.Value.LineIndex, - existingHtmlContent.Source.Value.CharacterIndex, - contentLength); + if (source != null && + existingHtmlContent.Source != null && + existingHtmlContent.Source.Value.FilePath == source.Value.FilePath && + existingHtmlContent.Source.Value.AbsoluteIndex + existingHtmlContent.Source.Value.Length == source.Value.AbsoluteIndex) + { + Combine(existingHtmlContent, span); + return; } } - else + + _builder.Add(new HtmlContentIRNode() { - _builder.Add(new HtmlContentIRNode() - { - Content = span.Content, - Source = BuildSourceSpanFromNode(span), - }); + Content = span.Content, + Source = BuildSourceSpanFromNode(span), + }); + } + private void Combine(HtmlContentIRNode node, Span span) + { + node.Content = node.Content + span.Content; + if (node.Source != null) + { + Debug.Assert(node.Source.Value.FilePath != null); + + node.Source = new SourceSpan( + node.Source.Value.FilePath, + node.Source.Value.AbsoluteIndex, + node.Source.Value.LineIndex, + node.Source.Value.CharacterIndex, + node.Content.Length); } } diff --git a/test/Microsoft.AspNetCore.Razor.Evolution.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/Await_Runtime.codegen.cs b/test/Microsoft.AspNetCore.Razor.Evolution.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/Await_Runtime.codegen.cs index d975175ede..3ce77efef7 100644 --- a/test/Microsoft.AspNetCore.Razor.Evolution.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/Await_Runtime.codegen.cs +++ b/test/Microsoft.AspNetCore.Razor.Evolution.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/Await_Runtime.codegen.cs @@ -27,7 +27,8 @@ namespace Microsoft.AspNetCore.Razor.Evolution.IntegrationTests.TestFiles #line default #line hidden - WriteLiteral("

\r\n

Basic Asynchronous Statement Nested: "); + WriteLiteral("

\r\n

Basic Asynchronous Statement Nested: "); + WriteLiteral(" "); #line 13 "TestFiles/IntegrationTests/CodeGenerationIntegrationTest/Await.cshtml" Write(await Foo()); @@ -70,7 +71,8 @@ namespace Microsoft.AspNetCore.Razor.Evolution.IntegrationTests.TestFiles #line default #line hidden - WriteLiteral("

\r\n

Advanced Asynchronous Statement Nested: "); + WriteLiteral("

\r\n

Advanced Asynchronous Statement Nested: "); + WriteLiteral(" "); #line 24 "TestFiles/IntegrationTests/CodeGenerationIntegrationTest/Await.cshtml" Write(await Foo(boolValue: false)); diff --git a/test/Microsoft.AspNetCore.Razor.Evolution.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/EscapedTagHelpers_Runtime.codegen.cs b/test/Microsoft.AspNetCore.Razor.Evolution.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/EscapedTagHelpers_Runtime.codegen.cs index 85a6d85850..b74f89b059 100644 --- a/test/Microsoft.AspNetCore.Razor.Evolution.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/EscapedTagHelpers_Runtime.codegen.cs +++ b/test/Microsoft.AspNetCore.Razor.Evolution.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/EscapedTagHelpers_Runtime.codegen.cs @@ -29,13 +29,18 @@ namespace Microsoft.AspNetCore.Razor.Evolution.IntegrationTests.TestFiles #pragma warning disable 1998 public async System.Threading.Tasks.Task ExecuteAsync() { - WriteLiteral("\r\n

\r\n

\r\n <"); + WriteLiteral("p class=\"Hello World\" "); #line 4 "TestFiles/IntegrationTests/CodeGenerationIntegrationTest/EscapedTagHelpers.cshtml" Write(DateTime.Now); #line default #line hidden - WriteLiteral(">\r\n \r\n Not a TagHelper: "); + WriteLiteral(">\r\n <"); + WriteLiteral("input type=\"text\" />\r\n <"); + WriteLiteral("em>Not a TagHelper: "); __tagHelperExecutionContext = __tagHelperScopeManager.Begin("input", global::Microsoft.AspNetCore.Razor.TagHelpers.TagMode.SelfClosing, "test", async() => { } ); @@ -66,7 +71,9 @@ __TestNamespace_InputTagHelper2.Checked = true; } Write(__tagHelperExecutionContext.Output); __tagHelperExecutionContext = __tagHelperScopeManager.End(); - WriteLiteral("\r\n

\r\n
"); + WriteLiteral("\r\n \r\n"); } #pragma warning restore 1998 } diff --git a/test/Microsoft.AspNetCore.Razor.Evolution.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/Instrumented_Runtime.codegen.cs b/test/Microsoft.AspNetCore.Razor.Evolution.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/Instrumented_Runtime.codegen.cs index 77ed6d1011..9fb4d00ade 100644 --- a/test/Microsoft.AspNetCore.Razor.Evolution.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/Instrumented_Runtime.codegen.cs +++ b/test/Microsoft.AspNetCore.Razor.Evolution.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/Instrumented_Runtime.codegen.cs @@ -26,7 +26,8 @@ namespace Microsoft.AspNetCore.Razor.Evolution.IntegrationTests.TestFiles #line default #line hidden - WriteLiteral(" Hello, World\r\n

Hello, World

\r\n"); + WriteLiteral(" "); + WriteLiteral("Hello, World\r\n

Hello, World

\r\n"); WriteLiteral("\r\n"); #line 8 "TestFiles/IntegrationTests/CodeGenerationIntegrationTest/Instrumented.cshtml" while(i <= 10) { diff --git a/test/Microsoft.AspNetCore.Razor.Evolution.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/RazorComments_Runtime.codegen.cs b/test/Microsoft.AspNetCore.Razor.Evolution.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/RazorComments_Runtime.codegen.cs index 71a0ec5ff4..a13d5273e9 100644 --- a/test/Microsoft.AspNetCore.Razor.Evolution.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/RazorComments_Runtime.codegen.cs +++ b/test/Microsoft.AspNetCore.Razor.Evolution.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/RazorComments_Runtime.codegen.cs @@ -9,7 +9,8 @@ namespace Microsoft.AspNetCore.Razor.Evolution.IntegrationTests.TestFiles #pragma warning disable 1998 public async System.Threading.Tasks.Task ExecuteAsync() { - WriteLiteral("\r\n

This should be shown

\r\n\r\n"); + WriteLiteral("\r\n

This should "); + WriteLiteral(" be shown

\r\n\r\n"); #line 5 "TestFiles/IntegrationTests/CodeGenerationIntegrationTest/RazorComments.cshtml" Exception foo = diff --git a/test/Microsoft.AspNetCore.Razor.Evolution.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/Templates_Runtime.codegen.cs b/test/Microsoft.AspNetCore.Razor.Evolution.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/Templates_Runtime.codegen.cs index 5a45748218..0ac8d076a4 100644 --- a/test/Microsoft.AspNetCore.Razor.Evolution.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/Templates_Runtime.codegen.cs +++ b/test/Microsoft.AspNetCore.Razor.Evolution.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/Templates_Runtime.codegen.cs @@ -115,7 +115,8 @@ WriteTo(__razor_template_writer, item); #line default #line hidden - WriteLiteralTo(__razor_template_writer, "
    \r\n
  • Child Items... ?
    • \r\n
\r\n "); + WriteLiteralTo(__razor_template_writer, "
    \r\n
  • Child Items... ?
    • \r\n"); + WriteLiteralTo(__razor_template_writer, "
\r\n "); } )));