Skip to content

ReadOnlyDictionary.DeepClone() drops the wrapped dictionary's key comparer: a case-insensitive source's clone can't find "KEY" #87

Closed

Description

@matt-edmondson

What's wrong

DeepClone(this ReadOnlyDictionary<TKey,TValue>) (DeepClone/DeepCloneContainerExtensions.cs:241-246) builds the inner dictionary with CloneDictionary(source, source). CloneDictionary (line ~286) chooses the comparer from the runtime type of its first argument, and it only recognises Dictionary<,> and SortedDictionary<,>. Here the first argument is the ReadOnlyDictionary wrapper, so the default arm always runs (_ => new Dictionary<TKey, TValue>()). The comparer of the dictionary being wrapped is never checked.

Cloning through IReadOnlyDictionary<,> (line 268) has the same problem.

This is not covered by #83, which lists ConcurrentDictionary, SortedList and the immutable dictionaries. Adding arms for those types to CloneDictionary would still leave the wrapper case broken, because the wrapper's own type is what gets inspected.

Reproduction

using System.Collections.ObjectModel;
using ktsu.DeepClone;

var inner = new Dictionary<string, int>(StringComparer.OrdinalIgnoreCase) { ["Key"] = 1 };
var ro = new ReadOnlyDictionary<string, int>(inner);

Console.WriteLine(ro.ContainsKey("KEY"));                                          // True
Console.WriteLine(ro.DeepClone().ContainsKey("KEY"));                              // False
Console.WriteLine(((IReadOnlyDictionary<string, int>)ro).DeepClone().ContainsKey("KEY")); // False

Observed on current main (net10.0): True / False / False. Expected: True / True / True.

If the wrapped dictionary uses a comparer that is stricter than the default (for example ReferenceEqualityComparer with two equal but distinct keys), the clone throws ArgumentException: An item with the same key has already been added. This is the same failure #83 describes for the other types.

Why it matters

A read-only view over a case-insensitive dictionary is a common way to expose configuration or lookup tables. The clone looks correct, but lookups that work on the source fail on the clone, and the missing key only shows up later.

Suggested fix / acceptance criteria

  • In the ReadOnlyDictionary overload, and in CloneDictionary when the source is a ReadOnlyDictionary<,>, look at the wrapped dictionary rather than the wrapper. ReadOnlyDictionary exposes it only through the protected Dictionary property, so either:
    • read it once through a cached reflection accessor and pass it to CloneDictionary as the object to inspect, or
    • if reflection isn't acceptable, document in the <remarks> that the clone of a ReadOnlyDictionary always uses the default comparer.
  • Add a test: a ReadOnlyDictionary over an OrdinalIgnoreCase dictionary clones to one where ContainsKey("KEY") is true, through both the concrete and the IReadOnlyDictionary overloads.

Activity

  1. matt-edmondson commented on Sep 28, 2026

    @matt-edmondson
    ContributorAuthor

    Triage


    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

bugSomething isn't workingreadyFully specified; implement as written

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions