From d83061b1df195caeb5eb515ff9028d37068c5350 Mon Sep 17 00:00:00 2001 From: Artak Mkrtchyan Date: Thu, 8 Feb 2018 15:50:38 -0800 Subject: [PATCH 01/34] Added a unit test to repro the issue --- .../Legacy/TagHelperParseTreeRewriter.cs | 2 +- .../Legacy/TagHelperParseTreeRewriterTest.cs | 49 ++++++++++++++++--- 2 files changed, 44 insertions(+), 7 deletions(-) diff --git a/src/Microsoft.AspNetCore.Razor.Language/Legacy/TagHelperParseTreeRewriter.cs b/src/Microsoft.AspNetCore.Razor.Language/Legacy/TagHelperParseTreeRewriter.cs index 2059522263..79b013f32d 100644 --- a/src/Microsoft.AspNetCore.Razor.Language/Legacy/TagHelperParseTreeRewriter.cs +++ b/src/Microsoft.AspNetCore.Razor.Language/Legacy/TagHelperParseTreeRewriter.cs @@ -488,7 +488,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy private void ValidateParentAllowsContent(Span child, ErrorSink errorSink) { - if (HasAllowedChildren()) + if (child.Kind == SpanKindInternal.Comment || HasAllowedChildren()) { var content = child.Content; if (!string.IsNullOrWhiteSpace(content)) diff --git a/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/TagHelperParseTreeRewriterTest.cs b/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/TagHelperParseTreeRewriterTest.cs index 2feccbe1a4..04892592dd 100644 --- a/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/TagHelperParseTreeRewriterTest.cs +++ b/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/TagHelperParseTreeRewriterTest.cs @@ -254,7 +254,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy var descriptors = new TagHelperDescriptor[] { TagHelperDescriptorBuilder.Create("InputTagHelper", "SomeAssembly") - .TagMatchingRuleDescriptor(rule => + .TagMatchingRuleDescriptor(rule => rule .RequireTagName("input") .RequireTagStructure(TagStructure.WithoutEndTag)) @@ -371,7 +371,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy .TagMatchingRuleDescriptor(rule => rule.RequireTagName("strong")) .Build(), }; - + // Act & Assert EvaluateData( descriptors, @@ -793,7 +793,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy var descriptors = new TagHelperDescriptor[] { TagHelperDescriptorBuilder.Create("StrongTagHelper", "SomeAssembly") - .TagMatchingRuleDescriptor(rule => + .TagMatchingRuleDescriptor(rule => rule .RequireTagName("strong") .RequireAttributeDescriptor(attribute => attribute.Name("required"))) @@ -830,7 +830,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy .TagMatchingRuleDescriptor(rule => rule.RequireTagName("strong")) .Build(), TagHelperDescriptorBuilder.Create("BRTagHelper", "SomeAssembly") - .TagMatchingRuleDescriptor(rule => + .TagMatchingRuleDescriptor(rule => rule .RequireTagName("br") .RequireTagStructure(TagStructure.WithoutEndTag)) @@ -1108,6 +1108,43 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy EvaluateData(descriptors, documentContent, (MarkupBlock)expectedOutput, (RazorDiagnostic[])expectedErrors); } + [Fact] + public void Rewrite_AllowsCommentsAsChildren() + { + // Arrangestring documentContent, + IEnumerable allowedChildren = new List { "b" }; + string literalPrefix = ""; + string commentOutput = $""; + string expectedOutput = $"

{literalPrefix}{commentOutput}asdf

"; + + var pTagHelperBuilder = TagHelperDescriptorBuilder + .Create("PTagHelper", "SomeAssembly") + .TagMatchingRuleDescriptor(rule => rule.RequireTagName("p")); + foreach (var childTag in allowedChildren) + { + pTagHelperBuilder.AllowChildTag(childTag); + } + + var descriptors = new TagHelperDescriptor[] + { + pTagHelperBuilder.Build() + }; + + var factory = new SpanFactory(); + var blockFactory = new BlockFactory(factory); + + var expectedMarkup = new MarkupBlock( + new MarkupTagHelperBlock("p", + factory.Markup($"{literalPrefix}{commentOutput}"))); + + // Act & Assert + EvaluateData( + descriptors, + expectedOutput, + expectedMarkup, + Array.Empty()); + } + [Fact] public void Rewrite_UnderstandsNullTagNameWithAllowedChildrenForCatchAll() { @@ -1173,7 +1210,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy var descriptors = new TagHelperDescriptor[] { TagHelperDescriptorBuilder.Create("InputTagHelper", "SomeAssembly") - .TagMatchingRuleDescriptor(rule => + .TagMatchingRuleDescriptor(rule => rule .RequireTagName("input") .RequireTagStructure(TagStructure.WithoutEndTag)) @@ -1646,7 +1683,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy var descriptors = new TagHelperDescriptor[] { TagHelperDescriptorBuilder.Create("pTagHelper", "SomeAssembly") - .TagMatchingRuleDescriptor(rule => + .TagMatchingRuleDescriptor(rule => rule .RequireTagName("p") .RequireAttributeDescriptor(attribute => attribute.Name("class"))) From e7076a291ddabd30aa80b98041fb6f00b4f29f4c Mon Sep 17 00:00:00 2001 From: Artak Mkrtchyan Date: Mon, 12 Feb 2018 22:03:53 -0800 Subject: [PATCH 02/34] Ignoring markup comments during validation --- .../Legacy/TagHelperParseTreeRewriter.cs | 7 ++++++- .../Legacy/TagHelperParseTreeRewriterTest.cs | 11 +++++++---- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/Microsoft.AspNetCore.Razor.Language/Legacy/TagHelperParseTreeRewriter.cs b/src/Microsoft.AspNetCore.Razor.Language/Legacy/TagHelperParseTreeRewriter.cs index 79b013f32d..73b545dd33 100644 --- a/src/Microsoft.AspNetCore.Razor.Language/Legacy/TagHelperParseTreeRewriter.cs +++ b/src/Microsoft.AspNetCore.Razor.Language/Legacy/TagHelperParseTreeRewriter.cs @@ -116,7 +116,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy continue; } } - else + else if (!IsCommentTag((Span)child)) { ValidateParentAllowsContent((Span)child, errorSink); } @@ -817,6 +817,11 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy return relevantSymbol.Type == HtmlSymbolType.ForwardSlash; } + private static bool IsCommentTag(Span span) + { + return span.Content.StartsWith(""; - string expectedOutput = $"

{literalPrefix}{commentOutput}asdf

"; + string expectedOutput = $"

{literal}{commentOutput}

"; var pTagHelperBuilder = TagHelperDescriptorBuilder .Create("PTagHelper", "SomeAssembly") @@ -1127,7 +1127,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy var descriptors = new TagHelperDescriptor[] { - pTagHelperBuilder.Build() + pTagHelperBuilder.Build() }; var factory = new SpanFactory(); @@ -1135,7 +1135,10 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy var expectedMarkup = new MarkupBlock( new MarkupTagHelperBlock("p", - factory.Markup($"{literalPrefix}{commentOutput}"))); + blockFactory.MarkupTagBlock(""), + factory.Markup(literal), + blockFactory.MarkupTagBlock(""), + factory.Markup(commentOutput))); // Act & Assert EvaluateData( From 900c927d72e29f7f92ff3f0f1d4f6cbd06e904ee Mon Sep 17 00:00:00 2001 From: Artak Mkrtchyan Date: Mon, 12 Feb 2018 22:14:01 -0800 Subject: [PATCH 03/34] Revert unnecessary change --- .../Legacy/TagHelperParseTreeRewriter.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.AspNetCore.Razor.Language/Legacy/TagHelperParseTreeRewriter.cs b/src/Microsoft.AspNetCore.Razor.Language/Legacy/TagHelperParseTreeRewriter.cs index 73b545dd33..f046a8eea4 100644 --- a/src/Microsoft.AspNetCore.Razor.Language/Legacy/TagHelperParseTreeRewriter.cs +++ b/src/Microsoft.AspNetCore.Razor.Language/Legacy/TagHelperParseTreeRewriter.cs @@ -488,7 +488,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy private void ValidateParentAllowsContent(Span child, ErrorSink errorSink) { - if (child.Kind == SpanKindInternal.Comment || HasAllowedChildren()) + if (HasAllowedChildren()) { var content = child.Content; if (!string.IsNullOrWhiteSpace(content)) From 7d8d56cc9b2148197ac8e470d70a5972a65c8790 Mon Sep 17 00:00:00 2001 From: Artak Mkrtchyan Date: Tue, 13 Feb 2018 13:34:16 -0800 Subject: [PATCH 04/34] Ignoring razor comments during validation --- .../Legacy/TagHelperParseTreeRewriter.cs | 9 ++-- .../Legacy/TagHelperParseTreeRewriterTest.cs | 47 ++++++++++++++++++- 2 files changed, 52 insertions(+), 4 deletions(-) diff --git a/src/Microsoft.AspNetCore.Razor.Language/Legacy/TagHelperParseTreeRewriter.cs b/src/Microsoft.AspNetCore.Razor.Language/Legacy/TagHelperParseTreeRewriter.cs index f046a8eea4..01361a9893 100644 --- a/src/Microsoft.AspNetCore.Razor.Language/Legacy/TagHelperParseTreeRewriter.cs +++ b/src/Microsoft.AspNetCore.Razor.Language/Legacy/TagHelperParseTreeRewriter.cs @@ -116,7 +116,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy continue; } } - else if (!IsCommentTag((Span)child)) + else if (!IsComment((Span)child)) { ValidateParentAllowsContent((Span)child, errorSink); } @@ -817,9 +817,12 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy return relevantSymbol.Type == HtmlSymbolType.ForwardSlash; } - private static bool IsCommentTag(Span span) + private static bool IsComment(Span span) { - return span.Content.StartsWith(""; + string commentOutput = ""; string expectedOutput = $"

{literal}{commentOutput}

"; var pTagHelperBuilder = TagHelperDescriptorBuilder @@ -1138,7 +1138,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy blockFactory.MarkupTagBlock(""), factory.Markup(literal), blockFactory.MarkupTagBlock(""), - factory.Markup(commentOutput))); + new HtmlCommentBlock(factory.Markup(commentOutput)))); // Act & Assert EvaluateData( @@ -1148,6 +1148,58 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy Array.Empty()); } + [Fact] + public void Rewrite_FailsForContentWithCommentsAsChildren() + { + // Arrangestring documentContent, + Func nestedTagError = + (childName, parentName, allowed, location, length) => + RazorDiagnosticFactory.CreateTagHelper_InvalidNestedTag( + new SourceSpan(absoluteIndex: location, lineIndex: 0, characterIndex: location, length: length), childName, parentName, allowed); + Func nestedContentError = + (parentName, allowed, location, length) => + RazorDiagnosticFactory.CreateTagHelper_CannotHaveNonTagContent( + new SourceSpan(absoluteIndex: location, lineIndex: 0, characterIndex: location, length: length), parentName, allowed); + + IEnumerable allowedChildren = new List { "b" }; + string comment1 = "Hello"; + string literal = "asdf"; + string comment2 = "World"; + string expectedOutput = $"

{literal}

"; + + var pTagHelperBuilder = TagHelperDescriptorBuilder + .Create("PTagHelper", "SomeAssembly") + .TagMatchingRuleDescriptor(rule => rule.RequireTagName("p")); + foreach (var childTag in allowedChildren) + { + pTagHelperBuilder.AllowChildTag(childTag); + } + + var descriptors = new TagHelperDescriptor[] + { + pTagHelperBuilder.Build() + }; + + var factory = new SpanFactory(); + var blockFactory = new BlockFactory(factory); + + var expectedMarkup = new MarkupBlock( + new MarkupTagHelperBlock("p", + new HtmlCommentBlock(factory.Markup($"")), + factory.Markup(literal), + new HtmlCommentBlock(factory.Markup($"")))); + + // Act & Assert + EvaluateData( + descriptors, + expectedOutput, + expectedMarkup, + new[] + { + nestedContentError("p", "b", 15, 4), + }); + } + [Fact] public void Rewrite_AllowsRazorCommentsAsChildren() { @@ -1193,6 +1245,54 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy Array.Empty()); } + [Fact] + public void Rewrite_AllowsRazorMarkupInHtmlComment() + { + // Arrangestring documentContent, + IEnumerable allowedChildren = new List { "b" }; + string literal = "asdf"; + string part1 = ""; + string expectedOutput = $"

{literal}{part1}@{part2}{part3}

"; + + var pTagHelperBuilder = TagHelperDescriptorBuilder + .Create("PTagHelper", "SomeAssembly") + .TagMatchingRuleDescriptor(rule => rule.RequireTagName("p")); + foreach (var childTag in allowedChildren) + { + pTagHelperBuilder.AllowChildTag(childTag); + } + + var descriptors = new TagHelperDescriptor[] + { + pTagHelperBuilder.Build() + }; + + var factory = new SpanFactory(); + var blockFactory = new BlockFactory(factory); + + var expectedMarkup = new MarkupBlock( + new MarkupTagHelperBlock("p", + blockFactory.MarkupTagBlock(""), + factory.Markup(literal), + blockFactory.MarkupTagBlock(""), + new HtmlCommentBlock(factory.Markup(part1), + new ExpressionBlock( + factory.CodeTransition(), + factory.Code(part2) + .AsImplicitExpression(CSharpCodeParser.DefaultKeywords) + .Accepts(AcceptedCharactersInternal.NonWhiteSpace)), + factory.Markup(part3)))); + + // Act & Assert + EvaluateData( + descriptors, + expectedOutput, + expectedMarkup, + Array.Empty()); + } + [Fact] public void Rewrite_UnderstandsNullTagNameWithAllowedChildrenForCatchAll() { @@ -3993,7 +4093,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy new MarkupBlock( new MarkupTagBlock( factory.Markup("")), - factory.Markup(""), + new HtmlCommentBlock( factory.Markup("")), new MarkupTagBlock( factory.Markup(""))) }; @@ -4003,13 +4103,13 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy new MarkupBlock( new MarkupTagBlock( factory.Markup("")), - factory.Markup(""), + new HtmlCommentBlock(factory.Markup("")), new MarkupTagBlock( factory.Markup(""))) }; diff --git a/test/Microsoft.AspNetCore.Razor.Test.Common/Language/Legacy/BlockTypes.cs b/test/Microsoft.AspNetCore.Razor.Test.Common/Language/Legacy/BlockTypes.cs index c08e88d56f..56deb6b460 100644 --- a/test/Microsoft.AspNetCore.Razor.Test.Common/Language/Legacy/BlockTypes.cs +++ b/test/Microsoft.AspNetCore.Razor.Test.Common/Language/Legacy/BlockTypes.cs @@ -217,4 +217,14 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy { } } + + internal class HtmlCommentBlock : Block + { + private const BlockKindInternal ThisBlockKind = BlockKindInternal.HtmlComment; + + public HtmlCommentBlock(params SyntaxTreeNode[] children) + : base(ThisBlockKind, children, ParentChunkGenerator.Null) + { + } + } } From f42754371eeefedfab9ec2ca2f2192ae3854bc32 Mon Sep 17 00:00:00 2001 From: Artak Mkrtchyan Date: Tue, 20 Feb 2018 18:24:27 -0800 Subject: [PATCH 07/34] HtmlMarkupParser is now aware of HtmlComment blocks. Majority (if not all, will confirm soon) tests have been updated to account for HtmlComment blocks. --- .../Legacy/HtmlMarkupParser.cs | 49 ++++++++++++++----- .../Legacy/CSharpSectionTest.cs | 11 +++-- .../Legacy/HtmlBlockTest.cs | 33 +++++++------ .../Legacy/TagHelperParseTreeRewriterTest.cs | 48 +++++++++--------- .../TagHelperParseTreeRewriterTests.cs | 26 ++++++++++ ...lCommentWithQuote_Double_DesignTime.ir.txt | 3 +- ...HtmlCommentWithQuote_Double_Runtime.ir.txt | 3 +- ...lCommentWithQuote_Single_DesignTime.ir.txt | 3 +- ...HtmlCommentWithQuote_Single_Runtime.ir.txt | 3 +- .../Language/Legacy/BlockFactory.cs | 21 ++++++++ 10 files changed, 144 insertions(+), 56 deletions(-) create mode 100644 test/Microsoft.AspNetCore.Razor.Language.Test/TagHelperParseTreeRewriterTests.cs diff --git a/src/Microsoft.AspNetCore.Razor.Language/Legacy/HtmlMarkupParser.cs b/src/Microsoft.AspNetCore.Razor.Language/Legacy/HtmlMarkupParser.cs index 332023b775..fc675b6e47 100644 --- a/src/Microsoft.AspNetCore.Razor.Language/Legacy/HtmlMarkupParser.cs +++ b/src/Microsoft.AspNetCore.Razor.Language/Legacy/HtmlMarkupParser.cs @@ -207,10 +207,6 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy if (last != null) { Accept(last); - if (At(HtmlSymbolType.OpenAngle) && last.Type == HtmlSymbolType.Text) - { - Output(SpanKindInternal.Markup); - } } } @@ -501,31 +497,57 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy using (Context.Builder.StartBlock(BlockKindInternal.HtmlComment)) { AcceptAndMoveNext(); + Output(SpanKindInternal.Markup, AcceptedCharactersInternal.None); - Span.EditHandler.AcceptedCharacters = AcceptedCharactersInternal.Any; + Span.EditHandler.AcceptedCharacters = AcceptedCharactersInternal.WhiteSpace; while (!EndOfFile) { SkipToAndParseCode(HtmlSymbolType.DoubleHyphen); if (At(HtmlSymbolType.DoubleHyphen)) { - AcceptWhile(HtmlSymbolType.DoubleHyphen); + var lastDoubleHyphen = CurrentSymbol; + AcceptWhile(s => + { + if (NextIs(HtmlSymbolType.DoubleHyphen)) + { + lastDoubleHyphen = s; + return true; + } + + NextToken(); + EnsureCurrent(); + return false; + }); if (At(HtmlSymbolType.Text) && string.Equals(CurrentSymbol.Content, "-", StringComparison.Ordinal)) { + // Doing this here to maintain the order of symbols + if (!NextIs(HtmlSymbolType.CloseAngle)) + { + Accept(lastDoubleHyphen); + lastDoubleHyphen = null; + } + AcceptAndMoveNext(); } if (At(HtmlSymbolType.CloseAngle)) { - // This is the end of a comment block - Accept(this.CurrentSymbol); - Output(SpanKindInternal.Markup); + // Output the content in the comment block as a separate markup + Output(SpanKindInternal.Markup, AcceptedCharactersInternal.WhiteSpace); + + // This is the end of a comment block + Accept(lastDoubleHyphen); + AcceptAndMoveNext(); + Output(SpanKindInternal.Markup, AcceptedCharactersInternal.None); - NextToken(); - //AcceptAndMoveNext(); return true; } + else if (lastDoubleHyphen != null) + { + Accept(lastDoubleHyphen); + } } } } @@ -1486,6 +1508,11 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy // Checking to see if we meet the conditions of a special '!' tag: ")), + BlockFactory.HtmlCommentBlock(" "), + Factory.EmptyHtml()), Factory.MetaCode("}").Accepts(AcceptedCharactersInternal.None)), Factory.EmptyHtml())); } @@ -630,7 +631,8 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy Factory.Span(SpanKindInternal.Markup, " ", CSharpSymbolType.WhiteSpace).Accepts(AcceptedCharactersInternal.AllWhiteSpace), Factory.MetaCode("{").AutoCompleteWith(null, atEndOfSpan: true).Accepts(AcceptedCharactersInternal.None), new MarkupBlock( - Factory.Markup("")), + BlockFactory.HtmlCommentBlock(" > \" '"), + Factory.EmptyHtml()), Factory.MetaCode("}").Accepts(AcceptedCharactersInternal.None)), Factory.EmptyHtml())); } @@ -655,7 +657,8 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy Factory.Markup(Environment.NewLine), new MarkupTagBlock( Factory.Markup(" \" '-->")), + BlockFactory.HtmlCommentBlock(" > \" '"), + Factory.EmptyHtml()), Factory.MetaCode("}").Accepts(AcceptedCharactersInternal.None)), Factory.EmptyHtml())); } diff --git a/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/HtmlBlockTest.cs b/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/HtmlBlockTest.cs index f54ba50d37..f8d4b0fcd9 100644 --- a/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/HtmlBlockTest.cs +++ b/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/HtmlBlockTest.cs @@ -23,7 +23,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy Factory.Code(Environment.NewLine).AsStatement().AutoCompleteWith(null), new MarkupBlock( Factory.Markup(" "), - Factory.Markup("").Accepts(AcceptedCharactersInternal.None), + BlockFactory.HtmlCommentBlock(" Hello, I'm a comment that shouldn't break razor -"), Factory.Markup(Environment.NewLine).Accepts(AcceptedCharactersInternal.None)), Factory.EmptyCSharp().AsStatement(), Factory.MetaCode("}").Accepts(AcceptedCharactersInternal.None)), @@ -333,7 +333,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy [Fact] public void ParseBlockSupportsCommentAsBlock() { - SingleSpanBlockTest("", BlockKindInternal.Markup, SpanKindInternal.Markup, acceptedCharacters: AcceptedCharactersInternal.None); + ParseBlockTest("", new MarkupBlock( BlockFactory.HtmlCommentBlock(" foo "))); } [Fact] @@ -344,8 +344,8 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy new MarkupTagBlock( Factory.Markup("").Accepts(AcceptedCharactersInternal.None)), Factory.Markup("bar"), - Factory.Markup("").Accepts(AcceptedCharactersInternal.None), - Factory.Markup("baz"), + BlockFactory.HtmlCommentBlock(" zoop "), + Factory.Markup("baz").Accepts(AcceptedCharactersInternal.None), new MarkupTagBlock( Factory.Markup("").Accepts(AcceptedCharactersInternal.None)))); } @@ -355,7 +355,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy get { var factory = new SpanFactory(); - + var blockFactory = new BlockFactory(factory); return new TheoryData { { @@ -363,7 +363,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy new MarkupBlock( new MarkupTagBlock( factory.Markup("
").Accepts(AcceptedCharactersInternal.None)), - factory.Markup("").Accepts(AcceptedCharactersInternal.None), + blockFactory.HtmlCommentBlock("- Hello World -"), new MarkupTagBlock( factory.Markup("
").Accepts(AcceptedCharactersInternal.None))) }, @@ -372,7 +372,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy new MarkupBlock( new MarkupTagBlock( factory.Markup("
").Accepts(AcceptedCharactersInternal.None)), - factory.Markup("").Accepts(AcceptedCharactersInternal.None), + blockFactory.HtmlCommentBlock("-- Hello World --"), new MarkupTagBlock( factory.Markup("
").Accepts(AcceptedCharactersInternal.None))) }, @@ -381,7 +381,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy new MarkupBlock( new MarkupTagBlock( factory.Markup("
").Accepts(AcceptedCharactersInternal.None)), - factory.Markup("").Accepts(AcceptedCharactersInternal.None), + blockFactory.HtmlCommentBlock("--- Hello World ---"), new MarkupTagBlock( factory.Markup("
").Accepts(AcceptedCharactersInternal.None))) }, @@ -390,7 +390,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy new MarkupBlock( new MarkupTagBlock( factory.Markup("
").Accepts(AcceptedCharactersInternal.None)), - factory.Markup("").Accepts(AcceptedCharactersInternal.None), + blockFactory.HtmlCommentBlock("--- Hello < --- > World
---"), new MarkupTagBlock( factory.Markup("").Accepts(AcceptedCharactersInternal.None))) }, @@ -410,19 +410,24 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy [Fact] public void ParseBlockProperlyBalancesCommentStartAndEndTags() { - SingleSpanBlockTest("", BlockKindInternal.Markup, SpanKindInternal.Markup, acceptedCharacters: AcceptedCharactersInternal.None); + ParseBlockTest("", new MarkupBlock(BlockFactory.HtmlCommentBlock(""))); } [Fact] public void ParseBlockTerminatesAtEOFWhenParsingComment() { - SingleSpanBlockTest("", BlockKindInternal.Markup, SpanKindInternal.Markup, acceptedCharacters: AcceptedCharactersInternal.None); + ParseBlockTest("", new MarkupBlock(BlockFactory.HtmlCommentBlock("--"))); } [Fact] @@ -432,8 +437,8 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy new MarkupBlock( new MarkupTagBlock( Factory.Markup("").Accepts(AcceptedCharactersInternal.None)), - Factory.Markup("").Accepts(AcceptedCharactersInternal.None), - Factory.Markup("-->"), + BlockFactory.HtmlCommentBlock("").Accepts(AcceptedCharactersInternal.None), new MarkupTagBlock( Factory.Markup("").Accepts(AcceptedCharactersInternal.None)))); } diff --git a/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/TagHelperParseTreeRewriterTest.cs b/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/TagHelperParseTreeRewriterTest.cs index 45a1ffa907..ceca9e86f2 100644 --- a/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/TagHelperParseTreeRewriterTest.cs +++ b/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/TagHelperParseTreeRewriterTest.cs @@ -1114,8 +1114,8 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy // Arrangestring documentContent, IEnumerable allowedChildren = new List { "b" }; string literal = "asdf"; - string commentOutput = ""; - string expectedOutput = $"

{literal}{commentOutput}

"; + string commentOutput = "Hello World"; + string expectedOutput = $"

{literal}

"; var pTagHelperBuilder = TagHelperDescriptorBuilder .Create("PTagHelper", "SomeAssembly") @@ -1138,7 +1138,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy blockFactory.MarkupTagBlock(""), factory.Markup(literal), blockFactory.MarkupTagBlock(""), - new HtmlCommentBlock(factory.Markup(commentOutput)))); + blockFactory.HtmlCommentBlock(commentOutput))); // Act & Assert EvaluateData( @@ -1185,9 +1185,9 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy var expectedMarkup = new MarkupBlock( new MarkupTagHelperBlock("p", - new HtmlCommentBlock(factory.Markup($"")), + blockFactory.HtmlCommentBlock(comment1), factory.Markup(literal), - new HtmlCommentBlock(factory.Markup($"")))); + blockFactory.HtmlCommentBlock(comment2))); // Act & Assert EvaluateData( @@ -1251,10 +1251,11 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy // Arrangestring documentContent, IEnumerable allowedChildren = new List { "b" }; string literal = "asdf"; - string part1 = ""; - string expectedOutput = $"

{literal}{part1}@{part2}{part3}

"; + string commentStart = ""; + string expectedOutput = $"

{literal}{commentStart}{part1}@{part2}{commentEnd}

"; var pTagHelperBuilder = TagHelperDescriptorBuilder .Create("PTagHelper", "SomeAssembly") @@ -1277,13 +1278,13 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy blockFactory.MarkupTagBlock(""), factory.Markup(literal), blockFactory.MarkupTagBlock(""), - new HtmlCommentBlock(factory.Markup(part1), - new ExpressionBlock( - factory.CodeTransition(), - factory.Code(part2) - .AsImplicitExpression(CSharpCodeParser.DefaultKeywords) - .Accepts(AcceptedCharactersInternal.NonWhiteSpace)), - factory.Markup(part3)))); + blockFactory.HtmlCommentBlock( + factory.Markup(part1).Accepts(AcceptedCharactersInternal.WhiteSpace), + new ExpressionBlock( + factory.CodeTransition(), + factory.Code(part2) + .AsImplicitExpression(CSharpCodeParser.DefaultKeywords) + .Accepts(AcceptedCharactersInternal.NonWhiteSpace))))); // Act & Assert EvaluateData( @@ -4086,14 +4087,14 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy get { var factory = new SpanFactory(); - + var blockFactory = new BlockFactory(factory); yield return new object[] { "", new MarkupBlock( new MarkupTagBlock( factory.Markup("")), - new HtmlCommentBlock( factory.Markup("")), + blockFactory.HtmlCommentBlock (" Hello World "), new MarkupTagBlock( factory.Markup(""))) }; @@ -4103,13 +4104,14 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy new MarkupBlock( new MarkupTagBlock( factory.Markup("")), - new HtmlCommentBlock(factory.Markup("")), + .AsImplicitExpression(CSharpCodeParser.DefaultKeywords) + .Accepts(AcceptedCharactersInternal.NonWhiteSpace)), + factory.Markup(" ").Accepts(AcceptedCharactersInternal.WhiteSpace)), new MarkupTagBlock( factory.Markup(""))) }; @@ -4185,8 +4187,8 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy new ExpressionBlock( factory.CodeTransition(), factory.Code("foo") - .AsImplicitExpression(CSharpCodeParser.DefaultKeywords) - .Accepts(AcceptedCharactersInternal.NonWhiteSpace)), + .AsImplicitExpression(CSharpCodeParser.DefaultKeywords) + .Accepts(AcceptedCharactersInternal.NonWhiteSpace)), factory.Markup(" ]]>"), new MarkupTagBlock( factory.Markup("
"))) diff --git a/test/Microsoft.AspNetCore.Razor.Language.Test/TagHelperParseTreeRewriterTests.cs b/test/Microsoft.AspNetCore.Razor.Language.Test/TagHelperParseTreeRewriterTests.cs new file mode 100644 index 0000000000..a24a9c8d4f --- /dev/null +++ b/test/Microsoft.AspNetCore.Razor.Language.Test/TagHelperParseTreeRewriterTests.cs @@ -0,0 +1,26 @@ +using System; +using System.Collections.Generic; +using System.Text; +using Microsoft.AspNetCore.Razor.Language.Legacy; +using Xunit; + +namespace Microsoft.AspNetCore.Razor.Language.Test +{ + public class TagHelperParseTreeRewriterTests + { + public void IsComment_ReturnsTrueForSpanInHtmlCommentBlock() + { + // Arrange + SpanFactory spanFactory = new SpanFactory(); + + Span content = spanFactory.Markup(""); + Block commentBlock = new HtmlCommentBlock(content); + + // Act + bool actualResult = TagHelperParseTreeRewriter.IsComment(content); + + // Assert + Assert.True(actualResult); + } + } +} diff --git a/test/Microsoft.AspNetCore.Razor.Language.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/HtmlCommentWithQuote_Double_DesignTime.ir.txt b/test/Microsoft.AspNetCore.Razor.Language.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/HtmlCommentWithQuote_Double_DesignTime.ir.txt index aa6a00864d..615f0fce97 100644 --- a/test/Microsoft.AspNetCore.Razor.Language.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/HtmlCommentWithQuote_Double_DesignTime.ir.txt +++ b/test/Microsoft.AspNetCore.Razor.Language.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/HtmlCommentWithQuote_Double_DesignTime.ir.txt @@ -10,7 +10,8 @@ Document - IntermediateToken - - CSharp - #pragma warning restore 0414 MethodDeclaration - - public async - System.Threading.Tasks.Task - ExecuteAsync HtmlContent - (0:0,0 [45] HtmlCommentWithQuote_Double.cshtml) - IntermediateToken - (0:0,0 [12] HtmlCommentWithQuote_Double.cshtml) - Html - \n + IntermediateToken - (0:0,0 [10] HtmlCommentWithQuote_Double.cshtml) - Html - + IntermediateToken - (10:0,10 [2] HtmlCommentWithQuote_Double.cshtml) - Html - \n IntermediateToken - (12:1,0 [4] HtmlCommentWithQuote_Double.cshtml) - Html - diff --git a/test/Microsoft.AspNetCore.Razor.Language.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/HtmlCommentWithQuote_Double_Runtime.ir.txt b/test/Microsoft.AspNetCore.Razor.Language.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/HtmlCommentWithQuote_Double_Runtime.ir.txt index 343985c189..7adfc1650b 100644 --- a/test/Microsoft.AspNetCore.Razor.Language.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/HtmlCommentWithQuote_Double_Runtime.ir.txt +++ b/test/Microsoft.AspNetCore.Razor.Language.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/HtmlCommentWithQuote_Double_Runtime.ir.txt @@ -5,7 +5,8 @@ Document - ClassDeclaration - - public - TestFiles_IntegrationTests_CodeGenerationIntegrationTest_HtmlCommentWithQuote_Double_Runtime - - MethodDeclaration - - public async - System.Threading.Tasks.Task - ExecuteAsync HtmlContent - (0:0,0 [45] HtmlCommentWithQuote_Double.cshtml) - IntermediateToken - (0:0,0 [12] HtmlCommentWithQuote_Double.cshtml) - Html - \n + IntermediateToken - (0:0,0 [10] HtmlCommentWithQuote_Double.cshtml) - Html - + IntermediateToken - (10:0,10 [2] HtmlCommentWithQuote_Double.cshtml) - Html - \n IntermediateToken - (12:1,0 [4] HtmlCommentWithQuote_Double.cshtml) - Html - diff --git a/test/Microsoft.AspNetCore.Razor.Language.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/HtmlCommentWithQuote_Single_DesignTime.ir.txt b/test/Microsoft.AspNetCore.Razor.Language.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/HtmlCommentWithQuote_Single_DesignTime.ir.txt index 04795676d1..0e342fcf51 100644 --- a/test/Microsoft.AspNetCore.Razor.Language.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/HtmlCommentWithQuote_Single_DesignTime.ir.txt +++ b/test/Microsoft.AspNetCore.Razor.Language.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/HtmlCommentWithQuote_Single_DesignTime.ir.txt @@ -10,7 +10,8 @@ Document - IntermediateToken - - CSharp - #pragma warning restore 0414 MethodDeclaration - - public async - System.Threading.Tasks.Task - ExecuteAsync HtmlContent - (0:0,0 [45] HtmlCommentWithQuote_Single.cshtml) - IntermediateToken - (0:0,0 [12] HtmlCommentWithQuote_Single.cshtml) - Html - \n + IntermediateToken - (0:0,0 [10] HtmlCommentWithQuote_Single.cshtml) - Html - + IntermediateToken - (10:0,10 [2] HtmlCommentWithQuote_Single.cshtml) - Html - \n IntermediateToken - (12:1,0 [4] HtmlCommentWithQuote_Single.cshtml) - Html - diff --git a/test/Microsoft.AspNetCore.Razor.Language.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/HtmlCommentWithQuote_Single_Runtime.ir.txt b/test/Microsoft.AspNetCore.Razor.Language.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/HtmlCommentWithQuote_Single_Runtime.ir.txt index 65aafb6fe7..137c2a042c 100644 --- a/test/Microsoft.AspNetCore.Razor.Language.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/HtmlCommentWithQuote_Single_Runtime.ir.txt +++ b/test/Microsoft.AspNetCore.Razor.Language.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/HtmlCommentWithQuote_Single_Runtime.ir.txt @@ -5,7 +5,8 @@ Document - ClassDeclaration - - public - TestFiles_IntegrationTests_CodeGenerationIntegrationTest_HtmlCommentWithQuote_Single_Runtime - - MethodDeclaration - - public async - System.Threading.Tasks.Task - ExecuteAsync HtmlContent - (0:0,0 [45] HtmlCommentWithQuote_Single.cshtml) - IntermediateToken - (0:0,0 [12] HtmlCommentWithQuote_Single.cshtml) - Html - \n + IntermediateToken - (0:0,0 [10] HtmlCommentWithQuote_Single.cshtml) - Html - + IntermediateToken - (10:0,10 [2] HtmlCommentWithQuote_Single.cshtml) - Html - \n IntermediateToken - (12:1,0 [4] HtmlCommentWithQuote_Single.cshtml) - Html - diff --git a/test/Microsoft.AspNetCore.Razor.Test.Common/Language/Legacy/BlockFactory.cs b/test/Microsoft.AspNetCore.Razor.Test.Common/Language/Legacy/BlockFactory.cs index 5c2ee95394..a58d3b0c87 100644 --- a/test/Microsoft.AspNetCore.Razor.Test.Common/Language/Legacy/BlockFactory.cs +++ b/test/Microsoft.AspNetCore.Razor.Test.Common/Language/Legacy/BlockFactory.cs @@ -55,6 +55,27 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy ); } + public HtmlCommentBlock HtmlCommentBlock(string content) + { + return new HtmlCommentBlock(new SyntaxTreeNode[] { + _factory.Markup("").Accepts(AcceptedCharactersInternal.None) }); + } + + public HtmlCommentBlock HtmlCommentBlock(params SyntaxTreeNode[] syntaxTreeNodes) + { + var nodes = new List(); + nodes.Add(_factory.Markup("").Accepts(AcceptedCharactersInternal.None)); + + return new HtmlCommentBlock(nodes.ToArray()); + } + public Block TagHelperBlock( string tagName, TagMode tagMode, From 2949470a4383fd22f89fb11be5b1c7beb08dab77 Mon Sep 17 00:00:00 2001 From: Artak Mkrtchyan Date: Tue, 20 Feb 2018 22:20:48 -0800 Subject: [PATCH 08/34] Fixed all the tests to reflect HtmlComment block type support --- .../Legacy/HtmlDocumentTest.cs | 8 +++++++- .../Legacy/HtmlTagsTest.cs | 2 +- .../Legacy/HtmlToCodeSwitchTest.cs | 15 +++++++------- .../Legacy/TagHelperParseTreeRewriterTest.cs | 20 +++++++++---------- ...lCommentWithQuote_Double_DesignTime.ir.txt | 4 +++- ...HtmlCommentWithQuote_Double_Runtime.ir.txt | 4 +++- ...lCommentWithQuote_Single_DesignTime.ir.txt | 4 +++- ...HtmlCommentWithQuote_Single_Runtime.ir.txt | 4 +++- .../Language/Legacy/BlockFactory.cs | 16 +++++++-------- 9 files changed, 45 insertions(+), 32 deletions(-) diff --git a/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/HtmlDocumentTest.cs b/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/HtmlDocumentTest.cs index 0f06fe578a..9f0e11218b 100644 --- a/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/HtmlDocumentTest.cs +++ b/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/HtmlDocumentTest.cs @@ -214,7 +214,13 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy [Fact] public void ParseDocumentReturnsOneMarkupSegmentIfNoCodeBlocksEncountered() { - SingleSpanDocumentTest("Foo BazBarBar Bar", new MarkupBlock( - Factory.Markup("").Accepts(AcceptedCharactersInternal.None), + BlockFactory.HtmlCommentBlock("Foo"), Factory.Markup(" ").Accepts(AcceptedCharactersInternal.None))); } diff --git a/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/HtmlToCodeSwitchTest.cs b/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/HtmlToCodeSwitchTest.cs index 29c1dfb81a..928762fbc6 100644 --- a/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/HtmlToCodeSwitchTest.cs +++ b/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/HtmlToCodeSwitchTest.cs @@ -112,13 +112,14 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy new MarkupBlock( new MarkupTagBlock( Factory.Markup("").Accepts(AcceptedCharactersInternal.None)), - Factory.Markup("").Accepts(AcceptedCharactersInternal.None), + BlockFactory.HtmlCommentBlock(Factory, f => new SyntaxTreeNode[] { + f.Markup(" ").Accepts(AcceptedCharactersInternal.WhiteSpace), + new ExpressionBlock( + f.CodeTransition(), + f.Code("foo") + .AsImplicitExpression(CSharpCodeParser.DefaultKeywords) + .Accepts(AcceptedCharactersInternal.NonWhiteSpace)), + f.Markup(" ").Accepts(AcceptedCharactersInternal.WhiteSpace) }), new MarkupTagBlock( Factory.Markup("").Accepts(AcceptedCharactersInternal.None)))); } diff --git a/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/TagHelperParseTreeRewriterTest.cs b/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/TagHelperParseTreeRewriterTest.cs index ceca9e86f2..425b728716 100644 --- a/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/TagHelperParseTreeRewriterTest.cs +++ b/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/TagHelperParseTreeRewriterTest.cs @@ -1278,13 +1278,13 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy blockFactory.MarkupTagBlock(""), factory.Markup(literal), blockFactory.MarkupTagBlock(""), - blockFactory.HtmlCommentBlock( - factory.Markup(part1).Accepts(AcceptedCharactersInternal.WhiteSpace), + BlockFactory.HtmlCommentBlock(factory, f => new SyntaxTreeNode[] { + f.Markup(part1).Accepts(AcceptedCharactersInternal.WhiteSpace), new ExpressionBlock( - factory.CodeTransition(), - factory.Code(part2) + f.CodeTransition(), + f.Code(part2) .AsImplicitExpression(CSharpCodeParser.DefaultKeywords) - .Accepts(AcceptedCharactersInternal.NonWhiteSpace))))); + .Accepts(AcceptedCharactersInternal.NonWhiteSpace)) }))); // Act & Assert EvaluateData( @@ -4104,14 +4104,14 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy new MarkupBlock( new MarkupTagBlock( factory.Markup("")), - blockFactory.HtmlCommentBlock( - factory.Markup(" ").Accepts(AcceptedCharactersInternal.WhiteSpace), + BlockFactory.HtmlCommentBlock(factory, f=> new SyntaxTreeNode[]{ + f.Markup(" ").Accepts(AcceptedCharactersInternal.WhiteSpace), new ExpressionBlock( - factory.CodeTransition(), - factory.Code("foo") + f.CodeTransition(), + f.Code("foo") .AsImplicitExpression(CSharpCodeParser.DefaultKeywords) .Accepts(AcceptedCharactersInternal.NonWhiteSpace)), - factory.Markup(" ").Accepts(AcceptedCharactersInternal.WhiteSpace)), + factory.Markup(" ").Accepts(AcceptedCharactersInternal.WhiteSpace) }), new MarkupTagBlock( factory.Markup(""))) }; diff --git a/test/Microsoft.AspNetCore.Razor.Language.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/HtmlCommentWithQuote_Double_DesignTime.ir.txt b/test/Microsoft.AspNetCore.Razor.Language.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/HtmlCommentWithQuote_Double_DesignTime.ir.txt index 615f0fce97..4523eae7a3 100644 --- a/test/Microsoft.AspNetCore.Razor.Language.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/HtmlCommentWithQuote_Double_DesignTime.ir.txt +++ b/test/Microsoft.AspNetCore.Razor.Language.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/HtmlCommentWithQuote_Double_DesignTime.ir.txt @@ -10,7 +10,9 @@ Document - IntermediateToken - - CSharp - #pragma warning restore 0414 MethodDeclaration - - public async - System.Threading.Tasks.Task - ExecuteAsync HtmlContent - (0:0,0 [45] HtmlCommentWithQuote_Double.cshtml) - IntermediateToken - (0:0,0 [10] HtmlCommentWithQuote_Double.cshtml) - Html - + IntermediateToken - (0:0,0 [4] HtmlCommentWithQuote_Double.cshtml) - Html - IntermediateToken - (10:0,10 [2] HtmlCommentWithQuote_Double.cshtml) - Html - \n IntermediateToken - (12:1,0 [4] HtmlCommentWithQuote_Double.cshtml) - Html - + IntermediateToken - (0:0,0 [4] HtmlCommentWithQuote_Double.cshtml) - Html - IntermediateToken - (10:0,10 [2] HtmlCommentWithQuote_Double.cshtml) - Html - \n IntermediateToken - (12:1,0 [4] HtmlCommentWithQuote_Double.cshtml) - Html - + IntermediateToken - (0:0,0 [4] HtmlCommentWithQuote_Single.cshtml) - Html - IntermediateToken - (10:0,10 [2] HtmlCommentWithQuote_Single.cshtml) - Html - \n IntermediateToken - (12:1,0 [4] HtmlCommentWithQuote_Single.cshtml) - Html - + IntermediateToken - (0:0,0 [4] HtmlCommentWithQuote_Single.cshtml) - Html - IntermediateToken - (10:0,10 [2] HtmlCommentWithQuote_Single.cshtml) - Html - \n IntermediateToken - (12:1,0 [4] HtmlCommentWithQuote_Single.cshtml) - Html - ").Accepts(AcceptedCharactersInternal.None) }); + return HtmlCommentBlock(_factory, f => new SyntaxTreeNode[] { f.Markup(content).Accepts(AcceptedCharactersInternal.WhiteSpace) }); } - public HtmlCommentBlock HtmlCommentBlock(params SyntaxTreeNode[] syntaxTreeNodes) + public static HtmlCommentBlock HtmlCommentBlock(SpanFactory factory, Func> nodesBuilder = null) { var nodes = new List(); - nodes.Add(_factory.Markup("").Accepts(AcceptedCharactersInternal.None)); + nodes.Add(factory.Markup("-->").Accepts(AcceptedCharactersInternal.None)); return new HtmlCommentBlock(nodes.ToArray()); } From 0c8dddcc40e1f0d9856ca380afd263a2d7f0308f Mon Sep 17 00:00:00 2001 From: Artak Mkrtchyan Date: Thu, 22 Feb 2018 17:05:13 -0800 Subject: [PATCH 09/34] Added the AllowHtmlCommentsInTagHelpers feature flag to control behavior of having comments in TagHelpers --- .../Legacy/TagHelperParseTreeRewriter.cs | 8 +++++++- .../RazorParserFeatureFlags.cs | 11 +++++++++-- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/src/Microsoft.AspNetCore.Razor.Language/Legacy/TagHelperParseTreeRewriter.cs b/src/Microsoft.AspNetCore.Razor.Language/Legacy/TagHelperParseTreeRewriter.cs index 6cac8b46ea..a4c9906b1a 100644 --- a/src/Microsoft.AspNetCore.Razor.Language/Legacy/TagHelperParseTreeRewriter.cs +++ b/src/Microsoft.AspNetCore.Razor.Language/Legacy/TagHelperParseTreeRewriter.cs @@ -488,7 +488,13 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy private void ValidateParentAllowsContent(Span child, ErrorSink errorSink) { - if (HasAllowedChildren() && !IsComment(child) && child.Kind != SpanKindInternal.Transition && child.Kind != SpanKindInternal.Code) + var isDisallowedContent = true; + if (_featureFlags.AllowHtmlCommentsInTagHelpers) + { + isDisallowedContent = !IsComment(child) && child.Kind != SpanKindInternal.Transition && child.Kind != SpanKindInternal.Code; + } + + if (HasAllowedChildren() && isDisallowedContent) { var content = child.Content; if (!string.IsNullOrWhiteSpace(content)) diff --git a/src/Microsoft.AspNetCore.Razor.Language/RazorParserFeatureFlags.cs b/src/Microsoft.AspNetCore.Razor.Language/RazorParserFeatureFlags.cs index 0629eb9af8..409718b640 100644 --- a/src/Microsoft.AspNetCore.Razor.Language/RazorParserFeatureFlags.cs +++ b/src/Microsoft.AspNetCore.Razor.Language/RazorParserFeatureFlags.cs @@ -8,26 +8,33 @@ namespace Microsoft.AspNetCore.Razor.Language public static RazorParserFeatureFlags Create(RazorLanguageVersion version) { var allowMinimizedBooleanTagHelperAttributes = false; + var allowHtmlCommentsInTagHelpers = false; if (version.CompareTo(RazorLanguageVersion.Version_2_1) >= 0) { // Added in 2.1 allowMinimizedBooleanTagHelperAttributes = true; + allowHtmlCommentsInTagHelpers = true; } - return new DefaultRazorParserFeatureFlags(allowMinimizedBooleanTagHelperAttributes); + return new DefaultRazorParserFeatureFlags(allowMinimizedBooleanTagHelperAttributes, allowHtmlCommentsInTagHelpers); } public abstract bool AllowMinimizedBooleanTagHelperAttributes { get; } + public abstract bool AllowHtmlCommentsInTagHelpers { get; } + private class DefaultRazorParserFeatureFlags : RazorParserFeatureFlags { - public DefaultRazorParserFeatureFlags(bool allowMinimizedBooleanTagHelperAttributes) + public DefaultRazorParserFeatureFlags(bool allowMinimizedBooleanTagHelperAttributes, bool allowHtmlCommentsInTagHelpers) { AllowMinimizedBooleanTagHelperAttributes = allowMinimizedBooleanTagHelperAttributes; + AllowHtmlCommentsInTagHelpers = allowHtmlCommentsInTagHelpers; } public override bool AllowMinimizedBooleanTagHelperAttributes { get; } + + public override bool AllowHtmlCommentsInTagHelpers { get; } } } } From 1796abcbcde83996b6b0da6ff5fa85f682b29e21 Mon Sep 17 00:00:00 2001 From: Artak Mkrtchyan Date: Thu, 22 Feb 2018 17:07:52 -0800 Subject: [PATCH 10/34] Updated the test TagHelperBlockRewriteTest class --- .../Legacy/TagHelperBlockRewriterTest.cs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/TagHelperBlockRewriterTest.cs b/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/TagHelperBlockRewriterTest.cs index 9b2adc4d07..88c774c93c 100644 --- a/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/TagHelperBlockRewriterTest.cs +++ b/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/TagHelperBlockRewriterTest.cs @@ -2950,7 +2950,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy intType), RazorDiagnosticFactory.CreateParsing_TagHelperIndexerAttributeNameMustIncludeKey( new SourceSpan(7, 0, 7, 11), - "int-prefix-", + "int-prefix-", "input"), } }, @@ -2973,7 +2973,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy stringType), RazorDiagnosticFactory.CreateParsing_TagHelperIndexerAttributeNameMustIncludeKey( new SourceSpan(7, 0, 7, 14), - "string-prefix-", + "string-prefix-", "input"), } }, @@ -3638,7 +3638,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy RazorDiagnosticFactory.CreateTagHelper_EmptyBoundAttribute( new SourceSpan(7, 0, 7, 21), "bound-required-string", - "input", + "input", stringType), } }, @@ -3962,7 +3962,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy .Build(), }; - var featureFlags = new TestRazorParserFeatureFlags(allowMinimizedBooleanTagHelperAttributes: false); + var featureFlags = new TestRazorParserFeatureFlags(allowMinimizedBooleanTagHelperAttributes: false, allowHtmlCommentsInTagHelper: false); var expectedOutput = new MarkupBlock( new MarkupTagHelperBlock( @@ -3994,12 +3994,15 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy private class TestRazorParserFeatureFlags : RazorParserFeatureFlags { - public TestRazorParserFeatureFlags(bool allowMinimizedBooleanTagHelperAttributes) + public TestRazorParserFeatureFlags(bool allowMinimizedBooleanTagHelperAttributes, bool allowHtmlCommentsInTagHelper) { AllowMinimizedBooleanTagHelperAttributes = allowMinimizedBooleanTagHelperAttributes; + AllowHtmlCommentsInTagHelpers = allowHtmlCommentsInTagHelper; } public override bool AllowMinimizedBooleanTagHelperAttributes { get; } + + public override bool AllowHtmlCommentsInTagHelpers { get; } } } } From 9c3adba40ff470c164cfccdcc895733648f8bec2 Mon Sep 17 00:00:00 2001 From: Artak Mkrtchyan Date: Mon, 5 Mar 2018 17:28:23 -0800 Subject: [PATCH 11/34] Updated the Comments detection to comply with HTML 5 specification --- .../Legacy/HtmlMarkupParser.cs | 151 ++++++++++++++---- .../Legacy/TokenizerBackedParser.cs | 47 ++++++ .../Legacy/HtmlBlockTest.cs | 4 +- .../Legacy/HtmlDocumentTest.cs | 3 +- 4 files changed, 170 insertions(+), 35 deletions(-) diff --git a/src/Microsoft.AspNetCore.Razor.Language/Legacy/HtmlMarkupParser.cs b/src/Microsoft.AspNetCore.Razor.Language/Legacy/HtmlMarkupParser.cs index fc675b6e47..2fe84b5a33 100644 --- a/src/Microsoft.AspNetCore.Razor.Language/Legacy/HtmlMarkupParser.cs +++ b/src/Microsoft.AspNetCore.Razor.Language/Legacy/HtmlMarkupParser.cs @@ -492,7 +492,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy if (AcceptAndMoveNext()) { - if (CurrentSymbol.Type == HtmlSymbolType.DoubleHyphen) + if (IsHtmlCommentAhead()) { using (Context.Builder.StartBlock(BlockKindInternal.HtmlComment)) { @@ -505,32 +505,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy SkipToAndParseCode(HtmlSymbolType.DoubleHyphen); if (At(HtmlSymbolType.DoubleHyphen)) { - var lastDoubleHyphen = CurrentSymbol; - AcceptWhile(s => - { - if (NextIs(HtmlSymbolType.DoubleHyphen)) - { - lastDoubleHyphen = s; - return true; - } - - NextToken(); - EnsureCurrent(); - return false; - }); - - if (At(HtmlSymbolType.Text) && - string.Equals(CurrentSymbol.Content, "-", StringComparison.Ordinal)) - { - // Doing this here to maintain the order of symbols - if (!NextIs(HtmlSymbolType.CloseAngle)) - { - Accept(lastDoubleHyphen); - lastDoubleHyphen = null; - } - - AcceptAndMoveNext(); - } + var lastDoubleHyphen = AcceptAllButLastDoubleHypens(); if (At(HtmlSymbolType.CloseAngle)) { @@ -541,7 +516,6 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy Accept(lastDoubleHyphen); AcceptAndMoveNext(); Output(SpanKindInternal.Markup, AcceptedCharactersInternal.None); - return true; } else if (lastDoubleHyphen != null) @@ -551,8 +525,6 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy } } } - - return false; } else if (CurrentSymbol.Type == HtmlSymbolType.LeftBracket) { @@ -571,6 +543,125 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy return false; } + private HtmlSymbol AcceptAllButLastDoubleHypens() + { + var lastDoubleHyphen = CurrentSymbol; + AcceptWhile(s => + { + if (NextIs(HtmlSymbolType.DoubleHyphen)) + { + lastDoubleHyphen = s; + return true; + } + + NextToken(); + EnsureCurrent(); + return false; + }); + + if (At(HtmlSymbolType.Text) && IsDashSymbol(CurrentSymbol)) + { + // Doing this here to maintain the order of symbols + if (!NextIs(HtmlSymbolType.CloseAngle)) + { + Accept(lastDoubleHyphen); + lastDoubleHyphen = null; + } + + AcceptAndMoveNext(); + } + + return lastDoubleHyphen; + } + + private static bool IsDashSymbol(HtmlSymbol symbol) + { + return string.Equals(symbol.Content, "-", StringComparison.Ordinal); + } + + private bool IsHtmlCommentAhead() + { + /* + * From HTML5 Specification, available at http://www.w3.org/TR/html52/syntax.html#comments + * + * Comments must have the following format: + * 1. The string "", or "--!>" + * 2.3 nor end with the string "" + * + * */ + + if (CurrentSymbol.Type != HtmlSymbolType.DoubleHyphen) + { + return false; + } + + // Check condition 2.1 + if (NextIs(HtmlSymbolType.CloseAngle) || NextIs(next => IsDashSymbol(next) && NextIs(HtmlSymbolType.CloseAngle))) + { + return false; + } + + // Check condition 2.2 + bool isValidComment = false; + LookaheadUntil((s, p) => + { + bool breakLookahead = false; + if (s.Type == HtmlSymbolType.DoubleHyphen) + { + if (NextIs(HtmlSymbolType.CloseAngle)) + { + // We're at the end of a comment. check the condition 2.3 to make sure the text ending is allowed. + isValidComment = !EndsWithSymbolsSequence(p, HtmlSymbolType.OpenAngle, HtmlSymbolType.Bang, HtmlSymbolType.DoubleHyphen); + breakLookahead = true; + } + else if (NextIs(ns => IsDashSymbol(ns) && NextIs(HtmlSymbolType.CloseAngle))) + { + // This is also a valid closing comment case, as the dashes lookup is treated with DoubleHyphen symbols first. + isValidComment = true; + breakLookahead = true; + } + else if (NextIs(ns => ns.Type == HtmlSymbolType.Bang && NextIs(HtmlSymbolType.CloseAngle))) + { + isValidComment = false; + breakLookahead = true; + } + } + else if (s.Type == HtmlSymbolType.OpenAngle) + { + if (NextIs(ns => ns.Type == HtmlSymbolType.Bang && NextIs(HtmlSymbolType.DoubleHyphen))) + { + isValidComment = false; + breakLookahead = true; + } + } + + return breakLookahead; + }); + + return isValidComment; + } + + private bool EndsWithSymbolsSequence(IEnumerable symbols, params HtmlSymbolType[] sequenceToMatchWith) + { + int index = sequenceToMatchWith.Length; + foreach (var previousSymbol in symbols) + { + if (index == 0) + { + break; + } + + if (sequenceToMatchWith[--index] != previousSymbol.Type) + return false; + } + + return index == 0; + } + private bool CData() { if (CurrentSymbol.Type == HtmlSymbolType.Text && string.Equals(CurrentSymbol.Content, "cdata", StringComparison.OrdinalIgnoreCase)) diff --git a/src/Microsoft.AspNetCore.Razor.Language/Legacy/TokenizerBackedParser.cs b/src/Microsoft.AspNetCore.Razor.Language/Legacy/TokenizerBackedParser.cs index 6f564eb442..1819407076 100644 --- a/src/Microsoft.AspNetCore.Razor.Language/Legacy/TokenizerBackedParser.cs +++ b/src/Microsoft.AspNetCore.Razor.Language/Legacy/TokenizerBackedParser.cs @@ -109,6 +109,53 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy return symbols[count]; } + /// + /// Looks forward until the specified condition is met. + /// + /// A predicate accepting the symbol being evaluated and the list of symbols which have been looped through. + /// true, if the condition was met. false - if the condition wasn't met and the last symbol has already been processed. + /// The list of previous symbols is passed in the reverse order. So the last processed element will be the first one in the list. + protected bool LookaheadUntil(Func, bool> condition) + { + if (condition == null) + { + throw new ArgumentNullException(nameof(condition)); + } + + bool matchFound = false; + + // We add 1 in order to store the current symbol. + var symbols = new List(); + symbols.Add(CurrentSymbol); + + while (true) + { + if (!NextToken()) + { + break; + } + + symbols.Add(CurrentSymbol); + if (condition(CurrentSymbol, symbols.Reverse())) + { + matchFound = true; + break; + } + } + + // Restore Tokenizer's location to where it was pointing before the look-ahead. + for (var i = symbols.Count - 1; i >= 0; i--) + { + PutBack(symbols[i]); + } + + // The PutBacks above will set CurrentSymbol to null. EnsureCurrent will set our CurrentSymbol to the + // next symbol. + EnsureCurrent(); + + return matchFound; + } + protected internal bool NextToken() { PreviousSymbol = CurrentSymbol; diff --git a/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/HtmlBlockTest.cs b/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/HtmlBlockTest.cs index f8d4b0fcd9..5d3d46a831 100644 --- a/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/HtmlBlockTest.cs +++ b/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/HtmlBlockTest.cs @@ -419,9 +419,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy ParseBlockTest( ""; + string expectedOutput = $"

{literalPrefix}{commentOutput}asdf

"; + + var pTagHelperBuilder = TagHelperDescriptorBuilder + .Create("PTagHelper", "SomeAssembly") + .TagMatchingRuleDescriptor(rule => rule.RequireTagName("p")); + foreach (var childTag in allowedChildren) + { + pTagHelperBuilder.AllowChildTag(childTag); + } + + var descriptors = new TagHelperDescriptor[] + { + pTagHelperBuilder.Build() + }; + + var factory = new SpanFactory(); + var blockFactory = new BlockFactory(factory); + + var expectedMarkup = new MarkupBlock( + new MarkupTagHelperBlock("p", + factory.Markup($"{literalPrefix}{commentOutput}"))); + + // Act & Assert + EvaluateData( + descriptors, + expectedOutput, + expectedMarkup, + Array.Empty()); + } + [Fact] public void Rewrite_UnderstandsNullTagNameWithAllowedChildrenForCatchAll() { @@ -1173,7 +1210,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy var descriptors = new TagHelperDescriptor[] { TagHelperDescriptorBuilder.Create("InputTagHelper", "SomeAssembly") - .TagMatchingRuleDescriptor(rule => + .TagMatchingRuleDescriptor(rule => rule .RequireTagName("input") .RequireTagStructure(TagStructure.WithoutEndTag)) @@ -1646,7 +1683,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy var descriptors = new TagHelperDescriptor[] { TagHelperDescriptorBuilder.Create("pTagHelper", "SomeAssembly") - .TagMatchingRuleDescriptor(rule => + .TagMatchingRuleDescriptor(rule => rule .RequireTagName("p") .RequireAttributeDescriptor(attribute => attribute.Name("class"))) From 41b7d90ea8bf4e633c2e24542e55424a8ab80072 Mon Sep 17 00:00:00 2001 From: Artak Mkrtchyan Date: Mon, 12 Feb 2018 22:03:53 -0800 Subject: [PATCH 16/34] Ignoring markup comments during validation --- .../Legacy/TagHelperParseTreeRewriter.cs | 7 ++++++- .../Legacy/TagHelperParseTreeRewriterTest.cs | 11 +++++++---- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/Microsoft.AspNetCore.Razor.Language/Legacy/TagHelperParseTreeRewriter.cs b/src/Microsoft.AspNetCore.Razor.Language/Legacy/TagHelperParseTreeRewriter.cs index 79b013f32d..73b545dd33 100644 --- a/src/Microsoft.AspNetCore.Razor.Language/Legacy/TagHelperParseTreeRewriter.cs +++ b/src/Microsoft.AspNetCore.Razor.Language/Legacy/TagHelperParseTreeRewriter.cs @@ -116,7 +116,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy continue; } } - else + else if (!IsCommentTag((Span)child)) { ValidateParentAllowsContent((Span)child, errorSink); } @@ -817,6 +817,11 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy return relevantSymbol.Type == HtmlSymbolType.ForwardSlash; } + private static bool IsCommentTag(Span span) + { + return span.Content.StartsWith(""; - string expectedOutput = $"

{literalPrefix}{commentOutput}asdf

"; + string expectedOutput = $"

{literal}{commentOutput}

"; var pTagHelperBuilder = TagHelperDescriptorBuilder .Create("PTagHelper", "SomeAssembly") @@ -1127,7 +1127,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy var descriptors = new TagHelperDescriptor[] { - pTagHelperBuilder.Build() + pTagHelperBuilder.Build() }; var factory = new SpanFactory(); @@ -1135,7 +1135,10 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy var expectedMarkup = new MarkupBlock( new MarkupTagHelperBlock("p", - factory.Markup($"{literalPrefix}{commentOutput}"))); + blockFactory.MarkupTagBlock(""), + factory.Markup(literal), + blockFactory.MarkupTagBlock(""), + factory.Markup(commentOutput))); // Act & Assert EvaluateData( From 47228dd8221dda317cb83b2178a23b3ddd704813 Mon Sep 17 00:00:00 2001 From: Artak Mkrtchyan Date: Mon, 12 Feb 2018 22:14:01 -0800 Subject: [PATCH 17/34] Revert unnecessary change --- .../Legacy/TagHelperParseTreeRewriter.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.AspNetCore.Razor.Language/Legacy/TagHelperParseTreeRewriter.cs b/src/Microsoft.AspNetCore.Razor.Language/Legacy/TagHelperParseTreeRewriter.cs index 73b545dd33..f046a8eea4 100644 --- a/src/Microsoft.AspNetCore.Razor.Language/Legacy/TagHelperParseTreeRewriter.cs +++ b/src/Microsoft.AspNetCore.Razor.Language/Legacy/TagHelperParseTreeRewriter.cs @@ -488,7 +488,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy private void ValidateParentAllowsContent(Span child, ErrorSink errorSink) { - if (child.Kind == SpanKindInternal.Comment || HasAllowedChildren()) + if (HasAllowedChildren()) { var content = child.Content; if (!string.IsNullOrWhiteSpace(content)) From 679acdc5d4aaf7591e5ca34296c553b415dc5caa Mon Sep 17 00:00:00 2001 From: Artak Mkrtchyan Date: Tue, 13 Feb 2018 13:34:16 -0800 Subject: [PATCH 18/34] Ignoring razor comments during validation --- .../Legacy/TagHelperParseTreeRewriter.cs | 9 ++-- .../Legacy/TagHelperParseTreeRewriterTest.cs | 47 ++++++++++++++++++- 2 files changed, 52 insertions(+), 4 deletions(-) diff --git a/src/Microsoft.AspNetCore.Razor.Language/Legacy/TagHelperParseTreeRewriter.cs b/src/Microsoft.AspNetCore.Razor.Language/Legacy/TagHelperParseTreeRewriter.cs index f046a8eea4..01361a9893 100644 --- a/src/Microsoft.AspNetCore.Razor.Language/Legacy/TagHelperParseTreeRewriter.cs +++ b/src/Microsoft.AspNetCore.Razor.Language/Legacy/TagHelperParseTreeRewriter.cs @@ -116,7 +116,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy continue; } } - else if (!IsCommentTag((Span)child)) + else if (!IsComment((Span)child)) { ValidateParentAllowsContent((Span)child, errorSink); } @@ -817,9 +817,12 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy return relevantSymbol.Type == HtmlSymbolType.ForwardSlash; } - private static bool IsCommentTag(Span span) + private static bool IsComment(Span span) { - return span.Content.StartsWith(""; + string commentOutput = ""; string expectedOutput = $"

{literal}{commentOutput}

"; var pTagHelperBuilder = TagHelperDescriptorBuilder @@ -1138,7 +1138,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy blockFactory.MarkupTagBlock(""), factory.Markup(literal), blockFactory.MarkupTagBlock(""), - factory.Markup(commentOutput))); + new HtmlCommentBlock(factory.Markup(commentOutput)))); // Act & Assert EvaluateData( @@ -1148,6 +1148,58 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy Array.Empty()); } + [Fact] + public void Rewrite_FailsForContentWithCommentsAsChildren() + { + // Arrangestring documentContent, + Func nestedTagError = + (childName, parentName, allowed, location, length) => + RazorDiagnosticFactory.CreateTagHelper_InvalidNestedTag( + new SourceSpan(absoluteIndex: location, lineIndex: 0, characterIndex: location, length: length), childName, parentName, allowed); + Func nestedContentError = + (parentName, allowed, location, length) => + RazorDiagnosticFactory.CreateTagHelper_CannotHaveNonTagContent( + new SourceSpan(absoluteIndex: location, lineIndex: 0, characterIndex: location, length: length), parentName, allowed); + + IEnumerable allowedChildren = new List { "b" }; + string comment1 = "Hello"; + string literal = "asdf"; + string comment2 = "World"; + string expectedOutput = $"

{literal}

"; + + var pTagHelperBuilder = TagHelperDescriptorBuilder + .Create("PTagHelper", "SomeAssembly") + .TagMatchingRuleDescriptor(rule => rule.RequireTagName("p")); + foreach (var childTag in allowedChildren) + { + pTagHelperBuilder.AllowChildTag(childTag); + } + + var descriptors = new TagHelperDescriptor[] + { + pTagHelperBuilder.Build() + }; + + var factory = new SpanFactory(); + var blockFactory = new BlockFactory(factory); + + var expectedMarkup = new MarkupBlock( + new MarkupTagHelperBlock("p", + new HtmlCommentBlock(factory.Markup($"")), + factory.Markup(literal), + new HtmlCommentBlock(factory.Markup($"")))); + + // Act & Assert + EvaluateData( + descriptors, + expectedOutput, + expectedMarkup, + new[] + { + nestedContentError("p", "b", 15, 4), + }); + } + [Fact] public void Rewrite_AllowsRazorCommentsAsChildren() { @@ -1193,6 +1245,54 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy Array.Empty()); } + [Fact] + public void Rewrite_AllowsRazorMarkupInHtmlComment() + { + // Arrangestring documentContent, + IEnumerable allowedChildren = new List { "b" }; + string literal = "asdf"; + string part1 = ""; + string expectedOutput = $"

{literal}{part1}@{part2}{part3}

"; + + var pTagHelperBuilder = TagHelperDescriptorBuilder + .Create("PTagHelper", "SomeAssembly") + .TagMatchingRuleDescriptor(rule => rule.RequireTagName("p")); + foreach (var childTag in allowedChildren) + { + pTagHelperBuilder.AllowChildTag(childTag); + } + + var descriptors = new TagHelperDescriptor[] + { + pTagHelperBuilder.Build() + }; + + var factory = new SpanFactory(); + var blockFactory = new BlockFactory(factory); + + var expectedMarkup = new MarkupBlock( + new MarkupTagHelperBlock("p", + blockFactory.MarkupTagBlock(""), + factory.Markup(literal), + blockFactory.MarkupTagBlock(""), + new HtmlCommentBlock(factory.Markup(part1), + new ExpressionBlock( + factory.CodeTransition(), + factory.Code(part2) + .AsImplicitExpression(CSharpCodeParser.DefaultKeywords) + .Accepts(AcceptedCharactersInternal.NonWhiteSpace)), + factory.Markup(part3)))); + + // Act & Assert + EvaluateData( + descriptors, + expectedOutput, + expectedMarkup, + Array.Empty()); + } + [Fact] public void Rewrite_UnderstandsNullTagNameWithAllowedChildrenForCatchAll() { @@ -3993,7 +4093,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy new MarkupBlock( new MarkupTagBlock( factory.Markup("")), - factory.Markup(""), + new HtmlCommentBlock( factory.Markup("")), new MarkupTagBlock( factory.Markup(""))) }; @@ -4003,13 +4103,13 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy new MarkupBlock( new MarkupTagBlock( factory.Markup("")), - factory.Markup(""), + new HtmlCommentBlock(factory.Markup("")), new MarkupTagBlock( factory.Markup(""))) }; diff --git a/test/Microsoft.AspNetCore.Razor.Test.Common/Language/Legacy/BlockTypes.cs b/test/Microsoft.AspNetCore.Razor.Test.Common/Language/Legacy/BlockTypes.cs index c08e88d56f..56deb6b460 100644 --- a/test/Microsoft.AspNetCore.Razor.Test.Common/Language/Legacy/BlockTypes.cs +++ b/test/Microsoft.AspNetCore.Razor.Test.Common/Language/Legacy/BlockTypes.cs @@ -217,4 +217,14 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy { } } + + internal class HtmlCommentBlock : Block + { + private const BlockKindInternal ThisBlockKind = BlockKindInternal.HtmlComment; + + public HtmlCommentBlock(params SyntaxTreeNode[] children) + : base(ThisBlockKind, children, ParentChunkGenerator.Null) + { + } + } } From e1fbea24f119f9526c61e227b8e96e16a9b56360 Mon Sep 17 00:00:00 2001 From: Artak Mkrtchyan Date: Tue, 20 Feb 2018 18:24:27 -0800 Subject: [PATCH 21/34] HtmlMarkupParser is now aware of HtmlComment blocks. Majority (if not all, will confirm soon) tests have been updated to account for HtmlComment blocks. --- .../Legacy/HtmlMarkupParser.cs | 49 ++++++++++++++----- .../Legacy/CSharpSectionTest.cs | 11 +++-- .../Legacy/HtmlBlockTest.cs | 33 +++++++------ .../Legacy/TagHelperParseTreeRewriterTest.cs | 48 +++++++++--------- .../TagHelperParseTreeRewriterTests.cs | 26 ++++++++++ ...lCommentWithQuote_Double_DesignTime.ir.txt | 3 +- ...HtmlCommentWithQuote_Double_Runtime.ir.txt | 3 +- ...lCommentWithQuote_Single_DesignTime.ir.txt | 3 +- ...HtmlCommentWithQuote_Single_Runtime.ir.txt | 3 +- .../Language/Legacy/BlockFactory.cs | 21 ++++++++ 10 files changed, 144 insertions(+), 56 deletions(-) create mode 100644 test/Microsoft.AspNetCore.Razor.Language.Test/TagHelperParseTreeRewriterTests.cs diff --git a/src/Microsoft.AspNetCore.Razor.Language/Legacy/HtmlMarkupParser.cs b/src/Microsoft.AspNetCore.Razor.Language/Legacy/HtmlMarkupParser.cs index 332023b775..fc675b6e47 100644 --- a/src/Microsoft.AspNetCore.Razor.Language/Legacy/HtmlMarkupParser.cs +++ b/src/Microsoft.AspNetCore.Razor.Language/Legacy/HtmlMarkupParser.cs @@ -207,10 +207,6 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy if (last != null) { Accept(last); - if (At(HtmlSymbolType.OpenAngle) && last.Type == HtmlSymbolType.Text) - { - Output(SpanKindInternal.Markup); - } } } @@ -501,31 +497,57 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy using (Context.Builder.StartBlock(BlockKindInternal.HtmlComment)) { AcceptAndMoveNext(); + Output(SpanKindInternal.Markup, AcceptedCharactersInternal.None); - Span.EditHandler.AcceptedCharacters = AcceptedCharactersInternal.Any; + Span.EditHandler.AcceptedCharacters = AcceptedCharactersInternal.WhiteSpace; while (!EndOfFile) { SkipToAndParseCode(HtmlSymbolType.DoubleHyphen); if (At(HtmlSymbolType.DoubleHyphen)) { - AcceptWhile(HtmlSymbolType.DoubleHyphen); + var lastDoubleHyphen = CurrentSymbol; + AcceptWhile(s => + { + if (NextIs(HtmlSymbolType.DoubleHyphen)) + { + lastDoubleHyphen = s; + return true; + } + + NextToken(); + EnsureCurrent(); + return false; + }); if (At(HtmlSymbolType.Text) && string.Equals(CurrentSymbol.Content, "-", StringComparison.Ordinal)) { + // Doing this here to maintain the order of symbols + if (!NextIs(HtmlSymbolType.CloseAngle)) + { + Accept(lastDoubleHyphen); + lastDoubleHyphen = null; + } + AcceptAndMoveNext(); } if (At(HtmlSymbolType.CloseAngle)) { - // This is the end of a comment block - Accept(this.CurrentSymbol); - Output(SpanKindInternal.Markup); + // Output the content in the comment block as a separate markup + Output(SpanKindInternal.Markup, AcceptedCharactersInternal.WhiteSpace); + + // This is the end of a comment block + Accept(lastDoubleHyphen); + AcceptAndMoveNext(); + Output(SpanKindInternal.Markup, AcceptedCharactersInternal.None); - NextToken(); - //AcceptAndMoveNext(); return true; } + else if (lastDoubleHyphen != null) + { + Accept(lastDoubleHyphen); + } } } } @@ -1486,6 +1508,11 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy // Checking to see if we meet the conditions of a special '!' tag: ")), + BlockFactory.HtmlCommentBlock(" "), + Factory.EmptyHtml()), Factory.MetaCode("}").Accepts(AcceptedCharactersInternal.None)), Factory.EmptyHtml())); } @@ -630,7 +631,8 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy Factory.Span(SpanKindInternal.Markup, " ", CSharpSymbolType.WhiteSpace).Accepts(AcceptedCharactersInternal.AllWhiteSpace), Factory.MetaCode("{").AutoCompleteWith(null, atEndOfSpan: true).Accepts(AcceptedCharactersInternal.None), new MarkupBlock( - Factory.Markup("")), + BlockFactory.HtmlCommentBlock(" > \" '"), + Factory.EmptyHtml()), Factory.MetaCode("}").Accepts(AcceptedCharactersInternal.None)), Factory.EmptyHtml())); } @@ -655,7 +657,8 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy Factory.Markup(Environment.NewLine), new MarkupTagBlock( Factory.Markup(" \" '-->")), + BlockFactory.HtmlCommentBlock(" > \" '"), + Factory.EmptyHtml()), Factory.MetaCode("}").Accepts(AcceptedCharactersInternal.None)), Factory.EmptyHtml())); } diff --git a/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/HtmlBlockTest.cs b/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/HtmlBlockTest.cs index f54ba50d37..f8d4b0fcd9 100644 --- a/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/HtmlBlockTest.cs +++ b/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/HtmlBlockTest.cs @@ -23,7 +23,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy Factory.Code(Environment.NewLine).AsStatement().AutoCompleteWith(null), new MarkupBlock( Factory.Markup(" "), - Factory.Markup("").Accepts(AcceptedCharactersInternal.None), + BlockFactory.HtmlCommentBlock(" Hello, I'm a comment that shouldn't break razor -"), Factory.Markup(Environment.NewLine).Accepts(AcceptedCharactersInternal.None)), Factory.EmptyCSharp().AsStatement(), Factory.MetaCode("}").Accepts(AcceptedCharactersInternal.None)), @@ -333,7 +333,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy [Fact] public void ParseBlockSupportsCommentAsBlock() { - SingleSpanBlockTest("", BlockKindInternal.Markup, SpanKindInternal.Markup, acceptedCharacters: AcceptedCharactersInternal.None); + ParseBlockTest("", new MarkupBlock( BlockFactory.HtmlCommentBlock(" foo "))); } [Fact] @@ -344,8 +344,8 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy new MarkupTagBlock( Factory.Markup("").Accepts(AcceptedCharactersInternal.None)), Factory.Markup("bar"), - Factory.Markup("").Accepts(AcceptedCharactersInternal.None), - Factory.Markup("baz"), + BlockFactory.HtmlCommentBlock(" zoop "), + Factory.Markup("baz").Accepts(AcceptedCharactersInternal.None), new MarkupTagBlock( Factory.Markup("").Accepts(AcceptedCharactersInternal.None)))); } @@ -355,7 +355,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy get { var factory = new SpanFactory(); - + var blockFactory = new BlockFactory(factory); return new TheoryData { { @@ -363,7 +363,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy new MarkupBlock( new MarkupTagBlock( factory.Markup("
").Accepts(AcceptedCharactersInternal.None)), - factory.Markup("").Accepts(AcceptedCharactersInternal.None), + blockFactory.HtmlCommentBlock("- Hello World -"), new MarkupTagBlock( factory.Markup("
").Accepts(AcceptedCharactersInternal.None))) }, @@ -372,7 +372,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy new MarkupBlock( new MarkupTagBlock( factory.Markup("
").Accepts(AcceptedCharactersInternal.None)), - factory.Markup("").Accepts(AcceptedCharactersInternal.None), + blockFactory.HtmlCommentBlock("-- Hello World --"), new MarkupTagBlock( factory.Markup("
").Accepts(AcceptedCharactersInternal.None))) }, @@ -381,7 +381,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy new MarkupBlock( new MarkupTagBlock( factory.Markup("
").Accepts(AcceptedCharactersInternal.None)), - factory.Markup("").Accepts(AcceptedCharactersInternal.None), + blockFactory.HtmlCommentBlock("--- Hello World ---"), new MarkupTagBlock( factory.Markup("
").Accepts(AcceptedCharactersInternal.None))) }, @@ -390,7 +390,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy new MarkupBlock( new MarkupTagBlock( factory.Markup("
").Accepts(AcceptedCharactersInternal.None)), - factory.Markup("").Accepts(AcceptedCharactersInternal.None), + blockFactory.HtmlCommentBlock("--- Hello < --- > World
---"), new MarkupTagBlock( factory.Markup("").Accepts(AcceptedCharactersInternal.None))) }, @@ -410,19 +410,24 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy [Fact] public void ParseBlockProperlyBalancesCommentStartAndEndTags() { - SingleSpanBlockTest("", BlockKindInternal.Markup, SpanKindInternal.Markup, acceptedCharacters: AcceptedCharactersInternal.None); + ParseBlockTest("", new MarkupBlock(BlockFactory.HtmlCommentBlock(""))); } [Fact] public void ParseBlockTerminatesAtEOFWhenParsingComment() { - SingleSpanBlockTest("", BlockKindInternal.Markup, SpanKindInternal.Markup, acceptedCharacters: AcceptedCharactersInternal.None); + ParseBlockTest("", new MarkupBlock(BlockFactory.HtmlCommentBlock("--"))); } [Fact] @@ -432,8 +437,8 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy new MarkupBlock( new MarkupTagBlock( Factory.Markup("").Accepts(AcceptedCharactersInternal.None)), - Factory.Markup("").Accepts(AcceptedCharactersInternal.None), - Factory.Markup("-->"), + BlockFactory.HtmlCommentBlock("").Accepts(AcceptedCharactersInternal.None), new MarkupTagBlock( Factory.Markup("").Accepts(AcceptedCharactersInternal.None)))); } diff --git a/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/TagHelperParseTreeRewriterTest.cs b/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/TagHelperParseTreeRewriterTest.cs index 45a1ffa907..ceca9e86f2 100644 --- a/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/TagHelperParseTreeRewriterTest.cs +++ b/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/TagHelperParseTreeRewriterTest.cs @@ -1114,8 +1114,8 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy // Arrangestring documentContent, IEnumerable allowedChildren = new List { "b" }; string literal = "asdf"; - string commentOutput = ""; - string expectedOutput = $"

{literal}{commentOutput}

"; + string commentOutput = "Hello World"; + string expectedOutput = $"

{literal}

"; var pTagHelperBuilder = TagHelperDescriptorBuilder .Create("PTagHelper", "SomeAssembly") @@ -1138,7 +1138,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy blockFactory.MarkupTagBlock(""), factory.Markup(literal), blockFactory.MarkupTagBlock(""), - new HtmlCommentBlock(factory.Markup(commentOutput)))); + blockFactory.HtmlCommentBlock(commentOutput))); // Act & Assert EvaluateData( @@ -1185,9 +1185,9 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy var expectedMarkup = new MarkupBlock( new MarkupTagHelperBlock("p", - new HtmlCommentBlock(factory.Markup($"")), + blockFactory.HtmlCommentBlock(comment1), factory.Markup(literal), - new HtmlCommentBlock(factory.Markup($"")))); + blockFactory.HtmlCommentBlock(comment2))); // Act & Assert EvaluateData( @@ -1251,10 +1251,11 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy // Arrangestring documentContent, IEnumerable allowedChildren = new List { "b" }; string literal = "asdf"; - string part1 = ""; - string expectedOutput = $"

{literal}{part1}@{part2}{part3}

"; + string commentStart = ""; + string expectedOutput = $"

{literal}{commentStart}{part1}@{part2}{commentEnd}

"; var pTagHelperBuilder = TagHelperDescriptorBuilder .Create("PTagHelper", "SomeAssembly") @@ -1277,13 +1278,13 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy blockFactory.MarkupTagBlock(""), factory.Markup(literal), blockFactory.MarkupTagBlock(""), - new HtmlCommentBlock(factory.Markup(part1), - new ExpressionBlock( - factory.CodeTransition(), - factory.Code(part2) - .AsImplicitExpression(CSharpCodeParser.DefaultKeywords) - .Accepts(AcceptedCharactersInternal.NonWhiteSpace)), - factory.Markup(part3)))); + blockFactory.HtmlCommentBlock( + factory.Markup(part1).Accepts(AcceptedCharactersInternal.WhiteSpace), + new ExpressionBlock( + factory.CodeTransition(), + factory.Code(part2) + .AsImplicitExpression(CSharpCodeParser.DefaultKeywords) + .Accepts(AcceptedCharactersInternal.NonWhiteSpace))))); // Act & Assert EvaluateData( @@ -4086,14 +4087,14 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy get { var factory = new SpanFactory(); - + var blockFactory = new BlockFactory(factory); yield return new object[] { "", new MarkupBlock( new MarkupTagBlock( factory.Markup("")), - new HtmlCommentBlock( factory.Markup("")), + blockFactory.HtmlCommentBlock (" Hello World "), new MarkupTagBlock( factory.Markup(""))) }; @@ -4103,13 +4104,14 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy new MarkupBlock( new MarkupTagBlock( factory.Markup("")), - new HtmlCommentBlock(factory.Markup("")), + .AsImplicitExpression(CSharpCodeParser.DefaultKeywords) + .Accepts(AcceptedCharactersInternal.NonWhiteSpace)), + factory.Markup(" ").Accepts(AcceptedCharactersInternal.WhiteSpace)), new MarkupTagBlock( factory.Markup(""))) }; @@ -4185,8 +4187,8 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy new ExpressionBlock( factory.CodeTransition(), factory.Code("foo") - .AsImplicitExpression(CSharpCodeParser.DefaultKeywords) - .Accepts(AcceptedCharactersInternal.NonWhiteSpace)), + .AsImplicitExpression(CSharpCodeParser.DefaultKeywords) + .Accepts(AcceptedCharactersInternal.NonWhiteSpace)), factory.Markup(" ]]>"), new MarkupTagBlock( factory.Markup("
"))) diff --git a/test/Microsoft.AspNetCore.Razor.Language.Test/TagHelperParseTreeRewriterTests.cs b/test/Microsoft.AspNetCore.Razor.Language.Test/TagHelperParseTreeRewriterTests.cs new file mode 100644 index 0000000000..a24a9c8d4f --- /dev/null +++ b/test/Microsoft.AspNetCore.Razor.Language.Test/TagHelperParseTreeRewriterTests.cs @@ -0,0 +1,26 @@ +using System; +using System.Collections.Generic; +using System.Text; +using Microsoft.AspNetCore.Razor.Language.Legacy; +using Xunit; + +namespace Microsoft.AspNetCore.Razor.Language.Test +{ + public class TagHelperParseTreeRewriterTests + { + public void IsComment_ReturnsTrueForSpanInHtmlCommentBlock() + { + // Arrange + SpanFactory spanFactory = new SpanFactory(); + + Span content = spanFactory.Markup(""); + Block commentBlock = new HtmlCommentBlock(content); + + // Act + bool actualResult = TagHelperParseTreeRewriter.IsComment(content); + + // Assert + Assert.True(actualResult); + } + } +} diff --git a/test/Microsoft.AspNetCore.Razor.Language.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/HtmlCommentWithQuote_Double_DesignTime.ir.txt b/test/Microsoft.AspNetCore.Razor.Language.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/HtmlCommentWithQuote_Double_DesignTime.ir.txt index aa6a00864d..615f0fce97 100644 --- a/test/Microsoft.AspNetCore.Razor.Language.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/HtmlCommentWithQuote_Double_DesignTime.ir.txt +++ b/test/Microsoft.AspNetCore.Razor.Language.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/HtmlCommentWithQuote_Double_DesignTime.ir.txt @@ -10,7 +10,8 @@ Document - IntermediateToken - - CSharp - #pragma warning restore 0414 MethodDeclaration - - public async - System.Threading.Tasks.Task - ExecuteAsync HtmlContent - (0:0,0 [45] HtmlCommentWithQuote_Double.cshtml) - IntermediateToken - (0:0,0 [12] HtmlCommentWithQuote_Double.cshtml) - Html - \n + IntermediateToken - (0:0,0 [10] HtmlCommentWithQuote_Double.cshtml) - Html - + IntermediateToken - (10:0,10 [2] HtmlCommentWithQuote_Double.cshtml) - Html - \n IntermediateToken - (12:1,0 [4] HtmlCommentWithQuote_Double.cshtml) - Html - diff --git a/test/Microsoft.AspNetCore.Razor.Language.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/HtmlCommentWithQuote_Double_Runtime.ir.txt b/test/Microsoft.AspNetCore.Razor.Language.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/HtmlCommentWithQuote_Double_Runtime.ir.txt index 343985c189..7adfc1650b 100644 --- a/test/Microsoft.AspNetCore.Razor.Language.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/HtmlCommentWithQuote_Double_Runtime.ir.txt +++ b/test/Microsoft.AspNetCore.Razor.Language.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/HtmlCommentWithQuote_Double_Runtime.ir.txt @@ -5,7 +5,8 @@ Document - ClassDeclaration - - public - TestFiles_IntegrationTests_CodeGenerationIntegrationTest_HtmlCommentWithQuote_Double_Runtime - - MethodDeclaration - - public async - System.Threading.Tasks.Task - ExecuteAsync HtmlContent - (0:0,0 [45] HtmlCommentWithQuote_Double.cshtml) - IntermediateToken - (0:0,0 [12] HtmlCommentWithQuote_Double.cshtml) - Html - \n + IntermediateToken - (0:0,0 [10] HtmlCommentWithQuote_Double.cshtml) - Html - + IntermediateToken - (10:0,10 [2] HtmlCommentWithQuote_Double.cshtml) - Html - \n IntermediateToken - (12:1,0 [4] HtmlCommentWithQuote_Double.cshtml) - Html - diff --git a/test/Microsoft.AspNetCore.Razor.Language.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/HtmlCommentWithQuote_Single_DesignTime.ir.txt b/test/Microsoft.AspNetCore.Razor.Language.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/HtmlCommentWithQuote_Single_DesignTime.ir.txt index 04795676d1..0e342fcf51 100644 --- a/test/Microsoft.AspNetCore.Razor.Language.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/HtmlCommentWithQuote_Single_DesignTime.ir.txt +++ b/test/Microsoft.AspNetCore.Razor.Language.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/HtmlCommentWithQuote_Single_DesignTime.ir.txt @@ -10,7 +10,8 @@ Document - IntermediateToken - - CSharp - #pragma warning restore 0414 MethodDeclaration - - public async - System.Threading.Tasks.Task - ExecuteAsync HtmlContent - (0:0,0 [45] HtmlCommentWithQuote_Single.cshtml) - IntermediateToken - (0:0,0 [12] HtmlCommentWithQuote_Single.cshtml) - Html - \n + IntermediateToken - (0:0,0 [10] HtmlCommentWithQuote_Single.cshtml) - Html - + IntermediateToken - (10:0,10 [2] HtmlCommentWithQuote_Single.cshtml) - Html - \n IntermediateToken - (12:1,0 [4] HtmlCommentWithQuote_Single.cshtml) - Html - diff --git a/test/Microsoft.AspNetCore.Razor.Language.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/HtmlCommentWithQuote_Single_Runtime.ir.txt b/test/Microsoft.AspNetCore.Razor.Language.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/HtmlCommentWithQuote_Single_Runtime.ir.txt index 65aafb6fe7..137c2a042c 100644 --- a/test/Microsoft.AspNetCore.Razor.Language.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/HtmlCommentWithQuote_Single_Runtime.ir.txt +++ b/test/Microsoft.AspNetCore.Razor.Language.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/HtmlCommentWithQuote_Single_Runtime.ir.txt @@ -5,7 +5,8 @@ Document - ClassDeclaration - - public - TestFiles_IntegrationTests_CodeGenerationIntegrationTest_HtmlCommentWithQuote_Single_Runtime - - MethodDeclaration - - public async - System.Threading.Tasks.Task - ExecuteAsync HtmlContent - (0:0,0 [45] HtmlCommentWithQuote_Single.cshtml) - IntermediateToken - (0:0,0 [12] HtmlCommentWithQuote_Single.cshtml) - Html - \n + IntermediateToken - (0:0,0 [10] HtmlCommentWithQuote_Single.cshtml) - Html - + IntermediateToken - (10:0,10 [2] HtmlCommentWithQuote_Single.cshtml) - Html - \n IntermediateToken - (12:1,0 [4] HtmlCommentWithQuote_Single.cshtml) - Html - diff --git a/test/Microsoft.AspNetCore.Razor.Test.Common/Language/Legacy/BlockFactory.cs b/test/Microsoft.AspNetCore.Razor.Test.Common/Language/Legacy/BlockFactory.cs index 5c2ee95394..a58d3b0c87 100644 --- a/test/Microsoft.AspNetCore.Razor.Test.Common/Language/Legacy/BlockFactory.cs +++ b/test/Microsoft.AspNetCore.Razor.Test.Common/Language/Legacy/BlockFactory.cs @@ -55,6 +55,27 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy ); } + public HtmlCommentBlock HtmlCommentBlock(string content) + { + return new HtmlCommentBlock(new SyntaxTreeNode[] { + _factory.Markup("").Accepts(AcceptedCharactersInternal.None) }); + } + + public HtmlCommentBlock HtmlCommentBlock(params SyntaxTreeNode[] syntaxTreeNodes) + { + var nodes = new List(); + nodes.Add(_factory.Markup("").Accepts(AcceptedCharactersInternal.None)); + + return new HtmlCommentBlock(nodes.ToArray()); + } + public Block TagHelperBlock( string tagName, TagMode tagMode, From 6f515bb7637fcf70b4a692a626c92e84e5ef208c Mon Sep 17 00:00:00 2001 From: Artak Mkrtchyan Date: Tue, 20 Feb 2018 22:20:48 -0800 Subject: [PATCH 22/34] Fixed all the tests to reflect HtmlComment block type support --- .../Legacy/HtmlDocumentTest.cs | 8 +++++++- .../Legacy/HtmlTagsTest.cs | 2 +- .../Legacy/HtmlToCodeSwitchTest.cs | 15 +++++++------- .../Legacy/TagHelperParseTreeRewriterTest.cs | 20 +++++++++---------- ...lCommentWithQuote_Double_DesignTime.ir.txt | 4 +++- ...HtmlCommentWithQuote_Double_Runtime.ir.txt | 4 +++- ...lCommentWithQuote_Single_DesignTime.ir.txt | 4 +++- ...HtmlCommentWithQuote_Single_Runtime.ir.txt | 4 +++- .../Language/Legacy/BlockFactory.cs | 16 +++++++-------- 9 files changed, 45 insertions(+), 32 deletions(-) diff --git a/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/HtmlDocumentTest.cs b/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/HtmlDocumentTest.cs index 0f06fe578a..9f0e11218b 100644 --- a/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/HtmlDocumentTest.cs +++ b/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/HtmlDocumentTest.cs @@ -214,7 +214,13 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy [Fact] public void ParseDocumentReturnsOneMarkupSegmentIfNoCodeBlocksEncountered() { - SingleSpanDocumentTest("Foo BazBarBar Bar", new MarkupBlock( - Factory.Markup("").Accepts(AcceptedCharactersInternal.None), + BlockFactory.HtmlCommentBlock("Foo"), Factory.Markup(" ").Accepts(AcceptedCharactersInternal.None))); } diff --git a/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/HtmlToCodeSwitchTest.cs b/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/HtmlToCodeSwitchTest.cs index 29c1dfb81a..928762fbc6 100644 --- a/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/HtmlToCodeSwitchTest.cs +++ b/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/HtmlToCodeSwitchTest.cs @@ -112,13 +112,14 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy new MarkupBlock( new MarkupTagBlock( Factory.Markup("").Accepts(AcceptedCharactersInternal.None)), - Factory.Markup("").Accepts(AcceptedCharactersInternal.None), + BlockFactory.HtmlCommentBlock(Factory, f => new SyntaxTreeNode[] { + f.Markup(" ").Accepts(AcceptedCharactersInternal.WhiteSpace), + new ExpressionBlock( + f.CodeTransition(), + f.Code("foo") + .AsImplicitExpression(CSharpCodeParser.DefaultKeywords) + .Accepts(AcceptedCharactersInternal.NonWhiteSpace)), + f.Markup(" ").Accepts(AcceptedCharactersInternal.WhiteSpace) }), new MarkupTagBlock( Factory.Markup("").Accepts(AcceptedCharactersInternal.None)))); } diff --git a/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/TagHelperParseTreeRewriterTest.cs b/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/TagHelperParseTreeRewriterTest.cs index ceca9e86f2..425b728716 100644 --- a/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/TagHelperParseTreeRewriterTest.cs +++ b/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/TagHelperParseTreeRewriterTest.cs @@ -1278,13 +1278,13 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy blockFactory.MarkupTagBlock(""), factory.Markup(literal), blockFactory.MarkupTagBlock(""), - blockFactory.HtmlCommentBlock( - factory.Markup(part1).Accepts(AcceptedCharactersInternal.WhiteSpace), + BlockFactory.HtmlCommentBlock(factory, f => new SyntaxTreeNode[] { + f.Markup(part1).Accepts(AcceptedCharactersInternal.WhiteSpace), new ExpressionBlock( - factory.CodeTransition(), - factory.Code(part2) + f.CodeTransition(), + f.Code(part2) .AsImplicitExpression(CSharpCodeParser.DefaultKeywords) - .Accepts(AcceptedCharactersInternal.NonWhiteSpace))))); + .Accepts(AcceptedCharactersInternal.NonWhiteSpace)) }))); // Act & Assert EvaluateData( @@ -4104,14 +4104,14 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy new MarkupBlock( new MarkupTagBlock( factory.Markup("")), - blockFactory.HtmlCommentBlock( - factory.Markup(" ").Accepts(AcceptedCharactersInternal.WhiteSpace), + BlockFactory.HtmlCommentBlock(factory, f=> new SyntaxTreeNode[]{ + f.Markup(" ").Accepts(AcceptedCharactersInternal.WhiteSpace), new ExpressionBlock( - factory.CodeTransition(), - factory.Code("foo") + f.CodeTransition(), + f.Code("foo") .AsImplicitExpression(CSharpCodeParser.DefaultKeywords) .Accepts(AcceptedCharactersInternal.NonWhiteSpace)), - factory.Markup(" ").Accepts(AcceptedCharactersInternal.WhiteSpace)), + factory.Markup(" ").Accepts(AcceptedCharactersInternal.WhiteSpace) }), new MarkupTagBlock( factory.Markup(""))) }; diff --git a/test/Microsoft.AspNetCore.Razor.Language.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/HtmlCommentWithQuote_Double_DesignTime.ir.txt b/test/Microsoft.AspNetCore.Razor.Language.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/HtmlCommentWithQuote_Double_DesignTime.ir.txt index 615f0fce97..4523eae7a3 100644 --- a/test/Microsoft.AspNetCore.Razor.Language.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/HtmlCommentWithQuote_Double_DesignTime.ir.txt +++ b/test/Microsoft.AspNetCore.Razor.Language.Test/TestFiles/IntegrationTests/CodeGenerationIntegrationTest/HtmlCommentWithQuote_Double_DesignTime.ir.txt @@ -10,7 +10,9 @@ Document - IntermediateToken - - CSharp - #pragma warning restore 0414 MethodDeclaration - - public async - System.Threading.Tasks.Task - ExecuteAsync HtmlContent - (0:0,0 [45] HtmlCommentWithQuote_Double.cshtml) - IntermediateToken - (0:0,0 [10] HtmlCommentWithQuote_Double.cshtml) - Html - + IntermediateToken - (0:0,0 [4] HtmlCommentWithQuote_Double.cshtml) - Html - IntermediateToken - (10:0,10 [2] HtmlCommentWithQuote_Double.cshtml) - Html - \n IntermediateToken - (12:1,0 [4] HtmlCommentWithQuote_Double.cshtml) - Html - + IntermediateToken - (0:0,0 [4] HtmlCommentWithQuote_Double.cshtml) - Html - IntermediateToken - (10:0,10 [2] HtmlCommentWithQuote_Double.cshtml) - Html - \n IntermediateToken - (12:1,0 [4] HtmlCommentWithQuote_Double.cshtml) - Html - + IntermediateToken - (0:0,0 [4] HtmlCommentWithQuote_Single.cshtml) - Html - IntermediateToken - (10:0,10 [2] HtmlCommentWithQuote_Single.cshtml) - Html - \n IntermediateToken - (12:1,0 [4] HtmlCommentWithQuote_Single.cshtml) - Html - + IntermediateToken - (0:0,0 [4] HtmlCommentWithQuote_Single.cshtml) - Html - IntermediateToken - (10:0,10 [2] HtmlCommentWithQuote_Single.cshtml) - Html - \n IntermediateToken - (12:1,0 [4] HtmlCommentWithQuote_Single.cshtml) - Html - ").Accepts(AcceptedCharactersInternal.None) }); + return HtmlCommentBlock(_factory, f => new SyntaxTreeNode[] { f.Markup(content).Accepts(AcceptedCharactersInternal.WhiteSpace) }); } - public HtmlCommentBlock HtmlCommentBlock(params SyntaxTreeNode[] syntaxTreeNodes) + public static HtmlCommentBlock HtmlCommentBlock(SpanFactory factory, Func> nodesBuilder = null) { var nodes = new List(); - nodes.Add(_factory.Markup("").Accepts(AcceptedCharactersInternal.None)); + nodes.Add(factory.Markup("-->").Accepts(AcceptedCharactersInternal.None)); return new HtmlCommentBlock(nodes.ToArray()); } From 0af42204eb05400ff5c505c0fa4d81b39298eb6d Mon Sep 17 00:00:00 2001 From: Artak Mkrtchyan Date: Thu, 22 Feb 2018 17:05:13 -0800 Subject: [PATCH 23/34] Added the AllowHtmlCommentsInTagHelpers feature flag to control behavior of having comments in TagHelpers --- .../Legacy/TagHelperParseTreeRewriter.cs | 8 +++++++- .../RazorParserFeatureFlags.cs | 11 +++++++++-- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/src/Microsoft.AspNetCore.Razor.Language/Legacy/TagHelperParseTreeRewriter.cs b/src/Microsoft.AspNetCore.Razor.Language/Legacy/TagHelperParseTreeRewriter.cs index 6cac8b46ea..a4c9906b1a 100644 --- a/src/Microsoft.AspNetCore.Razor.Language/Legacy/TagHelperParseTreeRewriter.cs +++ b/src/Microsoft.AspNetCore.Razor.Language/Legacy/TagHelperParseTreeRewriter.cs @@ -488,7 +488,13 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy private void ValidateParentAllowsContent(Span child, ErrorSink errorSink) { - if (HasAllowedChildren() && !IsComment(child) && child.Kind != SpanKindInternal.Transition && child.Kind != SpanKindInternal.Code) + var isDisallowedContent = true; + if (_featureFlags.AllowHtmlCommentsInTagHelpers) + { + isDisallowedContent = !IsComment(child) && child.Kind != SpanKindInternal.Transition && child.Kind != SpanKindInternal.Code; + } + + if (HasAllowedChildren() && isDisallowedContent) { var content = child.Content; if (!string.IsNullOrWhiteSpace(content)) diff --git a/src/Microsoft.AspNetCore.Razor.Language/RazorParserFeatureFlags.cs b/src/Microsoft.AspNetCore.Razor.Language/RazorParserFeatureFlags.cs index 0629eb9af8..409718b640 100644 --- a/src/Microsoft.AspNetCore.Razor.Language/RazorParserFeatureFlags.cs +++ b/src/Microsoft.AspNetCore.Razor.Language/RazorParserFeatureFlags.cs @@ -8,26 +8,33 @@ namespace Microsoft.AspNetCore.Razor.Language public static RazorParserFeatureFlags Create(RazorLanguageVersion version) { var allowMinimizedBooleanTagHelperAttributes = false; + var allowHtmlCommentsInTagHelpers = false; if (version.CompareTo(RazorLanguageVersion.Version_2_1) >= 0) { // Added in 2.1 allowMinimizedBooleanTagHelperAttributes = true; + allowHtmlCommentsInTagHelpers = true; } - return new DefaultRazorParserFeatureFlags(allowMinimizedBooleanTagHelperAttributes); + return new DefaultRazorParserFeatureFlags(allowMinimizedBooleanTagHelperAttributes, allowHtmlCommentsInTagHelpers); } public abstract bool AllowMinimizedBooleanTagHelperAttributes { get; } + public abstract bool AllowHtmlCommentsInTagHelpers { get; } + private class DefaultRazorParserFeatureFlags : RazorParserFeatureFlags { - public DefaultRazorParserFeatureFlags(bool allowMinimizedBooleanTagHelperAttributes) + public DefaultRazorParserFeatureFlags(bool allowMinimizedBooleanTagHelperAttributes, bool allowHtmlCommentsInTagHelpers) { AllowMinimizedBooleanTagHelperAttributes = allowMinimizedBooleanTagHelperAttributes; + AllowHtmlCommentsInTagHelpers = allowHtmlCommentsInTagHelpers; } public override bool AllowMinimizedBooleanTagHelperAttributes { get; } + + public override bool AllowHtmlCommentsInTagHelpers { get; } } } } From 6885fb15d0c7c9cb8cf7224f7a6ddec2bf6df427 Mon Sep 17 00:00:00 2001 From: Artak Mkrtchyan Date: Thu, 22 Feb 2018 17:07:52 -0800 Subject: [PATCH 24/34] Updated the test TagHelperBlockRewriteTest class --- .../Legacy/TagHelperBlockRewriterTest.cs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/TagHelperBlockRewriterTest.cs b/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/TagHelperBlockRewriterTest.cs index 9b2adc4d07..88c774c93c 100644 --- a/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/TagHelperBlockRewriterTest.cs +++ b/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/TagHelperBlockRewriterTest.cs @@ -2950,7 +2950,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy intType), RazorDiagnosticFactory.CreateParsing_TagHelperIndexerAttributeNameMustIncludeKey( new SourceSpan(7, 0, 7, 11), - "int-prefix-", + "int-prefix-", "input"), } }, @@ -2973,7 +2973,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy stringType), RazorDiagnosticFactory.CreateParsing_TagHelperIndexerAttributeNameMustIncludeKey( new SourceSpan(7, 0, 7, 14), - "string-prefix-", + "string-prefix-", "input"), } }, @@ -3638,7 +3638,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy RazorDiagnosticFactory.CreateTagHelper_EmptyBoundAttribute( new SourceSpan(7, 0, 7, 21), "bound-required-string", - "input", + "input", stringType), } }, @@ -3962,7 +3962,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy .Build(), }; - var featureFlags = new TestRazorParserFeatureFlags(allowMinimizedBooleanTagHelperAttributes: false); + var featureFlags = new TestRazorParserFeatureFlags(allowMinimizedBooleanTagHelperAttributes: false, allowHtmlCommentsInTagHelper: false); var expectedOutput = new MarkupBlock( new MarkupTagHelperBlock( @@ -3994,12 +3994,15 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy private class TestRazorParserFeatureFlags : RazorParserFeatureFlags { - public TestRazorParserFeatureFlags(bool allowMinimizedBooleanTagHelperAttributes) + public TestRazorParserFeatureFlags(bool allowMinimizedBooleanTagHelperAttributes, bool allowHtmlCommentsInTagHelper) { AllowMinimizedBooleanTagHelperAttributes = allowMinimizedBooleanTagHelperAttributes; + AllowHtmlCommentsInTagHelpers = allowHtmlCommentsInTagHelper; } public override bool AllowMinimizedBooleanTagHelperAttributes { get; } + + public override bool AllowHtmlCommentsInTagHelpers { get; } } } } From 33814fb6347ba4f27c3eb8da7d23ce4e8a06a974 Mon Sep 17 00:00:00 2001 From: Artak Mkrtchyan Date: Mon, 5 Mar 2018 17:28:23 -0800 Subject: [PATCH 25/34] Updated the Comments detection to comply with HTML 5 specification --- .../Legacy/HtmlMarkupParser.cs | 151 ++++++++++++++---- .../Legacy/TokenizerBackedParser.cs | 47 ++++++ .../Legacy/HtmlBlockTest.cs | 4 +- .../Legacy/HtmlDocumentTest.cs | 3 +- 4 files changed, 170 insertions(+), 35 deletions(-) diff --git a/src/Microsoft.AspNetCore.Razor.Language/Legacy/HtmlMarkupParser.cs b/src/Microsoft.AspNetCore.Razor.Language/Legacy/HtmlMarkupParser.cs index fc675b6e47..2fe84b5a33 100644 --- a/src/Microsoft.AspNetCore.Razor.Language/Legacy/HtmlMarkupParser.cs +++ b/src/Microsoft.AspNetCore.Razor.Language/Legacy/HtmlMarkupParser.cs @@ -492,7 +492,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy if (AcceptAndMoveNext()) { - if (CurrentSymbol.Type == HtmlSymbolType.DoubleHyphen) + if (IsHtmlCommentAhead()) { using (Context.Builder.StartBlock(BlockKindInternal.HtmlComment)) { @@ -505,32 +505,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy SkipToAndParseCode(HtmlSymbolType.DoubleHyphen); if (At(HtmlSymbolType.DoubleHyphen)) { - var lastDoubleHyphen = CurrentSymbol; - AcceptWhile(s => - { - if (NextIs(HtmlSymbolType.DoubleHyphen)) - { - lastDoubleHyphen = s; - return true; - } - - NextToken(); - EnsureCurrent(); - return false; - }); - - if (At(HtmlSymbolType.Text) && - string.Equals(CurrentSymbol.Content, "-", StringComparison.Ordinal)) - { - // Doing this here to maintain the order of symbols - if (!NextIs(HtmlSymbolType.CloseAngle)) - { - Accept(lastDoubleHyphen); - lastDoubleHyphen = null; - } - - AcceptAndMoveNext(); - } + var lastDoubleHyphen = AcceptAllButLastDoubleHypens(); if (At(HtmlSymbolType.CloseAngle)) { @@ -541,7 +516,6 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy Accept(lastDoubleHyphen); AcceptAndMoveNext(); Output(SpanKindInternal.Markup, AcceptedCharactersInternal.None); - return true; } else if (lastDoubleHyphen != null) @@ -551,8 +525,6 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy } } } - - return false; } else if (CurrentSymbol.Type == HtmlSymbolType.LeftBracket) { @@ -571,6 +543,125 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy return false; } + private HtmlSymbol AcceptAllButLastDoubleHypens() + { + var lastDoubleHyphen = CurrentSymbol; + AcceptWhile(s => + { + if (NextIs(HtmlSymbolType.DoubleHyphen)) + { + lastDoubleHyphen = s; + return true; + } + + NextToken(); + EnsureCurrent(); + return false; + }); + + if (At(HtmlSymbolType.Text) && IsDashSymbol(CurrentSymbol)) + { + // Doing this here to maintain the order of symbols + if (!NextIs(HtmlSymbolType.CloseAngle)) + { + Accept(lastDoubleHyphen); + lastDoubleHyphen = null; + } + + AcceptAndMoveNext(); + } + + return lastDoubleHyphen; + } + + private static bool IsDashSymbol(HtmlSymbol symbol) + { + return string.Equals(symbol.Content, "-", StringComparison.Ordinal); + } + + private bool IsHtmlCommentAhead() + { + /* + * From HTML5 Specification, available at http://www.w3.org/TR/html52/syntax.html#comments + * + * Comments must have the following format: + * 1. The string "", or "--!>" + * 2.3 nor end with the string "" + * + * */ + + if (CurrentSymbol.Type != HtmlSymbolType.DoubleHyphen) + { + return false; + } + + // Check condition 2.1 + if (NextIs(HtmlSymbolType.CloseAngle) || NextIs(next => IsDashSymbol(next) && NextIs(HtmlSymbolType.CloseAngle))) + { + return false; + } + + // Check condition 2.2 + bool isValidComment = false; + LookaheadUntil((s, p) => + { + bool breakLookahead = false; + if (s.Type == HtmlSymbolType.DoubleHyphen) + { + if (NextIs(HtmlSymbolType.CloseAngle)) + { + // We're at the end of a comment. check the condition 2.3 to make sure the text ending is allowed. + isValidComment = !EndsWithSymbolsSequence(p, HtmlSymbolType.OpenAngle, HtmlSymbolType.Bang, HtmlSymbolType.DoubleHyphen); + breakLookahead = true; + } + else if (NextIs(ns => IsDashSymbol(ns) && NextIs(HtmlSymbolType.CloseAngle))) + { + // This is also a valid closing comment case, as the dashes lookup is treated with DoubleHyphen symbols first. + isValidComment = true; + breakLookahead = true; + } + else if (NextIs(ns => ns.Type == HtmlSymbolType.Bang && NextIs(HtmlSymbolType.CloseAngle))) + { + isValidComment = false; + breakLookahead = true; + } + } + else if (s.Type == HtmlSymbolType.OpenAngle) + { + if (NextIs(ns => ns.Type == HtmlSymbolType.Bang && NextIs(HtmlSymbolType.DoubleHyphen))) + { + isValidComment = false; + breakLookahead = true; + } + } + + return breakLookahead; + }); + + return isValidComment; + } + + private bool EndsWithSymbolsSequence(IEnumerable symbols, params HtmlSymbolType[] sequenceToMatchWith) + { + int index = sequenceToMatchWith.Length; + foreach (var previousSymbol in symbols) + { + if (index == 0) + { + break; + } + + if (sequenceToMatchWith[--index] != previousSymbol.Type) + return false; + } + + return index == 0; + } + private bool CData() { if (CurrentSymbol.Type == HtmlSymbolType.Text && string.Equals(CurrentSymbol.Content, "cdata", StringComparison.OrdinalIgnoreCase)) diff --git a/src/Microsoft.AspNetCore.Razor.Language/Legacy/TokenizerBackedParser.cs b/src/Microsoft.AspNetCore.Razor.Language/Legacy/TokenizerBackedParser.cs index 6f564eb442..1819407076 100644 --- a/src/Microsoft.AspNetCore.Razor.Language/Legacy/TokenizerBackedParser.cs +++ b/src/Microsoft.AspNetCore.Razor.Language/Legacy/TokenizerBackedParser.cs @@ -109,6 +109,53 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy return symbols[count]; } + /// + /// Looks forward until the specified condition is met. + /// + /// A predicate accepting the symbol being evaluated and the list of symbols which have been looped through. + /// true, if the condition was met. false - if the condition wasn't met and the last symbol has already been processed. + /// The list of previous symbols is passed in the reverse order. So the last processed element will be the first one in the list. + protected bool LookaheadUntil(Func, bool> condition) + { + if (condition == null) + { + throw new ArgumentNullException(nameof(condition)); + } + + bool matchFound = false; + + // We add 1 in order to store the current symbol. + var symbols = new List(); + symbols.Add(CurrentSymbol); + + while (true) + { + if (!NextToken()) + { + break; + } + + symbols.Add(CurrentSymbol); + if (condition(CurrentSymbol, symbols.Reverse())) + { + matchFound = true; + break; + } + } + + // Restore Tokenizer's location to where it was pointing before the look-ahead. + for (var i = symbols.Count - 1; i >= 0; i--) + { + PutBack(symbols[i]); + } + + // The PutBacks above will set CurrentSymbol to null. EnsureCurrent will set our CurrentSymbol to the + // next symbol. + EnsureCurrent(); + + return matchFound; + } + protected internal bool NextToken() { PreviousSymbol = CurrentSymbol; diff --git a/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/HtmlBlockTest.cs b/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/HtmlBlockTest.cs index f8d4b0fcd9..5d3d46a831 100644 --- a/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/HtmlBlockTest.cs +++ b/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/HtmlBlockTest.cs @@ -419,9 +419,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy ParseBlockTest( ""); + + // Act + HtmlSymbol symbol = sut.AcceptAllButLastDoubleHypens(); + + // Assert + Assert.Equal(doubleHyphenSymbol, symbol); + Assert.True(sut.At(HtmlSymbolType.CloseAngle)); + Assert.Equal(doubleHyphenSymbol, sut.PreviousSymbol); + } + + [Fact] + public void AcceptAllButLastDoubleHypens_ReturnsTheDoubleHyphenSymbolAfterAcceptingTheDash() + { + // Arrange + TestHtmlMarkupParser sut = CreateTestParserForContent("--->"); + + // Act + HtmlSymbol symbol = sut.AcceptAllButLastDoubleHypens(); + + // Assert + Assert.Equal(doubleHyphenSymbol, symbol); + Assert.True(sut.At(HtmlSymbolType.CloseAngle)); + Assert.True(HtmlMarkupParser.IsDashSymbol(sut.PreviousSymbol)); + } + + [Fact] + public void IsHtmlCommentAhead_ReturnsTrueForEmptyCommentTag() + { + // Arrange + TestHtmlMarkupParser sut = CreateTestParserForContent("---->"); + + // Act & Assert + Assert.True(sut.IsHtmlCommentAhead()); + } + + [Fact] + public void IsHtmlCommentAhead_ReturnsTrueForValidCommentTag() + { + // Arrange + TestHtmlMarkupParser sut = CreateTestParserForContent("-- Some comment content in here -->"); + + // Act & Assert + Assert.True(sut.IsHtmlCommentAhead()); + } + + [Fact] + public void IsHtmlCommentAhead_ReturnsTrueForValidCommentTagWithExtraDashesAtClosingTag() + { + // Arrange + TestHtmlMarkupParser sut = CreateTestParserForContent("-- Some comment content in here ----->"); + + // Act & Assert + Assert.True(sut.IsHtmlCommentAhead()); + } + + [Fact] + public void IsHtmlCommentAhead_ReturnsTrueForValidCommentTagWithExtraInfoAfter() + { + // Arrange + TestHtmlMarkupParser sut = CreateTestParserForContent("-- comment --> the first part is a valid comment without the Open angle and bang symbols"); + + // Act & Assert + Assert.True(sut.IsHtmlCommentAhead()); + } + + [Fact] + public void IsHtmlCommentAhead_ReturnsFalseForNotClosedComment() + { + // Arrange + TestHtmlMarkupParser sut = CreateTestParserForContent("-- not closed comment"); + + // Act & Assert + Assert.False(sut.IsHtmlCommentAhead()); + } + + [Fact] + public void IsHtmlCommentAhead_ReturnsFalseForCommentWithoutLastClosingAngle() + { + // Arrange + TestHtmlMarkupParser sut = CreateTestParserForContent("-- not closed comment--"); + + // Act & Assert + Assert.False(sut.IsHtmlCommentAhead()); + } + + private class TestHtmlMarkupParser : HtmlMarkupParser + { + public new HtmlSymbol PreviousSymbol + { + get => base.PreviousSymbol; + } + + public new bool IsHtmlCommentAhead() + { + return base.IsHtmlCommentAhead(); + } + + public TestHtmlMarkupParser(ParserContext context) : base(context) + { + this.EnsureCurrent(); + } + + public new HtmlSymbol AcceptAllButLastDoubleHypens() + { + return base.AcceptAllButLastDoubleHypens(); + } + + public override void BuildSpan(SpanBuilder span, SourceLocation start, string content) + { + base.BuildSpan(span, start, content); + } + } + + private static TestHtmlMarkupParser CreateTestParserForContent(string content) + { + var source = TestRazorSourceDocument.Create(content); + var options = RazorParserOptions.CreateDefault(); + var context = new ParserContext(source, options); + + return new TestHtmlMarkupParser(context); + } + } +} From 1318a835119529db22a0602bd0e603bf16f771c3 Mon Sep 17 00:00:00 2001 From: Artak Mkrtchyan Date: Thu, 8 Mar 2018 17:13:34 -0800 Subject: [PATCH 30/34] Fixed the NextIs method to put back the symbol, when at the end of the file --- .../Legacy/TokenizerBackedParser.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/Microsoft.AspNetCore.Razor.Language/Legacy/TokenizerBackedParser.cs b/src/Microsoft.AspNetCore.Razor.Language/Legacy/TokenizerBackedParser.cs index 2b6e55090a..7911380d0f 100644 --- a/src/Microsoft.AspNetCore.Razor.Language/Legacy/TokenizerBackedParser.cs +++ b/src/Microsoft.AspNetCore.Razor.Language/Legacy/TokenizerBackedParser.cs @@ -308,6 +308,11 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy EnsureCurrent(); return result; } + else + { + PutBack(cur); + EnsureCurrent(); + } return false; } From 6c8c6a777cd78b332298c31e63e8d9e34919b1a8 Mon Sep 17 00:00:00 2001 From: Artak Mkrtchyan Date: Fri, 9 Mar 2018 09:16:17 -0800 Subject: [PATCH 31/34] - Clarified the code where the comment content ending is checked to be allowed or not. - Added more unit tests --- .../Legacy/HtmlMarkupParser.cs | 25 +++++--- .../Legacy/TokenizerBackedParser.cs | 2 +- .../Legacy/HtmlMarkupParserTests.cs | 61 +++++++++++++++---- .../Legacy/TokenizerLookaheadTest.cs | 24 +++++--- 4 files changed, 79 insertions(+), 33 deletions(-) diff --git a/src/Microsoft.AspNetCore.Razor.Language/Legacy/HtmlMarkupParser.cs b/src/Microsoft.AspNetCore.Razor.Language/Legacy/HtmlMarkupParser.cs index e9dcf36d5c..39841d4835 100644 --- a/src/Microsoft.AspNetCore.Razor.Language/Legacy/HtmlMarkupParser.cs +++ b/src/Microsoft.AspNetCore.Razor.Language/Legacy/HtmlMarkupParser.cs @@ -613,7 +613,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy if (NextIs(HtmlSymbolType.CloseAngle)) { // Check condition 2.3: We're at the end of a comment. Check to make sure the text ending is allowed. - isValidComment = !SymbolSequenceEndsWithItems(p, HtmlSymbolType.OpenAngle, HtmlSymbolType.Bang, HtmlSymbolType.DoubleHyphen); + isValidComment = !IsCommentContentDisallowed(p); return true; } else if (NextIs(ns => IsDashSymbol(ns) && NextIs(HtmlSymbolType.CloseAngle))) @@ -643,21 +643,28 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy return isValidComment; } - internal static bool SymbolSequenceEndsWithItems(IEnumerable sequence, params HtmlSymbolType[] items) + /// + /// Verifies, that the sequence doesn't end with the "<!-" HtmlSymbols. Note, the first symbol is an opening bracket symbol + /// + internal static bool IsCommentContentDisallowed(IEnumerable sequence) { - int index = items.Length; - foreach (var previousSymbol in sequence) + var reversedSequence = sequence.Reverse(); + var disallowEnding = new[] { new HtmlSymbol("-", HtmlSymbolType.Text), new HtmlSymbol("!", HtmlSymbolType.Bang), new HtmlSymbol("<", HtmlSymbolType.OpenAngle) }; + var index = 0; + foreach (var item in reversedSequence) { - if (index == 0) + if (!item.Equals(disallowEnding[index++])) { - break; + return false; } - if (items[--index] != previousSymbol.Type) - return false; + if (index == disallowEnding.Length) + { + return true; + } } - return index == 0; + return false; } private bool CData() diff --git a/src/Microsoft.AspNetCore.Razor.Language/Legacy/TokenizerBackedParser.cs b/src/Microsoft.AspNetCore.Razor.Language/Legacy/TokenizerBackedParser.cs index 7911380d0f..01fa0b11e1 100644 --- a/src/Microsoft.AspNetCore.Razor.Language/Legacy/TokenizerBackedParser.cs +++ b/src/Microsoft.AspNetCore.Razor.Language/Legacy/TokenizerBackedParser.cs @@ -135,7 +135,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy } symbols.Add(CurrentSymbol); - if (condition(CurrentSymbol, symbols.Reverse())) + if (condition(CurrentSymbol, symbols)) { matchFound = true; break; diff --git a/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/HtmlMarkupParserTests.cs b/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/HtmlMarkupParserTests.cs index db47d47b73..c3e0cb87e1 100644 --- a/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/HtmlMarkupParserTests.cs +++ b/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/HtmlMarkupParserTests.cs @@ -1,4 +1,6 @@ -using System.Collections.Generic; +using System; +using System.Collections.Generic; +using System.Linq; using Microsoft.AspNetCore.Razor.Language.Legacy; using Xunit; @@ -25,7 +27,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Test.Legacy public void IsDashSymbol_ReturnsFalseForNonDashSymbol(object symbol) { // Arrange - HtmlSymbol convertedSymbol = (HtmlSymbol)symbol; + var convertedSymbol = (HtmlSymbol)symbol; // Act & Assert Assert.False(HtmlMarkupParser.IsDashSymbol(convertedSymbol)); @@ -35,7 +37,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Test.Legacy public void IsDashSymbol_ReturnsTrueForADashSymbol() { // Arrange - HtmlSymbol dashSymbol = new HtmlSymbol("-", HtmlSymbolType.Text); + var dashSymbol = new HtmlSymbol("-", HtmlSymbolType.Text); // Act & Assert Assert.True(HtmlMarkupParser.IsDashSymbol(dashSymbol)); @@ -45,10 +47,10 @@ namespace Microsoft.AspNetCore.Razor.Language.Test.Legacy public void AcceptAllButLastDoubleHypens_ReturnsTheOnlyDoubleHyphenSymbol() { // Arrange - TestHtmlMarkupParser sut = CreateTestParserForContent("-->"); + var sut = CreateTestParserForContent("-->"); // Act - HtmlSymbol symbol = sut.AcceptAllButLastDoubleHypens(); + var symbol = sut.AcceptAllButLastDoubleHypens(); // Assert Assert.Equal(doubleHyphenSymbol, symbol); @@ -60,10 +62,10 @@ namespace Microsoft.AspNetCore.Razor.Language.Test.Legacy public void AcceptAllButLastDoubleHypens_ReturnsTheDoubleHyphenSymbolAfterAcceptingTheDash() { // Arrange - TestHtmlMarkupParser sut = CreateTestParserForContent("--->"); + var sut = CreateTestParserForContent("--->"); // Act - HtmlSymbol symbol = sut.AcceptAllButLastDoubleHypens(); + var symbol = sut.AcceptAllButLastDoubleHypens(); // Assert Assert.Equal(doubleHyphenSymbol, symbol); @@ -75,7 +77,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Test.Legacy public void IsHtmlCommentAhead_ReturnsTrueForEmptyCommentTag() { // Arrange - TestHtmlMarkupParser sut = CreateTestParserForContent("---->"); + var sut = CreateTestParserForContent("---->"); // Act & Assert Assert.True(sut.IsHtmlCommentAhead()); @@ -85,7 +87,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Test.Legacy public void IsHtmlCommentAhead_ReturnsTrueForValidCommentTag() { // Arrange - TestHtmlMarkupParser sut = CreateTestParserForContent("-- Some comment content in here -->"); + var sut = CreateTestParserForContent("-- Some comment content in here -->"); // Act & Assert Assert.True(sut.IsHtmlCommentAhead()); @@ -95,7 +97,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Test.Legacy public void IsHtmlCommentAhead_ReturnsTrueForValidCommentTagWithExtraDashesAtClosingTag() { // Arrange - TestHtmlMarkupParser sut = CreateTestParserForContent("-- Some comment content in here ----->"); + var sut = CreateTestParserForContent("-- Some comment content in here ----->"); // Act & Assert Assert.True(sut.IsHtmlCommentAhead()); @@ -105,7 +107,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Test.Legacy public void IsHtmlCommentAhead_ReturnsTrueForValidCommentTagWithExtraInfoAfter() { // Arrange - TestHtmlMarkupParser sut = CreateTestParserForContent("-- comment --> the first part is a valid comment without the Open angle and bang symbols"); + var sut = CreateTestParserForContent("-- comment --> the first part is a valid comment without the Open angle and bang symbols"); // Act & Assert Assert.True(sut.IsHtmlCommentAhead()); @@ -115,7 +117,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Test.Legacy public void IsHtmlCommentAhead_ReturnsFalseForNotClosedComment() { // Arrange - TestHtmlMarkupParser sut = CreateTestParserForContent("-- not closed comment"); + var sut = CreateTestParserForContent("-- not closed comment"); // Act & Assert Assert.False(sut.IsHtmlCommentAhead()); @@ -125,12 +127,45 @@ namespace Microsoft.AspNetCore.Razor.Language.Test.Legacy public void IsHtmlCommentAhead_ReturnsFalseForCommentWithoutLastClosingAngle() { // Arrange - TestHtmlMarkupParser sut = CreateTestParserForContent("-- not closed comment--"); + var sut = CreateTestParserForContent("-- not closed comment--"); // Act & Assert Assert.False(sut.IsHtmlCommentAhead()); } + [Fact] + public void IsCommentContentDisallowed_ReturnsFalseForAllowedContent() + { + // Arrange + var expectedSymbol1 = new HtmlSymbol("a", HtmlSymbolType.Text); + var sequence = Enumerable.Range((int)'a', 26).Select(item => new HtmlSymbol(((char)item).ToString(), HtmlSymbolType.Text)); + + // Act & Assert + Assert.False(HtmlMarkupParser.IsCommentContentDisallowed(sequence)); + } + + [Fact] + public void IsCommentContentDisallowed_ReturnsTrueForDisallowedContent() + { + // Arrange + var expectedSymbol1 = new HtmlSymbol("a", HtmlSymbolType.Text); + var sequence = new[] { new HtmlSymbol("<", HtmlSymbolType.OpenAngle), new HtmlSymbol("!", HtmlSymbolType.Bang), new HtmlSymbol("-", HtmlSymbolType.Text) }; + + // Act & Assert + Assert.True(HtmlMarkupParser.IsCommentContentDisallowed(sequence)); + } + + [Fact] + public void IsCommentContentDisallowed_ReturnsFalseForEmptyContent() + { + // Arrange + var expectedSymbol1 = new HtmlSymbol("a", HtmlSymbolType.Text); + var sequence = Array.Empty(); + + // Act & Assert + Assert.False(HtmlMarkupParser.IsCommentContentDisallowed(sequence)); + } + private class TestHtmlMarkupParser : HtmlMarkupParser { public new HtmlSymbol PreviousSymbol diff --git a/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/TokenizerLookaheadTest.cs b/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/TokenizerLookaheadTest.cs index 74d0c075f3..aa9ea560cc 100644 --- a/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/TokenizerLookaheadTest.cs +++ b/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/TokenizerLookaheadTest.cs @@ -3,7 +3,7 @@ using System; using System.Collections.Generic; -using System.IO; +using System.Linq; using System.Text; using Xunit; @@ -57,25 +57,29 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy } [Fact] - public void LookaheadUntil_PassesThePreviousSymbolsInReverseOrder() + public void LookaheadUntil_PassesThePreviousSymbolsInTheSameOrder() { // Arrange var tokenizer = CreateContentTokenizer("asdf--fvd--<"); // Act - Stack symbols = new Stack(); var i = 3; + IEnumerable previousSymbols = null; var symbolFound = tokenizer.LookaheadUntil((s, p) => { - symbols.Push(s); + previousSymbols = p; return --i == 0; }); // Assert - Assert.Equal(3, symbols.Count); - Assert.Equal(new HtmlSymbol("fvd", HtmlSymbolType.Text), symbols.Pop()); - Assert.Equal(new HtmlSymbol("--", HtmlSymbolType.DoubleHyphen), symbols.Pop()); - Assert.Equal(new HtmlSymbol("asdf", HtmlSymbolType.Text), symbols.Pop()); + Assert.Equal(4, previousSymbols.Count()); + + // For the very first element, there will be no previous items, so null is expected + var orderIndex = 0; + Assert.Null(previousSymbols.ElementAt(orderIndex++)); + Assert.Equal(new HtmlSymbol("asdf", HtmlSymbolType.Text), previousSymbols.ElementAt(orderIndex++)); + Assert.Equal(new HtmlSymbol("--", HtmlSymbolType.DoubleHyphen), previousSymbols.ElementAt(orderIndex++)); + Assert.Equal(new HtmlSymbol("fvd", HtmlSymbolType.Text), previousSymbols.ElementAt(orderIndex++)); } [Fact] @@ -85,7 +89,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy var tokenizer = CreateContentTokenizer("asdf--fvd"); // Act - Stack symbols = new Stack(); + var symbols = new Stack(); var symbolFound = tokenizer.LookaheadUntil((s, p) => { symbols.Push(s); @@ -107,7 +111,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy var tokenizer = CreateContentTokenizer("asdf--fvd"); // Act - Stack symbols = new Stack(); + var symbols = new Stack(); var symbolFound = tokenizer.LookaheadUntil((s, p) => { symbols.Push(s); From e8eb3b7b1c7c373f36febe6cd7e6a8351da7cfd2 Mon Sep 17 00:00:00 2001 From: Artak Mkrtchyan Date: Fri, 9 Mar 2018 11:52:23 -0800 Subject: [PATCH 32/34] Added extra test to cover extra dash in closing comment --- .../Legacy/HtmlBlockTest.cs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/HtmlBlockTest.cs b/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/HtmlBlockTest.cs index 5d3d46a831..3abd53b105 100644 --- a/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/HtmlBlockTest.cs +++ b/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/HtmlBlockTest.cs @@ -333,7 +333,13 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy [Fact] public void ParseBlockSupportsCommentAsBlock() { - ParseBlockTest("", new MarkupBlock( BlockFactory.HtmlCommentBlock(" foo "))); + ParseBlockTest("", new MarkupBlock(BlockFactory.HtmlCommentBlock(" foo "))); + } + + [Fact] + public void ParseBlockSupportsCommentWithExtraDashAsBlock() + { + ParseBlockTest("", new MarkupBlock(BlockFactory.HtmlCommentBlock(" foo -"))); } [Fact] From 3e22c169304a8c23bb108dbf063056ae4b890cd4 Mon Sep 17 00:00:00 2001 From: Artak Mkrtchyan Date: Fri, 9 Mar 2018 17:18:57 -0800 Subject: [PATCH 33/34] Added extra tests for non-covered scenarios: - For older version of Razor the HTML comments will be complained about by TahHelperRewriter - RazorParserFeatureFlags tests now ensure that AllowHtmlCommentsInTagHelpers is true in 2.1 version and false in older versions - Added extra test for IsHtmlCommentAhead to make sure Razor code transition is allowed in comment tag - Moved the unallowed html comment ending to a static array. --- .../Legacy/HtmlMarkupParser.cs | 7 ++- .../Legacy/HtmlMarkupParserTests.cs | 10 ++++ .../Legacy/TagHelperParseTreeRewriterTest.cs | 59 +++++++++++++++++-- .../RazorParserFeatureFlagsTest.cs | 2 + 4 files changed, 71 insertions(+), 7 deletions(-) diff --git a/src/Microsoft.AspNetCore.Razor.Language/Legacy/HtmlMarkupParser.cs b/src/Microsoft.AspNetCore.Razor.Language/Legacy/HtmlMarkupParser.cs index 39841d4835..48c79c83e1 100644 --- a/src/Microsoft.AspNetCore.Razor.Language/Legacy/HtmlMarkupParser.cs +++ b/src/Microsoft.AspNetCore.Razor.Language/Legacy/HtmlMarkupParser.cs @@ -12,6 +12,8 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy { private const string ScriptTagName = "script"; + private static readonly HtmlSymbol[] nonAllowedHtmlCommentEnding = new[] { new HtmlSymbol("-", HtmlSymbolType.Text), new HtmlSymbol("!", HtmlSymbolType.Bang), new HtmlSymbol("<", HtmlSymbolType.OpenAngle) }; + private static readonly char[] ValidAfterTypeAttributeNameCharacters = { ' ', '\t', '\r', '\n', '\f', '=' }; private SourceLocation _lastTagStart = SourceLocation.Zero; private HtmlSymbol _bufferedOpenAngle; @@ -649,16 +651,15 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy internal static bool IsCommentContentDisallowed(IEnumerable sequence) { var reversedSequence = sequence.Reverse(); - var disallowEnding = new[] { new HtmlSymbol("-", HtmlSymbolType.Text), new HtmlSymbol("!", HtmlSymbolType.Bang), new HtmlSymbol("<", HtmlSymbolType.OpenAngle) }; var index = 0; foreach (var item in reversedSequence) { - if (!item.Equals(disallowEnding[index++])) + if (!item.Equals(nonAllowedHtmlCommentEnding[index++])) { return false; } - if (index == disallowEnding.Length) + if (index == nonAllowedHtmlCommentEnding.Length) { return true; } diff --git a/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/HtmlMarkupParserTests.cs b/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/HtmlMarkupParserTests.cs index c3e0cb87e1..a40ec2e422 100644 --- a/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/HtmlMarkupParserTests.cs +++ b/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/HtmlMarkupParserTests.cs @@ -133,6 +133,16 @@ namespace Microsoft.AspNetCore.Razor.Language.Test.Legacy Assert.False(sut.IsHtmlCommentAhead()); } + [Fact] + public void IsHtmlCommentAhead_ReturnsTrueForCommentWithCodeInside() + { + // Arrange + var sut = CreateTestParserForContent("-- not closed @DateTime.Now comment-->"); + + // Act & Assert + Assert.True(sut.IsHtmlCommentAhead()); + } + [Fact] public void IsCommentContentDisallowed_ReturnsFalseForAllowedContent() { diff --git a/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/TagHelperParseTreeRewriterTest.cs b/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/TagHelperParseTreeRewriterTest.cs index 425b728716..3f37c82c4d 100644 --- a/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/TagHelperParseTreeRewriterTest.cs +++ b/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/TagHelperParseTreeRewriterTest.cs @@ -1111,7 +1111,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy [Fact] public void Rewrite_AllowsSimpleHtmlCommentsAsChildren() { - // Arrangestring documentContent, + // Arrange IEnumerable allowedChildren = new List { "b" }; string literal = "asdf"; string commentOutput = "Hello World"; @@ -1148,10 +1148,61 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy Array.Empty()); } + [Fact] + public void Rewrite_DoesntAllowSimpleHtmlCommentsAsChildrenWhenFeatureFlagIsOff() + { + // Arrange + Func nestedTagError = + (childName, parentName, allowed, location, length) => + RazorDiagnosticFactory.CreateTagHelper_InvalidNestedTag( + new SourceSpan(absoluteIndex: location, lineIndex: 0, characterIndex: location, length: length), childName, parentName, allowed); + Func nestedContentError = + (parentName, allowed, location, length) => + RazorDiagnosticFactory.CreateTagHelper_CannotHaveNonTagContent( + new SourceSpan(absoluteIndex: location, lineIndex: 0, characterIndex: location, length: length), parentName, allowed); + + IEnumerable allowedChildren = new List { "b" }; + string comment1 = "Hello"; + string expectedOutput = $"

"; + + var pTagHelperBuilder = TagHelperDescriptorBuilder + .Create("PTagHelper", "SomeAssembly") + .TagMatchingRuleDescriptor(rule => rule.RequireTagName("p")); + foreach (var childTag in allowedChildren) + { + pTagHelperBuilder.AllowChildTag(childTag); + } + + var descriptors = new TagHelperDescriptor[] + { + pTagHelperBuilder.Build() + }; + + var factory = new SpanFactory(); + var blockFactory = new BlockFactory(factory); + + var expectedMarkup = new MarkupBlock( + new MarkupTagHelperBlock("p", + blockFactory.HtmlCommentBlock(comment1))); + + // Act & Assert + EvaluateData( + descriptors, + expectedOutput, + expectedMarkup, + new[] + { + nestedContentError("p", "b", 3, 4), + nestedContentError("p", "b", 7, 5), + nestedContentError("p", "b", 12, 3), + }, + featureFlags: RazorParserFeatureFlags.Create(RazorLanguageVersion.Version_2_0)); + } + [Fact] public void Rewrite_FailsForContentWithCommentsAsChildren() { - // Arrangestring documentContent, + // Arrange Func nestedTagError = (childName, parentName, allowed, location, length) => RazorDiagnosticFactory.CreateTagHelper_InvalidNestedTag( @@ -1203,7 +1254,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy [Fact] public void Rewrite_AllowsRazorCommentsAsChildren() { - // Arrangestring documentContent, + // Arrange IEnumerable allowedChildren = new List { "b" }; string literal = "asdf"; string commentOutput = $"@*{literal}*@"; @@ -1248,7 +1299,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy [Fact] public void Rewrite_AllowsRazorMarkupInHtmlComment() { - // Arrangestring documentContent, + // Arrange IEnumerable allowedChildren = new List { "b" }; string literal = "asdf"; string part1 = "Hello "; diff --git a/test/Microsoft.AspNetCore.Razor.Language.Test/RazorParserFeatureFlagsTest.cs b/test/Microsoft.AspNetCore.Razor.Language.Test/RazorParserFeatureFlagsTest.cs index db57a33fdf..cfdb9f023d 100644 --- a/test/Microsoft.AspNetCore.Razor.Language.Test/RazorParserFeatureFlagsTest.cs +++ b/test/Microsoft.AspNetCore.Razor.Language.Test/RazorParserFeatureFlagsTest.cs @@ -16,6 +16,7 @@ namespace Microsoft.AspNetCore.Razor.Language // Assert Assert.True(context.AllowMinimizedBooleanTagHelperAttributes); + Assert.True(context.AllowHtmlCommentsInTagHelpers); } [Fact] @@ -26,6 +27,7 @@ namespace Microsoft.AspNetCore.Razor.Language // Assert Assert.False(context.AllowMinimizedBooleanTagHelperAttributes); + Assert.False(context.AllowHtmlCommentsInTagHelpers); } } } From ae94c3c4524dda877fd4c3d3b893157c314f0435 Mon Sep 17 00:00:00 2001 From: Artak Mkrtchyan Date: Fri, 9 Mar 2018 18:18:16 -0800 Subject: [PATCH 34/34] - Updated naming for methods to be more intuitive. - Added extra tests for clarity - Added extra comments to clarify complex areas --- .../Legacy/BlockKindInternal.cs | 1 - .../Legacy/HtmlMarkupParser.cs | 40 +++++++++++-------- .../Legacy/HtmlSymbol.cs | 2 + .../Legacy/HtmlMarkupParserTests.cs | 40 ++++++++++++------- 4 files changed, 51 insertions(+), 32 deletions(-) diff --git a/src/Microsoft.AspNetCore.Razor.Language/Legacy/BlockKindInternal.cs b/src/Microsoft.AspNetCore.Razor.Language/Legacy/BlockKindInternal.cs index 9f9886164c..6c37427ca7 100644 --- a/src/Microsoft.AspNetCore.Razor.Language/Legacy/BlockKindInternal.cs +++ b/src/Microsoft.AspNetCore.Razor.Language/Legacy/BlockKindInternal.cs @@ -19,7 +19,6 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy // Special Comment = 8, Tag = 9, - HtmlComment = 10 } } diff --git a/src/Microsoft.AspNetCore.Razor.Language/Legacy/HtmlMarkupParser.cs b/src/Microsoft.AspNetCore.Razor.Language/Legacy/HtmlMarkupParser.cs index 48c79c83e1..a23adbb28f 100644 --- a/src/Microsoft.AspNetCore.Razor.Language/Legacy/HtmlMarkupParser.cs +++ b/src/Microsoft.AspNetCore.Razor.Language/Legacy/HtmlMarkupParser.cs @@ -12,7 +12,8 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy { private const string ScriptTagName = "script"; - private static readonly HtmlSymbol[] nonAllowedHtmlCommentEnding = new[] { new HtmlSymbol("-", HtmlSymbolType.Text), new HtmlSymbol("!", HtmlSymbolType.Bang), new HtmlSymbol("<", HtmlSymbolType.OpenAngle) }; + private static readonly HtmlSymbol[] nonAllowedHtmlCommentEnding = new[] { HtmlSymbol.Hyphen, new HtmlSymbol("!", HtmlSymbolType.Bang), new HtmlSymbol("<", HtmlSymbolType.OpenAngle) }; + private static readonly HtmlSymbol[] singleHyphenArray = new[] { HtmlSymbol.Hyphen }; private static readonly char[] ValidAfterTypeAttributeNameCharacters = { ' ', '\t', '\r', '\n', '\f', '=' }; private SourceLocation _lastTagStart = SourceLocation.Zero; @@ -506,7 +507,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy while (!EndOfFile) { SkipToAndParseCode(HtmlSymbolType.DoubleHyphen); - var lastDoubleHyphen = AcceptAllButLastDoubleHypens(); + var lastDoubleHyphen = AcceptAllButLastDoubleHyphens(); if (At(HtmlSymbolType.CloseAngle)) { @@ -543,7 +544,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy return false; } - protected HtmlSymbol AcceptAllButLastDoubleHypens() + protected HtmlSymbol AcceptAllButLastDoubleHyphens() { var lastDoubleHyphen = CurrentSymbol; AcceptWhile(s => @@ -558,9 +559,8 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy }); NextToken(); - EnsureCurrent(); - if (At(HtmlSymbolType.Text) && IsDashSymbol(CurrentSymbol)) + if (At(HtmlSymbolType.Text) && IsHyphen(CurrentSymbol)) { // Doing this here to maintain the order of symbols if (!NextIs(HtmlSymbolType.CloseAngle)) @@ -575,9 +575,9 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy return lastDoubleHyphen; } - internal static bool IsDashSymbol(HtmlSymbol symbol) + internal static bool IsHyphen(HtmlSymbol symbol) { - return string.Equals(symbol.Content, "-", StringComparison.Ordinal); + return symbol.Equals(HtmlSymbol.Hyphen); } protected bool IsHtmlCommentAhead() @@ -589,7 +589,10 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy * 1. The string "", or "--!>" + * 2.2 nor contain the strings + * 2.2.1 "" // As we will be treating this as a comment ending, there is no need to handle this case at all. + * 2.2.3 "--!>" * 2.3 nor end with the string "" * @@ -601,37 +604,42 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy } // Check condition 2.1 - if (NextIs(HtmlSymbolType.CloseAngle) || NextIs(next => IsDashSymbol(next) && NextIs(HtmlSymbolType.CloseAngle))) + if (NextIs(HtmlSymbolType.CloseAngle) || NextIs(next => IsHyphen(next) && NextIs(HtmlSymbolType.CloseAngle))) { return false; } // Check condition 2.2 var isValidComment = false; - LookaheadUntil((s, p) => + LookaheadUntil((symbol, prevSymbols) => { - if (s.Type == HtmlSymbolType.DoubleHyphen) + if (symbol.Type == HtmlSymbolType.DoubleHyphen) { if (NextIs(HtmlSymbolType.CloseAngle)) { // Check condition 2.3: We're at the end of a comment. Check to make sure the text ending is allowed. - isValidComment = !IsCommentContentDisallowed(p); + isValidComment = !IsCommentContentEndingInvalid(prevSymbols); return true; } - else if (NextIs(ns => IsDashSymbol(ns) && NextIs(HtmlSymbolType.CloseAngle))) + else if (NextIs(ns => IsHyphen(ns) && NextIs(HtmlSymbolType.CloseAngle))) { - // This is also a valid closing comment case, as the dashes lookup is treated with DoubleHyphen symbols first. + // Check condition 2.3: we're at the end of a comment, which has an extra dash. + // Need to treat the dash as part of the content and check the ending. + // However, that case would have already been checked as part of check from 2.2.1 which + // would already fail this iteration and we wouldn't get here isValidComment = true; return true; } else if (NextIs(ns => ns.Type == HtmlSymbolType.Bang && NextIs(HtmlSymbolType.CloseAngle))) { + // This is condition 2.2.3 isValidComment = false; return true; } } - else if (s.Type == HtmlSymbolType.OpenAngle) + else if (symbol.Type == HtmlSymbolType.OpenAngle) { + // Checking condition 2.2.1 if (NextIs(ns => ns.Type == HtmlSymbolType.Bang && NextIs(HtmlSymbolType.DoubleHyphen))) { isValidComment = false; @@ -648,7 +656,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy /// /// Verifies, that the sequence doesn't end with the "<!-" HtmlSymbols. Note, the first symbol is an opening bracket symbol /// - internal static bool IsCommentContentDisallowed(IEnumerable sequence) + internal static bool IsCommentContentEndingInvalid(IEnumerable sequence) { var reversedSequence = sequence.Reverse(); var index = 0; diff --git a/src/Microsoft.AspNetCore.Razor.Language/Legacy/HtmlSymbol.cs b/src/Microsoft.AspNetCore.Razor.Language/Legacy/HtmlSymbol.cs index f41f323d1d..217e293704 100644 --- a/src/Microsoft.AspNetCore.Razor.Language/Legacy/HtmlSymbol.cs +++ b/src/Microsoft.AspNetCore.Razor.Language/Legacy/HtmlSymbol.cs @@ -8,6 +8,8 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy { internal class HtmlSymbol : SymbolBase { + internal static readonly HtmlSymbol Hyphen = new HtmlSymbol("-", HtmlSymbolType.Text); + public HtmlSymbol(string content, HtmlSymbolType type) : base(content, type, RazorDiagnostic.EmptyArray) { diff --git a/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/HtmlMarkupParserTests.cs b/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/HtmlMarkupParserTests.cs index a40ec2e422..cc12a33914 100644 --- a/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/HtmlMarkupParserTests.cs +++ b/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/HtmlMarkupParserTests.cs @@ -24,23 +24,23 @@ namespace Microsoft.AspNetCore.Razor.Language.Test.Legacy [Theory] [MemberData(nameof(NonDashSymbols))] - public void IsDashSymbol_ReturnsFalseForNonDashSymbol(object symbol) + public void IsHyphen_ReturnsFalseForNonDashSymbol(object symbol) { // Arrange var convertedSymbol = (HtmlSymbol)symbol; // Act & Assert - Assert.False(HtmlMarkupParser.IsDashSymbol(convertedSymbol)); + Assert.False(HtmlMarkupParser.IsHyphen(convertedSymbol)); } [Fact] - public void IsDashSymbol_ReturnsTrueForADashSymbol() + public void IsHyphen_ReturnsTrueForADashSymbol() { // Arrange var dashSymbol = new HtmlSymbol("-", HtmlSymbolType.Text); // Act & Assert - Assert.True(HtmlMarkupParser.IsDashSymbol(dashSymbol)); + Assert.True(HtmlMarkupParser.IsHyphen(dashSymbol)); } [Fact] @@ -50,7 +50,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Test.Legacy var sut = CreateTestParserForContent("-->"); // Act - var symbol = sut.AcceptAllButLastDoubleHypens(); + var symbol = sut.AcceptAllButLastDoubleHyphens(); // Assert Assert.Equal(doubleHyphenSymbol, symbol); @@ -65,12 +65,12 @@ namespace Microsoft.AspNetCore.Razor.Language.Test.Legacy var sut = CreateTestParserForContent("--->"); // Act - var symbol = sut.AcceptAllButLastDoubleHypens(); + var symbol = sut.AcceptAllButLastDoubleHyphens(); // Assert Assert.Equal(doubleHyphenSymbol, symbol); Assert.True(sut.At(HtmlSymbolType.CloseAngle)); - Assert.True(HtmlMarkupParser.IsDashSymbol(sut.PreviousSymbol)); + Assert.True(HtmlMarkupParser.IsHyphen(sut.PreviousSymbol)); } [Fact] @@ -103,6 +103,16 @@ namespace Microsoft.AspNetCore.Razor.Language.Test.Legacy Assert.True(sut.IsHtmlCommentAhead()); } + [Fact] + public void IsHtmlCommentAhead_ReturnsFalseForContentWithBadEndingAndExtraDash() + { + // Arrange + var sut = CreateTestParserForContent("-- Some comment content in here "); + + // Act & Assert + Assert.False(sut.IsHtmlCommentAhead()); + } + [Fact] public void IsHtmlCommentAhead_ReturnsTrueForValidCommentTagWithExtraInfoAfter() { @@ -144,36 +154,36 @@ namespace Microsoft.AspNetCore.Razor.Language.Test.Legacy } [Fact] - public void IsCommentContentDisallowed_ReturnsFalseForAllowedContent() + public void IsCommentContentEndingInvalid_ReturnsFalseForAllowedContent() { // Arrange var expectedSymbol1 = new HtmlSymbol("a", HtmlSymbolType.Text); var sequence = Enumerable.Range((int)'a', 26).Select(item => new HtmlSymbol(((char)item).ToString(), HtmlSymbolType.Text)); // Act & Assert - Assert.False(HtmlMarkupParser.IsCommentContentDisallowed(sequence)); + Assert.False(HtmlMarkupParser.IsCommentContentEndingInvalid(sequence)); } [Fact] - public void IsCommentContentDisallowed_ReturnsTrueForDisallowedContent() + public void IsCommentContentEndingInvalid_ReturnsTrueForDisallowedContent() { // Arrange var expectedSymbol1 = new HtmlSymbol("a", HtmlSymbolType.Text); var sequence = new[] { new HtmlSymbol("<", HtmlSymbolType.OpenAngle), new HtmlSymbol("!", HtmlSymbolType.Bang), new HtmlSymbol("-", HtmlSymbolType.Text) }; // Act & Assert - Assert.True(HtmlMarkupParser.IsCommentContentDisallowed(sequence)); + Assert.True(HtmlMarkupParser.IsCommentContentEndingInvalid(sequence)); } [Fact] - public void IsCommentContentDisallowed_ReturnsFalseForEmptyContent() + public void IsCommentContentEndingInvalid_ReturnsFalseForEmptyContent() { // Arrange var expectedSymbol1 = new HtmlSymbol("a", HtmlSymbolType.Text); var sequence = Array.Empty(); // Act & Assert - Assert.False(HtmlMarkupParser.IsCommentContentDisallowed(sequence)); + Assert.False(HtmlMarkupParser.IsCommentContentEndingInvalid(sequence)); } private class TestHtmlMarkupParser : HtmlMarkupParser @@ -193,9 +203,9 @@ namespace Microsoft.AspNetCore.Razor.Language.Test.Legacy this.EnsureCurrent(); } - public new HtmlSymbol AcceptAllButLastDoubleHypens() + public new HtmlSymbol AcceptAllButLastDoubleHyphens() { - return base.AcceptAllButLastDoubleHypens(); + return base.AcceptAllButLastDoubleHyphens(); } public override void BuildSpan(SpanBuilder span, SourceLocation start, string content)