From 7afd78b36a0a1f7bcda276c115bb88b29982a15a Mon Sep 17 00:00:00 2001 From: "N. Taylor Mullen" Date: Mon, 26 Jan 2015 16:35:21 -0800 Subject: [PATCH] Fix empty attribute projections for TagHelpers. - Added TagHelperParseTreeRewriter tests, attempted to cover all empty attribute edge cases. - Added Codegen tests to validate output and DesignTimeLineMappings. #271 --- .../TagHelpers/TagHelperBlockRewriter.cs | 61 ++++++-- .../Generator/CSharpTagHelperRenderingTest.cs | 37 ++++- .../TagHelperParseTreeRewriterTest.cs | 71 +++++++++ .../EmptyAttributeTagHelpers.DesignTime.cs | 62 ++++++++ .../CS/Output/EmptyAttributeTagHelpers.cs | 141 ++++++++++++++++++ .../CS/Source/EmptyAttributeTagHelpers.cshtml | 8 + 6 files changed, 365 insertions(+), 15 deletions(-) create mode 100644 test/Microsoft.AspNet.Razor.Test/TestFiles/CodeGenerator/CS/Output/EmptyAttributeTagHelpers.DesignTime.cs create mode 100644 test/Microsoft.AspNet.Razor.Test/TestFiles/CodeGenerator/CS/Output/EmptyAttributeTagHelpers.cs create mode 100644 test/Microsoft.AspNet.Razor.Test/TestFiles/CodeGenerator/CS/Source/EmptyAttributeTagHelpers.cshtml diff --git a/src/Microsoft.AspNet.Razor/Parser/TagHelpers/TagHelperBlockRewriter.cs b/src/Microsoft.AspNet.Razor/Parser/TagHelpers/TagHelperBlockRewriter.cs index 4fa31a7889..49023e37c2 100644 --- a/src/Microsoft.AspNet.Razor/Parser/TagHelpers/TagHelperBlockRewriter.cs +++ b/src/Microsoft.AspNet.Razor/Parser/TagHelpers/TagHelperBlockRewriter.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.Linq; using Microsoft.AspNet.Razor.Generator; using Microsoft.AspNet.Razor.Parser.SyntaxTree; @@ -90,10 +91,15 @@ namespace Microsoft.AspNet.Razor.Parser.TagHelpers.Internal EditHandler = span.EditHandler, Kind = span.Kind }; + + // Will contain symbols that represent a single attribute value: var htmlSymbols = span.Symbols.OfType().ToArray(); var capturedAttributeValueStart = false; var attributeValueStartLocation = span.Start; - var symbolOffset = 1; + + // The symbolOffset is initialized to 0 to expect worst case: "class=". If a quote is found later on for + // the attribute value the symbolOffset is adjusted accordingly. + var symbolOffset = 0; string name = null; // Iterate down through the symbols to find the name and the start of the value. @@ -104,6 +110,11 @@ namespace Microsoft.AspNet.Razor.Parser.TagHelpers.Internal if (afterEquals) { + // We've captured all leading whitespace, the attribute name, and an equals with an optional + // quote/double quote. We're now at: " asp-for='|...'" or " asp-for=|..." + // The goal here is to capture all symbols until the end of the attribute. Note this will not + // consume an ending quote due to the symbolOffset. + // When symbols are accepted into SpanBuilders, their locations get altered to be offset by the // parent which is why we need to mark our start location prior to adding the symbol. // This is needed to know the location of the attribute value start within the document. @@ -118,13 +129,24 @@ namespace Microsoft.AspNet.Razor.Parser.TagHelpers.Internal } else if (name == null && symbol.Type == HtmlSymbolType.Text) { + // We've captured all leading whitespace prior to the attribute name. + // We're now at: " |asp-for='...'" or " |asp-for=..." + // The goal here is to capture the attribute name. + name = symbol.Content; - attributeValueStartLocation = SourceLocation.Advance(span.Start, name); + attributeValueStartLocation = SourceLocation.Advance(attributeValueStartLocation, name); } else if (symbol.Type == HtmlSymbolType.Equals) { - // We've found an '=' symbol, this means that the coming symbols will either be a quote - // or value (in the case that the value is unquoted). + Debug.Assert( + name != null, + "Name should never be null here. The parser should guaruntee an attribute has a name."); + + // We've captured all leading whitespace and the attribute name. + // We're now at: " asp-for|='...'" or " asp-for|=..." + // The goal here is to consume the equal sign and the optional single/double-quote. + + // The coming symbols will either be a quote or value (in the case that the value is unquoted). // Spaces after/before the equal symbol are not yet supported: // https://github.com/aspnet/Razor/issues/123 @@ -134,27 +156,38 @@ namespace Microsoft.AspNet.Razor.Parser.TagHelpers.Internal SourceLocation symbolStartLocation; // Check for attribute start values, aka single or double quote - if (IsQuote(htmlSymbols[i + 1])) + if ((i + 1) < htmlSymbols.Length && IsQuote(htmlSymbols[i + 1])) { // Move past the attribute start so we can accept the true value. i++; - symbolStartLocation = htmlSymbols[i + 1].Start; + symbolStartLocation = htmlSymbols[i].Start; + + // If there's a start quote then there must be an end quote to be valid, skip it. + symbolOffset = 1; } else { symbolStartLocation = symbol.Start; - - // Set the symbol offset to 0 so we don't attempt to skip an end quote that doesn't exist. - symbolOffset = 0; } - attributeValueStartLocation = symbolStartLocation + - span.Start + - new SourceLocation(absoluteIndex: 1, - lineIndex: 0, - characterIndex: 1); + attributeValueStartLocation = + span.Start + + symbolStartLocation + + new SourceLocation(absoluteIndex: 1, lineIndex: 0, characterIndex: 1); + afterEquals = true; } + else if (symbol.Type == HtmlSymbolType.WhiteSpace) + { + // We're at the start of the attribute, this branch may be hit on the first iterations of + // the loop since the parser separates attributes with their spaces included as symbols. + // We're at: "| asp-for='...'" or "| asp-for=..." + // Note: This will not be hit even for situations like asp-for ="..." because the core Razor + // parser currently does not know how to handle attributes in that format. This will be addressed + // by https://github.com/aspnet/Razor/issues/123. + + attributeValueStartLocation = SourceLocation.Advance(attributeValueStartLocation, symbol.Content); + } } // After all symbols have been added we need to set the builders start position so we do not indirectly diff --git a/test/Microsoft.AspNet.Razor.Test/Generator/CSharpTagHelperRenderingTest.cs b/test/Microsoft.AspNet.Razor.Test/Generator/CSharpTagHelperRenderingTest.cs index a706e68e50..75128bc2be 100644 --- a/test/Microsoft.AspNet.Razor.Test/Generator/CSharpTagHelperRenderingTest.cs +++ b/test/Microsoft.AspNet.Razor.Test/Generator/CSharpTagHelperRenderingTest.cs @@ -213,7 +213,41 @@ namespace Microsoft.AspNet.Razor.Test.Generator BuildLineMapping(1094, 30, 18, 5179, 187, 19, 29), BuildLineMapping(1231, 34, 5279, 192, 0, 1), } - } + }, + { + "EmptyAttributeTagHelpers", + "EmptyAttributeTagHelpers.DesignTime", + PAndInputTagHelperDescriptors, + new List + { + BuildLineMapping(documentAbsoluteIndex: 14, + documentLineIndex: 0, + generatedAbsoluteIndex: 493, + generatedLineIndex: 15, + characterOffsetIndex: 14, + contentLength: 11), + BuildLineMapping(documentAbsoluteIndex: 62, + documentLineIndex: 3, + documentCharacterOffsetIndex: 26, + generatedAbsoluteIndex: 1289, + generatedLineIndex: 39, + generatedCharacterOffsetIndex: 28, + contentLength: 0), + BuildLineMapping(documentAbsoluteIndex: 122, + documentLineIndex: 5, + generatedAbsoluteIndex: 1634, + generatedLineIndex: 48, + characterOffsetIndex: 30, + contentLength: 0), + BuildLineMapping(documentAbsoluteIndex: 88, + documentLineIndex: 4, + documentCharacterOffsetIndex: 12, + generatedAbsoluteIndex: 1789, + generatedLineIndex: 54, + generatedCharacterOffsetIndex: 19, + contentLength: 0) + } + }, }; } } @@ -245,6 +279,7 @@ namespace Microsoft.AspNet.Razor.Test.Generator { "BasicTagHelpers", PAndInputTagHelperDescriptors }, { "BasicTagHelpers.RemoveTagHelper", PAndInputTagHelperDescriptors }, { "ComplexTagHelpers", PAndInputTagHelperDescriptors }, + { "EmptyAttributeTagHelpers", PAndInputTagHelperDescriptors }, }; } } diff --git a/test/Microsoft.AspNet.Razor.Test/TagHelpers/TagHelperParseTreeRewriterTest.cs b/test/Microsoft.AspNet.Razor.Test/TagHelpers/TagHelperParseTreeRewriterTest.cs index 2425da44e1..68d0c1746f 100644 --- a/test/Microsoft.AspNet.Razor.Test/TagHelpers/TagHelperParseTreeRewriterTest.cs +++ b/test/Microsoft.AspNet.Razor.Test/TagHelpers/TagHelperParseTreeRewriterTest.cs @@ -19,6 +19,77 @@ namespace Microsoft.AspNet.Razor.Test.TagHelpers { public class TagHelperParseTreeRewriterTest : CsHtmlMarkupParserTestBase { + public static TheoryData EmptyAttributeTagHelperData + { + get + { + var factory = CreateDefaultSpanFactory(); + + // documentContent, expectedOutput + return new TheoryData + { + { + "

", + new MarkupBlock( + new MarkupTagHelperBlock("p", + new Dictionary + { + { "class", new MarkupBlock() } + })) + }, + { + "

", + new MarkupBlock( + new MarkupTagHelperBlock("p", + new Dictionary + { + { "class", new MarkupBlock() } + })) + }, + { + "

", + new MarkupBlock( + new MarkupTagHelperBlock("p", + new Dictionary + { + // We expected a markup node here because attribute values without quotes can only ever + // be a single item, hence don't need to be enclosed by a block. + { "class", factory.Markup("").With(SpanCodeGenerator.Null) }, + })) + }, + { + "

", + new MarkupBlock( + new MarkupTagHelperBlock("p", + new Dictionary + { + { "class1", new MarkupBlock() }, + { "class2", factory.Markup("").With(SpanCodeGenerator.Null) }, + { "class3", new MarkupBlock() }, + })) + }, + { + "

", + new MarkupBlock( + new MarkupTagHelperBlock("p", + new Dictionary + { + { "class1", new MarkupBlock() }, + { "class2", new MarkupBlock() }, + { "class3", factory.Markup("").With(SpanCodeGenerator.Null) }, + })) + }, + }; + } + } + + [Theory] + [MemberData(nameof(EmptyAttributeTagHelperData))] + public void Rewrite_UnderstandsEmptyAttributeTagHelpers(string documentContent, MarkupBlock expectedOutput) + { + RunParseTreeRewriterTest(documentContent, expectedOutput, new RazorError[0], "p"); + } + public static TheoryData MalformedTagHelperAttributeBlockData { get diff --git a/test/Microsoft.AspNet.Razor.Test/TestFiles/CodeGenerator/CS/Output/EmptyAttributeTagHelpers.DesignTime.cs b/test/Microsoft.AspNet.Razor.Test/TestFiles/CodeGenerator/CS/Output/EmptyAttributeTagHelpers.DesignTime.cs new file mode 100644 index 0000000000..931181397d --- /dev/null +++ b/test/Microsoft.AspNet.Razor.Test/TestFiles/CodeGenerator/CS/Output/EmptyAttributeTagHelpers.DesignTime.cs @@ -0,0 +1,62 @@ +namespace TestOutput +{ + using Microsoft.AspNet.Razor.Runtime.TagHelpers; + using System; + using System.Threading.Tasks; + + public class EmptyAttributeTagHelpers + { + private static object @__o; + private void @__RazorDesignTimeHelpers__() + { + #pragma warning disable 219 + string __tagHelperDirectiveSyntaxHelper = null; + __tagHelperDirectiveSyntaxHelper = +#line 1 "EmptyAttributeTagHelpers.cshtml" + "something" + +#line default +#line hidden + ; + #pragma warning restore 219 + } + #line hidden + private InputTagHelper __InputTagHelper = null; + private InputTagHelper2 __InputTagHelper2 = null; + private PTagHelper __PTagHelper = null; + #line hidden + public EmptyAttributeTagHelpers() + { + } + + #pragma warning disable 1998 + public override async Task ExecuteAsync() + { + __InputTagHelper = CreateTagHelper(); + __InputTagHelper.Type = ""; + __InputTagHelper2 = CreateTagHelper(); + __InputTagHelper2.Type = __InputTagHelper.Type; +#line 4 "EmptyAttributeTagHelpers.cshtml" +__InputTagHelper2.Checked = ; + +#line default +#line hidden + __InputTagHelper = CreateTagHelper(); + __InputTagHelper.Type = ""; + __InputTagHelper2 = CreateTagHelper(); + __InputTagHelper2.Type = __InputTagHelper.Type; +#line 6 "EmptyAttributeTagHelpers.cshtml" + __InputTagHelper2.Checked = ; + +#line default +#line hidden + __PTagHelper = CreateTagHelper(); +#line 5 "EmptyAttributeTagHelpers.cshtml" +__PTagHelper.Age = ; + +#line default +#line hidden + } + #pragma warning restore 1998 + } +} diff --git a/test/Microsoft.AspNet.Razor.Test/TestFiles/CodeGenerator/CS/Output/EmptyAttributeTagHelpers.cs b/test/Microsoft.AspNet.Razor.Test/TestFiles/CodeGenerator/CS/Output/EmptyAttributeTagHelpers.cs new file mode 100644 index 0000000000..aba79edb5e --- /dev/null +++ b/test/Microsoft.AspNet.Razor.Test/TestFiles/CodeGenerator/CS/Output/EmptyAttributeTagHelpers.cs @@ -0,0 +1,141 @@ +#pragma checksum "EmptyAttributeTagHelpers.cshtml" "{ff1816ec-aa5e-4d10-87f7-6f4963833460}" "828f744feb52d497814b7a018f817f31d92085ce" +namespace TestOutput +{ + using Microsoft.AspNet.Razor.Runtime.TagHelpers; + using System; + using System.Threading.Tasks; + + public class EmptyAttributeTagHelpers + { + #line hidden + #pragma warning disable 0414 + private System.IO.TextWriter __tagHelperStringValueBuffer = null; + #pragma warning restore 0414 + private TagHelperExecutionContext __tagHelperExecutionContext = null; + private TagHelperRunner __tagHelperRunner = new TagHelperRunner(); + private TagHelperScopeManager __tagHelperScopeManager = new TagHelperScopeManager(); + private InputTagHelper __InputTagHelper = null; + private InputTagHelper2 __InputTagHelper2 = null; + private PTagHelper __PTagHelper = null; + #line hidden + public EmptyAttributeTagHelpers() + { + } + + #pragma warning disable 1998 + public override async Task ExecuteAsync() + { + Instrumentation.BeginContext(27, 13, true); + WriteLiteral("\r\n

\r\n "); + Instrumentation.EndContext(); + __tagHelperExecutionContext = __tagHelperScopeManager.Begin("input", "test", async() => { + } + , StartWritingScope, EndWritingScope); + __InputTagHelper = CreateTagHelper(); + __tagHelperExecutionContext.Add(__InputTagHelper); + __InputTagHelper.Type = ""; + __tagHelperExecutionContext.AddTagHelperAttribute("type", __InputTagHelper.Type); + __InputTagHelper2 = CreateTagHelper(); + __tagHelperExecutionContext.Add(__InputTagHelper2); + __InputTagHelper2.Type = __InputTagHelper.Type; +#line 4 "EmptyAttributeTagHelpers.cshtml" +__InputTagHelper2.Checked = ; + +#line default +#line hidden + __tagHelperExecutionContext.AddTagHelperAttribute("checked", __InputTagHelper2.Checked); + __tagHelperExecutionContext.AddHtmlAttribute("class", ""); + __tagHelperExecutionContext.Output = __tagHelperRunner.RunAsync(__tagHelperExecutionContext).Result; + WriteLiteral(__tagHelperExecutionContext.Output.GenerateStartTag()); + WriteLiteral(__tagHelperExecutionContext.Output.GeneratePreContent()); + if (__tagHelperExecutionContext.Output.ContentSet) + { + WriteLiteral(__tagHelperExecutionContext.Output.GenerateContent()); + } + else if (__tagHelperExecutionContext.ChildContentRetrieved) + { + WriteLiteral(__tagHelperExecutionContext.GetChildContentAsync().Result); + } + else + { + __tagHelperExecutionContext.ExecuteChildContentAsync().Wait(); + } + WriteLiteral(__tagHelperExecutionContext.Output.GeneratePostContent()); + WriteLiteral(__tagHelperExecutionContext.Output.GenerateEndTag()); + __tagHelperExecutionContext = __tagHelperScopeManager.End(); + Instrumentation.BeginContext(74, 6, true); + WriteLiteral("\r\n "); + Instrumentation.EndContext(); + __tagHelperExecutionContext = __tagHelperScopeManager.Begin("p", "test", async() => { + WriteLiteral("\r\n "); + __tagHelperExecutionContext = __tagHelperScopeManager.Begin("input", "test", async() => { + } + , StartWritingScope, EndWritingScope); + __InputTagHelper = CreateTagHelper(); + __tagHelperExecutionContext.Add(__InputTagHelper); + __InputTagHelper.Type = ""; + __tagHelperExecutionContext.AddTagHelperAttribute("type", __InputTagHelper.Type); + __InputTagHelper2 = CreateTagHelper(); + __tagHelperExecutionContext.Add(__InputTagHelper2); + __InputTagHelper2.Type = __InputTagHelper.Type; +#line 6 "EmptyAttributeTagHelpers.cshtml" + __InputTagHelper2.Checked = ; + +#line default +#line hidden + __tagHelperExecutionContext.AddTagHelperAttribute("checked", __InputTagHelper2.Checked); + __tagHelperExecutionContext.AddHtmlAttribute("class", ""); + __tagHelperExecutionContext.Output = __tagHelperRunner.RunAsync(__tagHelperExecutionContext).Result; + WriteLiteral(__tagHelperExecutionContext.Output.GenerateStartTag()); + WriteLiteral(__tagHelperExecutionContext.Output.GeneratePreContent()); + if (__tagHelperExecutionContext.Output.ContentSet) + { + WriteLiteral(__tagHelperExecutionContext.Output.GenerateContent()); + } + else if (__tagHelperExecutionContext.ChildContentRetrieved) + { + WriteLiteral(__tagHelperExecutionContext.GetChildContentAsync().Result); + } + else + { + __tagHelperExecutionContext.ExecuteChildContentAsync().Wait(); + } + WriteLiteral(__tagHelperExecutionContext.Output.GeneratePostContent()); + WriteLiteral(__tagHelperExecutionContext.Output.GenerateEndTag()); + __tagHelperExecutionContext = __tagHelperScopeManager.End(); + WriteLiteral("\r\n "); + } + , StartWritingScope, EndWritingScope); + __PTagHelper = CreateTagHelper(); + __tagHelperExecutionContext.Add(__PTagHelper); +#line 5 "EmptyAttributeTagHelpers.cshtml" +__PTagHelper.Age = ; + +#line default +#line hidden + __tagHelperExecutionContext.AddTagHelperAttribute("age", __PTagHelper.Age); + __tagHelperExecutionContext.Output = __tagHelperRunner.RunAsync(__tagHelperExecutionContext).Result; + WriteLiteral(__tagHelperExecutionContext.Output.GenerateStartTag()); + WriteLiteral(__tagHelperExecutionContext.Output.GeneratePreContent()); + if (__tagHelperExecutionContext.Output.ContentSet) + { + WriteLiteral(__tagHelperExecutionContext.Output.GenerateContent()); + } + else if (__tagHelperExecutionContext.ChildContentRetrieved) + { + WriteLiteral(__tagHelperExecutionContext.GetChildContentAsync().Result); + } + else + { + __tagHelperExecutionContext.ExecuteChildContentAsync().Wait(); + } + WriteLiteral(__tagHelperExecutionContext.Output.GeneratePostContent()); + WriteLiteral(__tagHelperExecutionContext.Output.GenerateEndTag()); + __tagHelperExecutionContext = __tagHelperScopeManager.End(); + Instrumentation.BeginContext(144, 8, true); + WriteLiteral("\r\n
"); + Instrumentation.EndContext(); + } + #pragma warning restore 1998 + } +} diff --git a/test/Microsoft.AspNet.Razor.Test/TestFiles/CodeGenerator/CS/Source/EmptyAttributeTagHelpers.cshtml b/test/Microsoft.AspNet.Razor.Test/TestFiles/CodeGenerator/CS/Source/EmptyAttributeTagHelpers.cshtml new file mode 100644 index 0000000000..9a294f796f --- /dev/null +++ b/test/Microsoft.AspNet.Razor.Test/TestFiles/CodeGenerator/CS/Source/EmptyAttributeTagHelpers.cshtml @@ -0,0 +1,8 @@ +@addtaghelper "something" + +
+ +

+ +

+
\ No newline at end of file