xunit/xunit#2828: Prefer IEquatable<> over custom collection equality

xunit · Nov 28, 2023 · 6e0a7cd · 6e0a7cd
1 parent c35ef46
commit 6e0a7cd
Show file tree

Hide file tree

Showing 2 changed files with 59 additions and 14 deletions.
diff --git a/EqualityAsserts.cs b/EqualityAsserts.cs
@@ -176,7 +176,15 @@ partial class Assert
 					{
 						try
 						{
-							if (CollectionTracker.AreCollectionsEqual(expectedTracker, actualTracker, itemComparer, itemComparer == AssertEqualityComparer<T>.DefaultInnerComparer, out mismatchedIndex))
+							bool result;
+
+							// Call AssertEqualityComparer.Equals because it checks for IEquatable<> before using CollectionTracker
+							if (aec != null)
+								result = aec.Equals(expected, expectedTracker, actual, actualTracker, out mismatchedIndex);
+							else
+								result = CollectionTracker.AreCollectionsEqual(expectedTracker, actualTracker, itemComparer, itemComparer == AssertEqualityComparer<T>.DefaultInnerComparer, out mismatchedIndex);
+
+							if (result)
 								return;
 						}
 						catch (Exception ex)
@@ -651,7 +659,15 @@ partial class Assert
 					{
 						try
 						{
-							if (!CollectionTracker.AreCollectionsEqual(expectedTracker, actualTracker, itemComparer, itemComparer == AssertEqualityComparer<T>.DefaultInnerComparer, out mismatchedIndex))
+							bool result;
+
+							// Call AssertEqualityComparer.Equals because it checks for IEquatable<> before using CollectionTracker
+							if (aec != null)
+								result = aec.Equals(expected, expectedTracker, actual, actualTracker, out mismatchedIndex);
+							else
+								result = CollectionTracker.AreCollectionsEqual(expectedTracker, actualTracker, itemComparer, itemComparer == AssertEqualityComparer<T>.DefaultInnerComparer, out mismatchedIndex);
+
+							if (!result)
 								return;
 
 							// For NotEqual that doesn't throw, pointers are irrelevant, because

diff --git a/Sdk/AssertEqualityComparer.cs b/Sdk/AssertEqualityComparer.cs
@@ -126,23 +126,46 @@ public AssertEqualityComparer(IEqualityComparer innerComparer = null)
 			T y)
 #endif
 		{
-			// Null?
-			if (x == null && y == null)
-				return true;
-			if (x == null || y == null)
-				return false;
+			int? _;
 
-#if !XUNIT_FRAMEWORK
-			// Collections?
+#if XUNIT_FRAMEWORK
+			return Equals(x, y, out _);
+#else
 			using (var xTracker = x.AsNonStringTracker())
 			using (var yTracker = y.AsNonStringTracker())
-			{
-				int? _;
+				return Equals(x, xTracker, y, yTracker, out _);
+#endif
+		}
 
-				if (xTracker != null && yTracker != null)
-					return CollectionTracker.AreCollectionsEqual(xTracker, yTracker, InnerComparer, InnerComparer == DefaultInnerComparer, out _);
-			}
+		internal bool Equals(
+#if XUNIT_NULLABLE
+			[AllowNull] T x,
+#if !XUNIT_FRAMEWORK
+			CollectionTracker? xTracker,
+#endif
+			[AllowNull] T y,
+#if !XUNIT_FRAMEWORK
+			CollectionTracker? yTracker,
 #endif
+#else
+			T x,
+#if !XUNIT_FRAMEWORK
+			CollectionTracker xTracker,
+#endif
+			T y,
+#if !XUNIT_FRAMEWORK
+			CollectionTracker yTracker,
+#endif
+#endif
+			out int? mismatchedIndex)
+		{
+			mismatchedIndex = null;
+
+			// Null?
+			if (x == null && y == null)
+				return true;
+			if (x == null || y == null)
+				return false;
 
 			// Implements IEquatable<T>?
 			var equatable = x as IEquatable<T>;
@@ -171,6 +194,12 @@ public AssertEqualityComparer(IEqualityComparer innerComparer = null)
 				}
 			}
 
+#if !XUNIT_FRAMEWORK
+			// Special case collections (before IStructuralEquatable because arrays implement that in a way we don't want to call)
+			if (xTracker != null && yTracker != null)
+				return CollectionTracker.AreCollectionsEqual(xTracker, yTracker, InnerComparer, InnerComparer == DefaultInnerComparer, out mismatchedIndex);
+#endif
+
 			// Implements IStructuralEquatable?
 			var structuralEquatable = x as IStructuralEquatable;
 			if (structuralEquatable != null && structuralEquatable.Equals(y, new TypeErasedEqualityComparer(innerComparer.Value)))