feat: covariant assertions for interfaces and non-sealed classes

[thomhurst]

Summary

• Source-generated assertion extension methods now use generic type parameters with constraints for interface and non-sealed class target types, enabling assertions to work on derived types without explicit casts
• Both [GenerateAssertion] (MethodAssertionGenerator) and [AssertionExtension] (AssertionExtensionGenerator) support covariance
• Covariance is safely excluded for value types, sealed classes, arrays, type parameters, and types with unresolved generic parameters (e.g., Lazy<T>)

Example

// User-defined assertion targeting an interface
[GenerateAssertion(ExpectationMessage = "to have foo '{expected}'")]
public static bool HasFoo(this IInterface target, string expected)
    => target.Foo == expected;

// Now works seamlessly on derived types:
Impl item = new Impl { Foo = "bar" };
await Assert.That(item).HasFoo("bar"); // Previously required cast to IInterface

Generated code change

// Before:
public static HasFoo_Assertion HasFoo(this IAssertionSource<IInterface> source, ...)

// After:
public static HasFoo_Assertion HasFoo<TActual>(this IAssertionSource<TActual> source, ...)
    where TActual : IInterface

Test plan

• All 56 source generator snapshot tests pass (3 snapshots updated)
• All 1931 assertion tests pass
• TUnit.Assertions and TUnit.TestProject build cleanly for net10.0
• Value types (HttpStatusCode), sealed classes (string), arrays, and generic types (Lazy) correctly excluded from covariance

Closes #4830

[claude]

Code Review: Covariant Assertions for Interfaces and Non-Sealed Classes

This is a well-designed feature that solves a real usability pain point. The core approach — introducing a generic TActual type parameter with a constraint — is the right solution for enabling covariant assertion chaining without explicit casts. The PR description is thorough, and the exclusion logic (sealed classes, value types, arrays, open generics) is sound.

---

Issue 1: AssertionExtensionGenerator is missing the ContainsTypeParameter guard

MethodAssertionGenerator explicitly excludes types that contain unresolved type parameters:

var isCovariantCandidate = (targetType.TypeKind == TypeKind.Interface || targetType.TypeKind == TypeKind.Class)
    && !targetType.IsSealed
    && !ContainsTypeParameter(targetType);  // ← guards against Lazy<T>, IList<T>, etc.

AssertionExtensionGenerator does not have this check:

var isCovariantCandidate = !isNullableOverload
    && (typeParam.TypeKind == TypeKind.Interface || typeParam.TypeKind == TypeKind.Class)
    && !typeParam.IsSealed;  // ← no ContainsTypeParameter guard

The ITypeParameterSymbol check handles direct type parameters (e.g. T), but a named type like IList<T> or Lazy<T> would still pass as a covariant candidate if it appears as the assertion target type in an [AssertionExtension]-decorated class. This is an inconsistency that could produce broken generated code.

Suggestion: Add the same ContainsTypeParameter helper to AssertionExtensionGenerator, or extract it to a shared static utility, and include it in the isCovariantCandidate check.

---

Issue 2: Potential TActual name collision with user-defined generic parameters

When generating the combined generic declaration:

var allGenericParams = new List<string>();
if (isCovariant)
{
    allGenericParams.Add("TActual");
}
allGenericParams.AddRange(genericParams);  // ← user method's own type params

If a user writes an assertion method that already has a generic type parameter named TActual:

[GenerateAssertion(ExpectationMessage = "...")]
public static bool HasItem<TActual>(this IMyCollection target, TActual expected) => ...;

The generated code would produce HasItem<TActual, TActual>(...) — a compilation error.

Suggestion: Either document that TActual is a reserved name for the generator, or pick a name that is unlikely to conflict (e.g. TActualSource__), or add a diagnostic error when the conflict is detected.

---

Minor: Use modern C# range syntax instead of Substring

CLAUDE.md specifies LangVersion is preview and to use the latest syntax. Both generators contain:

constraintType = typeParamDisplay.Substring(0, typeParamDisplay.Length - 1);

Prefer:

constraintType = typeParamDisplay[..^1];

---

What's done well

• The ContainsTypeParameter recursive helper in MethodAssertionGenerator correctly handles arrays, nested generics, and direct type parameters.
• Using source.Context.Map<T>(static x => (T?)x) for the context narrowing is clean — the static lambda avoids closures, and explicitly specifying the TNew type argument preserves nullability correctly.
• The `[UnconditionalSuppressMessage("Trimming", "IL2091")]" attribute is correctly applied wherever a new generic parameter is introduced, maintaining AOT compatibility.
• Snapshot tests are properly updated.
• Correct exclusion of nullable overloads from the covariant path avoids duplicate-overload errors.

The two issues above (missing ContainsTypeParameter in AssertionExtensionGenerator and the TActual name collision) are worth addressing before merge, particularly the first one since it's a consistency gap between the two generators.

[claude]

Follow-up Review: Refactor Commit (4da27a1)

Reviewing the second commit against the previous review issues.

---

Issue 1 (from previous review): FIXED ✓

The missing ContainsTypeParameter guard in AssertionExtensionGenerator has been addressed by extracting the shared CovarianceHelper static class. Both generators now go through the same IsCovariantCandidate logic consistently. The extraction is clean — well-documented methods, focused responsibilities, and the static modifier is appropriate since there's no state.

---

Issue 2 (from previous review): Still open

The TActual name collision with user-defined generic parameters is still present. In MethodAssertionGenerator.cs:

var genericDeclaration = isCovariant
    ? genericParams.Count > 0
        ? $"<TActual, {string.Join(", ", genericParams)}>"
        : "<TActual>"
    : ...

If a user writes:

[GenerateAssertion(ExpectationMessage = "...")]
public static bool Contains<TActual>(this IMyCollection target, TActual expected) => ...;

The generated output becomes Contains<TActual, TActual>(...) — a compile error. The same applies to AssertionExtensionGenerator where genericTypeParam = "TActual" is hardcoded.

Suggested fix: Check for collision before emitting, or use a mangled name that's clearly reserved (e.g. TActualSource):

// In CovarianceHelper or at the call site:
private static string ChooseTActualName(IEnumerable<string> existingTypeParams)
{
    const string preferred = "TActual";
    var name = preferred;
    int i = 0;
    while (existingTypeParams.Contains(name))
        name = $"{preferred}{++i}";
    return name;
}

Alternatively, document TActual as a reserved name (similar to how some generators reserve __-prefixed identifiers) and emit a Diagnostic error if the conflict is detected. The diagnostic approach would be more discoverable.

---

New observation: Substring vs range syntax in CovarianceHelper

CovarianceHelper.GetConstraintTypeName uses:

return typeName.Substring(0, typeName.Length - 1);

CLAUDE.md specifies LangVersion = preview. Prefer:

return typeName[..^1];

Minor point, but consistent with the codebase's style guidelines.

---

CovarianceHelper quality

The extracted helper is well-structured:

• ContainsTypeParameter is correctly recursive over named type arguments, array element types, and direct type parameter symbols
• The XML doc comments are clear and accurate
• static lambda in GetCovariantContextExpr avoids closure allocation
• Nullable annotations are properly propagated through GetNullableCastType

The main outstanding item before merge remains the TActual name collision potential.

[thomhurst]

Thanks for the thorough review!

Issue 1 (missing ContainsTypeParameter in AssertionExtensionGenerator): Fixed in 4da27a1 — extracted a shared CovarianceHelper class that both generators now use, including the ContainsTypeParameter guard.

Issue 2 (TActual name collision): Fixed in ff70b54 — added CovarianceHelper.GetCovariantTypeParamName() which checks existing generic parameters and falls back to TActual_ (appending underscores) if TActual is already taken.

Minor (range syntax): The source generator targets netstandard2.0, which doesn't support [..^1] range syntax (System.Index/System.Range require .NET Core 3.0+). The Substring call is correct here.

[claude]

Final Review: All Previous Issues Addressed

Issue 1 (ContainsTypeParameter in AssertionExtensionGenerator): RESOLVED ✓

Both generators now route through CovarianceHelper.IsCovariantCandidate, which includes the ContainsTypeParameter guard. The extraction to a shared static class was the right architectural call — single responsibility, no duplication.

Issue 2 (TActual name collision): RESOLVED ✓

CovarianceHelper.GetCovariantTypeParamName correctly handles collisions by appending underscores:

var candidate = PreferredName + "_";
while (existing.Contains(candidate))
    candidate += "_";

This is functionally correct. A user would have to name their parameter TActual or TActual_ to encounter the fallback, which is extremely unlikely in practice.

Minor (Substring vs range syntax): Correctly unchanged

The source generator targets netstandard2.0, where System.Index/System.Range are unavailable. The Substring call is the correct approach here — good catch on the explanation.

---

Summary

The implementation is solid:

• Correct exclusion criteria (sealed, value types, arrays, open generics) via IsCovariantCandidate
• Clean Map<T>(static x => (T?)x) pattern for context narrowing — static lambda avoids allocations
• [UnconditionalSuppressMessage("Trimming", "IL2091")] applied consistently to all covariant methods, with an accurate justification
• Snapshot tests updated correctly
• CovarianceHelper is well-documented and focused

This is ready to merge. The feature delivers a genuine ergonomic improvement for users working with interface-typed assertions, and the implementation is conservative about when covariance applies.

[claude]

Follow-up Review: Commit aeb032d (chore: update public API snapshots)

This 4th commit is purely a public API snapshot update — no logic changes. All 4 modified files follow the same pattern: extending the verified API signatures to include the new <TActual> generic parameter and where TActual : <ConstraintType> constraint, consistent with the covariant generation changes from the earlier commits.

Snapshot Quality Check

The new signatures correctly match what the source generator now emits:

• [UnconditionalSuppressMessage("Trimming", "IL2091", ...)] attribute is present on each covariant method ✓
• Generic parameter <TActual> appears in method signature ✓
• where TActual : <BaseType> constraint is correctly specified ✓
• Non-covariant types (sealed classes, value types) retain the old non-generic signature ✓

All three previous issues (missing ContainsTypeParameter guard, TActual name collision, Substring vs range syntax) were addressed in commits 2 and 3. This final commit is consistent with those fixes.

Summary

All previous review feedback has been addressed. The PR is in good shape and ready to merge. The public API snapshots correctly reflect the breaking change in method signatures (this is a source-compatible change for users — existing call sites continue to work due to type inference, while derived types no longer require explicit casts).

[claude]

Follow-up Review: Commit 2faa66e (refactor: minor cleanup in covariance helpers)

Reviewing the 5th and latest commit against the previous review state (which concluded the PR was ready to merge after commit aeb032dfd9).

Changes in this commit

1. Cache typeParam.ToDisplayString() in AssertionExtensionGenerator

Previously the method was called up to 3 times across different code paths. Caching it as typeParamDisplay at the top of the method before the if/else if/else block is correct and clean — the value is the same regardless of which branch is taken.

2. Replace HashSet<string> with linear Contains() in GetCovariantTypeParamName

The reasoning in the commit message is correct — for 0–2 items, HashSet<T> allocation overhead exceeds the cost of linear search. One thing worth noting: the original HashSet approach materialized the enumerable once, while the new code can iterate it multiple times (once for the initial check, and once per loop iteration). In practice this is safe because the call sites pass a List<string> (which supports multiple enumeration), but if the signature ever changes to accept a lazily-evaluated IEnumerable<string>, the while-loop would re-evaluate on each iteration. This is a very low risk given the usage pattern, but worth a brief code comment if you want to guard against future misuse.

3. Inline GetNullableCastType into GetCovariantContextExpr

Removing a single-use private helper and inlining its one-liner body (typeName.EndsWith("?") ? typeName : $"{typeName}?") is the right call — it reduces indirection without losing clarity. The moved comment ("Map's Func takes TValue? and returns TNew?") is preserved on the calling method.

---

Summary

All three changes are correct and appropriately scoped. No regressions introduced. The PR remains in good shape — all issues from prior reviews have been resolved across the 5-commit history. Ready to merge.

[claude]

Follow-up Review: Commit 2faa66e (refactor: minor cleanup in covariance helpers)

Reviewing the 5th and latest commit against the previous review state (which concluded the PR was ready to merge after commit aeb032dfd9).

Changes in this commit

1. Cache typeParam.ToDisplayString() in AssertionExtensionGenerator

Previously called up to 3 times across different code paths. Caching it as typeParamDisplay at the top of the method before the if/else chain is correct and clean — the value is the same regardless of which branch is taken.

2. Replace HashSet<string> with linear Contains() in GetCovariantTypeParamName

The reasoning in the commit message is correct — for 0–2 items, HashSet allocation overhead exceeds the cost of linear search. One thing worth noting: the original HashSet approach materialized the enumerable once, while the new code can iterate existingGenericParams multiple times (once for the initial check, once per loop iteration in the collision-avoidance path). This is safe in all current call sites since the callers pass a List<string>, but if the signature ever changes to accept a lazily-evaluated IEnumerable<string>, the while-loop would re-evaluate on each iteration. Low risk given usage patterns, but a brief code comment would guard against future misuse.

3. Inline GetNullableCastType into GetCovariantContextExpr

Removing a single-use private helper and inlining its one-liner body is the right call — it reduces indirection without losing clarity. The explanatory comment ("Map's Func takes TValue? and returns TNew?") is preserved on the calling method.

---

Summary

All three changes are correct and appropriately scoped. No regressions introduced. The PR remains in good shape — all issues from prior reviews have been resolved across the 5-commit history. Ready to merge.

[claude]

Follow-up Review: Commit 3c2cd47 (fix: ensure netstandard2.0 TUnit.Mocks built before SourceGenerator.Tests)

This is a single-file change to TUnit.Mocks.SourceGenerator.Tests/TUnit.Mocks.SourceGenerator.Tests.csproj, adding a build-order dependency.

What the change does

A second ProjectReference to the same TUnit.Mocks.csproj is added with:

• SetTargetFramework="TargetFramework=netstandard2.0" — forces the netstandard2.0 TFM to be built before this test project starts
• ReferenceOutputAssembly="false" — prevents it from being added as a code reference (avoiding conflict with the existing unrestricted reference on line 7)
• Private="false" — prevents the DLL from being copied to the output directory

This is the standard MSBuild idiom for expressing a TFM-specific build-order dependency without creating a duplicate assembly reference, and it's correct here.

Observation: Duplicate ProjectReference is intentional but slightly unusual

The csproj now references TUnit.Mocks.csproj twice:

<!-- Line 7 - assembly reference (builds the test project's TFM) -->
<ProjectReference Include="..\TUnit.Mocks\TUnit.Mocks.csproj" />

<!-- Lines 19-22 - build-order dependency only (forces netstandard2.0 build) -->
<ProjectReference Include="..\TUnit.Mocks\TUnit.Mocks.csproj"
                  SetTargetFramework="TargetFramework=netstandard2.0"
                  ReferenceOutputAssembly="false"
                  Private="false" />

This is valid MSBuild behavior — the two references have different effective targets and the ReferenceOutputAssembly="false" prevents collisions. The comment on the new ItemGroup explains the intent clearly. No issues here.

Summary

The fix is correct and uses the right MSBuild pattern. All previous review issues remain resolved. The PR is ready to merge.