feat(assertions): inference-friendly pinned overload for covariant [AssertionExtension] with own generic (#5922) by thomhurst · Pull Request #6196 · thomhurst/TUnit

thomhurst · 2026-06-08T19:42:38Z

Summary

Resolves the call-site ergonomics issue in #5922 (the follow-up to the #5918 emit fix).

When an [AssertionExtension] class declares its own generic parameter and derives from Assertion<TConcrete> for a non-sealed concrete (covariance-candidate) receiver, the generator emitted only a covariant overload:

public static MatchesAssertion<T> Matches<TActual, T>(this IAssertionSource<TActual> source, Func<T, bool> predicate, ...)
    where TActual : System.Exception

Because IAssertionSource<T> is invariant, the covariant TActual is genuinely required so Assert.That(subclass) can bind. But C# forbids partial type-argument specification, so a caller naming the class's own (non-inferable) argument had to repeat the receiver type:

Assert.That(ex).Matches<Exception, MyPayload>(p => p.IsValid);   // verbose

Change

The generator now additionally emits an inference-friendly pinned-receiver overload — but only for this exact shape (covariance-candidate receiver and own generic params):

public static MatchesAssertion<T> Matches<T>(this IAssertionSource<System.Exception> source, Func<T, bool> predicate, ...)

So the common exact-receiver call site only needs the class's own type argument, while the covariant overload is kept for subclass receivers:

Assert.That(ex).Matches<MyPayload>(p => p.IsValid);              // exact type — clean
Assert.That(argEx).Matches<ArgumentException, MyPayload>(...);   // subclass — still explicit

No OverloadResolutionPriority is needed: C#'s "more specific parameter types" rule prefers the concrete-receiver overload when both apply (the same way M(int) beats M<T>(T)). Dropping the attribute also avoids a polyfill dependency in the generated code.

MethodAssertionGenerator is intentionally left unchanged — it already suppresses covariance for own-generic methods (documented there, citing CS1929).

Files

AssertionExtensionGenerator.cs — add pinnedReceiver path; emit the extra overload only for the covariance-candidate + own-generics shape (via a small local EmitMethod helper).
ConcreteReceiverWithExtraGeneric.*.verified.txt — updated snapshots (all TFMs); strengthened the generator test to pin both overload signatures alongside the compile-clean gate.
CovariantGenericExtensionInferenceTests.cs (new) — compile-time guard proving overload resolution: pinned binds for exact type, covariant binds for subclass, and no CS0121 ambiguity when both apply.

Verification

Generator tests: 267/267
PublicAPI snapshots: 18/18 (no shipping assertion surface changed)
Inference-proof runtime tests: 3/3 on net8.0 + net10.0

…tension] with own generic (#5922) When an [AssertionExtension] class declares its own generic parameter over a non-sealed concrete (covariance-candidate) receiver, the generator emitted only a covariant Matches<TActual, T>(this IAssertionSource<TActual>, ...) overload. Because C# forbids partial type-argument specification, callers that must name the class's own argument (e.g. a non-inferable lambda type) were also forced to repeat the receiver type: Assert.That(ex).Matches<Exception, MyPayload>(...). Additionally emit an inference-friendly pinned-receiver overload Matches<T>(this IAssertionSource<Exception>, ...) for this shape. The common exact-receiver call site now needs only the class's own type argument (Assert.That(ex).Matches<MyPayload>(...)); the covariant overload is kept so a more-derived static receiver can still bind. No OverloadResolutionPriority is needed - C#'s "more specific parameter types" rule prefers the concrete-receiver overload when both apply. - AssertionExtensionGenerator: add pinnedReceiver path; emit the extra overload only for the covariance-candidate + own-generics shape. - Update ConcreteReceiverWithExtraGeneric snapshots (all TFMs); strengthen the test to pin both overload signatures alongside the compile-clean gate. - Add CovariantGenericExtensionInferenceTests: compile-time guard proving overload resolution (pinned for exact type, covariant for subclass, no CS0121 ambiguity).

codacy-production · 2026-06-08T19:44:44Z

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 41 complexity

Metric Results

Complexity 41

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

This is a well-scoped, well-motivated fix for a real C# ergonomics pain point. The approach is sound and the implementation is correct.

What's good

Conservative scope: The pinned-receiver overload is emitted only when both conditions are true — covariance-candidate receiver AND own generic params. Emitting it more broadly would risk ambiguity or breaking changes.

No OverloadResolutionPriority polyfill needed: Relying on C#'s native "more specific parameter types" rule to prefer IAssertionSource<Exception> over IAssertionSource<TActual> is the right call. It avoids a generated-code dependency on a net9.0/polyfill attribute and is a more principled disambiguation.

Three-scenario test coverage: Exact-type receiver, subclass receiver, and both-applicable-with-inference all have explicit tests. The class-level XML comment clearly communicates why these are primarily compile-time guards.

Snapshot pinned-overload uses source.Context directly (not source.Context.Map<...>): correct, since the receiver is already pinned to IAssertionSource<Exception>.

Things to consider

1. `EmitMethod` local vs. parameters — implicit capture

void EmitMethod(IMethodSymbol constructor, bool negated)
{
    GenerateExtensionMethod(sourceBuilder, data, constructor, negated, isNullableOverload: false);
    if (needsPinnedOverload)
        GenerateExtensionMethod(sourceBuilder, data, constructor, negated, isNullableOverload: false, pinnedReceiver: true);
}

EmitMethod silently closes over sourceBuilder, data, and needsPinnedOverload. For a reader who jumps to the constructor loop call sites without reading upward, the captured state isn't obvious. This isn't a bug — the existing code already threads sourceBuilder and data through all helpers — but consider whether a static helper method EmitMethod(sourceBuilder, data, constructor, negated, needsPinnedOverload) would be cleaner, making the dependencies explicit rather than captured. Minor point, leave or change as you prefer.

2. `OverloadResolutionPriority` on both overloads

If a user annotates their assertion class with [OverloadResolutionPriority(n)] (non-zero), that priority is applied to both the covariant and pinned overloads (because effectivePriority = data.OverloadResolutionPriority is not pinnedReceiver-aware). When both overloads carry the same non-zero ORP tier, C#'s "more specific parameter types" rule still applies within that tier, so there's no ambiguity issue. But it's worth a comment near the effectivePriority block noting that ORP is intentionally the same on both overloads so that the "more specific receiver" rule remains the tiebreaker.

3. Nullable overloads and the new path

Nullable overloads already suppress covariance (isNullableOverload → exact receiver). They are generated separately from the EmitMethod loop (in the section that follows the constructor loop). Verifying: does the pinned-receiver shape ever interact with nullable overloads? The answer looks like "no" — the nullable path is a distinct branch and EmitMethod only controls the non-nullable path. Worth confirming with a quick scan to make sure no constructor accidentally ends up in both paths.

4. Test predicates always return `Passed` — acceptable but worth a comment

InferencePayloadMatchesAssertion and InferenceTaggedExceptionAssertion both return AssertionResult.Passed unconditionally, so p => p.Ok where Ok defaults to false never fails. The class-level XML doc explains this, but someone maintaining Pinned_Overload_Exact_Receiver_Only_Needs_Own_Type_Argument years from now might be confused why a failing predicate passes. A single inline comment on CheckAsync ("always passes; these fixtures are compile-time guards only") would close that gap. This is a minor nit.

Summary

The design is correct and the implementation is minimal. The two main things worth a second look are (1) whether the EmitMethod closure vs. explicit-parameter shape is intentional, and (2) a small clarification comment near the OverloadResolutionPriority assignment to explain why both overloads share the same priority. Neither is a blocker. Overall this is a solid, well-tested change.

…on-inferable (#5922) The pinned-receiver overload added in #6196 only helps the call site when the caller is forced to name a type argument - i.e. when an own generic parameter is NOT inferable from the constructor's value arguments (e.g. it appears only inside a Func<T, bool>). When every own generic is inferable (e.g. a plain T tag), the caller writes no type arguments at all and the covariant overload binds on its own, so the pinned overload is pure dead weight. Gate emission on CovarianceHelper.OwnGenericsAreInferable so the pinned overload is skipped for the inferable case. This also keeps the two overloads arity-disjoint (pinned <T...> vs covariant <TActual, T...>), so they are never both applicable and no OverloadResolutionPriority tiebreaker is needed. Also addresses review feedback: EmitMethod is now an explicit static method (no implicit closure capture), and the compile-guard test fixtures are commented as always-passing / compile-time-only.

thomhurst · 2026-06-08T20:15:18Z

Pushed a follow-up (04954d2) that narrows when the pinned overload is emitted, after a fair challenge that it rarely helps in practice.

The problem with always emitting it

The pinned overload only earns its place when the caller is forced to name a type argument — i.e. when an own generic parameter is not inferable from the constructor's value arguments (it appears only inside a Func<T, bool> / Action<T> / Expression<...>, where a lambda can't fix it). When every own generic is inferable (e.g. a plain T tag), the caller writes no type arguments at all, the covariant overload binds on its own, and the pinned overload is pure dead weight — extra API/IntelliSense surface for zero benefit.

Change

Emission is now gated on CovarianceHelper.OwnGenericsAreInferable(constructor, ownTypeParameters):

non-inferable own generic (e.g. Func<T,bool>) → pinned overload emitted (the real ergonomic win).
inferable own generic (e.g. T tag) → pinned overload skipped.
Detection is conservative: anything not provably inferable is treated as non-inferable, so it keeps the prior behaviour rather than risk an ergonomic regression.

A nice side effect: with the inferable case excluded, the pinned (<T...>) and covariant (<TActual, T...>) overloads are arity-disjoint and never both applicable, so the "more specific receiver" tiebreaker is no longer exercised and no OverloadResolutionPriority is involved — which moots review concern #2.

Review feedback addressed

Repeat attributes #1 (closure capture): EmitMethod is now an explicit private static method; dependencies are passed in rather than captured.
Retry attribute #2 (ORP comment): moot — overloads are arity-disjoint, documented in the comment block.
Ignore attribute #3 (nullable path): confirmed non-issue — GenerateExtensionMethod is never called with isNullableOverload: true in this generator, so pinned + nullable can't co-occur.
And assertion conditions #4 (always-passing test predicate): added inline comments on the compile-guard fixtures.

Tests

New snapshot fixture ConcreteReceiverWithInferableGenericAssertion (T tag) proves the pinned overload is not emitted; existing ConcreteReceiverWithExtraGeneric (Func<T,bool>) snapshots are unchanged.
Generator tests 52/52, compile-guard inference tests 9/9 (net8/9/10), PublicAPI 18/18.

claude

Code Review — PR #6196: Inference-friendly pinned overload for covariant AssertionExtension

Following up on my previous review (2026-06-08). Checking whether prior concerns were addressed and doing a fresh pass on the diff.

Previous comment corrections

Item 1 from prior review was wrong. I flagged EmitMethod as a "closure that silently closes over sourceBuilder, data", but looking at the actual code it's a private static void EmitMethod(StringBuilder sourceBuilder, AssertionExtensionData data, IMethodSymbol constructor, bool negated, bool emitPinned) — all explicit parameters. No closure at all. That concern is a non-issue; the implementation is clean.

New findings

1. Redundant double-check in hasOwnGenerics (AssertionExtensionGenerator.cs)

var hasOwnGenerics = data.ClassSymbol.IsGenericType && data.ClassSymbol.TypeParameters.Length > 0;

IsGenericType == true for a named type symbol already implies TypeParameters.Length > 0. The && TypeParameters.Length > 0 guard is always-true when IsGenericType is true — it cannot add any protection. Fine to leave, but reads as unintentional.

2. OverloadResolutionPriority note — still unaddressed (AssertionExtensionGenerator.cs, effectivePriority block)

From the prior review: if a user annotates their class with [OverloadResolutionPriority(n)], effectivePriority is applied to both overloads because the pinned-path check happens after priority is read. The covariant overload's "more specific parameter types" wins naturally when both are applicable, so this doesn't break correctness today. But it's worth a short comment near the effectivePriority calculation noting that ORP is intentionally the same on both overloads — "more specific receiver wins by C# spec, no tiebreaker needed" — so a future maintainer doesn't try to add pinnedReceiver-aware priority logic.

3. Conservative treatment of T inside generic collections of delegates (CovarianceHelper.cs, AppearsInInferablePosition)

When a parameter type is IEnumerable<Func<T, bool>>, the method returns false for T (the outer IEnumerable<> is not a delegate, so it recurses into its type arg Func<T, bool>; Func<T, bool> is a delegate, so it stops and returns false). In practice a caller could pass a List<Func<int, bool>> and the compiler would infer T = int. So the analysis is conservative here — the pinned overload gets emitted even when it's technically redundant.

This is intentional and documented ("The bias is conservative…"), and the worst outcome is one extra overload. Worth noting explicitly in the method summary that this is a known conservatism for delegate-inside-collection cases, not a bug, so future readers don't "fix" it into an over-aggressive heuristic.

4. No test for negated assertion with pinned overload

EmitMethod emits the pinned overload for both positive and negated variants. There's no call-site test confirming the negated pinned path compiles and resolves correctly. The snapshot tests cover the generated text, but a small addition to CovariantGenericExtensionInferenceTests for a negated call (e.g. await Assert.That(ex).Not.MatchesPayload<Payload>(...)) would close this gap.

What's working well

The logic for when to emit the pinned overload is precise and well-documented: covariance candidate and has own generics and those generics are not all inferable.
The pinned overload correctly uses source.Context directly (no .Map<>() call) since the receiver is already the concrete type.
IsExpressionTree correctly gates on System.Linq.Expressions.Expression<T> — the usual expression-tree pattern — without false-positives on other expression-tree types.
The arity-disjoint property (pinned <T> vs covariant <TActual, T>) means no OverloadResolutionPriority polyfill is needed. That's a clean design.
Three-scenario test coverage in CovariantGenericExtensionInferenceTests (exact-type, subclass, fully-inferable) covers the critical correctness surface.

Summary

Items 1 and 3 are informational. Item 2 (ORP comment) is a low-cost documentation improvement. Item 4 (negated test) is the only actionable gap. None are blockers — the core design is sound and the implementation is minimal and well-tested. Good PR.

…review #6196)

thomhurst · 2026-06-08T20:25:54Z

Thanks for the second pass. Went through all four findings (commit 94d3737):

#2 — ORP comment near effectivePriority → ✅ Applied. Added a comment at the effectivePriority calculation explaining that the pinned and covariant overloads intentionally share the same priority, since they're arity-disjoint and the "more specific receiver" rule is the tiebreaker (not ORP).

#3 — delegate-inside-collection conservatism → ✅ Applied. Documented the known conservatism in AppearsInInferablePosition's summary: a T reachable only via IEnumerable<Func<T,bool>> is reported non-inferable even though the compiler could infer it; the only consequence is one redundant overload (still resolved by the more-specific-receiver rule), so it shouldn't be "fixed" into an over-aggressive heuristic.

#1 — IsGenericType && TypeParameters.Length > 0 "always-true" → Pushing back: it's not redundant. INamedTypeSymbol.IsGenericType walks containing types — it returns true if the type or any containing type has type parameters. So a non-generic assertion nested in a generic outer class (class Outer<T> { class Inner : Assertion<SomeConcrete> }) has IsGenericType == true but TypeParameters.Length == 0. The && TypeParameters.Length > 0 is what restricts hasOwnGenerics to the type's own type parameters. Removing it would set hasOwnGenerics true for that nested case with an empty ownTypeParameters. Leaving as-is.

#4 — no negated-pinned compile-guard test → Pushing back: the negated path is the same emit code — negated only flips the method name (methodName = negated ? data.NegatedMethodName : data.MethodName); the generic-parameter handling, pinnedReceiver logic, and emitPinned value are identical for the positive and negated calls. So a negated call-site guard would exercise the exact same overload-resolution machinery the existing positive Pinned_Overload_Exact_Receiver_Only_Needs_Own_Type_Argument test already covers — it'd be duplicate coverage rather than new protection. Happy to add one if you'd still prefer the explicit belt-and-braces.

Also noting your correction on prior item #1 (the EmitMethod closure) — yep, it's now an explicit static method, so that's resolved.

claude

Code Review — PR #6196 (third pass)

This is a follow-up pass after the author's response to the previous two reviews. All prior findings were either addressed or pushed back on with solid reasoning. This pass checks the current state of the code for anything remaining.

Prior items — status

Closure vs explicit parameters (#1 from first review): Resolved. EmitMethod is a proper private static void EmitMethod(StringBuilder, AssertionExtensionData, IMethodSymbol, bool, bool) — all dependencies explicit.

IsGenericType && TypeParameters.Length > 0 (#1 from second review): The author's rebuttal is correct. In Roslyn, INamedTypeSymbol.IsGenericType returns true if the type itself or any enclosing type has type parameters. So for a non-generic nested class class Outer<T> { class Inner : Assertion<Concrete> }, Inner.IsGenericType == true but Inner.TypeParameters.Length == 0. The dual check is load-bearing, not redundant. The same pattern already appears at line 289 (pre-existing code), confirming it's intentional.

ORP comment (#2 from second review): Addressed — comment added at the effectivePriority block.

Delegate-inside-collection conservatism (#3 from second review): Addressed — documented explicitly in AppearsInInferablePosition's XML summary.

Negated pinned test (#4 from second review): The author's pushback is correct. The negated flag only flips the emitted method name; the emitPinned decision and type-parameter handling are identical for both the positive and negated arms. A negated call-site test would cover the exact same overload-resolution machinery, so this is intentional shared coverage rather than a gap.

Remaining observations (informational only, not blockers)

isNullableOverload is dead parameter state. GenerateExtensionMethod accepts bool isNullableOverload but it is never called with true in this generator — only EmitMethod calls it, and both call sites pass isNullableOverload: false. The parameter exists for historical reasons (it may be future scaffolding), but as-is, the three if (isNullableOverload) branches inside GenerateExtensionMethod are unreachable. This is pre-existing and out of scope for this PR, but worth noting for a future cleanup pass.

OwnGenericsAreInferable with a context-only constructor. If a generic Assertion<T> subclass has only the AssertionContext<T> parameter (no value params), valueParameters after Skip(1) is empty. The method then returns false for each own type parameter, setting emitPinned = true. The resulting pinned overload Foo<T>(this IAssertionSource<Exception> source) has no value params to infer T from, so T remains unresolvable at the call site. This is a degenerate / nonsensical assertion design that wouldn't be useful in practice, and the conservative bias is documented, so this is not a real concern — just noting the edge case is handled consistently with the conservative policy.

No test for a class with multiple constructors where one has inferable own generics and another does not. For example, class Foo<T> : Assertion<Exception> with ctor1(ctx, Func<T,bool>) (non-inferable) and ctor2(ctx, T value) (inferable). This would emit a pinned overload for ctor1 but not ctor2. The resulting API is correct (the per-constructor emitPinned computation is independent and sound), but no test covers this mixed case. Low risk.

Summary

The implementation is correct and well-motivated. All actionable findings from prior reviews were addressed. The rebuttals on items #1 (IsGenericType) and #4 (negated test) are technically sound and I agree with both. The remaining observations above are informational — none warrant changes. This PR is ready to merge.

Updated [TUnit](https://github.com/thomhurst/TUnit) from 1.51.0 to 1.53.0. <details> <summary>Release notes</summary> _Sourced from [TUnit's releases](https://github.com/thomhurst/TUnit/releases)._ ## 1.53.0  ## What's Changed ### Other Changes * feat(assertions): return typed value from IsAssignableTo<T> (#6184) by @thomhurst in thomhurst/TUnit#6187 * fix: stop doubling backslashes in source-gen emitted FilePath (breaks HTML report source links) by @thomhurst in thomhurst/TUnit#6193 * feat(assertions): add ContainsKey().And.Value drill-in for dictionaries (#6185) by @thomhurst in thomhurst/TUnit#6188 * fix(tests): snapshot ExecutionLog under lock to fix parallel race by @thomhurst in thomhurst/TUnit#6194 * fix(engine): run lifecycle hooks before test class construction (#6192) by @thomhurst in thomhurst/TUnit#6195 * feat(assertions): inference-friendly pinned overload for covariant [AssertionExtension] with own generic (#5922) by @thomhurst in thomhurst/TUnit#6196 * feat: add DeferEnumeration to defer data-source expansion to runtime (#5833) by @thomhurst in thomhurst/TUnit#6197 ### Dependencies * chore(deps): update tunit to 1.51.0 by @thomhurst in thomhurst/TUnit#6186 * chore(deps): update microsoft.testing to 18.8.0 by @thomhurst in thomhurst/TUnit#6191 * chore(deps): update aspire to 13.4.3 by @thomhurst in thomhurst/TUnit#6198 **Full Changelog**: thomhurst/TUnit@v1.51.0...v1.53.0 Commits viewable in [compare view](thomhurst/TUnit@v1.51.0...v1.53.0). </details> Updated [TUnit.AspNetCore](https://github.com/thomhurst/TUnit) from 1.51.0 to 1.53.0. <details> <summary>Release notes</summary> _Sourced from [TUnit.AspNetCore's releases](https://github.com/thomhurst/TUnit/releases)._ ## 1.53.0  ## What's Changed ### Other Changes * feat(assertions): return typed value from IsAssignableTo<T> (#6184) by @thomhurst in thomhurst/TUnit#6187 * fix: stop doubling backslashes in source-gen emitted FilePath (breaks HTML report source links) by @thomhurst in thomhurst/TUnit#6193 * feat(assertions): add ContainsKey().And.Value drill-in for dictionaries (#6185) by @thomhurst in thomhurst/TUnit#6188 * fix(tests): snapshot ExecutionLog under lock to fix parallel race by @thomhurst in thomhurst/TUnit#6194 * fix(engine): run lifecycle hooks before test class construction (#6192) by @thomhurst in thomhurst/TUnit#6195 * feat(assertions): inference-friendly pinned overload for covariant [AssertionExtension] with own generic (#5922) by @thomhurst in thomhurst/TUnit#6196 * feat: add DeferEnumeration to defer data-source expansion to runtime (#5833) by @thomhurst in thomhurst/TUnit#6197 ### Dependencies * chore(deps): update tunit to 1.51.0 by @thomhurst in thomhurst/TUnit#6186 * chore(deps): update microsoft.testing to 18.8.0 by @thomhurst in thomhurst/TUnit#6191 * chore(deps): update aspire to 13.4.3 by @thomhurst in thomhurst/TUnit#6198 **Full Changelog**: thomhurst/TUnit@v1.51.0...v1.53.0 Commits viewable in [compare view](thomhurst/TUnit@v1.51.0...v1.53.0). </details> Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) </details> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

thomhurst temporarily deployed to Pull Requests June 8, 2026 19:42 — with GitHub Actions Inactive

claude Bot reviewed Jun 8, 2026

View reviewed changes

thomhurst had a problem deploying to Pull Requests June 8, 2026 20:15 — with GitHub Actions Error

claude Bot reviewed Jun 8, 2026

View reviewed changes

docs(assertions): clarify ORP-sharing and inferability conservatism (…

94d3737

…review #6196)

thomhurst temporarily deployed to Pull Requests June 8, 2026 20:26 — with GitHub Actions Inactive

claude Bot reviewed Jun 8, 2026

View reviewed changes

thomhurst enabled auto-merge (squash) June 8, 2026 20:32

thomhurst merged commit 3bada16 into main Jun 8, 2026
14 checks passed

thomhurst deleted the fix/5922-pinned-overload-ergonomics branch June 8, 2026 20:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(assertions): inference-friendly pinned overload for covariant [AssertionExtension] with own generic (#5922)#6196

feat(assertions): inference-friendly pinned overload for covariant [AssertionExtension] with own generic (#5922)#6196
thomhurst merged 3 commits into
mainfrom
fix/5922-pinned-overload-ergonomics

thomhurst commented Jun 8, 2026

Uh oh!

codacy-production Bot commented Jun 8, 2026 •

edited

Loading

Uh oh!

claude Bot left a comment

Uh oh!

thomhurst commented Jun 8, 2026

Uh oh!

claude Bot left a comment

Uh oh!

thomhurst commented Jun 8, 2026

Uh oh!

claude Bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

thomhurst commented Jun 8, 2026

Summary

Change

Files

Verification

Uh oh!

codacy-production Bot commented Jun 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Up to standards ✅

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Code Review

What's good

Things to consider

1. EmitMethod local vs. parameters — implicit capture

2. OverloadResolutionPriority on both overloads

3. Nullable overloads and the new path

4. Test predicates always return Passed — acceptable but worth a comment

Summary

Uh oh!

thomhurst commented Jun 8, 2026

The problem with always emitting it

Change

Review feedback addressed

Tests

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Code Review — PR #6196: Inference-friendly pinned overload for covariant AssertionExtension

Previous comment corrections

New findings

What's working well

Summary

Uh oh!

thomhurst commented Jun 8, 2026

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Code Review — PR #6196 (third pass)

Prior items — status

Remaining observations (informational only, not blockers)

Summary

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

codacy-production Bot commented Jun 8, 2026 •

edited

Loading

1. `EmitMethod` local vs. parameters — implicit capture

2. `OverloadResolutionPriority` on both overloads

4. Test predicates always return `Passed` — acceptable but worth a comment