feat(assertions): return typed value from IsAssignableTo<T> (#6184)

[thomhurst]

Summary

Closes #6184.

IsTypeOf<T>() returns the value cast to T, but IsAssignableTo<T>() previously returned the original (untyped) value, forcing a second manual cast:

// Before
await Assert.That(okResult.Value).IsAssignableTo<IEnumerable<MyDto>>();
IEnumerable<MyDto> data = (IEnumerable<MyDto>)okResult.Value!;

// After
IEnumerable<MyDto> data = (await Assert.That(okResult.Value).IsAssignableTo<IEnumerable<MyDto>>())!;

IsAssignableTo<T>() now returns the value cast to T, matching IsTypeOf<T>().

Implementation

IsAssignableToAssertion<TTarget, TValue> now derives from Assertion<TTarget> (was Assertion<TValue>) and maps the context to the target type via context.Map<TTarget>(value => value is TTarget c ? c : default), so awaiting it yields TTarget?.

Validation still runs against the retained original context, so the existing behaviour is fully preserved:

• dual-mode checks (value type vs. thrown-exception type, e.g. .Throws<X>().And.IsAssignableTo<Y>()),
• null handling,
• error messages ("type X is not assignable to Y").

Both the mapped and original contexts share the same cached underlying evaluation, so the source is still evaluated exactly once. No extension-method signatures changed — the awaited type simply flips from TValue? to TTarget?.

Scope

IsAssignableTo only. IsAssignableFrom<TSource> and the negated variants (IsNotAssignableTo/IsNotAssignableFrom/IsNotTypeOf) are intentionally unchanged — they have no meaningful typed return.

Tests

• New TUnit.Assertions.Tests/IsAssignableToTypedReturnTests.cs: cast-to-interface (the issue scenario), cast-to-base, exact type, plus not-assignable and null failure cases.
• Full assertions suite green (2130 tests), including TypeAssertionAmbiguityTests, TypeOfTests, and Throws* regression suites.

[codacy-production]

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 9 complexity

Metric	Results
Complexity	9

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

The feature itself is well-motivated and the overall approach is sound. A few points worth considering:

---

Double-evaluation risk in CheckAsync

The most architecturally interesting part of the change is calling _sourceContext.GetAsync() directly inside CheckAsync instead of using the mapped metadata parameter that the base class provides:

protected override async Task<AssertionResult> CheckAsync(EvaluationMetadata<TTarget> metadata)
{
    var (value, exception) = await _sourceContext.GetAsync();  // bypasses metadata
    ...
}

The comment in the code notes that both contexts share the same cached evaluation, which is correct — EvaluationContext<TValue>.GetAsync() caches after first call. However, the metadata parameter passed by the base class ExecuteCoreAsync() already contains the evaluated Value and Exception from Context.GetAsync() (the mapped context). By ignoring metadata and calling _sourceContext.GetAsync() separately, this works today because the cache is shared at the EvaluationContext level (the mapped context's evaluator calls GetAsync() on the source, so both are populated together).

The subtle risk: if someone later changes the evaluation flow (e.g., adds retry/re-evaluation logic) the two calls could potentially diverge, since _sourceContext and Context have independent EvaluationContext instances even though they share the underlying source evaluation chain. The caching only prevents double execution of the original delegate; it does not enforce that both contexts return identical snapshots if the evaluation is ever invalidated.

A safer alternative would be to store the original cast result alongside the assertion and validate using the mapped metadata only, eliminating the need to reach back to _sourceContext at all:

// In constructor:
: base(context.Map<TTarget>(value => {
    _castSucceeded = value is TTarget casted ? (true, casted) : (false, default);
    return _castSucceeded.Item2;
}))

// In CheckAsync: use metadata.Value/metadata.Exception directly

Or, keep the current approach but add a [MethodImpl(MethodImplOptions.AggressiveInlining)] comment or test that makes the coupling to the caching guarantee explicit.

---

Missing test coverage for the Throws chaining scenario

The PR description specifically calls out that dual-mode checks (value type vs. thrown-exception type, e.g., .Throws<X>().And.IsAssignableTo<Y>()) are preserved. However, the new test file IsAssignableToTypedReturnTests.cs does not include a test exercising this path directly. The existing tests in Throws.ExactlyTests.cs use IsAssignableTo on an already-awaited exception reference ((object)ex), not chained directly on the Throws assertion.

A test like the following would give direct regression coverage for the exception path through CheckAsync:

[Test]
public async Task IsAssignableTo_OnThrown_ExceptionType_ReturnsTypedException()
{
    var result = await Assert.That(async () =>
    {
        throw new InvalidOperationException("test");
    }).ThrowsAsync<InvalidOperationException>()
      .And
      .IsAssignableTo<Exception>();

    // result should be Exception-typed
    await Assert.That(result!.Message).IsEqualTo("test");
}

Without this test, the exception branch (if (exception != null)) in CheckAsync is exercised only indirectly.

---

Minor: inconsistent use of AssertionResult.Passed vs AssertionResult._passedTask

This PR converts the async overrides to return AssertionResult.Passed (correct for async Task<AssertionResult> methods), and removes the now-unnecessary Task.FromResult(AssertionResult.Failed(...)) wrappers — both clean improvements. However, the sibling synchronous assertion classes in the same file (IsNotAssignableToAssertion, IsAssignableFromAssertion, IsNotAssignableFromAssertion, TypeOfAssertion, IsNotTypeOfAssertion) still use the internal AssertionResult._passedTask instead of the public AssertionResult.Passed wrapped in a task. This is not a bug but creates a stylistic inconsistency within the file. If there's an intent to clean this up across the codebase, this file would be a natural starting point.

---

Overall assessment

The feature is correct, the use of context.Map<TTarget>() for type projection is consistent with how TypeOfAssertion works, and the fix eliminates a real user pain point. The implementation is clean. The points above are concerns worth addressing — particularly the missing exception-path test — but none are blockers for the current functionality.

[thomhurst]

Thanks for the thorough review. Addressed in 7f1c3d7:

1. Missing Throws-chaining test coverage — Good catch, added two direct tests plus a single-evaluation guard:

• IsAssignableTo_ChainedAfterThrows_ChecksAndReturnsExceptionType — …Throws<InvalidOperationException>().And.IsAssignableTo<Exception>(). In the Throws chain the asserted context value is the thrown exception (ThrowsAssertion<TException> : Assertion<TException>), so the map casts it and the awaited result is the exception typed as Exception (the test asserts result!.Message == "boom"). The exception branch of CheckAsync is now exercised directly.
• IsAssignableTo_ChainedAfterThrows_Fails_WhenExceptionNotAssignable — covers the failing exception path.
• IsAssignableTo_EvaluatesSourceOnlyOnce — counts delegate invocations to lock the caching guarantee (see point 2).

(Note: the API is Throws<T>(), not ThrowsAsync<T>().)

2. Double-evaluation / coupling to caching — Now covered by IsAssignableTo_EvaluatesSourceOnlyOnce, which fails if the source delegate runs more than once, so a future re-evaluation change can't silently regress it.

I'd push back on the suggested closure alternative (capturing the cast result into a field from inside the Map lambda): the map runs lazily during evaluation, so mutating assertion state from it is a worse coupling than reading the original context, and it still wouldn't supply the original type needed for the "type X is not assignable to Y" message on a failed cast (the mapped value is null there). _sourceContext is required specifically for that message and is genuinely needed, not just convenient.

3. _passedTask vs Passed — Intentional, not changing it. The sync sibling overrides return Task<AssertionResult> directly, so _passedTask (a cached, pre-allocated Task) avoids a Task.FromResult(...) allocation on every call. AssertionResult.Passed is correct only because my overrides became async. Converting the sync siblings would add allocations, so the current mix is the efficient choice rather than an inconsistency to clean up.

[claude]

Follow-up Review

The three concerns from the previous review round have been addressed:

• Throws-chaining test coverage — Added and exercising the exception != null branch in CheckAsync directly.
• Double-evaluation guard — IsAssignableTo_EvaluatesSourceOnlyOnce locks the caching guarantee against future regression.
• _passedTask vs Passed — The explanation (sync siblings use a pre-allocated task to avoid a Task.FromResult allocation; the async override can return the struct directly) is sound. No change needed.

The pushback on the closure-mutating-field alternative is also correct: mutating assertion state from inside a lazy Map lambda would be a worse coupling, and it still wouldn't supply the original type name for the failure message when the cast returns null.

---

One remaining nit: unused metadata parameter

CheckAsync now ignores its parameter entirely:

protected override async Task<AssertionResult> CheckAsync(EvaluationMetadata<TTarget> metadata)
{
    var (value, exception) = await _sourceContext.GetAsync();  // metadata never read

If the project compiles with warnings-as-errors (or has an analyzer that flags unused parameters), this will break the build. Consider using a discard name to signal the intentional bypass:

protected override async Task<AssertionResult> CheckAsync(EvaluationMetadata<TTarget> _)

This documents the intentional bypass — readers immediately understand that the mapped metadata isn't used here, not that someone forgot to use it.

---

Overall

The implementation is correct, the test coverage is thorough, and the dual-context design is well-explained. The unused parameter is the only remaining nit, and it's a minor one. Good change.

[thomhurst]

Addressed the remaining nit in 897671f — renamed the unused CheckAsync parameter to _ with a short comment explaining the intentional bypass (validation runs against the original context for the failure message). Build green, all 8 tests still passing.

[claude]

Final Review — All Previous Concerns Addressed

This is a clean, well-executed improvement. All findings from prior rounds have been resolved:

Round 1 → Round 2 (resolved):

• Exception-path test coverage — and now exercise the exception != null branch in CheckAsync directly.
• Double-evaluation guard — IsAssignableTo_EvaluatesSourceOnlyOnce locks the caching guarantee with a real assertion that fails if the source delegate runs more than once.
• _passedTask vs Passed — The performance rationale (sync siblings use the pre-allocated task; the async override can return the struct directly) was correct. No change needed and the explanation is sound.

Round 2 → Round 3 (resolved):

• Unused parameter name — CheckAsync(EvaluationMetadata<TTarget> _) now uses a discard with an explanatory comment, clearly documenting the intentional bypass.

---

The final state of the implementation is solid:

• The dual-context design (_sourceContext for validation, mapped context for the return type) is correctly motivated and well-commented.
• The context.Map<TTarget>() approach is consistent with how TypeOfAssertion already works.
• 9 test cases cover the golden path, failure cases, null, Throws-chaining, and the evaluation-count regression guard.
• The async Task<AssertionResult> return type change allows clean direct returns instead of Task.FromResult allocations.

Approved.

[claude]

Final Review — All Previous Concerns Addressed

This is a clean, well-executed improvement. All findings from prior rounds have been resolved:

Round 1 → Round 2 (resolved):

• Exception-path test coverage — IsAssignableTo_ChainedAfterThrows_ChecksAndReturnsExceptionType and IsAssignableTo_ChainedAfterThrows_Fails_WhenExceptionNotAssignable now exercise the exception != null branch in CheckAsync directly.
• Double-evaluation guard — IsAssignableTo_EvaluatesSourceOnlyOnce locks the caching guarantee with a real assertion that fails if the source delegate runs more than once.
• _passedTask vs Passed — The performance rationale (sync siblings use the pre-allocated task; the async override can return the struct directly) was correct. No change needed and the explanation is sound.

Round 2 → Round 3 (resolved):

• Unused parameter name — CheckAsync(EvaluationMetadata<TTarget> _) now uses a discard with an explanatory comment, clearly documenting the intentional bypass.

---

The final state of the implementation is solid:

• The dual-context design (_sourceContext for validation, mapped context for the return type) is correctly motivated and well-commented.
• The context.Map<TTarget>() approach is consistent with how TypeOfAssertion already works.
• 9 test cases cover the golden path, failure cases, null, Throws-chaining, and the evaluation-count regression guard.
• The async Task<AssertionResult> return type change allows clean direct returns instead of Task.FromResult allocations.

Approved.

[claude]

Code Review — PR #6187: feat(assertions): return typed value from IsAssignableTo<T>

This PR went through three prior review rounds and all previously raised concerns have been addressed. This is a fresh pass over the final code.

---

What is correct and well-done

Design consistency. The context.Map<TTarget>() pattern is already used by TypeOfAssertion<TFrom, TTo>. Using the same mechanism here keeps the two type-narrowing assertions architecturally symmetric.

Dual-mode preservation. The exception != null branch in CheckAsync reads from _sourceContext directly, so the assertion correctly works when chained after .Throws<X>().And.

Single-evaluation guarantee. The IsAssignableTo_EvaluatesSourceOnlyOnce test locks this as a regression guard.

Discard parameter. CheckAsync(EvaluationMetadata<TTarget> _) correctly signals the intentional bypass of the mapped metadata — the right idiom.

Async cleanup. Converting CheckAsync to async Task<AssertionResult> eliminates Task.FromResult(...) wrappers on all three return paths, which is cleaner.

Test coverage. 9 test cases across interface cast, base-type cast, exact type, not-assignable failure, null failure, Throws-chained success, Throws-chained failure, and single-evaluation guard. Thorough.

Public API snapshots. All four .verified.txt files updated to reflect the base class change and discard parameter rename.

---

Minor observations (non-blocking)

1. Scope decision is correct. Intentionally leaving IsAssignableFrom<TSource>, IsNotAssignableTo<TTarget>, and IsNotAssignableFrom<TSource> unchanged is the right call — IsAssignableFrom tests a capability of the asserted type rather than narrowing it, and negated forms returning a narrowed value would be misleading. A one-line XML doc comment on IsAssignableFromAssertion explaining why it differs from IsAssignableToAssertion would help future contributors, but not required.

2. Pre-existing pattern inconsistency (not introduced here). Sibling classes like IsNotAssignableToAssertion still use the internal _passedTask field while this class now uses async/await. This is a pre-existing performance tradeoff, not a bug, but worth noting if there's ever a cleanup pass.

---

Verdict

The implementation is correct and complete. The feature eliminates a real user pain point (manual casting after IsAssignableTo<T>()), the design is consistent with the existing TypeOfAssertion pattern, all edge cases are covered by tests, and all prior review findings have been resolved.

Approved. ✅