From 1c9c74c801a605554826703382ee425cf1b2692e Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Tue, 30 Jan 2018 15:36:17 +0000 Subject: [PATCH] In RazorCompiler, fix bug about attribute nodes having wrong sequence numbers. Make tests explicit about sequence numbers. --- .../Engine/BlazorIntermediateNodeWriter.cs | 15 +-- .../RazorCompilerTest.cs | 97 +++++++++++-------- test/shared/AssertNode.cs | 36 +++++-- 3 files changed, 88 insertions(+), 60 deletions(-) diff --git a/src/Microsoft.AspNetCore.Blazor.Build/Core/RazorCompilation/Engine/BlazorIntermediateNodeWriter.cs b/src/Microsoft.AspNetCore.Blazor.Build/Core/RazorCompilation/Engine/BlazorIntermediateNodeWriter.cs index cda515963f..d6de896b0a 100644 --- a/src/Microsoft.AspNetCore.Blazor.Build/Core/RazorCompilation/Engine/BlazorIntermediateNodeWriter.cs +++ b/src/Microsoft.AspNetCore.Blazor.Build/Core/RazorCompilation/Engine/BlazorIntermediateNodeWriter.cs @@ -36,13 +36,11 @@ namespace Microsoft.AspNetCore.Blazor.Build.Core.RazorCompilation.Engine private struct PendingAttribute { - public int SourceSequence; public object AttributeValue; } private struct PendingAttributeToken { - public int SourceSequence; public IntermediateToken AttributeValue; } @@ -114,7 +112,6 @@ namespace Microsoft.AspNetCore.Blazor.Build.Core.RazorCompilation.Engine var token = (IntermediateToken)node.Children.Single(); _currentElementAttributeTokens.Add(new PendingAttributeToken { - SourceSequence = _sourceSequence++, AttributeValue = token }); return; @@ -165,12 +162,10 @@ namespace Microsoft.AspNetCore.Blazor.Build.Core.RazorCompilation.Engine public override void WriteHtmlAttribute(CodeRenderingContext context, HtmlAttributeIntermediateNode node) { - var attributeSourceSequence = _sourceSequence++; _currentAttributeValues = new List(); context.RenderChildren(node); _currentElementAttributes[node.AttributeName] = new PendingAttribute { - SourceSequence = attributeSourceSequence, AttributeValue = _currentAttributeValues }; _currentAttributeValues = null; @@ -249,14 +244,14 @@ namespace Microsoft.AspNetCore.Blazor.Build.Core.RazorCompilation.Engine foreach (var attribute in nextTag.Attributes) { - WriteAttribute(codeWriter, _sourceSequence++, attribute.Key, attribute.Value); + WriteAttribute(codeWriter, attribute.Key, attribute.Value); } if (_currentElementAttributes.Count > 0) { foreach (var pair in _currentElementAttributes) { - WriteAttribute(codeWriter, pair.Value.SourceSequence, pair.Key, pair.Value.AttributeValue); + WriteAttribute(codeWriter, pair.Key, pair.Value.AttributeValue); } _currentElementAttributes.Clear(); } @@ -267,7 +262,7 @@ namespace Microsoft.AspNetCore.Blazor.Build.Core.RazorCompilation.Engine { codeWriter .WriteStartMethodInvocation($"{builderVarName}.{nameof(RenderTreeBuilder.AddAttribute)}") - .Write(token.SourceSequence.ToString()) + .Write((_sourceSequence++).ToString()) .WriteParameterSeparator() .Write(token.AttributeValue.Content) .WriteEndMethodInvocation(); @@ -330,11 +325,11 @@ namespace Microsoft.AspNetCore.Blazor.Build.Core.RazorCompilation.Engine } } - private static void WriteAttribute(CodeWriter codeWriter, int sourceSequence, string key, object value) + private void WriteAttribute(CodeWriter codeWriter, string key, object value) { codeWriter .WriteStartMethodInvocation($"{builderVarName}.{nameof(RenderTreeBuilder.AddAttribute)}") - .Write(sourceSequence.ToString()) + .Write((_sourceSequence++).ToString()) .WriteParameterSeparator() .WriteStringLiteral(key) .WriteParameterSeparator(); diff --git a/test/Microsoft.AspNetCore.Blazor.Build.Test/RazorCompilerTest.cs b/test/Microsoft.AspNetCore.Blazor.Build.Test/RazorCompilerTest.cs index 823997fb94..8c47c253bb 100644 --- a/test/Microsoft.AspNetCore.Blazor.Build.Test/RazorCompilerTest.cs +++ b/test/Microsoft.AspNetCore.Blazor.Build.Test/RazorCompilerTest.cs @@ -97,7 +97,7 @@ namespace Microsoft.AspNetCore.Blazor.Build.Test // Assert Assert.Collection(treeBuilder.GetNodes(), - node => AssertNode.Text(node, "Some plain text")); + node => AssertNode.Text(node, "Some plain text", 0)); } [Fact] @@ -112,11 +112,17 @@ namespace Microsoft.AspNetCore.Blazor.Build.Test "); // Assert - var nodes = GetRenderTree(component).Where(NotWhitespace); + var nodes = GetRenderTree(component); Assert.Collection(nodes, - node => AssertNode.Text(node, "Hello"), - node => AssertNode.Text(node, "123"), - node => AssertNode.Text(node, new object().ToString())); + node => AssertNode.Whitespace(node, 0), + node => AssertNode.Text(node, "Hello", 1), + node => AssertNode.Whitespace(node, 2), + node => AssertNode.Whitespace(node, 3), // @((object)null) + node => AssertNode.Whitespace(node, 4), + node => AssertNode.Text(node, "123", 5), + node => AssertNode.Whitespace(node, 6), + node => AssertNode.Text(node, new object().ToString(), 7), + node => AssertNode.Whitespace(node, 8)); } [Fact] @@ -133,11 +139,14 @@ namespace Microsoft.AspNetCore.Blazor.Build.Test "); // Assert - var nodes = GetRenderTree(component).Where(NotWhitespace); + var nodes = GetRenderTree(component); Assert.Collection(nodes, - node => AssertNode.Text(node, "First"), - node => AssertNode.Text(node, "Second"), - node => AssertNode.Text(node, "Third")); + node => AssertNode.Whitespace(node, 0), + node => AssertNode.Text(node, "First", 1), + node => AssertNode.Text(node, "Second", 1), + node => AssertNode.Text(node, "Third", 1), + node => AssertNode.Whitespace(node, 2), + node => AssertNode.Whitespace(node, 3)); } [Fact] @@ -148,8 +157,8 @@ namespace Microsoft.AspNetCore.Blazor.Build.Test // Assert Assert.Collection(GetRenderTree(component), - node => AssertNode.Element(node, "myelem", 1), - node => AssertNode.Text(node, "Hello")); + node => AssertNode.Element(node, "myelem", 1, 0), + node => AssertNode.Text(node, "Hello", 1)); } [Fact] @@ -160,8 +169,8 @@ namespace Microsoft.AspNetCore.Blazor.Build.Test // Assert Assert.Collection(GetRenderTree(component), - node => AssertNode.Text(node, "Some text so elem isn't at position 0 "), - node => AssertNode.Element(node, "myelem", 1)); + node => AssertNode.Text(node, "Some text so elem isn't at position 0 ", 0), + node => AssertNode.Element(node, "myelem", 1, 1)); } [Fact] @@ -172,8 +181,8 @@ namespace Microsoft.AspNetCore.Blazor.Build.Test // Assert Assert.Collection(GetRenderTree(component), - node => AssertNode.Text(node, "Some text so elem isn't at position 0 "), - node => AssertNode.Element(node, "img", 1)); + node => AssertNode.Text(node, "Some text so elem isn't at position 0 ", 0), + node => AssertNode.Element(node, "img", 1, 1)); } [Fact] @@ -184,9 +193,9 @@ namespace Microsoft.AspNetCore.Blazor.Build.Test // Assert Assert.Collection(GetRenderTree(component), - node => AssertNode.Element(node, "elem", 2), - node => AssertNode.Attribute(node, "attrib-one", "Value 1"), - node => AssertNode.Attribute(node, "a2", "v2")); + node => AssertNode.Element(node, "elem", 2, 0), + node => AssertNode.Attribute(node, "attrib-one", "Value 1", 1), + node => AssertNode.Attribute(node, "a2", "v2", 2)); } [Fact] @@ -199,8 +208,8 @@ namespace Microsoft.AspNetCore.Blazor.Build.Test // Assert Assert.Collection(GetRenderTree(component), - node => AssertNode.Element(node, "elem", 1), - node => AssertNode.Attribute(node, "attr", "My string")); + node => AssertNode.Element(node, "elem", 1, 0), + node => AssertNode.Attribute(node, "attr", "My string", 1)); } [Fact] @@ -213,8 +222,8 @@ namespace Microsoft.AspNetCore.Blazor.Build.Test // Assert Assert.Collection(GetRenderTree(component), - node => AssertNode.Element(node, "elem", 1), - node => AssertNode.Attribute(node, "attr", "123")); + node => AssertNode.Element(node, "elem", 1, 0), + node => AssertNode.Attribute(node, "attr", "123", 1)); } [Fact] @@ -227,8 +236,8 @@ namespace Microsoft.AspNetCore.Blazor.Build.Test // Assert Assert.Collection(GetRenderTree(component), - node => AssertNode.Element(node, "elem", 1), - node => AssertNode.Attribute(node, "attr", "Hello, WORLD with number 246!")); + node => AssertNode.Element(node, "elem", 1, 0), + node => AssertNode.Attribute(node, "attr", "Hello, WORLD with number 246!", 1)); } [Fact] @@ -249,16 +258,18 @@ namespace Microsoft.AspNetCore.Blazor.Build.Test // Assert Assert.False((bool)handlerWasCalledProperty.GetValue(component)); - Assert.Collection(GetRenderTree(component).Where(NotWhitespace), - node => AssertNode.Element(node, "elem", 1), + Assert.Collection(GetRenderTree(component), + node => AssertNode.Element(node, "elem", 1, 0), node => { Assert.Equal(RenderTreeNodeType.Attribute, node.NodeType); + Assert.Equal(1, node.Sequence); Assert.NotNull(node.AttributeValue); ((UIEventHandler)node.AttributeValue)(null); Assert.True((bool)handlerWasCalledProperty.GetValue(component)); - }); + }, + node => AssertNode.Whitespace(node, 2)); } [Fact] @@ -271,19 +282,22 @@ namespace Microsoft.AspNetCore.Blazor.Build.Test public bool DidInvokeCode { get; set; } = false; }"); var didInvokeCodeProperty = component.GetType().GetProperty("DidInvokeCode"); + var nodes = GetRenderTree(component); // Assert Assert.False((bool)didInvokeCodeProperty.GetValue(component)); - Assert.Collection(GetRenderTree(component).Where(NotWhitespace), - node => AssertNode.Element(node, "elem", 1), + Assert.Collection(nodes, + node => AssertNode.Element(node, "elem", 1, 0), node => { Assert.Equal(RenderTreeNodeType.Attribute, node.NodeType); Assert.NotNull(node.AttributeValue); + Assert.Equal(1, node.Sequence); ((UIEventHandler)node.AttributeValue)(null); Assert.True((bool)didInvokeCodeProperty.GetValue(component)); - }); + }, + node => AssertNode.Whitespace(node, 2)); } [Fact] @@ -293,14 +307,15 @@ namespace Microsoft.AspNetCore.Blazor.Build.Test var treeBuilder = new RenderTreeBuilder(new TestRenderer()); // Arrange/Act - var component = CompileToComponent(@" - @using System.Collections.Generic + var component = CompileToComponent( + @"@using System.Collections.Generic @(typeof(List).FullName)"); component.BuildRenderTree(treeBuilder); // Assert - Assert.Collection(treeBuilder.GetNodes().Where(NotWhitespace), - node => AssertNode.Text(node, typeof(List).FullName)); + Assert.Collection(treeBuilder.GetNodes(), + node => AssertNode.Whitespace(node, 0), + node => AssertNode.Text(node, typeof(List).FullName, 1)); } [Fact] @@ -320,16 +335,18 @@ namespace Microsoft.AspNetCore.Blazor.Build.Test // Assert Assert.False((bool)didInvokeCodeProperty.GetValue(component)); - Assert.Collection(GetRenderTree(component).Where(NotWhitespace), - node => AssertNode.Element(node, "elem", 1), + Assert.Collection(GetRenderTree(component), + node => AssertNode.Element(node, "elem", 1, 0), node => { Assert.Equal(RenderTreeNodeType.Attribute, node.NodeType); Assert.NotNull(node.AttributeValue); + Assert.Equal(1, node.Sequence); ((UIEventHandler)node.AttributeValue)(null); Assert.True((bool)didInvokeCodeProperty.GetValue(component)); - }); + }, + node => AssertNode.Whitespace(node, 2)); } [Fact] @@ -345,13 +362,9 @@ namespace Microsoft.AspNetCore.Blazor.Build.Test // Assert Assert.Collection(treeBuilder.GetNodes(), - node => AssertNode.Component(node)); + node => AssertNode.Component(node, 0)); } - private static bool NotWhitespace(RenderTreeNode node) - => node.NodeType != RenderTreeNodeType.Text - || !string.IsNullOrWhiteSpace(node.TextContent); - private static ArrayRange GetRenderTree(IComponent component) { var treeBuilder = new RenderTreeBuilder(new TestRenderer()); diff --git a/test/shared/AssertNode.cs b/test/shared/AssertNode.cs index b8a39e6fc4..afd040832d 100644 --- a/test/shared/AssertNode.cs +++ b/test/shared/AssertNode.cs @@ -1,6 +1,7 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using System; using Microsoft.AspNetCore.Blazor.Components; using Microsoft.AspNetCore.Blazor.RenderTree; using Xunit; @@ -9,42 +10,61 @@ namespace Microsoft.AspNetCore.Blazor.Test.Shared { internal static class AssertNode { - public static void Text(RenderTreeNode node, string textContent) + public static void Sequence(RenderTreeNode node, int? sequence = null) + { + if (sequence.HasValue) + { + Assert.Equal(sequence.Value, node.Sequence); + } + } + + public static void Text(RenderTreeNode node, string textContent, int? sequence = null) { Assert.Equal(RenderTreeNodeType.Text, node.NodeType); Assert.Equal(textContent, node.TextContent); Assert.Equal(0, node.ElementDescendantsEndIndex); + AssertNode.Sequence(node, sequence); } - public static void Element(RenderTreeNode node, string elementName, int descendantsEndIndex) + public static void Element(RenderTreeNode node, string elementName, int descendantsEndIndex, int? sequence = null) { Assert.Equal(RenderTreeNodeType.Element, node.NodeType); Assert.Equal(elementName, node.ElementName); Assert.Equal(descendantsEndIndex, node.ElementDescendantsEndIndex); + AssertNode.Sequence(node, sequence); } - public static void Attribute(RenderTreeNode node, string attributeName) + public static void Attribute(RenderTreeNode node, string attributeName, int? sequence = null) { Assert.Equal(RenderTreeNodeType.Attribute, node.NodeType); Assert.Equal(attributeName, node.AttributeName); + AssertNode.Sequence(node, sequence); } - public static void Attribute(RenderTreeNode node, string attributeName, string attributeValue) + public static void Attribute(RenderTreeNode node, string attributeName, string attributeValue, int? sequence = null) { - AssertNode.Attribute(node, attributeName); + AssertNode.Attribute(node, attributeName, sequence); Assert.Equal(attributeValue, node.AttributeValue); } - public static void Attribute(RenderTreeNode node, string attributeName, UIEventHandler attributeEventHandlerValue) + public static void Attribute(RenderTreeNode node, string attributeName, UIEventHandler attributeEventHandlerValue, int? sequence = null) { - AssertNode.Attribute(node, attributeName); + AssertNode.Attribute(node, attributeName, sequence); Assert.Equal(attributeEventHandlerValue, node.AttributeValue); } - public static void Component(RenderTreeNode node) where T : IComponent + public static void Component(RenderTreeNode node, int? sequence = null) where T : IComponent { Assert.Equal(RenderTreeNodeType.Component, node.NodeType); Assert.Equal(typeof(T), node.ComponentType); + AssertNode.Sequence(node, sequence); + } + + public static void Whitespace(RenderTreeNode node, int? sequence = null) + { + Assert.Equal(RenderTreeNodeType.Text, node.NodeType); + AssertNode.Sequence(node, sequence); + Assert.True(string.IsNullOrWhiteSpace(node.TextContent)); } } }