fix(sourcegen): drop covariant TActual when [GenerateAssertion] method has its own type parameters

[JohnVerheij]

Description

Suppresses receiver-type covariance in MethodAssertionGenerator when the source method declares its own type parameters. The covariant TActual parameter previously prepended to the generated extension caused CS1929 at call sites supplying explicit type arguments, because C# does not permit partial type-argument specification. The fix is a single boolean conjunct (genericParams.Count == 0) plus an explanatory comment. Receivers of a more-derived static type can still reach the assertion via upcast.

Related Issue

Fixes #5934

Type of Change

• Bug fix (non-breaking change that fixes an issue)
• New feature (non-breaking change that adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Documentation update
• Performance improvement
• Refactoring (no functional changes)

Checklist

Required

• I have read the Contributing Guidelines
• If this is a new feature, I started a discussion first and received agreement
• My code follows the project's code style (modern C# syntax, proper naming conventions)
• I have written tests that prove my fix is effective or my feature works

TUnit-Specific Requirements

• Dual-Mode Implementation: If this change affects test discovery/execution, I have implemented it in BOTH:
  • Source Generator path (TUnit.Core.SourceGenerator)
  • Reflection path (TUnit.Engine)
• Snapshot Tests: If I changed source generator output or public APIs:
  • I ran TUnit.Core.SourceGenerator.Tests and/or TUnit.PublicAPI tests
  • I reviewed the .received.txt files and accepted them as .verified.txt
  • I committed the updated .verified.txt files
• Performance: If this change affects hot paths (test discovery, execution, assertions):
  • I minimized allocations and avoided LINQ in hot paths
  • I cached reflection results where appropriate
• AOT Compatibility: If this change uses reflection:
  • I added appropriate [DynamicallyAccessedMembers] annotations
  • I verified the change works with dotnet publish -p:PublishAot=true

Testing

• All existing tests pass (dotnet test)
• I have added tests that cover my changes
• I have tested both source-generated and reflection modes (if applicable)

Additional Notes

Decision matrix covered by the regression tests

TUnit.Assertions.Tests/GenerateAssertionGenericMethodOnNonSealedReceiverTests.cs adds nine cases:

1. generic method on non-sealed receiver with explicit type arg: was CS1929, now compiles
2. generic method on sealed receiver: control, still works (covariance was already disabled for sealed)
3. non-generic method on non-sealed receiver: control, still works
4. generic method on non-sealed receiver with full inference: control, still works
5. generic method with multiple type parameters (<TFirst, TSecond>): uniform behaviour, asserts the genericParams.Count > 0 guard is not 1-specific
6. generic method on an interface receiver: IsCovariantCandidate accepts both class and interface; the fix applies to both
7. generic method with where T : IParsable<T> constraint: the constraint itself is incidental to the trigger, but represents the dominant real-world consumer shape
8. async result (Task<AssertionResult>) return type: orthogonal to the extension signature
9. derived static receiver via upcast: verifies the documented tradeoff

Generator-level coverage

• MethodAssertionGeneratorTests.MethodOnConcreteNonSealedReceiver asserts the new emit shape: single <T> type parameter, exact-receiver IAssertionSource<NonSealedReceiverType>, no TActual, no where TActual : ....
• Four new *.verified.txt snapshots (one per TFM: net472, net8.0, net9.0, net10.0). All four are byte-identical.

Impact on existing snapshots

None. The TUnit codebase has no [GenerateAssertion] method that matches the affected shape (generic source method on a concrete non-sealed reference-type receiver). All existing snapshots regenerated identically. Full TUnit.Assertions.Tests, TUnit.Assertions.SourceGenerator.Tests, TUnit.Assertions.Should.Tests, and TUnit.Assertions.Should.SourceGenerator.Tests pass green locally.

Related

TUnit.Assertions.Should.SourceGenerator was inspected; it does not use the covariance helper, so the bug is not present there.

The analogous case for the other generator path is covered by AssertionExtensionGeneratorTests.ConcreteReceiverWithExtraGeneric, which asserts the single merged generic-parameter list (no adjacent <X><Y> blocks) for [AssertionExtension] classes. The bug in MethodAssertionGenerator is the same family (two type parameters on the call signature versus one explicit argument at the call site), and the fix follows the same instinct.

Issue #5922 reports the user-visible friction in the [AssertionExtension] path; that is out of scope for this PR but tracked there.

[codacy-production]

Not up to standards ⛔

🔴 Issues 2 minor

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

Results:
2 new issues

Category	Results
CodeStyle	2 minor

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

Summary

This is a clean, surgical bug fix with excellent test coverage. The root cause diagnosis and fix are both correct.

---

The Fix (MethodAssertionGenerator.cs line 999)

The single conjunct && genericParams.Count == 0 is the right approach. With covariance enabled on a generic method, the generator produces a two-type-parameter extension (e.g. HasItem<TActual, T>). C# forbids partial type-argument specification, so a call site supplying one explicit argument — the common case — fails with CS1929. Disabling covariance for this shape yields a single-generic extension that callers can bind without restriction, and the existing downstream logic (generic declaration, where clause, source type name) already handles the isCovariant = false path correctly.

One architectural question worth considering: the fix ties the covariance decision to a count (genericParams.Count == 0). This is a proxy for "the method has no user-visible type parameters of its own." If a future scenario needs to suppress covariance for a different reason, the condition may become harder to read. An alternative would be an explicit SuppressCovariance flag on AssertionMethodData, computed once during collection. That said, for the current scope this is not a blocker — the comment explains the WHY clearly enough.

---

Test Coverage

Nine integration cases in GenerateAssertionGenericMethodOnNonSealedReceiverTests.cs are well chosen:

• The failing case, two sealed/non-generic control cases, full-inference control, multi-param generics, interface receiver, constrained type param, async result, and the upcast workaround.
• The GenericMethod_DerivedStaticReceiver_UpcastWorkaround test explicitly documents the tradeoff rather than hiding it, which is the right call.

Generator-level test (MethodAssertionGeneratorTests.MethodOnConcreteNonSealedReceiver) is appropriately specific: it asserts the single-parameter shape and negatively asserts TActual and where TActual : are absent.

---

Minor Observations (non-blocking)

1. Test type namespace pollution — NonSealedContainer, SealedContainer, IContainerInterface, etc. are defined directly in the TUnit.Assertions.Tests namespace root. Other test files follow this pattern so it is consistent, but a nested namespace (e.g. TUnit.Assertions.Tests.GenerateAssertionGenericMethodOnNonSealedReceiver) would reduce the chance of future name collisions with other test data files.

2. Verbose class-level XML doc — The lengthy <summary> on GenerateAssertionGenericMethodOnNonSealedReceiverTests restates the commit message. The test method names and individual test bodies already communicate intent; the class doc does not add information a future reader could not derive from the code.

3. Four identical snapshot files — All four TFM snapshots are byte-identical (expected, since the generator emits the same output across TFMs). This is a project convention and not a problem, just worth being aware of when generator behaviour starts diverging per TFM in future.

---

Verdict

Approve. The fix is minimal and correct, the downstream logic handles the isCovariant = false path without modification, and the test suite covers the affected shape thoroughly including the documented tradeoff. No blocking issues.

[thomhurst]

Thanks!