Fix item-at Satisfies source typing

[thomhurst]

Summary

• add specialised ItemAt(...).Satisfies(...) overloads for collection-like item values
• keep generic Satisfies behavior while allowing type-erased specialised assertions internally
• add issue .HasItemAt(i).Satisfies() loses specialised assertion source for item #5706 regression coverage and public API snapshots

Verification

• dotnet test TUnit.Assertions.Tests\TUnit.Assertions.Tests.csproj --framework net10.0 --no-restore -- --treenode-filter "///Issue5706Tests/*"
• dotnet test TUnit.PublicAPI\TUnit.PublicAPI.csproj --no-restore

Closes #5706

[codacy-production]

Not up to standards ⛔

🔴 Issues 1 minor

Alerts:
⚠ 1 issue (≤ 0 issues of at least minor severity)

Results:
1 new issue

Category	Results
BestPractice	1 minor

View in Codacy

🟢 Metrics 38 complexity

Metric	Results
Complexity	38

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]

Code Review: Fix item-at Satisfies source typing (#5706)

The fix is correct and clearly addresses the root cause: Satisfies was constructing a ValueAssertion<TItem> (a bare wrapper over IAssertionSource<TItem>) rather than a richly-typed assertion source, so users of collection-type items lost all the specialized assertion API.

---

Architecture concern: combinatorial overload explosion

ListItemAtSatisfiesExtensions.cs is 311 lines covering 11 concrete collection types × 2 source types = 22 overloads. This approach works, but it won't scale. Every time a new assertion type is added to TUnit (e.g. SortedSetAssertion, ImmutableArrayAssertion, or any user-defined collection assertion type), this file must be updated with 2 more overloads. The pattern also doesn't compose — it says nothing about IList<IList<IList<T>>> or other nested structures.

A more maintainable design would expose a single generic overload that accepts a source factory:

// Single entry point — no enumeration of concrete types needed
public static ListItemAtSatisfiesAssertion<TList, TItem> Satisfies<TList, TItem, TSource>(
    this ListItemAtSource<TList, TItem> source,
    Func<TItem, string, TSource> sourceFactory,
    Func<TSource, IAssertion?> assertion,
    [CallerArgumentExpression(nameof(assertion))] string? expression = null)
    where TList : IList<TItem>
    where TSource : IAssertionSource<TItem>
{
    // one implementation
}

Usage becomes:

await Assert.That(items).ItemAt(0)
    .Satisfies(
        (item, label) => new CollectionAssertion<int>(item, label),
        a => a.Count().IsEqualTo(3));

This pushes the factory responsibility to the call site, but TUnit could still ship convenience helpers (e.g. static factory methods on each assertion type) rather than enumerating every combination. The key benefit: users can bring their own assertion source types and they'll work automatically — the current approach gives them no way to do that.

---

internal accessors as a design smell

// ListAssertions.cs (and ReadOnlyListAssertions.cs)
internal AssertionContext<TList> InternalListContext => _listContext;
internal int InternalIndex => _index;

These were private before; they're now exposed internal solely to allow the extension class (in the same assembly) to bypass normal construction. The Internal prefix signals this awkwardness. A cleaner alternative: a factory/builder method directly on ListItemAtSource<TList, TItem> that creates the assertion given a raw factory:

internal ListItemAtSatisfiesAssertion<TList, TItem> CreateSatisfiesAssertion(
    Func<TItem, int, IAssertion?> factory)
{
    _listContext.ExpressionBuilder.Append(...)
    return new ListItemAtSatisfiesAssertion<TList, TItem>(_listContext, _index, factory);
}

This hides the internals and keeps the responsibility for mutating ExpressionBuilder inside the source type, where it belongs.

---

The internal assertion constructor is unguarded on the expression side

The new internal constructor of ListItemAtSatisfiesAssertion skips the expression-appending that SatisfiesSpecialised does manually:

// Extension helper appends expression here...
source.InternalListContext.ExpressionBuilder.Append($".Satisfies({expression})");

// ...then constructs via internal ctor which does NOT append anything
return new ListItemAtSatisfiesAssertion<TList, TItem>(
    source.InternalListContext,
    source.InternalIndex,
    assertionFactory);

If anyone constructs ListItemAtSatisfiesAssertion via the internal ctor without calling ExpressionBuilder.Append first, the expression will be silently incomplete. The factory-method approach above would eliminate this risk by co-locating both steps.

---

Test coverage gaps

The regression tests cover String, Collection, Dictionary, and Set for both IList and IReadOnlyList — solid baseline. Missing:

• T[] (the ArrayAssertion overload is present but untested)
• HashSet<T> (the HashSetAssertion overload exists)
• Dictionary<K,V> concrete type (there's a MutableDictionaryAssertion overload for it)

Given this is a regression fix, having at least a smoke test for each overload variant that was added would make future regressions much easier to catch.

---

Summary

The core fix is sound — changing the internal delegate from Func<IAssertionSource<TItem>, Assertion<TItem>?> to Func<TItem, int, IAssertion?> and building the specialized source at dispatch time is exactly right. The backward-compatible forwarding constructor preserves the existing public API cleanly.

The concern is that the mechanism chosen (enumerating all known concrete types as overloads) trades one problem (missing specialization) for another (a maintenance tax on every new assertion type). Worth considering whether a single generic sourceFactory parameter would be a better long-term design before this pattern spreads to other assertion combinators.

[thomhurst]

Addressed the review feedback in ce722f9:

• moved specialised Satisfies construction behind source-owned CreateSatisfiesAssertion helpers
• removed the exposed InternalListContext/InternalIndex accessors
• added smoke coverage for array, concrete Dictionary, HashSet, read-only dictionary, and read-only set item overloads

Verification rerun:

• dotnet test TUnit.Assertions.Tests\TUnit.Assertions.Tests.csproj --framework net10.0 --no-restore -- --treenode-filter "///Issue5706Tests/*" (14/14 passed)
• dotnet test TUnit.PublicAPI\TUnit.PublicAPI.csproj --no-restore (15/15 passed after cleaning stale PublicAPI outputs)

[claude]

Code Review: Fix item-at Satisfies source typing (#5706)

The PR correctly identifies and fixes the root cause: ListItemAtSatisfiesAssertion was storing Func<IAssertionSource<TItem>, Assertion<TItem>?> which forced the lambda return type to be Assertion<TItem>? — making it impossible to use collection assertions that chain to a different Assertion<T> (e.g., Count().IsEqualTo(3) returns Assertion<int>, not Assertion<IEnumerable<int>>).

What's done well

• The internal refactor to Func<TItem, int, IAssertion?> is clean and separates item retrieval from assertion creation correctly.
• Constructor chaining preserves backward compatibility elegantly.
• The CreateSatisfiesAssertion bridge approach (following feedback from the previous review) correctly avoids exposing internal index/context accessors.
• Good test coverage across diverse concrete types.
• Public API snapshots updated properly.

---

Architectural Concern: The Root Fix May Be Simpler

The 303-line extension file is working around a return-type covariance constraint on the existing overload. The original signature requires Func<IAssertionSource<TItem>, Assertion<TItem>?>, but collection assertion chains return Assertion<int> (not Assertion<IEnumerable<int>>), so the lambda won't compile.

If most collection assertion extension methods (like Count()) are already defined on IAssertionSource<IEnumerable<T>> — rather than only on the concrete CollectionAssertion<T> — then adding a single overload with a relaxed return type directly on the Satisfies method would handle the majority of cases without any per-type dispatch:

// One new overload on ListItemAtSource<TList, TItem>
public ListItemAtSatisfiesAssertion<TList, TItem> Satisfies(
    Func<IAssertionSource<TItem>, IAssertion?> assertion,
    [CallerArgumentExpression(nameof(assertion))] string? expression = null)
{
    return CreateSatisfiesAssertion(
        (item, index) => assertion(new ValueAssertion<TItem>(item, $"item[{index}]")),
        expression);
}

With this, users can write item => item.Count().IsEqualTo(3) and the lambda compiles because IAssertion? is satisfied by the Assertion<int> return. The full 303-line extension file would only be needed if users specifically need the concrete assertion type (e.g., CollectionAssertion<int> has methods not available through IAssertionSource<IEnumerable<int>>).

Is the extension file truly necessary, or is the IAssertion? overload sufficient for all the test cases?

---

Scalability of the Extension File

If the per-type extension methods are genuinely needed, the static dispatch approach has inherent growth pressure:

• Missing today: ICollection<T>, Queue<T>, Stack<T>, LinkedList<T>, SortedList<TKey,TValue>, ImmutableArray<T>, ImmutableList<T>, etc.
• Any user-defined assertion source type will never work with this approach.

A more future-proof alternative for strongly-typed sources would be a factory overload:

public ListItemAtSatisfiesAssertion<TList, TItem> Satisfies<TSource>(
    Func<TItem, string, TSource> sourceFactory,
    Func<TSource, IAssertion?> assertion,
    [CallerArgumentExpression(nameof(assertion))] string? expression = null)
    where TSource : IAssertionSource<TItem>

Usage:

.Satisfies(
    (item, name) => new CollectionAssertion<int>(item, name),
    col => col.Count().IsEqualTo(3))

This covers all current and future assertion source types without adding overloads, at the cost of a slightly more verbose call site. The per-type extension methods could still be provided as ergonomic wrappers for the most common types.

---

Minor

• The string item path works correctly through the existing Func<IAssertionSource<TItem>, Assertion<TItem>?> overload (string assertions return Assertion<string>). The behavior asymmetry between string items and collection items is worth a code comment noting why no specialized overload is needed for string.
• #if NET5_0_OR_GREATER on IReadOnlySet<T> is correct; no issues there.

---

Overall the implementation is functionally correct and the previous review feedback has been addressed. The main open question is whether the simpler single-overload approach (Func<IAssertionSource<TItem>, IAssertion?>) would suffice, which would let you drop most of the extension file. If the specialized concrete types are genuinely needed for their extra methods, the factory overload pattern would scale better than the current per-type enumeration.

[claude]

Code Review

Previous feedback (constructor chaining, CreateSatisfiesAssertion encapsulation, test coverage gaps) has been addressed. The architecture is sound.

What's working well

IAssertionSourceFor<TItem, TSelf> CRTP interface — The right foundation. Using C# 11 static abstract members for a per-item factory enables fully generic dispatch without per-type overloads at the core level. The constraint where TSource : IAssertionSourceFor<TItem, TSource> correctly closes the loop.

CreateSatisfiesAssertion encapsulation — Resolves the previous concern about leaking InternalListContext/InternalIndex. The extension methods call an internal method from within the same assembly, which is the correct scope.

Backward-compatible constructor chaining — The old public ListItemAtSatisfiesAssertion(context, index, Func<IAssertionSource<TItem>, Assertion<TItem>?>) constructor chains to the new internal one. No binary breaking change, and the wrapping via new ValueAssertion<TItem> is transparent to callers.

Func<TItem, int, IAssertion?> — Passing index alongside item is essential for constructing the label $"item[{index}]" in the right place, rather than deferring label construction to CheckAsync. This is the key fix for #5706.

---

Remaining design concern: List<T> / Dictionary<TKey,TValue> bridging asymmetry

The comment explains the why well, but there is a cleanable root cause. ListAssertion<T> implements IAssertionSourceFor<IList<T>, ListAssertion<T>> (interface-typed), not IAssertionSourceFor<List<T>, ListAssertion<T>> (concrete-typed). So the generic Satisfies<TSource> path can't bind when the item is List<T>, forcing the explicit bridge:

// Current — two code paths
=> source.Satisfies<ListAssertion<TInner>>(assertion, expression);   // via IAssertionSourceFor
=> source.CreateSatisfiesAssertion(                                   // manual bridge
       (item, index) => assertion(new ListAssertion<TInner>(item, ...)), expression);

ListAssertion<T> could implement both interface variants without conflict, since the Create static methods differ in parameter type (IList<T> vs List<T>):

public class ListAssertion<TItem>
    : ListAssertionBase<IList<TItem>, TItem>
#if !NETSTANDARD2_0
    , IAssertionSourceFor<IList<TItem>, ListAssertion<TItem>>   // existing
    , IAssertionSourceFor<List<TItem>, ListAssertion<TItem>>    // new — eliminates bridge
#endif
{
    public static ListAssertion<TItem> Create(IList<TItem> item, string label) => new(item, label);
    public static ListAssertion<TItem> Create(List<TItem> item, string label)  => new(item, label);
}

C# can implement two parameterizations of the same generic interface when the static abstract member signatures are distinct (different TItem → different parameter type). With this, the concrete List<T> extension overloads would reduce to simple Satisfies<ListAssertion<TInner>> calls, eliminating the asymmetric CreateSatisfiesAssertion bridge and making the code more consistent. Same pattern applies to MutableDictionaryAssertion. Not a blocker, but worth a follow-up — the inconsistency will likely surface again when users try to add their own concrete-type overloads.

---

Minor: extension duplication between list / readonly-list source types

Each of the 11 logical type mappings appears twice in ListItemAtSatisfiesExtensions.cs — once for ListItemAtSource<TList, TItem> and once for ReadOnlyListItemAtSource<TList, TItem>. This is intrinsic because the return types differ (ListItemAtSatisfiesAssertion vs ReadOnlyListItemAtSatisfiesAssertion). A shared interface over both source types exposing CreateSatisfiesAssertion could halve this, but the return-type variance makes that non-trivial. Noting for visibility rather than blocking the PR.

---

Test coverage note

Tests cover IEnumerable<int>, int[], IDictionary<string,int>, Dictionary<string,int>, ISet<int>, HashSet<int>, IReadOnlyDictionary<string,int>, IReadOnlySet<int>, and string. The IList<T> and IReadOnlyList<T> item overloads (which are among the cleanest — pure Satisfies<TSource> delegation) don't appear to have dedicated test cases. Low risk, but worth adding for completeness.

---

Summary

Functionally correct, all previous feedback has been addressed, and the IAssertionSourceFor CRTP foundation is genuinely extensible — users can write Satisfies<MyCustomAssertion>(...) for types TUnit doesn't know about. The main open item is the List<T>/Dictionary<TKey,TValue> concrete-type bridging asymmetry, which is cosmetic now but becomes a maintenance tax as the pattern propagates.

[claude]

Code Review: Fix item-at Satisfies source typing — iteration 4

All prior feedback has been fully addressed. This review covers only the new commit (0080687e).

---

What changed since the last review

The latest commit does three things:

1. IAssertionSourceFor<TItem, TSelf> no longer extends IAssertionSource<TItem> — required to allow ListAssertion<T> and MutableDictionaryAssertion<T> to implement two parameterizations of the interface simultaneously.
2. ListAssertion<T> and MutableDictionaryAssertion<TKey,TValue> implement both the interface-typed and concrete-typed variants (IAssertionSourceFor<IList<T>> and IAssertionSourceFor<List<T>>), so the concrete overloads in the extension file now delegate through the generic Satisfies<TSource> path without any CreateSatisfiesAssertion bypass.
3. Test coverage added for IList<IList<T>>, IList<IReadOnlyList<T>>, IReadOnlyList<IList<T>>, and IReadOnlyList<IReadOnlyList<T>> — directly addresses the gap flagged in the last review.

All three of these were explicitly flagged as open items in the previous review, so well done on the quick turnaround.

---

Removing IAssertionSourceFor : IAssertionSource<TItem>

This is the right call. The commit message explains the invariance problem precisely, and the <remarks> doc on the interface explains the design rationale for future maintainers. The public API snapshot reflects the change correctly. Since IAssertionSourceFor was never shipped in a release, removing the inheritance is not a breaking change.

One small thing to verify: the API snapshots show that ArrayAssertion<TItem>, CollectionAssertion<TItem>, ReadOnlyListAssertion<TItem>, etc. now list fewer interfaces in their type hierarchy (the transitively-inherited IAssertionSource and IAssertionSource<TItem> that came from IAssertionSourceFor are no longer shown). The underlying implementations are unaffected — those interfaces are still satisfied through the base class hierarchy — so this is a snapshot artifact, not a behavior regression.

---

Minor gap: IList<List<T>> case not tested

The refactored List<T> overloads now route through Satisfies<ListAssertion<TInner>> (via the new IAssertionSourceFor<List<T>, ListAssertion<T>> implementation). This is the code path most changed by this commit, yet it has no dedicated test. All existing tests use IList<IList<T>>, not IList<List<T>>. A minimal test case:

[Test]
public async Task List_ItemAt_Satisfies_Preserves_ConcreteList_Item_Source()
{
    IList<List<int>> items = new List<List<int>> { new() { 1, 2, 3 } };
    await Assert.That(items).ItemAt(0).Satisfies(item => item.Count().IsEqualTo(3));
}

Same applies to IList<Dictionary<string,int>> (the concrete dictionary path). Low risk since the types are equivalent at the IList<T> / IDictionary<K,V> level, but a regression test for exactly the code path changed this round would be good practice.

---

Summary

The implementation is clean, the internal Satisfies<TSource> path is now used consistently for both concrete and interface item types, the CreateSatisfiesAssertion bypass is gone, and the test coverage is substantially improved. The only remaining suggestion is smoke tests for the List<T> and Dictionary<K,V> concrete-item paths that are specifically enabled by this commit. No blockers.

[claude]

Code Review: Fix item-at Satisfies source typing (#5764)

This is a well-structured fix for a real usability problem. The core issue is that ItemAt(...).Satisfies(item => item.Count().IsEqualTo(3)) was silently giving callers only a ValueAssertion<TItem> (the generic fallback), losing the specialized collection-aware assertion API surface. The fix is correct and the design is mostly sound.

---

What the PR does well

Backward compatibility is preserved correctly. The original public constructor ListItemAtSatisfiesAssertion(context, index, Func<IAssertionSource<TItem>, Assertion<TItem>?>) is retained and delegates to the new internal raw-item constructor. Any code that compiled before will continue to compile and behave identically.

IAssertionSourceFor<TItem, TSelf> is a clean abstraction. Using the CRTP static-abstract pattern (static abstract TSelf Create(TItem item, string label)) is idiomatic for this kind of factory-without-instance dispatch. The decision not to extend IAssertionSource<TItem> (documented in the XML doc remarks) correctly avoids the conflicting Context property problem when a class implements multiple parameterisations of the interface — e.g., ListAssertion<TItem> implementing both IAssertionSourceFor<IList<TItem>, ...> and IAssertionSourceFor<List<TItem>, ...>.

Expression string is correctly passed through. The $"item[{index}]" label is consistent with what the old code produced when it created new ValueAssertion<TItem>(actualItem, $"item[{_index}]") inline in CheckAsync, so error messages will not change.

#if !NETSTANDARD2_0 guards are correct. Static abstract interface members require C# 11 / .NET 7+. The project targets netstandard2.0;net8.0;net9.0;net10.0, so !NETSTANDARD2_0 is the right predicate — it compiles out the feature on the legacy target while enabling it everywhere the project actually runs.

CreateSatisfiesAssertion centralises the expression-builder append. Extracting the shared step into an internal helper prevents the expression string from being appended once by each Satisfies overload, which would have doubled the text in error messages.

---

Issues and observations

1. LastItem().Satisfies() has the same problem and is untouched

ListLastItemSource<TList, TItem>.Satisfies still uses the old Func<IAssertionSource<TItem>, Assertion<TItem>?> signature and creates a ValueAssertion<TItem> inside ListLastItemSatisfiesAssertion.CheckAsync. A user writing:

await Assert.That(nestedLists).LastItem().Satisfies(item => item.Count().IsEqualTo(3));

hits the exact same problem as issue #5706 but for the last item. The same fix (a Satisfies<TSource> overload on ListLastItemSource + a corresponding internal constructor on ListLastItemSatisfiesAssertion) should be applied. ReadOnlyListLastItemSource and ReadOnlyListLastItemSatisfiesAssertion have identical pre-existing gaps. This PR's scope is explicitly ItemAt, so it is not a blocker, but it should be tracked as a follow-up.

2. The extension class ListItemAtSatisfiesExtensions will need perpetual manual maintenance

The 22-overload file works, but it encodes a fixed enumeration of "known collection shapes". Every new assertion source type (e.g., a future SortedListAssertion<T>, LinkedListAssertion<T>, or user-defined custom source) requires both:

• Implementing IAssertionSourceFor<TItem, TSelf> on the new class, and
• Adding two overloads to ListItemAtSatisfiesExtensions for the ListItemAtSource and ReadOnlyListItemAtSource variants.

The Satisfies<TSource> generic overload already exists on ListItemAtSource and handles the dispatch. The extension methods exist only because C# type inference can't infer TSource from Func<TSource, IAssertion?> when the parameter is a collection type like IList<TInner> — the compiler can't match TItem = IList<TInner> and TSource = ListAssertion<TInner> automatically.

A comment in the file explaining this trade-off and directing future assertion source authors to add overloads here would reduce confusion. For example:

// NOTE: Each overload here exists solely because C# type inference cannot infer TSource
// from Func<TSource, IAssertion?> when TSource does not directly match TItem.
// If you add a new IAssertionSourceFor<T, TSource> implementation, add corresponding
// List/ReadOnlyList overloads below, or users will need to spell out the type argument:
//   .Satisfies<MyAssertion<T>>(item => item.SomeMethod())

This does not block the PR but would prevent "why isn't my source being picked up" confusion for contributors.

3. Minor: the internal CreateSatisfiesAssertion is internal but lives on a public class

ListItemAtSource<TList, TItem>.CreateSatisfiesAssertion is internal, which means it is not accessible to consumers outside the assembly — that is intentional and correct. Just worth noting that the internal visibility also means the extension methods in ListItemAtSatisfiesExtensions can call source.Satisfies<TSource>(...) (the public generic overload), not CreateSatisfiesAssertion directly. The routing is: extension method → Satisfies<TSource> (public) → CreateSatisfiesAssertion (internal). This chain is correct.

4. Public API snapshot represents static abstract as an instance member

public interface IAssertionSourceFor<TItem, TSelf>
    where TSelf : .<TItem, TSelf>
{
    TSelf Create(TItem item, string label);   // ← missing 'static'
}

This is a known limitation of the public API diffing tooling (it does not distinguish static-abstract from instance), not a defect in the PR itself. Worth being aware of if anyone reads the snapshot literally.

---

Summary

The core fix is correct, well-tested (18 regression tests spanning both IList and IReadOnlyList variants across all major collection shapes), and backward compatible. The IAssertionSourceFor<TItem, TSelf> interface is a clean extensibility seam. The main structural concern is that ListItemAtSatisfiesExtensions will need to grow alongside any new collection assertion sources — a comment documenting this would make the pattern self-explanatory for future contributors. The LastItem().Satisfies() gap is pre-existing and out of scope here, but worth a follow-up issue.

Good work overall — the fix does exactly what it says and doesn't introduce any regressions I can identify.