[patch] Keep the wrapped dictionary's comparer when cloning a ReadOnlyDictionary

DeepClone.Test/DictionaryComparerTests.cs

@@ -4,6 +4,7 @@ namespace ktsu.DeepClone.Test;
 
 using System.Collections.Concurrent;
 using System.Collections.Immutable;
+using System.Collections.ObjectModel;
 
 /// <summary>
 /// Tests that cloning a dictionary through <see cref="IDictionary{TKey, TValue}"/> or
@@ -122,4 +123,82 @@ public void ImmutableSortedDictionary_ThroughIDictionary_KeepsComparerAndOrder()
 
 		CollectionAssert.AreEqual(DescendingKeys, clone.Keys.ToArray());
 	}
+
+	/// <summary>
+	/// 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).
+	/// </summary>
+	[TestMethod]
+	public void ReadOnlyDictionary_KeepsWrappedComparer()
+	{
+		ReadOnlyDictionary<string, int> original = new(new Dictionary<string, int>(StringComparer.OrdinalIgnoreCase) { ["Key"] = 1 });
+
+		ReadOnlyDictionary<string, int> clone = original.DeepClone();
+
+		Assert.IsTrue(clone.ContainsKey("KEY"));
+	}
+
+	/// <summary>
+	/// Tests that a read-only dictionary over a case-insensitive dictionary keeps the wrapped dictionary's comparer
+	/// when cloned through IReadOnlyDictionary.
+	/// </summary>
+	[TestMethod]
+	public void ReadOnlyDictionary_ThroughIReadOnlyDictionary_KeepsWrappedComparer()
+	{
+		ReadOnlyDictionary<string, int> original = new(new Dictionary<string, int>(StringComparer.OrdinalIgnoreCase) { ["Key"] = 1 });
+
+		IReadOnlyDictionary<string, int> clone = ((IReadOnlyDictionary<string, int>)original).DeepClone();
+
+		Assert.IsTrue(clone.ContainsKey("KEY"));
+	}
+
+	/// <summary>
+	/// Tests that a read-only dictionary cloned through IDictionary keeps the wrapped dictionary's comparer.
+	/// </summary>
+	[TestMethod]
+	public void ReadOnlyDictionary_ThroughIDictionary_KeepsWrappedComparer()
+	{
+		ReadOnlyDictionary<string, int> original = new(new Dictionary<string, int>(StringComparer.OrdinalIgnoreCase) { ["Key"] = 1 });
+
+		IDictionary<string, int> clone = ((IDictionary<string, int>)original).DeepClone();
+
+		Assert.IsTrue(clone.ContainsKey("KEY"));
+	}
+
+	/// <summary>
+	/// Tests that a read-only dictionary over a reference-equality dictionary holding two equal but distinct keys
+	/// clones without throwing and keeps both entries.
+	/// </summary>
+	[TestMethod]
+	public void ReadOnlyDictionary_WithReferenceEquality_ClonesEqualKeys()
+	{
+		Dictionary<object, int> inner = new(ReferenceEqualityComparer.Instance)
+		{
+			[new string('k', 1)] = 1,
+			[new string('k', 1)] = 2,
+		};
+		ReadOnlyDictionary<object, int> original = new(inner);
+
+		ReadOnlyDictionary<object, int> clone = original.DeepClone();
+
+		Assert.HasCount(2, clone);
+	}
+
+	/// <summary>
+	/// 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.
+	/// </summary>
+	[TestMethod]
+	public void NestedReadOnlyDictionary_OverSortedDictionary_KeepsComparerAndOrder()
+	{
+		Comparer<string> descending = Comparer<string>.Create((x, y) => string.CompareOrdinal(y, x));
+		SortedDictionary<string, int> inner = new(descending) { ["a"] = 1, ["c"] = 3, ["b"] = 2 };
+		ReadOnlyDictionary<string, int> original = new(new ReadOnlyDictionary<string, int>(inner));
+
+		IDictionary<string, int> clone = ((IDictionary<string, int>)original).DeepClone();
+		clone["d"] = 4;
+
+		Assert.IsInstanceOfType<SortedDictionary<string, int>>(clone);
+		Assert.AreSequenceEqual(DescendingKeys, clone.Keys);
+	}
 }

DeepClone/DeepCloneContainerExtensions.cs

@@ -4,6 +4,7 @@
 
 using System.Collections.Concurrent;
 using System.Collections.ObjectModel;
+using System.Reflection;
 #if NET
 using System.Collections.Immutable;
 #endif
@@ -174,7 +175,7 @@
 	/// </code>
 	/// </remarks>
 	/// <exception cref="ArgumentNullException">Thrown if source is null.</exception>
 	public static IEnumerable<T> DeepClone<T>(this IEnumerable<T> source)
 	{
 		Ensure.NotNull(source);
 
@@ -196,7 +197,8 @@
 	/// <see cref="SortedList{TKey, TValue}"/>, <see cref="ConcurrentDictionary{TKey, TValue}"/> (.NET 6 and later),
 	/// and, on .NET, <c>ImmutableDictionary</c> and <c>ImmutableSortedDictionary</c>. 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 <see cref="ReadOnlyDictionary{TKey, TValue}"/> is cloned according to the
+	/// dictionary it wraps.
 	///
 	/// Example usage:
 	/// <code>
@@ -294,7 +296,9 @@
 	/// <remarks>
 	/// <see cref="ReadOnlyDictionary{TKey, TValue}"/> implements both <see cref="IDictionary{TKey, TValue}"/> and
 	/// <see cref="IReadOnlyDictionary{TKey, TValue}"/>, so this overload is also what lets
-	/// <c>readOnlyDictionary.DeepClone()</c> compile without a cast.
+	/// <c>readOnlyDictionary.DeepClone()</c> 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
+	/// <see cref="IDictionary{TKey, TValue}"/> overload.
 	/// </remarks>
 	/// <exception cref="ArgumentNullException">Thrown if source is null.</exception>
 	public static ReadOnlyDictionary<TKey, TValue> DeepClone<TKey, TValue>(this ReadOnlyDictionary<TKey, TValue> source)
@@ -341,14 +345,20 @@
 	/// </summary>
 	/// <typeparam name="TKey">The type of keys in the dictionary.</typeparam>
 	/// <typeparam name="TValue">The type of values in the dictionary.</typeparam>
-	/// <param name="source">The dictionary being cloned, inspected for its runtime type and comparer.</param>
+	/// <param name="source">The dictionary being cloned, inspected for its runtime type and comparer. A
+	/// <see cref="ReadOnlyDictionary{TKey, TValue}"/> is looked through to the dictionary it wraps.</param>
 	/// <param name="pairs">The key-value pairs of the dictionary being cloned.</param>
 	/// <returns>A <see cref="SortedList{TKey, TValue}"/> or <see cref="ConcurrentDictionary{TKey, TValue}"/> for a source of
 	/// that type, a <see cref="SortedDictionary{TKey, TValue}"/> for any other sorted source, otherwise a
 	/// <see cref="Dictionary{TKey, TValue}"/>. All of them implement <see cref="IReadOnlyDictionary{TKey, TValue}"/>.</returns>
 	private static IDictionary<TKey, TValue> CloneDictionary<TKey, TValue>(object source, IEnumerable<KeyValuePair<TKey, TValue>> pairs)
 		where TKey : notnull
 	{
+		while (source is ReadOnlyDictionary<TKey, TValue> readOnly)
+		{
+			source = WrappedDictionary<TKey, TValue>.Of(readOnly);
+		}
+
 		IDictionary<TKey, TValue> clone = source switch
 		{
 			SortedDictionary<TKey, TValue> sorted => new SortedDictionary<TKey, TValue>(sorted.Comparer),
@@ -368,6 +378,33 @@
 		return clone;
 	}
 
+	/// <summary>
+	/// Reads the dictionary a <see cref="ReadOnlyDictionary{TKey, TValue}"/> wraps, which it exposes only through
+	/// its protected <c>Dictionary</c> property, so a clone can keep that dictionary's kind and comparer.
+	/// </summary>
+	/// <typeparam name="TKey">The type of keys in the dictionary.</typeparam>
+	/// <typeparam name="TValue">The type of values in the dictionary.</typeparam>
+	private static class WrappedDictionary<TKey, TValue>
+		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<ReadOnlyDictionary<TKey, TValue>, IDictionary<TKey, TValue>> Getter =
+			typeof(ReadOnlyDictionary<TKey, TValue>)
+				.GetProperty("Dictionary", BindingFlags.Instance | BindingFlags.NonPublic)!
+				.GetGetMethod(nonPublic: true)!
+				.CreateDelegate<Func<ReadOnlyDictionary<TKey, TValue>, IDictionary<TKey, TValue>>>();
+
+		/// <summary>
+		/// Gets the dictionary that <paramref name="readOnly"/> wraps.
+		/// </summary>
+		/// <param name="readOnly">The read-only wrapper.</param>
+		/// <returns>The wrapped dictionary.</returns>
+		internal static IDictionary<TKey, TValue> Of(ReadOnlyDictionary<TKey, TValue> readOnly) => Getter(readOnly);
+	}
+
 	/// <summary>
 	/// Adds a deep clone of each key-value pair to a destination dictionary.
 	/// </summary>