Skip to content

feat(mocks): accept an async factory in Returns() on async members - #6503

Merged
thomhurst merged 3 commits into
mainfrom
fix/6495-async-returns-factory
Jul 28, 2026
Merged

feat(mocks): accept an async factory in Returns() on async members#6503
thomhurst merged 3 commits into
mainfrom
fix/6495-async-returns-factory

Conversation

@thomhurst

Copy link
Copy Markdown
Owner

Fixes #6495

Problem

mock.SomeMethodAsync().Returns(async () => { await Task.Delay(1000); return 42; });
// error CS4010: Cannot convert async lambda expression to delegate type 'Func<int>'

Returns on an async member is generated over the unwrapped return type, so its factory overload is Func<int> — an async lambda has nowhere to bind. The shape that does compile, Returns(() => { Thread.Sleep(1000); return 42; }), blocks the caller and hands back an already-completed task, so await Task.WhenAny(mock.SomeMethodAsync(), Task.Delay(timeout)) can never let the timeout branch win. That makes timeout-handling tests unwritable — the reporter kept NSubstitute for one test because of it.

ReturnsAsync(Func<Task<T>>) already did the right thing, but nothing at the Returns call site pointed there.

Fix

Emit a Returns overload alongside every existing ReturnsAsync factory overload — the parameterless form in EmitReturnsAsyncOverloads and the typed-parameter form in GenerateTypedReturnsAsyncOverload — routing through the same ReturnsRaw path. The factory's task is handed back as-is, so it stays pending until it completes.

Covers Task<T>, ValueTask<T>, Task and ValueTask members. The synchronous Func<T> overload is untouched; ReturnsAsync is unchanged.

Tests

tests/TUnit.Mocks.Tests/Issue6495Tests.cs — 8 cases: the timeout race (asserting the returned task is incomplete and that a 50ms timeout beats it), value propagation, per-call invocation, the typed-parameter form, ValueTask<T> and Task members, plus guards that the synchronous Returns factory and ReturnsAsync still behave as before.

TUnit.Mocks.Tests 1168 passed. Generator snapshots regenerated (8 files, additive only) and passing on net8.0/net9.0/net10.0. Analyzers 30, Http 54, Logging 31.

Returns() on an async member only offered the synchronous Func<T>
overload, so `.Returns(async () => { await Task.Delay(x); return y; })`
failed with CS4010. The only shape that compiled was a blocking
Thread.Sleep, which returns an already-completed task to the caller —
so production code racing the call against a timeout could never let the
timeout win, making that whole class of test unwritable.

Emit a Returns overload alongside each existing ReturnsAsync factory
overload, parameterless and typed-parameter alike, routing through the
same ReturnsRaw path. The factory's task is handed back as-is and stays
pending until it completes.

Fixes #6495
Comment thread src/TUnit.Mocks.SourceGenerator/Builders/MockMembersBuilder.cs
@greptile-apps

greptile-apps Bot commented Jul 28, 2026

Copy link
Copy Markdown

Greptile Summary

This PR adds async-factory Returns aliases for generated async mock members.

  • Routes parameterless and typed factories through the existing raw async-return path.
  • Uses overload-resolution priority to preserve synchronous null- and throw-lambda binding.
  • Emits the new aliases and their behavioral tests only for .NET 9 or later.
  • Adds regenerated source-generator snapshots and regression tests.

Confidence Score: 3/5

The PR is not yet safe to merge because the reported Returns(async ...) use case remains unavailable to .NET 8 consumers.

Both new overload forms are enclosed in NET9_0_OR_GREATER, so generated .NET 8 mocks retain the original overload set and continue rejecting async lambdas at the Returns call site.

Files Needing Attention: src/TUnit.Mocks.SourceGenerator/Builders/MockMembersBuilder.cs, tests/TUnit.Mocks.Tests/Issue6495Tests.cs

Important Files Changed

Filename Overview
src/TUnit.Mocks.SourceGenerator/Builders/MockMembersBuilder.cs Adds the async-factory aliases with ambiguity protection, but conditionally removes the advertised feature from .NET 8 generated mocks.
tests/TUnit.Mocks.Tests/Issue6495Tests.cs Covers pending-task behavior, values, invocation frequency, typed factories, and overload binding on .NET 9+, while leaving the new API untested and unavailable on .NET 8.
tests/TUnit.Mocks.SourceGenerator.Tests/Snapshots/Interface_With_Async_Methods.verified.txt Confirms the generated overload shapes and their .NET 9 conditional compilation boundaries.

Reviews (3): Last reviewed commit: "fix(mocks): gate the async Returns alias..." | Re-trigger Greptile

@claude

claude Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Code review

Nice, focused fix — routing the new Returns overloads through the existing ReturnsRaw path (rather than duplicating the async-completion logic) keeps this consistent with ReturnsAsync, and the added tests correctly pin down the behavior that actually mattered for #6495 (the mocked call must stay pending so a timeout can race it).

Confirmed issue: the new overload is source-breaking for reference-typed async results

MockMembersBuilder.cs now emits Returns(Func<T> factory) and Returns(Func<Task<T>> factory) side-by-side for every async member (same for the parameterless and ValueTask variants). Both are also emitted for members where T is a reference type or nullable — e.g. IDialogService_ShowDialogAsync_M1_MockCall<TDialog> gets both Returns(Func<object, DialogParameters, IDialogReference>) and Returns(Func<object, DialogParameters, Task<IDialogReference>>) (see tests/TUnit.Mocks.SourceGenerator.Tests/Snapshots/Interface_FluentUI_Shape_Nullable_Warnings.verified.txt lines 55-65 in the diff).

When T is a reference type, any call whose lambda body doesn't itself pin the return type — Returns(() => null) or Returns(() => throw new SomeException()) — becomes ambiguous overload resolution (CS0121), because null/throw convert equally well to T and to Task<T>. I verified this compiles today and breaks after the change:

public Test Returns(Func<IDialogReference?> factory) { ... }
public Test Returns(Func<Task<IDialogReference?>> factory) { ... }
...
t.Returns(() => null);              // CS0121: ambiguous
t.Returns(() => throw new InvalidOperationException()); // CS0121: ambiguous

Both are idiomatic mocking patterns (stub a null result, or stub a call that should throw) that existing consumers may already be using against reference-typed async members. This wouldn't be caught by the PR's own test suite because Issue6495Tests.cs only exercises int-returning members — for a non-nullable value type there's no overlap (null/throw can't implicitly convert to int, and a genuine async () => ... lambda can only bind to the Task<T> overload in the first place, since async lambdas never convert to a non-Task delegate type). So the feature this PR is aimed at (real async lambdas) never hits the ambiguity — only pre-existing synchronous lambdas against reference-typed results do.

Why this matters architecturally: the collision isn't a one-off bug in this diff, it's a consequence of overloading Returns on two Func<> shapes that differ only in return type. That pattern is inherently fragile in C# overload resolution (ambiguous whenever the lambda body doesn't itself disambiguate), and it will keep resurfacing if more shapes are added later. Two more targeted alternatives avoid the collision entirely:

  • Only emit the new Returns(Func<Task<T>>) alias when T is a non-nullable value type, where null/throw-only lambdas can never collide with the sync overload. This narrows the fix (reference-typed async results would still need ReturnsAsync for factory lambdas), but it's a compile-safe subset rather than a blanket change.
  • Keep the blanket alias but make it additive-safe by having the generator detect when a member's unwrapped T is a reference/nullable type and skip generating the conflicting sync Returns(Func<T>) overload's ambiguous overlap — e.g. by not calling it Returns for those cases specifically. More invasive, but keeps the fix uniform.

Either way, this is worth resolving before merge since it's a compile-breaking regression for existing callers, not just a theoretical edge case — greptile-apps flagged the same root cause above and I independently confirmed it compiles-then-breaks with a minimal repro.

Everything else looks solid

  • ReturnsRaw reuse is the right call — no new completion-handling logic to keep in sync with ReturnsAsync.
  • Snapshot updates are additive-only and consistent across all affected fixtures; no stray .received.txt files.
  • Doc comments on the new overloads correctly describe the "handed back as-is / stays pending" semantics that make the timeout-race scenario work.

…das unambiguous

Review finding: for a reference-typed async result, a lambda whose body
pins nothing — Returns(() => null), Returns(() => throw ...) — converts
equally well to Func<T> and to the new Func<Task<T>>, making previously
valid setups CS0121. Reproduced against IReferenceResultService before
adding the attribute.

[OverloadResolutionPriority(-1)] on the async factory overloads breaks
that tie back to the pre-existing synchronous meaning. A genuine async
lambda is unaffected: it is not convertible to Func<T> at all, so it
remains the only applicable candidate.
@thomhurst

Copy link
Copy Markdown
Owner Author

Both reviews are right — confirmed the ambiguity before fixing it. With the new overloads and no tie-break, three of the new reference-typed cases fail to compile:

Issue6495Tests.cs(134,29): error CS0121: The call is ambiguous between
  '...GetNameAsync_M0_MockCall.Returns(System.Func<string?>)' and
  '...GetNameAsync_M0_MockCall.Returns(System.Func<System.Threading.Tasks.Task<string?>>)'

Fixed in bb4843d by putting [OverloadResolutionPriority(-1)] on the async factory overloads (both the parameterless and typed-parameter forms) rather than narrowing which members get the alias. The priority only breaks ties among applicable candidates, so:

  • Returns(() => null) / Returns(() => throw ...) — both overloads applicable, priority sends it back to the synchronous Func<T> and its pre-existing meaning (null is the value; the member still returns a completed task).
  • Returns(async () => ...) — an async lambda is not convertible to Func<T> at all, so the task overload is the only candidate and the feature still works.

This keeps the fix uniform across all async members instead of restricting it to non-nullable value types, which would have left the reference-typed case — the more common one for a mocked service — still needing ReturnsAsync.

[OverloadResolutionPriority] ranks only within a single containing type, which is satisfied here: both overloads are emitted into the same generated call wrapper. TUnit.Mocks already requires C# 14 (TM004), so the Roslyn ≥4.12 floor for honouring it is guaranteed.

Added to Issue6495Tests.cs against a new IReferenceResultService (Task<string?> members): null-returning lambda, throwing lambda, null-returning typed lambda, and an async lambda on a reference-typed result asserting it still stays pending. TUnit.Mocks.Tests 1172 passed; snapshots regenerated and green on net8.0/net9.0/net10.0.

Comment thread src/TUnit.Mocks.SourceGenerator/Builders/MockMembersBuilder.cs
@claude

claude Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Code review

Follow-up on my previous review: the source-breaking ambiguity I (and Greptile) flagged for Returns(() => null) / Returns(() => throw ...) on reference-typed async results is resolved in bb4843d.

Verified the fix:

  • [OverloadResolutionPriority(-1)] is applied uniformly to every new Returns async-factory alias (parameterless and typed-parameter forms, Task<T>/ValueTask<T>/Task/ValueTask), consistently deprioritizing them against the pre-existing synchronous Func<T> overload. This is the same pattern already used in src/TUnit.Assertions.Should/ShouldExtensions.cs, so it's consistent with an established convention rather than a new one-off technique.
  • Confirmed the attribute resolves on net8.0 (no #if NET9_0_OR_GREATER guard needed) — Directory.Build.props pulls in the Polyfill package for all internal projects, which supplies OverloadResolutionPriorityAttribute on down-level TFMs. Built TUnit.Mocks.Tests for net8.0 locally: 0 errors.
  • Ran the new Issue6495Tests on net8.0: all 12 pass, including the added IReferenceResultService cases (Returns(() => null), throwing lambda, typed null lambda, and an async lambda on a reference-typed result) that specifically exercise the collision scenario.

No remaining issues from either review. This is good to merge as far as the two previously-raised concerns go.

Review finding: OverloadResolutionPriorityAttribute only reaches a
consumer's compilation on net9.0+. TUnit.Mocks polyfills it internally for
its own build, so the type is not visible to a net8.0 consumer and the
generated attribute would be CS0246 — which is why the framework-operation
polyfills in this file already sit behind #if NET9_0_OR_GREATER.

The alias is inseparable from the priority (without it, reference-typed
results get CS0121 on null/throw lambdas), so the whole overload is now
net9.0+. net8.0 keeps ReturnsAsync, which already hands the task back
as-is and covers the timeout-race case from the issue.

Not reachable by an in-repo test: Directory.Build.props adds Polyfill to
every project here, so our own net8.0 build resolves the attribute that an
external consumer would not.
// See issue #6495.
writer.AppendLine();
writer.AppendLine("#if NET9_0_OR_GREATER");
writer.AppendLine("/// <summary>Configure a typed computed async return value using the actual method parameters. The returned task is handed back as-is, so an async factory stays pending until it completes.</summary>");

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Async Returns remains unavailable on net8

When a .NET 8 consumer configures an async member with Returns(async (...) => ...), this conditional removes both new async-factory overload forms from the generated mock, so the lambda is still considered against the synchronous factory overload and compilation fails with CS4010.

@claude

claude Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Code review

Follow-up on the two prior reviews on this PR: the only change since my last comment is a51c7ee ("gate the async Returns alias to net9.0+"), which is a correct and well-justified fix.

What changed and why it's right:

  • [OverloadResolutionPriority(-1)] is emitted into the consumer's generated source, not TUnit's own build. OverloadResolutionPriorityAttribute only ships in the BCL on net9.0+; TUnit.Mocks resolves it internally via its Polyfill package reference, but that package doesn't flow downstream to a consumer's own net8.0 compilation. My previous "built on net8.0: 0 errors" verification was against TUnit.Mocks.Tests, which itself pulls in Polyfill via Directory.Build.props — not representative of an external net8.0 consumer, so that check didn't actually cover this case. This commit's finding is correct and the fix (#if NET9_0_OR_GREATER around both the parameterless and typed-parameter Returns aliases) matches the pattern already used for the other framework-operation polyfills earlier in the same file.
  • The trade-off is explicit and reasonable: net8.0 consumers lose the new Returns(Func<Task<T>>) alias but keep ReturnsAsync, which already hands the task back as-is and covers the original timeout-race scenario from [TUnit.Mocks] No way to make an async member's .Returns() genuinely pending (only synchronous Func<T> supported) - breaks timeout-race tests #6495. That's a real capability gap for net8.0 (same one Greptile flagged on the first pass), but it's now a deliberate, documented trade-off rather than a silent compile break — the right call given the alias is inseparable from the priority attribute.
  • Snapshot and test changes are consistent with the source change across all affected fixtures — #if NET9_0_OR_GREATER added in matching locations in every .verified.txt, and the corresponding test cases in Issue6495Tests.cs are gated the same way. No stray .received.txt files.

No remaining issues. Good to merge as far as all three reviews (Greptile's and my two) go.

@thomhurst
thomhurst merged commit 800828a into main Jul 28, 2026
15 checks passed
@thomhurst
thomhurst deleted the fix/6495-async-returns-factory branch July 28, 2026 10:58
thomhurst added a commit that referenced this pull request Jul 28, 2026
…factories

Rebase conflict resolution: #6503 added the Func<Task<T>> Returns
overloads to the same generated call-wrapper types this branch moved
into TUnit.Mocks.Generated. Regenerated rather than hand-merged.
thomhurst added a commit that referenced this pull request Jul 28, 2026
…ce (#6504)

* fix(mocks): emit the setup surface into the globally-imported namespace

Setup, verify and event extensions were generated beside the mocked type.
An extension member is only visible when its namespace is imported, so
mocking a type whose namespace the test had not `using`'d compiled the
Mock() call — that entry point already lives in TUnit.Mocks — but hid
every member setup behind CS1061, which reads as "this member isn't
supported". Reported as a name collision between two same-named ICluster
interfaces; it reproduces with a single mock and no collision involved.

Emit the extension classes and the call wrappers they return into
TUnit.Mocks.Generated, which TUnit.Mocks.targets adds as a global using.
Their names are derived from the fully qualified type name, so types
sharing a short name across namespaces stay distinct. The impl, factory
and wrapper types keep their current placement, as do the out/ref setter
delegates the impl references by short name.

Fixes #6494

* test(mocks): snapshot the non-span ref-struct out/ref delegate emission

Review nit: no snapshot covered the one construct that emits a
namespace-scoped delegate, which is precisely the path this PR split out
of the members loop. The new fixture pins both halves — the delegates stay
in the mocked type's namespace (the impl references them by short name)
while the setup surface moves to TUnit.Mocks.Generated.

* test(mocks): refresh snapshots after rebasing onto the async Returns factories

Rebase conflict resolution: #6503 added the Func<Task<T>> Returns
overloads to the same generated call-wrapper types this branch moved
into TUnit.Mocks.Generated. Regenerated rather than hand-merged.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[TUnit.Mocks] No way to make an async member's .Returns() genuinely pending (only synchronous Func<T> supported) - breaks timeout-race tests

1 participant