fix: element comparer includes toggle to enforce check of closing tag

Author: egil

Diff for: CHANGELOG.md

@@ -1,3 +1,7 @@
+# 0.18.1
+
+- Fixed element comparer such that it can strictly check if the closing tags in the source markup is the same.
+ 
 # 0.18.0
 
 - Added a new comparer, which ensures element tags are closed the same way, e.g. `<br> and <br />` would not be considered equal, but `<br>` and `<br>` would be.

Diff for: src/AngleSharp.Diffing.Tests/Strategies/DiffingStrategyPipelineTest.cs

@@ -0,0 +1,53 @@
+namespace AngleSharp.Diffing.Strategies.ElementStrategies;
+
+public class ElementComparerTest : DiffingTestBase
+{
+    public ElementComparerTest(DiffingTestFixture fixture) : base(fixture)
+    {
+    }
+
+    [Theory(DisplayName = "When control and test nodes have the same type and name and enforceTagClosing is false, the result is Same")]
+    [InlineData("<p>", "<p />")]
+    [InlineData("<br>", "<br/>")]
+    [InlineData("<br>", "<br>")]
+    [InlineData("<p>", "<p>")]
+    public void Test001(string controlHtml, string testHtml)
+    {
+        var comparison = ToComparison(controlHtml, testHtml);
+        new ElementComparer(enforceTagClosing: false)
+            .Compare(comparison, CompareResult.Unknown)
+            .ShouldBe(CompareResult.Same);
+    }
+
+    [Theory(DisplayName = "When control and test nodes have the a different type and name, the result is Different")]
+    [InlineData("<div>", "<p>", false)]
+    [InlineData("<div>", "textnode", false)]
+    [InlineData("<div>", "<!--comment-->", false)]
+    [InlineData("<!--comment-->", "textnode", false)]
+    [InlineData("<br>", "<br/>", true)]
+    [InlineData("<input>", "<input/>", true)]
+    [InlineData("<div>", "<p>", true)]
+    [InlineData("<div>", "textnode", true)]
+    [InlineData("<div>", "<!--comment-->", true)]
+    [InlineData("<!--comment-->", "textnode", true)]
+    public void Test002(string controlHtml, string testHtml, bool enforceTagClosing)
+    {
+        var comparison = ToComparison(controlHtml, testHtml);
+        new ElementComparer(enforceTagClosing)
+            .Compare(comparison, CompareResult.Unknown)
+            .ShouldBe(CompareResult.Different);
+    }
+
+    [Theory(DisplayName = "When unknown node is used in comparison, but node name is equal, the result is Same")]
+    [InlineData("<svg><path></path></svg>", "<path/>")]
+    public void HandleUnknownNodeDuringComparison(string controlHtml, string testHtml)
+    {
+        var knownNode = ToNode(controlHtml).FirstChild.ToComparisonSource(0, ComparisonSourceType.Control);
+        var unknownNode = ToNode(testHtml).ToComparisonSource(0, ComparisonSourceType.Test);
+        var comparison = new Comparison(knownNode, unknownNode);
+
+        new ElementComparer(enforceTagClosing: false)
+            .Compare(comparison, CompareResult.Unknown)
+            .ShouldBe(CompareResult.Same);
+    }
+}

Diff for: src/AngleSharp.Diffing.Tests/Strategies/ElementStrategies/ElementClosingComparerTest.cs

@@ -5,24 +5,19 @@ namespace AngleSharp.Diffing;
 
 public static partial class DiffingStrategyPipelineBuilderExtensions
 {
+    /// <summary>
+    /// Enables the basic element comparer, that checks if two nodes are element nodes and have the same name.
+    /// </summary>
+    /// <param name="builder">The collection to add the comparer to.</param>
+    /// <param name="enforceTagClosing">
+    /// Whether or not the closing style of
+    /// an element is considered during comparison.
+    /// </param>
+    public static IDiffingStrategyCollection AddElementComparer(this IDiffingStrategyCollection builder, bool enforceTagClosing)
     {
-        /// <summary>
-        /// Enables the basic element comparer, that checks if two nodes are element nodes and have the same name.
-        /// </summary>
-        public static IDiffingStrategyCollection AddElementComparer(this IDiffingStrategyCollection builder)
-        {
-            builder.AddComparer(ElementComparer.Compare, StrategyType.Generalized);
-            return builder;
-        }
-
-        /// <summary>
-        /// Adds an element comparer that ensures element tags are closed the same way, e.g. `&lt;br&gt;` and `&lt;br /&gt;` would not be considered equal, but `&lt;br&gt;` and `&lt;br&gt;` would be.
-        /// </summary>
-        public static IDiffingStrategyCollection AddElementClosingComparer(this IDiffingStrategyCollection builder)
-        {
-            builder.AddComparer(ElementClosingComparer.Compare, StrategyType.Generalized);
-            return builder;
-        }
+        builder.AddComparer(new ElementComparer(enforceTagClosing).Compare, StrategyType.Generalized);
+        return builder;
+    }
 
     /// <summary>
     /// Enables the CSS-selector matcher strategy during diffing.

Diff for: src/AngleSharp.Diffing.Tests/Strategies/ElementStrategies/ElementComparerTest.cs

@@ -1,23 +1,55 @@
-using AngleSharp.Diffing.Core;
-using AngleSharp.Dom;
+using AngleSharp.Html.Parser.Tokens;
 
-namespace AngleSharp.Diffing.Strategies.ElementStrategies
+namespace AngleSharp.Diffing.Strategies.ElementStrategies;
+
+/// <summary>
+/// Represents the element comparer strategy.
+/// </summary>
+public class ElementComparer
 {
     /// <summary>
-    /// Represents the element comparer strategy.
+    /// Gets whether or not the closing style of
+    /// an element is considered during comparison.
+    /// </summary>
+    public bool EnforceTagClosing { get; }
+
+    /// <summary>
+    /// Creates an instance of the <see cref="ElementComparer"/>.
+    /// </summary>
+    /// <param name="enforceTagClosing">
+    /// Whether or not the closing style of
+    /// an element is considered during comparison.
+    /// </param>
+    public ElementComparer(bool enforceTagClosing)
+    {
+        EnforceTagClosing = enforceTagClosing;
+    }
+
+    /// <summary>
+    /// The element comparer strategy.
     /// </summary>
-    public static class ElementComparer
+    public CompareResult Compare(in Comparison comparison, CompareResult currentDecision)
     {
-        /// <summary>
-        /// The element comparer strategy.
-        /// </summary>
-        public static CompareResult Compare(in Comparison comparison, CompareResult currentDecision)
+        if (currentDecision.IsSameOrSkip())
+            return currentDecision;
+
+        var result = comparison.Control.Node.NodeType == NodeType.Element && comparison.AreNodeTypesEqual
+            ? CompareResult.Same
+            : CompareResult.Different;
+
+        if (EnforceTagClosing && result == CompareResult.Same)
         {
-            if (currentDecision.IsSameOrSkip())
-                return currentDecision;
-            return comparison.Control.Node.NodeType == NodeType.Element && comparison.AreNodeTypesEqual
-                ? CompareResult.Same
+            if (comparison.Test.Node is not IElement testElement || testElement.SourceReference is not HtmlTagToken testTag)
+                throw new InvalidOperationException("No source reference attached to test element, cannot determine element tag closing style.");
+
+            if (comparison.Control.Node is not IElement controlElement || controlElement.SourceReference is not HtmlTagToken controlTag)
+                throw new InvalidOperationException("No source reference attached to test element, cannot determine element tag closing style.");
+
+            return testTag.IsSelfClosing == controlTag.IsSelfClosing
+                ? result
                 : CompareResult.Different;
         }
+
+        return result;
     }
 }