New baselines for TagHelperParseTreeRewriterTest #2639

ajaybhargavb · 2018-10-09T19:37:37Z

Part of #2584

Converted all of the TagHelperParseTreeRewriter tests. I understand it is impossible to look through each and every test. I suggest looking at as many different types of test as possible.

I'm only including the test changes here because the source may change a lot and it isn't worth reviewing at this point. If anyone is interested the source is here.

Here is the syntax for tag helpers,

  <Node Name="MarkupTagHelperElementSyntax" Base="MarkupSyntaxNode">
    <Kind Name="MarkupTagHelperElement" />
    <Field Name="StartTag" Type="MarkupTagHelperStartTagSyntax" />
    <Field Name="Body" Type="SyntaxList&lt;RazorSyntaxNode&gt;" Optional="true" />
    <Field Name="EndTag" Type="MarkupTagHelperEndTagSyntax" Optional="true" />
  </Node>
  <Node Name="MarkupTagHelperStartTagSyntax" Base="RazorBlockSyntax">
    <Kind Name="MarkupTagHelperStartTag" />
    <Field Name="Children" Type="SyntaxList&lt;RazorSyntaxNode&gt;" Override="true" />
  </Node>
  <Node Name="MarkupTagHelperEndTagSyntax" Base="RazorBlockSyntax">
    <Kind Name="MarkupTagHelperEndTag" />
    <Field Name="Children" Type="SyntaxList&lt;RazorSyntaxNode&gt;" Override="true" />
  </Node>
  <Node Name="MarkupTagHelperAttributeSyntax" Base="MarkupSyntaxNode">
    <Kind Name="MarkupTagHelperAttribute" />
    <Field Name="NamePrefix" Type="MarkupTextLiteralSyntax" Optional="true" />
    <Field Name="Name" Type="MarkupTextLiteralSyntax" />
    <Field Name="NameSuffix" Type="MarkupTextLiteralSyntax" Optional="true" />
    <Field Name="EqualsToken" Type="SyntaxToken">
      <Kind Name="Equals" />
    </Field>
    <Field Name="ValuePrefix" Type="MarkupTextLiteralSyntax" Optional="true" />
    <Field Name="Value" Type="MarkupTagHelperAttributeValueSyntax" />
    <Field Name="ValueSuffix" Type="MarkupTextLiteralSyntax" Optional="true" />
  </Node>
  <Node Name="MarkupMinimizedTagHelperAttributeSyntax" Base="MarkupSyntaxNode">
    <Kind Name="MarkupMinimizedTagHelperAttribute" />
    <Field Name="NamePrefix" Type="MarkupTextLiteralSyntax" Optional="true" />
    <Field Name="Name" Type="MarkupTextLiteralSyntax" />
  </Node>
  <Node Name="MarkupTagHelperAttributeValueSyntax" Base="RazorBlockSyntax">
    <Kind Name="MarkupTagHelperAttributeValue" />
    <Field Name="Children" Type="SyntaxList&lt;RazorSyntaxNode&gt;" Override="true" />
  </Node>

ajaybhargavb · 2018-10-09T19:38:01Z

test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/TagHelperParseTreeRewriterTest.cs

@@ -16,15 +16,6 @@ public TagHelperParseTreeRewriterTest()
            UseNewSyntaxTree = true;
        }

-        [Fact]


Ignore. Just something I added in a previous commit.

ajaybhargavb · 2018-10-09T19:40:57Z

...st/TestFiles/ParserTests/TagHelperParseTreeRewriterTest/AllowsPrefixedTagHelpers10.stree.txt

+                Text;[and];
+                Whitespace;[ ];
+                Text;[spaces];
+            MarkupTagHelperEndTag - [37..47)::10


This is the main structural change. Tag helper end tags are now well represented in the tree.
I am currently not generating the corresponding classified spans for this to keep them consistent with the old tree. But ideally we should generate those. We should discuss this at some point. I've added it to the tracking list https://github.com/aspnet/Razor/issues/2619. @rynowak this is what I was trying to explain earlier.

If the behaviour isn't changing right now, then that sounds great!

ajaybhargavb · 2018-10-09T19:45:06Z

...st/TestFiles/ParserTests/TagHelperParseTreeRewriterTest/AllowsPrefixedTagHelpers10.stree.txt

+                    Equals;[=];
+                    MarkupTextLiteral - [15..16)::1 - ["] - Gen<None> - SpanEditHandler;Accepts:Any
+                        DoubleQuote;["];
+                    MarkupTagHelperAttributeValue - [16..19)::3


Note, the attibute value of tag helpers use a specific type (MarkupTagHelperAttributeValue) as opposed to using a GenericBlock.

ajaybhargavb · 2018-10-09T19:47:56Z

...st/TestFiles/ParserTests/TagHelperParseTreeRewriterTest/AllowsPrefixedTagHelpers11.stree.txt

+                        DoubleQuote;["];
+                    MarkupTagHelperAttributeValue - [17..30)::13
+                        MarkupDynamicAttributeValue - [17..30)::13 - [@DateTime.Now]
+                            GenericBlock - [17..30)::13


As we discussed earlier, we will get rid of GenericBlock at some point. I've added that to the feedback list https://github.com/aspnet/Razor/issues/2619

ajaybhargavb · 2018-10-09T19:53:12Z

...iles/ParserTests/TagHelperParseTreeRewriterTest/DoesNotRewriteSpecialTagTagHelpers1.diag.txt

+(0,0): Error RZ3008: Tag helpers cannot target tag name '?xml' because it contains a '?' character.
+(0,0): Error RZ3008: Tag helpers cannot target tag name '![CDATA[' because it contains a '!' character.
+(0,0): Error RZ3008: Tag helpers cannot target tag name '![CDATA[' because it contains a '[' character.
+(0,0): Error RZ3008: Tag helpers cannot target tag name '!DOCTYPE' because it contains a '!' character.


This is not a bug. These are the errors generated when the tag helper descriptors are found. Previously, TagHelperParseTreeRewriter was not responsible for combing these errors so test code never saw that. But I had to refactor some of that code and now the errors are combined in TagHelperParseTreeRewriter. Hence these errors show up.

ajaybhargavb · 2018-10-09T19:56:24Z

...st/TestFiles/ParserTests/TagHelperParseTreeRewriterTest/AllowsPrefixedTagHelpers11.stree.txt

+                                            Transition;[@];
+                                        CSharpImplicitExpressionBody - [18..30)::12
+                                            CSharpCodeBlock - [18..30)::12
+                                                CSharpExpressionLiteral - [18..30)::12 - [DateTime.Now] - Gen<Expr> - ImplicitExpressionEditHandler;Accepts:AnyExceptNewline;ImplicitExpression[ATD];K14


This is an example of where we rewrite the EditHandler and AcceptedCharacters for a bound non-string attribute. Before the rewrite the AcceptedCharacters for this would be NonWhitespace.

rynowak · 2018-10-09T20:45:37Z

...ewriterTest/InvalidStructure_UnderstandsTHPrefixAndAllowedChildrenAndRequireParent.stree.txt

+                    OpenAngle;[<];
+                    ForwardSlash;[/];
+                    Text;[th:p];
+                    CloseAngle;[>];


So here's an example of how this syntax representation is SO MUCH BETTER and more accurate than what we had before. 👍

rynowak

I did a random sampling of this, and WOW so much better. I want @NTaylorMullen to look at this as well.

NTaylorMullen

Super dope, looking sweet man.

NTaylorMullen · 2018-10-09T21:45:53Z

...st/TestFiles/ParserTests/TagHelperParseTreeRewriterTest/AllowsPrefixedTagHelpers10.stree.txt

+                MarkupTextLiteral - [0..8)::8 - [<th:myth] - Gen<Markup> - SpanEditHandler;Accepts:Any
+                    OpenAngle;[<];
+                    Text;[th:myth];
+                MarkupTagHelperAttribute - [8..20)::12 - class - DoubleQuotes


Should spit out the content of this like you do in MarkupTextLiteral

ajaybhargavb commented Oct 9, 2018

View reviewed changes

ajaybhargavb requested review from NTaylorMullen and rynowak October 9, 2018 19:43

ajaybhargavb assigned rynowak and NTaylorMullen Oct 9, 2018

ajaybhargavb commented Oct 9, 2018

View reviewed changes

rynowak reviewed Oct 9, 2018

View reviewed changes

rynowak approved these changes Oct 9, 2018

View reviewed changes

NTaylorMullen approved these changes Oct 9, 2018

View reviewed changes

New baselines for TagHelperParseTreeRewriterTest

25c7a17

ajaybhargavb force-pushed the ajbaaska/th-parsetree-rewriter-test branch from 23ad206 to 25c7a17 Compare October 9, 2018 22:43

ajaybhargavb merged commit 25c7a17 into ajbaaska/taghelper-parser Oct 9, 2018

ajaybhargavb deleted the ajbaaska/th-parsetree-rewriter-test branch October 9, 2018 22:46

ajaybhargavb mentioned this pull request Oct 12, 2018

New baselines for TagHelperBlockRewriterTest #2646

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

New baselines for TagHelperParseTreeRewriterTest #2639

New baselines for TagHelperParseTreeRewriterTest #2639

Uh oh!

ajaybhargavb commented Oct 9, 2018 •

edited

Loading

Uh oh!

ajaybhargavb Oct 9, 2018

Uh oh!

ajaybhargavb Oct 9, 2018 •

edited

Loading

Uh oh!

rynowak Oct 9, 2018

Uh oh!

ajaybhargavb Oct 9, 2018

Uh oh!

ajaybhargavb Oct 9, 2018 •

edited

Loading

Uh oh!

ajaybhargavb Oct 9, 2018

Uh oh!

ajaybhargavb Oct 9, 2018

Uh oh!

rynowak Oct 9, 2018

Uh oh!

rynowak left a comment

Uh oh!

NTaylorMullen left a comment

Uh oh!

NTaylorMullen Oct 9, 2018

Uh oh!

Uh oh!

New baselines for TagHelperParseTreeRewriterTest #2639

New baselines for TagHelperParseTreeRewriterTest #2639

Uh oh!

Conversation

ajaybhargavb commented Oct 9, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ajaybhargavb Oct 9, 2018

Choose a reason for hiding this comment

Uh oh!

ajaybhargavb Oct 9, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rynowak Oct 9, 2018

Choose a reason for hiding this comment

Uh oh!

ajaybhargavb Oct 9, 2018

Choose a reason for hiding this comment

Uh oh!

ajaybhargavb Oct 9, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ajaybhargavb Oct 9, 2018

Choose a reason for hiding this comment

Uh oh!

ajaybhargavb Oct 9, 2018

Choose a reason for hiding this comment

Uh oh!

rynowak Oct 9, 2018

Choose a reason for hiding this comment

Uh oh!

rynowak left a comment

Choose a reason for hiding this comment

Uh oh!

NTaylorMullen left a comment

Choose a reason for hiding this comment

Uh oh!

NTaylorMullen Oct 9, 2018

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ajaybhargavb commented Oct 9, 2018 •

edited

Loading

ajaybhargavb Oct 9, 2018 •

edited

Loading

ajaybhargavb Oct 9, 2018 •

edited

Loading