feat(assertions): add ContainsKey().And.Value drill-in for dictionaries (#6185)

[thomhurst]

What

Closes #6185.

Adds a fluent way to assert on a dictionary entry's value, instead of the two-step ContainsKey + re-index dance:

// before
await Assert.That(result).ContainsKey("Key");
await Assert.That(result["Key"]).IsEqualTo(1234L);

// after
await Assert.That(result).ContainsKey("Key").And.Value.IsEqualTo(1234L);
await Assert.That(result).ContainsKey("Key").And.Value.Member(x => x.Prop, p => p.IsEqualTo(1234L));

Design

• The value accessor is a property .Value gated behind .And (reads "contains key Key, and value is equal to …"), per maintainer preference in the issue.
• ContainsKey(key).And now returns a specialized continuation (DictionaryContainsKeyAndContinuation / mutable twin) that carries the asserted key and exposes .Value.
• .Value returns a DictionaryValueSource<TValue> (a thin ValueAssertion<TValue> wrapper). Because it's an IAssertionSource<TValue>, the entire existing value-assertion surface works for free — IsEqualTo, IsGreaterThan, IsNotEqualTo, Member, Satisfies, type checks, further .And/.Or, etc. No per-method reimplementation.
• Key existence is validated first: .And registers the ContainsKey assertion as a pending link, so AssertionContext.Map runs it as pre-work before the value is read. A missing key fails with the standard Expected ... to contain key "X" message, not a raw KeyNotFoundException.
• Works for both read-only (IReadOnlyDictionary) and mutable (IDictionary / concrete Dictionary<,>) dictionaries, including the custom-comparer ContainsKey overload.

Bonus

IsEmpty() / IsNotEmpty() now preserve the dictionary continuation (previously they dropped to a generic collection continuation), so chains like IsNotEmpty().And.ContainsKey("k").And.Value... keep working.

Tests

Added coverage in DictionaryCollectionTests: value equality (pass/fail), missing-key message, nested Member, other value assertions, value-level .And, longer dict chains, custom comparer, read-only + IDictionary paths, and IsEmpty/IsNotEmpty continuation preservation.

• Focused: 58 passed
• Full TUnit.Assertions.Tests (net10.0): 2139 passed, 0 failed

Pure runtime assertions-library change — no source generator, snapshot, or AOT impact.

[codacy-production]

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 89 complexity

Metric	Results
Complexity	89

View in Codacy

_{NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.}

[claude]

Review: feat(assertions): add ContainsKey().And.Value drill-in for dictionaries (#6185)

Overall this is a well-designed feature. The fluent API reads naturally, and piggybacking on ValueAssertion<TValue> to get the full assertion surface for free is the right architectural call. Test coverage is solid (17 new tests covering pass/fail paths, missing-key message, chaining, custom comparers, and interface variants). A few things worth addressing:

---

Moderate: ExtractValue is duplicated verbatim

DictionaryContainsKeyAndContinuation<TDictionary, TKey, TValue> and MutableDictionaryContainsKeyAndContinuation<TDictionary, TKey, TValue> each contain an identical private ExtractValue static method (lines 65–88 and 127–148 of DictionaryValueSource.cs). The only difference is the type constraint on the first parameter.

Both IReadOnlyDictionary<K,V> and IDictionary<K,V> expose TryGetValue and both implement IEnumerable<KeyValuePair<K,V>>, so a shared internal static helper is straightforward:

// e.g. DictionaryValueExtractor.cs (internal)
internal static class DictionaryValueExtractor
{
    internal static TValue? Extract<TKey, TValue>(
        IEnumerable<KeyValuePair<TKey, TValue>> dictionary,
        TKey key,
        IEqualityComparer<TKey>? comparer)
        where TKey : notnull
    {
        if (comparer is not null)
        {
            foreach (var pair in dictionary)
                if (comparer.Equals(pair.Key, key)) return pair.Value;
            return default;
        }
        // Both IDictionary<K,V> and IReadOnlyDictionary<K,V> have TryGetValue
        // so we can cast here for the fast path
        if (dictionary is IReadOnlyDictionary<TKey, TValue> rod)
            return rod.TryGetValue(key, out var v) ? v : default;
        foreach (var pair in dictionary)
            if (EqualityComparer<TKey>.Default.Equals(pair.Key, key)) return pair.Value;
        return default;
    }
}

Why this matters: the two copies will inevitably drift. If a bug is fixed in one, the other is forgotten.

---

Minor: 4 new classes for IsEmpty/IsNotEmpty that could be 2

The PR adds DictionaryIsEmptyAssertion, DictionaryIsNotEmptyAssertion, MutableDictionaryIsEmptyAssertion, and MutableDictionaryIsNotEmptyAssertion. Each Empty/NotEmpty pair has identical CheckAsync bodies — the only difference is CollectionChecks.CheckIsEmpty vs CollectionChecks.CheckIsNotEmpty and the expectation string.

The existing DictionaryNullAssertion (in CollectionNullAssertion.cs) uses bool expectNull to merge IsNull/IsNotNull into one class. The AsyncEnumerableIsEmptyAssertion uses the same bool expectEmpty trick. Applying the same approach here would reduce 4 classes to 2:

public class DictionaryIsEmptyAssertion<TDictionary, TKey, TValue>
    : Sources.DictionaryAssertionBase<TDictionary, TKey, TValue>
    where TDictionary : IReadOnlyDictionary<TKey, TValue>
    where TKey : notnull
{
    private readonly bool _expectEmpty;

    public DictionaryIsEmptyAssertion(AssertionContext<TDictionary> context, bool expectEmpty)
        : base(context) => _expectEmpty = expectEmpty;

    protected override Task<AssertionResult> CheckAsync(EvaluationMetadata<TDictionary> metadata)
    {
        // ... null/exception guards ...
        var adapter = new EnumerableAdapter<KeyValuePair<TKey, TValue>>(metadata.Value);
        return Task.FromResult(_expectEmpty
            ? CollectionChecks.CheckIsEmpty(adapter)
            : CollectionChecks.CheckIsNotEmpty(adapter));
    }

    protected override string GetExpectation() => _expectEmpty ? "to be empty" : "to not be empty";
}

Note: the existing CollectionIsEmptyAssertion/CollectionIsNotEmptyAssertion pair does use separate classes, so I acknowledge there's existing inconsistency in the codebase. Still worth calling out.

---

Minor: Test section-header comments

The block comment at the top of the new test section:

// ===================================
// ContainsKey(...).And.Value drill-in (#6185)
// ===================================

Per the project's CLAUDE.md convention, comments should be avoided — test method names are self-documenting. The #6185 reference also rots over time. I'd drop the comment block.

---

Informational: Value property is not idempotent

Accessing .Value twice on the same continuation instance would double-append to ExpressionBuilder and create a double-mapped context. This is consistent with how .And/.Or work throughout the codebase (none of them cache), but it's worth being aware of if continuation instances ever get reused. No action needed — just documenting.

---

Summary: The core design is solid. The ExtractValue duplication is the most actionable finding — it will bite maintainers when the two copies diverge. The IsEmpty/IsNotEmpty class explosion is a cleanliness concern. The test comments are a style nit. Great work overall on the feature itself.

[claude]

Review of feat(assertions): ContainsKey().And.Value drill-in (#6185)

The core design is sound and the ergonomics improvement is genuine. The detached-context delegation pattern for reusing collection assertion logic while preserving dictionary-specific chaining is the right approach. DictionaryValueSource<TValue> as a thin ValueAssertion<TValue> wrapper giving the full assertion surface for free is a highlight. That said, there are open items from the previous review and some new concerns from the expanded scope.

---

Previous review items still unresolved

1. ExtractValue duplication — still present

The method body (null check → comparer loop → TryGetValue) is verbatim in both DictionaryContainsKeyAndContinuation and MutableDictionaryContainsKeyAndContinuation. This was flagged as "Moderate" last time and hasn't changed.

The fix is straightforward — a single internal static helper works for both because IDictionary<K,V> and IReadOnlyDictionary<K,V> both support foreach over KeyValuePair and both have TryGetValue:

// In a new file: DictionaryValueExtractor.cs
internal static class DictionaryValueExtractor
{
    internal static TValue? Extract<TKey, TValue>(
        IEnumerable<KeyValuePair<TKey, TValue>>? source,
        TKey key,
        IEqualityComparer<TKey>? comparer)
        where TKey : notnull
    {
        if (source is null) return default;
        if (comparer is not null)
        {
            foreach (var pair in source)
                if (comparer.Equals(pair.Key, key))
                    return pair.Value;
            return default;
        }
        // Both IReadOnlyDictionary and IDictionary have TryGetValue via their own interface
        if (source is IReadOnlyDictionary<TKey, TValue> rod)
            return rod.TryGetValue(key, out var v1) ? v1 : default;
        if (source is IDictionary<TKey, TValue> d)
            return d.TryGetValue(key, out var v2) ? v2 : default;
        return default;
    }
}

Then both private ExtractValue methods become one-liners delegating to this helper.

2. Test section-header comments — still present

Lines with // =================================== blocks are still in the test file. CLAUDE.md: comments should only explain a non-obvious WHY. These explain WHAT the section contains (which test names already convey). Remove them.

---

New concerns (scope expansion)

The PR grew to include Count(), HasSingleItem(), HasAtLeast(), HasAtMost(), HasCountBetween() preservation — which is useful but introduces new duplication.

3. DictionaryCountSource / MutableDictionaryCountSource are near-identical clones

Both classes have the same 8 methods differing only in return type. Compare with CollectionCountSource — the pattern is now implemented three times. Adding a new count comparison (e.g. IsEven()) would require editing all three.

A simpler approach: make DictionaryCountSource extend CollectionCountSource and new-override only the methods where the return type needs to change. Or extract the shared Compare() helper as a static that takes a factory delegate for the return type.

4. DetachedContext() is duplicated

The one-liner new(Context.Evaluation, new System.Text.StringBuilder()) appears as a private method in both DictionaryAssertionBase and MutableDictionaryAssertionBase. This belongs on a shared base or as a static extension method. The logic is trivially identical.

---

Minor

5. Block comments in both base classes

The multi-line // The collection methods below ... comment blocks (in DictionaryAssertionBase.cs lines 700-705 and MutableDictionaryAssertionBase.cs) explain the public new strategy. This rationale belongs in the PR description (where it is already well-documented), not in the source. Per project convention, comments in code should capture a hidden constraint or non-obvious invariant — the design rationale is neither.

6. DictionaryContainsKeyAndContinuation is not sealed

The class is public class, not public sealed class. Given it carries _expectedKey state that drives Value's extraction, a subclass that calls the base Value getter could silently inherit a stale key. Sealing it is a defensive choice here (the mutable twin is the same).

---

What's working well

• The "ContainsKey check as pre-work" design means a missing key gets the friendly "to contain key" error, not a raw KeyNotFoundException. Correct.
• Test coverage hits all the important paths: pass, fail (wrong value), fail (missing key message), Member drill-in, custom comparer, IReadOnlyDictionary, IDictionary, value-level .And, continuation preservation. Solid.
• DictionaryDelegatingAssertion — delegating to an inner assertion built on a detached context so it doesn't disturb the And/Or pending link — is an elegant mechanism that sidesteps the type-hierarchy problem cleanly.

---

Summary: The feature is correct and well-tested. The two items from the prior review (ExtractValue duplication, test comments) need to be addressed before merging. The new scope additions also introduced the same duplication pattern in DictionaryCountSource — worth fixing alongside ExtractValue since the fix shape is the same.

[claude]

Follow-up review (third commit — refactor: dedup dictionary value lookup)

Thanks for the quick follow-up commit. The ExtractValue duplication is now fixed via DictionaryValueLookup, which was the most important item from the previous reviews.

What's been addressed:

• ExtractValue duplication → resolved; single DictionaryValueLookup.Extract shared by both continuations. Good.

What's still open / worth noting before merge:

---

1. DictionaryValueLookup.Extract is public on an internal class

In DictionaryValueSource.cs, the class is internal static class DictionaryValueLookup but the method is public static TValue? Extract(...). On an internal type, public and internal have identical effective accessibility, but public is misleading — it implies an intentional API surface. Change to internal to match the class intent.

---

2. DictionaryCountSource / MutableDictionaryCountSource are still verbatim clones

The two classes in DictionaryCollectionContinuations.cs have 8 identical methods that differ only in their return type (DictionaryAssertionBase vs MutableDictionaryAssertionBase) and the concrete delegating assertion they instantiate. The Compare helper is essentially the same body with different factory lines.

This is the same duplication pattern that DictionaryValueLookup just fixed in the value-source case. One approach:

// Both sources could share a private generic core via a factory delegate:
// (already both sealed, so no subclassing needed)
private TResult Compare<TResult>(
    int expected, CountComparison comparison, string expressionFragment,
    Func<AssertionContext<TDictionary>, Assertion<TDictionary>, TResult> factory)
{
    _context.ExpressionBuilder.Append(expressionFragment);
    var detached = new AssertionContext<TDictionary>(_context.Evaluation, new StringBuilder());
    var inner = new CollectionCountEqualsAssertion<TDictionary, KeyValuePair<TKey, TValue>>(
        detached, null, expected, comparison);
    return factory(_context, inner);
}

But given the type-parameter constraint differences (IReadOnlyDictionary vs IDictionary), the two classes can't share a common generic base class without a significant refactor. The duplication is tolerable for now, but if more count methods are added in the future, both classes must be updated in sync. Worth a // NOTE: keep in sync with MutableDictionaryCountSource comment at minimum, or a future consolidation task.

---

3. DetachedContext() private method is duplicated in both base classes

DictionaryAssertionBase (line 70) and MutableDictionaryAssertionBase both define:

private AssertionContext<TDictionary> DetachedContext()
    => new(Context.Evaluation, new System.Text.StringBuilder());

These are identical one-liners. The duplication is low-risk since the method is trivial and private, but it's the same pattern that was flagged for ExtractValue. Since both classes inherit from CollectionAssertionBase<TDictionary, KeyValuePair<TKey, TValue>>, this could be pushed to a shared extension method or to the base class as a protected helper.

---

4. DictionaryContainsKeyAndContinuation / MutableDictionaryContainsKeyAndContinuation are not sealed

Both are public class not public sealed class. These carry _expectedKey and _comparer state that drives the Value accessor. There's no designed extension point. Sealing them prevents a subclass from accidentally calling Value with a stale key captured in a different _expectedKey field. Not a blocking issue, but defensive practice here.

---

Summary: The PR is in good shape. Items 1 (accessibility modifier) and 4 (sealing) are trivial fixes. Items 2 and 3 are acceptable for now given the constraint mismatch, but worth a comment or a follow-up issue. If 1 and 4 are addressed, this looks mergeable.

[claude]

Follow-up review (fourth commit — refactor: address review - share CreateDetached, seal continuations)

Good iteration. Items 3 and 4 from the previous review are now resolved. Tracking the full state of open items:

---

Resolved since last review

CreateDetached duplication (item 3) — fixed. The private DetachedContext() helper in both base classes is gone; both now call Context.CreateDetached() directly, which is the right home for it.

Continuations not sealed (item 4) — fixed. Both DictionaryContainsKeyAndContinuation and MutableDictionaryContainsKeyAndContinuation are now sealed. Good.

---

Still open

1. DictionaryValueLookup.Extract is public on an internal class — not yet fixed.

DictionaryValueSource.cs line 31: public static TValue? Extract<TKey, TValue>(...) on internal static class DictionaryValueLookup. On an internal type, public and internal have the same effective accessibility — the method is invisible outside the assembly either way. However the public modifier is misleading: it implies this is an intentional API surface. Change to internal to match the class's declared intent. One-line fix.

2. DictionaryCountSource / MutableDictionaryCountSource verbatim clone — still present.

Both classes in DictionaryCollectionContinuations.cs have 8 identical methods differing only in their return type. This was flagged in reviews 2 and 3. The type-parameter constraint difference (IReadOnlyDictionary vs IDictionary) prevents a shared base class without a larger refactor, so the duplication is structurally constrained. At minimum, add a // NOTE: keep in sync with MutableDictionaryCountSource cross-reference comment so future maintainers don't update one and miss the other. A follow-up issue tracking the consolidation would also be appropriate.

3. Test section-header comments — still present.

// =================================== blocks remain in DictionaryCollectionTests.cs at lines 203–204, 260–261, 304–305, 331–332 (and so on through the file). These pre-date this PR and are already present in main. The new blocks added by this PR are at lines 630–631 and 812–813. Removing the two newly added ones is the minimum expected from the previous reviews; removing all of them would be a bonus cleanup.

---

Summary

Items 1 and 3 are trivial one-liners (modifier change, remove comment blocks). Item 2 is architecturally constrained and acceptable with a cross-reference comment. With those three addressed, this PR is in good shape to merge. The core feature — ContainsKey(key).And.Value.<any assertion> — is correctly designed, well-tested, and handles the edge cases (missing key, custom comparer, IReadOnlyDictionary, IDictionary, value-level chaining) properly.

[claude]

Follow-up review (fifth commit — refactor: internal Extract, sync-note, drop test headers)

All items from the previous four rounds of review have been addressed. This PR is ready to merge.

---

Resolved in latest commit

1. DictionaryValueLookup.Extract accessibility — fixed.
The method is now internal static to match the internal class. Correct.

2. DictionaryCountSource / MutableDictionaryCountSource sync — addressed.
Both classes now carry cross-reference // NOTE: keep the method set in sync with ... comments in their XML-doc summaries, which is the right minimum given the constraint mismatch (IReadOnlyDictionary vs IDictionary) prevents a shared base class. Future maintainers will see the note.

3. Test section-header comment blocks — removed.
The // =================================== decorative headers are gone from the test file.

---

Overall assessment

The design is solid end-to-end:

• Core feature: ContainsKey(key).And.Value.<any assertion> reads naturally and gives the full assertion surface for free by delegating to ValueAssertion<TValue>. No per-method reimplementation needed for value assertions.
• Pre-work ordering: ContainsKey runs before the value is read, so a missing key produces the friendly "to contain key" message rather than a raw KeyNotFoundException.
• Detached-context delegation: The approach for preserving dictionary-specific chaining through IsEmpty/IsNotEmpty/Count/etc. is elegant — inner assertions built on a detached context leave the And/Or pending link undisturbed.
• Test coverage: 21 new tests covering pass/fail, missing-key message, Member drill-in, value-level And, custom comparers, IReadOnlyDictionary, IDictionary, and continuation preservation through size methods.

No blocking issues remain. Mergeable.