From 0821e77e0ad3603d749597295dd5ec0b9e520581 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 26 Sep 2026 04:22:51 +0000 Subject: [PATCH 1/2] fix: remove set items from storage using the set's comparer [patch] ContiguousSet.Remove and InsertionOrderSet.Remove removed the item from the comparer-aware uniqueness HashSet, then searched the ordered backing store with default equality. With a custom comparer such as OrdinalIgnoreCase, Remove("APPLE") on a set holding "Apple" dropped the hash entry but left the stored item, so it still enumerated, Count did not change, and a later Add stored a duplicate. Both now locate the stored item with uniquenessSet.Comparer. Fixes ktsu-dev/Containers#61 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01UHW69XTeuuLbfVdQdJ8uQh --- Containers.Test/ContiguousSetTests.cs | 33 +++++++++++++++++++++++ Containers.Test/InsertionOrderSetTests.cs | 33 +++++++++++++++++++++++ Containers/ContiguousSet.cs | 15 +++++++++-- Containers/InsertionOrderSet.cs | 10 ++++++- 4 files changed, 88 insertions(+), 3 deletions(-) diff --git a/Containers.Test/ContiguousSetTests.cs b/Containers.Test/ContiguousSetTests.cs index 321ff2f..8c39568 100644 --- a/Containers.Test/ContiguousSetTests.cs +++ b/Containers.Test/ContiguousSetTests.cs @@ -452,4 +452,37 @@ public void WorksWithStrings() Assert.Contains("alpha", set); Assert.Contains("bravo", set); } + + [TestMethod] + public void Remove_WithCustomComparer_RemovesItemEqualOnlyUnderComparer() + { + // Arrange + ContiguousSet set = new(StringComparer.OrdinalIgnoreCase) { "Apple", "Banana" }; + + // Act + bool removed = set.Remove("APPLE"); + + // Assert + Assert.IsTrue(removed); + Assert.HasCount(1, set); + Assert.IsFalse(set.Contains("apple")); + string[] expected = ["Banana"]; + Assert.AreSequenceEqual(expected, set); + } + + [TestMethod] + public void Remove_WithCustomComparer_ThenAdd_KeepsSingleElement() + { + // Arrange + ContiguousSet set = new(StringComparer.OrdinalIgnoreCase) { "Apple" }; + + // Act + set.Remove("APPLE"); + set.Add("apple"); + + // Assert + Assert.HasCount(1, set); + string[] expected = ["apple"]; + Assert.AreSequenceEqual(expected, set); + } } diff --git a/Containers.Test/InsertionOrderSetTests.cs b/Containers.Test/InsertionOrderSetTests.cs index 7e198bc..e849580 100644 --- a/Containers.Test/InsertionOrderSetTests.cs +++ b/Containers.Test/InsertionOrderSetTests.cs @@ -322,4 +322,37 @@ public void WorksWithStrings_MaintainsInsertionOrder() Assert.AreEqual("alpha", set.ElementAt(1)); // Second unique Assert.AreEqual("bravo", set.ElementAt(2)); // Third unique } + + [TestMethod] + public void Remove_WithCustomComparer_RemovesItemEqualOnlyUnderComparer() + { + // Arrange + InsertionOrderSet set = new(StringComparer.OrdinalIgnoreCase) { "Apple", "Banana" }; + + // Act + bool removed = set.Remove("APPLE"); + + // Assert + Assert.IsTrue(removed); + Assert.HasCount(1, set); + Assert.IsFalse(set.Contains("apple")); + string[] expected = ["Banana"]; + Assert.AreSequenceEqual(expected, set); + } + + [TestMethod] + public void Remove_WithCustomComparer_ThenAdd_KeepsSingleElement() + { + // Arrange + InsertionOrderSet set = new(StringComparer.OrdinalIgnoreCase) { "Apple" }; + + // Act + set.Remove("APPLE"); + set.Add("apple"); + + // Assert + Assert.HasCount(1, set); + string[] expected = ["apple"]; + Assert.AreSequenceEqual(expected, set); + } } diff --git a/Containers/ContiguousSet.cs b/Containers/ContiguousSet.cs index bc60ff1..37c3f9b 100644 --- a/Containers/ContiguousSet.cs +++ b/Containers/ContiguousSet.cs @@ -281,8 +281,19 @@ public bool Remove(T item) return false; } - // Find and remove from the array - int index = Array.IndexOf(items, item, 0, Count); + // Find and remove from the array, matching with the set's comparer so an item that is + // equal only under a custom comparer is removed from storage as well as the hash set + IEqualityComparer comparer = uniquenessSet.Comparer; + int index = -1; + for (int i = 0; i < Count; i++) + { + if (comparer.Equals(items[i], item)) + { + index = i; + break; + } + } + if (index >= 0) { Count--; diff --git a/Containers/InsertionOrderSet.cs b/Containers/InsertionOrderSet.cs index 9b92842..cf6105e 100644 --- a/Containers/InsertionOrderSet.cs +++ b/Containers/InsertionOrderSet.cs @@ -237,7 +237,15 @@ public bool Remove(T item) return false; } - items.Remove(item); + // Match with the set's comparer so an item that is equal only under a custom comparer + // is removed from the list as well as the hash set + IEqualityComparer comparer = uniquenessSet.Comparer; + int index = items.FindIndex(x => comparer.Equals(x, item)); + if (index >= 0) + { + items.RemoveAt(index); + } + return true; } From 78a6afb7440d9428ddfdc9e34dffde0632aa12bd Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 26 Sep 2026 04:28:24 +0000 Subject: [PATCH 2/2] Rename test locals that shadow the expected field Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01UHW69XTeuuLbfVdQdJ8uQh --- Containers.Test/ContiguousSetTests.cs | 8 ++++---- Containers.Test/InsertionOrderSetTests.cs | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/Containers.Test/ContiguousSetTests.cs b/Containers.Test/ContiguousSetTests.cs index 8c39568..dd99782 100644 --- a/Containers.Test/ContiguousSetTests.cs +++ b/Containers.Test/ContiguousSetTests.cs @@ -466,8 +466,8 @@ public void Remove_WithCustomComparer_RemovesItemEqualOnlyUnderComparer() Assert.IsTrue(removed); Assert.HasCount(1, set); Assert.IsFalse(set.Contains("apple")); - string[] expected = ["Banana"]; - Assert.AreSequenceEqual(expected, set); + string[] expectedItems = ["Banana"]; + Assert.AreSequenceEqual(expectedItems, set); } [TestMethod] @@ -482,7 +482,7 @@ public void Remove_WithCustomComparer_ThenAdd_KeepsSingleElement() // Assert Assert.HasCount(1, set); - string[] expected = ["apple"]; - Assert.AreSequenceEqual(expected, set); + string[] expectedItems = ["apple"]; + Assert.AreSequenceEqual(expectedItems, set); } } diff --git a/Containers.Test/InsertionOrderSetTests.cs b/Containers.Test/InsertionOrderSetTests.cs index e849580..9eb498c 100644 --- a/Containers.Test/InsertionOrderSetTests.cs +++ b/Containers.Test/InsertionOrderSetTests.cs @@ -336,8 +336,8 @@ public void Remove_WithCustomComparer_RemovesItemEqualOnlyUnderComparer() Assert.IsTrue(removed); Assert.HasCount(1, set); Assert.IsFalse(set.Contains("apple")); - string[] expected = ["Banana"]; - Assert.AreSequenceEqual(expected, set); + string[] expectedItems = ["Banana"]; + Assert.AreSequenceEqual(expectedItems, set); } [TestMethod] @@ -352,7 +352,7 @@ public void Remove_WithCustomComparer_ThenAdd_KeepsSingleElement() // Assert Assert.HasCount(1, set); - string[] expected = ["apple"]; - Assert.AreSequenceEqual(expected, set); + string[] expectedItems = ["apple"]; + Assert.AreSequenceEqual(expectedItems, set); } }