From 9f13f9920ebbf142532e9ab1d6d52b9910cafb25 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 6 Oct 2026 00:30:26 +0000 Subject: [PATCH 1/2] [patch] Keep the wrapped dictionary's comparer when cloning a ReadOnlyDictionary CloneDictionary picked the clone's kind and comparer from the runtime type of the object it was given. For a ReadOnlyDictionary that is the wrapper, which matched none of the known types, so the clone always fell back to a default Dictionary. A read-only view over a case-insensitive dictionary therefore cloned to one where "KEY" no longer matched "Key". A stricter comparer also made the clone throw on duplicate keys. Look through ReadOnlyDictionary wrappers to the dictionary they wrap, read once per closed type through a cached delegate over the protected Dictionary property, before choosing the clone's kind and comparer. Fixes ktsu-dev/DeepClone#87 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01UoSUYMQMykgfsb5y6AhnKb --- DeepClone.Test/DictionaryComparerTests.cs | 79 +++++++++++++++++++++++ DeepClone/DeepCloneContainerExtensions.cs | 39 ++++++++++- 2 files changed, 115 insertions(+), 3 deletions(-) diff --git a/DeepClone.Test/DictionaryComparerTests.cs b/DeepClone.Test/DictionaryComparerTests.cs index 0c1f2f1..a9046d8 100644 --- a/DeepClone.Test/DictionaryComparerTests.cs +++ b/DeepClone.Test/DictionaryComparerTests.cs @@ -4,6 +4,7 @@ namespace ktsu.DeepClone.Test; using System.Collections.Concurrent; using System.Collections.Immutable; +using System.Collections.ObjectModel; /// /// Tests that cloning a dictionary through or @@ -122,4 +123,82 @@ public void ImmutableSortedDictionary_ThroughIDictionary_KeepsComparerAndOrder() CollectionAssert.AreEqual(DescendingKeys, clone.Keys.ToArray()); } + + /// + /// Tests that a read-only dictionary over a case-insensitive dictionary keeps the wrapped dictionary's comparer + /// when cloned through its concrete overload (ktsu-dev/DeepClone#87). + /// + [TestMethod] + public void ReadOnlyDictionary_KeepsWrappedComparer() + { + ReadOnlyDictionary original = new(new Dictionary(StringComparer.OrdinalIgnoreCase) { ["Key"] = 1 }); + + ReadOnlyDictionary clone = original.DeepClone(); + + Assert.IsTrue(clone.ContainsKey("KEY")); + } + + /// + /// Tests that a read-only dictionary over a case-insensitive dictionary keeps the wrapped dictionary's comparer + /// when cloned through IReadOnlyDictionary. + /// + [TestMethod] + public void ReadOnlyDictionary_ThroughIReadOnlyDictionary_KeepsWrappedComparer() + { + ReadOnlyDictionary original = new(new Dictionary(StringComparer.OrdinalIgnoreCase) { ["Key"] = 1 }); + + IReadOnlyDictionary clone = ((IReadOnlyDictionary)original).DeepClone(); + + Assert.IsTrue(clone.ContainsKey("KEY")); + } + + /// + /// Tests that a read-only dictionary cloned through IDictionary keeps the wrapped dictionary's comparer. + /// + [TestMethod] + public void ReadOnlyDictionary_ThroughIDictionary_KeepsWrappedComparer() + { + ReadOnlyDictionary original = new(new Dictionary(StringComparer.OrdinalIgnoreCase) { ["Key"] = 1 }); + + IDictionary clone = ((IDictionary)original).DeepClone(); + + Assert.IsTrue(clone.ContainsKey("KEY")); + } + + /// + /// Tests that a read-only dictionary over a reference-equality dictionary holding two equal but distinct keys + /// clones without throwing and keeps both entries. + /// + [TestMethod] + public void ReadOnlyDictionary_WithReferenceEquality_ClonesEqualKeys() + { + Dictionary inner = new(ReferenceEqualityComparer.Instance) + { + [new string('k', 1)] = 1, + [new string('k', 1)] = 2, + }; + ReadOnlyDictionary original = new(inner); + + ReadOnlyDictionary clone = original.DeepClone(); + + Assert.HasCount(2, clone); + } + + /// + /// Tests that a read-only dictionary over a sorted dictionary, itself wrapped in a second read-only dictionary, + /// clones to a dictionary that stays sorted by the innermost comparer. + /// + [TestMethod] + public void NestedReadOnlyDictionary_OverSortedDictionary_KeepsComparerAndOrder() + { + Comparer descending = Comparer.Create((x, y) => string.CompareOrdinal(y, x)); + SortedDictionary inner = new(descending) { ["a"] = 1, ["c"] = 3, ["b"] = 2 }; + ReadOnlyDictionary original = new(new ReadOnlyDictionary(inner)); + + IDictionary clone = ((IDictionary)original).DeepClone(); + clone["d"] = 4; + + Assert.IsInstanceOfType>(clone); + Assert.AreSequenceEqual(DescendingKeys, clone.Keys); + } } diff --git a/DeepClone/DeepCloneContainerExtensions.cs b/DeepClone/DeepCloneContainerExtensions.cs index 1f677e8..92fcbce 100644 --- a/DeepClone/DeepCloneContainerExtensions.cs +++ b/DeepClone/DeepCloneContainerExtensions.cs @@ -4,6 +4,7 @@ namespace ktsu.DeepClone; using System.Collections.Concurrent; using System.Collections.ObjectModel; +using System.Reflection; #if NET using System.Collections.Immutable; #endif @@ -196,7 +197,8 @@ public static IEnumerable DeepClone(this IEnumerable source) /// , (.NET 6 and later), /// and, on .NET, ImmutableDictionary and ImmutableSortedDictionary. A sorted source stays sorted, /// a sorted list or concurrent dictionary is cloned as the same type, and an immutable source is cloned as the - /// mutable dictionary of the same kind. + /// mutable dictionary of the same kind. A is cloned according to the + /// dictionary it wraps. /// /// Example usage: /// @@ -294,7 +296,9 @@ public static ConcurrentDictionary DeepClone(this Co /// /// implements both and /// , so this overload is also what lets - /// readOnlyDictionary.DeepClone() compile without a cast. + /// readOnlyDictionary.DeepClone() compile without a cast. The new wrapper wraps a dictionary of the same + /// kind, and with the same key comparer, as the dictionary the source wraps, as described for the + /// overload. /// /// Thrown if source is null. public static ReadOnlyDictionary DeepClone(this ReadOnlyDictionary source) @@ -341,7 +345,8 @@ public static IReadOnlyDictionary DeepClone(this IRe /// /// The type of keys in the dictionary. /// The type of values in the dictionary. - /// The dictionary being cloned, inspected for its runtime type and comparer. + /// The dictionary being cloned, inspected for its runtime type and comparer. A + /// is looked through to the dictionary it wraps. /// The key-value pairs of the dictionary being cloned. /// A or for a source of /// that type, a for any other sorted source, otherwise a @@ -349,6 +354,11 @@ public static IReadOnlyDictionary DeepClone(this IRe private static IDictionary CloneDictionary(object source, IEnumerable> pairs) where TKey : notnull { + while (source is ReadOnlyDictionary readOnly) + { + source = WrappedDictionary.Of(readOnly); + } + IDictionary clone = source switch { SortedDictionary sorted => new SortedDictionary(sorted.Comparer), @@ -368,6 +378,29 @@ private static IDictionary CloneDictionary(object so return clone; } + /// + /// Reads the dictionary a wraps, which it exposes only through + /// its protected Dictionary property, so a clone can keep that dictionary's kind and comparer. + /// + /// The type of keys in the dictionary. + /// The type of values in the dictionary. + private static class WrappedDictionary + where TKey : notnull + { + private static readonly Func, IDictionary> Getter = + typeof(ReadOnlyDictionary) + .GetProperty("Dictionary", BindingFlags.Instance | BindingFlags.NonPublic)! + .GetGetMethod(nonPublic: true)! + .CreateDelegate, IDictionary>>(); + + /// + /// Gets the dictionary that wraps. + /// + /// The read-only wrapper. + /// The wrapped dictionary. + internal static IDictionary Of(ReadOnlyDictionary readOnly) => Getter(readOnly); + } + /// /// Adds a deep clone of each key-value pair to a destination dictionary. /// From 4de6b87d46699e6801e187a5997e4125c847c4a6 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 6 Oct 2026 00:38:49 +0000 Subject: [PATCH 2/2] Justify the reflection that reads ReadOnlyDictionary's wrapped dictionary SonarCloud flags the protected-property getter under S3011. It only reads the wrapped dictionary, and the issue's triage chose that approach over losing the comparer, so suppress the rule on that field with the reason. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01UoSUYMQMykgfsb5y6AhnKb --- DeepClone/DeepCloneContainerExtensions.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/DeepClone/DeepCloneContainerExtensions.cs b/DeepClone/DeepCloneContainerExtensions.cs index 92fcbce..aeca58d 100644 --- a/DeepClone/DeepCloneContainerExtensions.cs +++ b/DeepClone/DeepCloneContainerExtensions.cs @@ -387,6 +387,10 @@ private static IDictionary CloneDictionary(object so private static class WrappedDictionary where TKey : notnull { + [System.Diagnostics.CodeAnalysis.SuppressMessage( + "Major Code Smell", + "S3011:Reflection should not be used to increase accessibility of classes, methods, or fields", + Justification = "ReadOnlyDictionary exposes the dictionary it wraps only through a protected property, and the getter is only read, never used to modify it. Without it the clone cannot keep the wrapped dictionary's comparer (ktsu-dev/DeepClone#87).")] private static readonly Func, IDictionary> Getter = typeof(ReadOnlyDictionary) .GetProperty("Dictionary", BindingFlags.Instance | BindingFlags.NonPublic)!