feat(mocks): make the async-factory Returns alias available below net9.0 - #6518
Conversation
The Returns(Func<Task<T>>) alias added for #6495 carried [OverloadResolutionPriority(-1)] to stay unambiguous against the synchronous Returns(Func<T>) sibling for typeless lambdas like Returns(() => null). The attribute type only reaches a consumer's compilation on net9.0+, so the whole alias was gated there — on net8.0 and net472, .Returns(async () => ...) did not exist at all and a mocked async call could only be handed back already-completed (issue #6515). Below net9.0 the alias is now generic instead: Returns<TAsyncFactoryResult>(Func<Task<TAsyncFactoryResult>>). A typeless lambda body (null / throw / default) cannot infer the type parameter, which excludes the alias from the candidate set and resolves the tie to the synchronous overload exactly as ORP does — while a genuine async lambda with an inferable body binds the alias, ValueTask included. The net9.0+ ORP shape is unchanged (it keeps the compile-time result-type check the generic form trades for inference). Bare Task/ValueTask members have no synchronous sibling to collide with, so their alias is now emitted unconditionally without either mechanism. The Issue6495 tests drop their net9.0+ gates and now run on every target framework, and Issue6515Tests adds the issue's timeout-race consumer shape. Fixes #6515
Greptile SummaryAdds cross-target-framework support for async-factory
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| src/TUnit.Mocks.SourceGenerator/Builders/MockMembersBuilder.cs | Generates the cross-TFM async Returns overloads and declared-result conversion helpers. |
| src/TUnit.Mocks.SourceGenerator/Builders/MockImplBuilder.cs | Updates raw async-return consumption to support outer-nullable task references. |
| tests/TUnit.Mocks.Tests/Issue6515Tests.cs | Adds runtime coverage for pending factories, overload resolution, result conversion, and nullable task behavior. |
| tests/TUnit.Mocks.Tests/Issue6495Tests.cs | Enables existing async-factory alias tests across all supported target frameworks. |
| tests/TUnit.Mocks.SourceGenerator.Tests/MockGeneratorTests.cs | Adds compilation and snapshot coverage for the generated overload and conversion shapes. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Async factory passed to Returns] --> B{Generic task result?}
B -- No --> C[Store bare Task or ValueTask as raw return]
B -- Yes --> D[Infer factory result type]
D --> E{Matches declared result type?}
E -- Yes --> F[Preserve original task]
E -- No --> G[Await and convert result]
F --> H[Store raw async return]
G --> H
H --> I[Mock invocation consumes raw return]
I --> J[Return declared Task or ValueTask]
Reviews (11): Last reviewed commit: "fix(mocks): skip typed-local dynamic con..." | Re-trigger Greptile
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8b45df102a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
|
|
||
| /// <summary>True for Task<T>/ValueTask<T> type strings, false for bare Task/ValueTask.</summary> | ||
| private static bool IsGenericTaskType(string taskType) => taskType.EndsWith(">"); |
There was a problem hiding this comment.
Detect nullable generic task return types
When a mocked member returns an outer-nullable type such as Task<string>?, taskType ends in ?, so this check misclassifies it as a non-generic Task. The generator then emits an unprioritized Returns(Func<Task<string>?>) overload alongside Returns(Func<string?>), making an existing setup such as .Returns(() => null) fail with CS0121 on every target framework, including net9.0 where the previous alias was prioritized. Determine genericity from the type model or account for the trailing nullable annotation.
Useful? React with 👍 / 👎.
Review finding (#6518): IsGenericTaskType tested EndsWith(">"), so an outer-nullable member type like Task<string?>? — which carries a trailing '?' — was misread as bare Task and got the ungated, unprioritized alias next to the synchronous factory, turning an existing Returns(() => null) setup into CS0121 on every TFM. Trim the annotation before testing. The new regression coverage surfaced a second, pre-existing break on the same member shape: EmitRawReturnCheck used the annotated return type in an pattern, which is never legal (CS8116) — so outer-nullable async members could not be mocked at all. The pattern now drops the outer annotation; a null raw value falls through to the existing informative throw. Covered by a generator test compiling the ambiguous call site and runtime tests on the Task<string?>? shape (sync null-lambda binding and pending async factory).
Review finding (#6518): IsGenericTaskType tested EndsWith(">"), so an outer-nullable member type like Task<string?>? — which carries a trailing '?' — was misread as bare Task and got the ungated, unprioritized alias next to the synchronous factory, turning an existing Returns(() => null) setup into CS0121 on every TFM. Trim the annotation before testing. The new regression coverage surfaced a second, pre-existing break on the same member shape: EmitRawReturnCheck used the annotated return type in an `is` pattern, which is never legal (CS8116) — so outer-nullable async members could not be mocked at all. The pattern now drops the outer annotation; a null raw value falls through to the existing informative throw. Covered by a generator test compiling the ambiguous call site and runtime tests on the Task<string?>? shape (sync null-lambda binding and pending async factory).
e89d696 to
d7f1558
Compare
|
Codex's P2 was valid — The regression coverage for that shape also surfaced a pre-existing break: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d7f1558061
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| writer.AppendLine($"public {wrapperName} Returns(global::System.Func<{taskType}> taskFactory) {{ EnsureSetup().ReturnsRaw(() => (object?)taskFactory()); return this; }}"); | ||
| writer.AppendLine("#else"); | ||
| writer.AppendLine(aliasDoc); | ||
| writer.AppendLine($"public {wrapperName} Returns<TAsyncFactoryResult>(global::System.Func<{taskKind}<TAsyncFactoryResult>> taskFactory) {{ EnsureSetup().ReturnsRaw(() => (object?)taskFactory()); return this; }}"); |
There was a problem hiding this comment.
Preserve the mocked result type in the generic alias
On the below-net9 branch, an async lambda returning a subtype infers that subtype rather than the mocked member's declared result type. For example, for Task<object> GetAsync(), .Returns(async () => { await Task.Yield(); return "value"; }) registers a Task<string>; the generated implementation then tests the raw value against Task<object>, which fails because Task<T> is invariant, and returns a faulted task with InvalidOperationException. The same regression affects interface/base return types, ValueTask<T>, and the typed-parameter alias.
Useful? React with 👍 / 👎.
…make the generic alias primary on every TFM Review finding (#6518): below net9.0 the generic alias infers the async lambda's own result type, which may be a SUBTYPE of the member's declared result (Task<object> member, async () => "value" infers Task<string>) — and Task<T>/ValueTask<T> are invariant, so the implementation's raw-return check rejected the stored task and produced a faulted task. The generic alias now converts to the declared task type: an exact-typed factory task passes through untouched (identity preserved), anything else is mirrored by an async wrapper that stays pending until the inner task completes. Writing regression coverage exposed that the net9.0+ ORP branch was WORSE on the same member shape: a natural-typed async lambda converts to object, ORP(-1) pruning runs before betterness, and Returns(object value) silently boxed the delegate as the member's value. The generic alias wins that contest (lambda-to-delegate beats lambda-to-object), so it is now the primary alias on EVERY target framework. The ORP(-1) non-generic alias remains, net9.0+-only, for the one shape generics cannot express — async () => null pins no type, so the deprioritised alias is the sole applicable candidate — and ORP pruning guarantees the two aliases are never ambiguous with each other. Second review finding: for an outer-nullable member, ReturnsAsync accepts a null task, but the raw-return pattern can never match null — the generated check now returns null there instead of throwing. Runtime coverage: subtype conversion (Task, ValueTask, typed parameters, pending-ness), exact-task identity, null-task on an outer-nullable member, and the async-null-body corner on net9.0+. Suites: 1193/1193 net10.0, 1192/1192 net9.0, 1176/1176 net8.0; generator 91/91 with 7 snapshots updated.
|
Both new findings verified and fixed in 1f5efff: P1 (subtype inference) — valid, and it went deeper than reported. The generic alias now converts the factory task to the declared task type: exact-typed tasks pass through untouched (reference identity preserved — covered by a test), subtypes are mirrored by an async wrapper that stays pending until the inner task completes, and a wrong-typed factory surfaces as While writing the regression tests I found the net9.0+ ORP branch was worse on the same P2 (null task on outer-nullable member) — valid, fixed. The raw-return check now returns null for outer-nullable members instead of falling into the mismatch throw; non-nullable members keep the informative throw. Coverage: subtype conversion across Task/ValueTask/typed-parameters/pending-ness, exact-task identity, null-task round-trip, async-null-body on net9.0+. 1193/1192/1176 across net10/net9/net8, generator 91/91. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1f5efffe27
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| writer.AppendLine($"private static async {taskKind}<{resultType}> __TUnitMocksAwaitAndConvert<TAsyncFactoryResult>({taskKind}<TAsyncFactoryResult> task)"); | ||
| writer.AppendLine($" => ({resultType})(object?)(await task.ConfigureAwait(false))!;"); |
There was a problem hiding this comment.
Preserve implicit async-result conversions
When a member returns Task<long> or ValueTask<long> and the async factory returns an int, this generic overload infers TAsyncFactoryResult as int, although the normal contextual conversion to the declared delegate permits int to long. The helper then casts the boxed int directly to long, so the mocked call faults with InvalidCastException; this also affects user-defined implicit conversions and occurs on net9+ because the generic overload outranks the non-generic alias.
Useful? React with 👍 / 👎.
| writer.AppendLine($"public {wrapperName} Returns<TAsyncFactoryResult>(global::System.Func<{taskKind}<TAsyncFactoryResult>> taskFactory) {{ EnsureSetup().ReturnsRaw(() => (object?)__TUnitMocksConvertAsyncResult<TAsyncFactoryResult>(taskFactory())); return this; }}"); | ||
| writer.AppendLine(); | ||
| writer.AppendLine($"private static {taskKind}<{resultType}> __TUnitMocksConvertAsyncResult<TAsyncFactoryResult>({taskKind}<TAsyncFactoryResult> task)"); |
There was a problem hiding this comment.
Generate a collision-free factory type parameter
If the mocked interface or generic method already declares TAsyncFactoryResult, the generated method and helpers shadow that wrapper type parameter. For a Task<TAsyncFactoryResult> member instantiated with object, an async factory returning string therefore leaves the helper typed as Task<string> instead of converting to Task<object>, and the implementation rejects the raw task. Generate a name that is unique against all captured type parameters.
Useful? React with 👍 / 👎.
| writer.AppendLine($"private static {taskKind}<{resultType}> __TUnitMocksConvertAsyncResult<TAsyncFactoryResult>({taskKind}<TAsyncFactoryResult> task)"); | ||
| writer.AppendLine($" => task is {taskKind}<{resultType}> exact ? exact : __TUnitMocksAwaitAndConvert(task);"); |
There was a problem hiding this comment.
Pass through null tasks from outer-nullable factories
For an outer-nullable member such as Task<string?>?, .Returns(() => (Task<string?>?)null) binds the new generic alias, but the exact-type pattern cannot match null and sends it to the awaiting helper, producing a non-null faulted task instead of the contractually valid null. Although the raw-return check now accepts null, this new conversion path intercepts it first and needs its own null pass-through.
Useful? React with 👍 / 👎.
- Replay implicit numeric widening in the conversion helper: a boxed int cannot unbox as long, so numeric-primitive result types get a guarded Convert.ChangeType branch (string/bool excluded to avoid silent parsing); anything else throws an informative InvalidCastException naming both types - Uniquify the alias's type parameter against the mocked type's and method's own type parameters (a user-declared TAsyncFactoryResult shadowed it and rebound the helper's result type) - Pass a null factory task straight to the raw-return check (valid for outer-nullable members) instead of faulting inside the conversion helper
|
Third round — all three P2s verified and fixed in a483b22. Numeric widening (P2) — valid.
Null task through the generic alias (P2) — valid. When the alias binds (factory task type differs from the declared one, so the synchronous factory is inapplicable) and the factory yields a null task, the alias now passes null straight to the raw-return check — which accepts it for outer-nullable members — instead of faulting inside the conversion helper. Covered on a Suites: mocks 1199/1198/1182 across net10/net9/net8; generator 91/91 on all three TFMs (snapshots updated). |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a483b22284
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| writer.AppendLine("switch (value)"); | ||
| using (writer.Block()) | ||
| { | ||
| writer.AppendLine($"case {patternType} exact: return exact;"); |
There was a problem hiding this comment.
Avoid emitting a dynamic type pattern
For a mocked member returning Task<dynamic> or ValueTask<dynamic>, patternType is dynamic, so the generated helper contains case dynamic exact, which is illegal (CS8208). Merely mocking such an interface therefore makes the consumer compilation fail; handle dynamic as object when generating the pattern and diagnostic type.
Useful? React with 👍 / 👎.
| using (writer.Block()) | ||
| { | ||
| writer.AppendLine($"case {patternType} exact: return exact;"); | ||
| writer.AppendLine($"case null: return default({resultType})!;"); |
There was a problem hiding this comment.
Reject null when converting to non-nullable value results
When the declared member returns Task<int> but the generic alias infers Task<int?>, a factory result of null reaches this branch and is silently converted to default(int) (zero). Null is not implicitly convertible from int? to int, so the mock returns corrupted data instead of reporting the incompatible factory result.
Useful? React with 👍 / 👎.
| { | ||
| // string/bool are IConvertible but have no numeric conversion in C# — | ||
| // excluding them keeps ChangeType from silently parsing "123" to a number. | ||
| writer.AppendLine($"case global::System.IConvertible convertible when convertible is not string && convertible is not bool: return ({patternType})global::System.Convert.ChangeType(convertible, typeof({patternType}), global::System.Globalization.CultureInfo.InvariantCulture);"); |
There was a problem hiding this comment.
Restrict numeric conversion to implicit widening pairs
Convert.ChangeType accepts conversions that C# does not consider implicit, such as long to int, double to int, or decimal to double. Thus a wrongly typed async factory can silently narrow or round its value rather than taking the advertised InvalidCastException path; check the source/destination numeric pair before converting.
Useful? React with 👍 / 👎.
…conversions Codex round 2 on #6518, all in the generated __TUnitMocksAwaitAndConvert helper: - Task<dynamic>/ValueTask<dynamic> members no longer break consumer compilation: `dynamic` is illegal as a pattern type (CS8208) and as a typeof operand (CS1962), so the helper's pattern/typeof spell it `object` (an identity conversion at the return site). Constructed types containing dynamic (Task<dynamic>) are legal and keep their spelling. - A null factory result on a non-nullable value-type member (Task<int> member, alias inferring Task<int?>) now throws the informative InvalidCastException naming the inferred and declared types instead of silently producing default (zero). The check is a runtime one so generic result types are judged by their instantiation; null remains valid for reference/nullable results. - Convert.ChangeType is gone: only C#'s implicit numeric widening table is replayed, each source type as a dedicated compiler-verified cast case. Narrowing/rounding pairs C# rejects (long->int, double->int, double->decimal) now surface via the informative InvalidCastException instead of silently truncating or rounding.
|
Fourth round - all three findings verified and fixed in 4c42b6b.
Silent
Suites: mocks 1208/1207/1191 across net10/net9/net8; generator 92/92 on all three TFMs (snapshots updated). |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4c42b6bd2c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…-factory helper `async () => 1` on a Task<byte> member compiles against the declared delegate via C#'s implicit constant expression conversion, but the generic alias infers int and the widening table had no int -> byte entry, so the awaited boxed int faulted with InvalidCastException. Constant-ness is erased at runtime, so the helper now replays these conversions as range-guarded exact-value cases (int -> sbyte/byte/ short/ushort/uint/ulong, long -> ulong when non-negative); out-of-range values keep taking the informative InvalidCastException path.
|
Fifth round addressed in f09916d: Widening table omits implicit constant conversions (Codex P2) — valid, fixed. Tests: runtime coverage for byte/ Suites: TUnit.Mocks.Tests 1213/1213, generator snapshots 93/93. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f09916ddf7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| using (writer.Block()) | ||
| { | ||
| writer.AppendLine($"case {patternType} exact: return exact;"); | ||
| if (!resultType.EndsWith("?") && patternType is not ("object" or "string")) |
There was a problem hiding this comment.
Guard null for unconstrained nullable result types
For a generated Task<T?> DoAsync<T>() instantiated with T = int, T? is int, not Nullable<int>, but this textual EndsWith("?") check suppresses the runtime value-type guard. If the generic alias infers int? and its factory produces null, the following null case returns default(T?)—zero—rather than rejecting the incompatible result, silently corrupting the mocked value. Base the guard on the runtime/type-model nullability instead of the trailing annotation.
Useful? React with 👍 / 👎.
…t-zero-to-enum replay A trailing '?' on an unconstrained (or class-constrained) type parameter is the defaultable annotation, not Nullable<T> — Task<T?> with T = int is Task<int>, so the textual EndsWith check wrongly suppressed the runtime value-type null guard and a null factory result became 0. The guard skip is now decided from the type-parameter model: only struct-constrained T? (genuine Nullable<T>) and concrete annotated types skip it. The conversion helper also replays C#'s implicit constant-zero-to-enum conversion as a value-guarded case: integral sources only (never another enum), exactly zero, enum destinations only — judged by typeof(T).IsEnum so generic instantiations participate. Non-zero values keep the informative InvalidCastException.
|
Sixth round — both findings verified and fixed in ebb933a. Defaultable Constant zero-to-enum omitted (P2) — valid. Suites: TUnit.Mocks.Tests 1220/1219/1203 across net10/net9/net8; generator snapshots 94/94 on all three TFMs (two updated, one new). |
…s-all-tfms # Conflicts: # tests/TUnit.Mocks.SourceGenerator.Tests/MockGeneratorTests.cs
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 02ca50075f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "long" or "Int64" => ["sbyte", "byte", "short", "ushort", "int", "uint", "char"], | ||
| "ulong" or "UInt64" => ["byte", "ushort", "uint", "char"], |
There was a problem hiding this comment.
Include native integers in widening conversions
Fresh evidence after the earlier numeric-conversion review is that this newly added widening table omits the predefined nint/nuint conversions. For example, an async factory returning nint for a Task<long> member is valid because nint implicitly converts to long, but the generic alias infers nint; the boxed result matches none of these cases and the mocked call throws InvalidCastException. The same regression affects conversions such as nuint to ulong and ordinary integral values to native-integer result types.
Useful? React with 👍 / 👎.
Code reviewRe-reviewed at The only change since then is test-only: All previously identified issues (tuple result types, |
Summary
Fixes #6515 — on net8.0 (and net472) consumers,
.Returns(async () => ...)on an async member did not exist, so a mocked async call could never be handed back still-pending and a caller's timeout/cancellation could never genuinely race it. (ReturnsAsync(Func<Task<T>>)did cover this on all TFMs, but theReturnsspelling — the one migrators from NSubstitute reach for — was net9.0+ only.)Why it was gated
The alias from #6503 needs to stay unambiguous against the synchronous
Returns(Func<T>)sibling: for a reference-typed result,Returns(() => null)/Returns(() => throw ...)convert equally well toFunc<T>andFunc<Task<T>>→ CS0121.[OverloadResolutionPriority(-1)]broke that tie, but the attribute type only exists in a consumer's compilation on net9.0+, so the whole alias was emitted under#if NET9_0_OR_GREATER.Fix
Below net9.0 the alias is now generic:
Returns<TAsyncFactoryResult>(Func<Task<TAsyncFactoryResult>>)(and theValueTask<...>/typed-parameter variants). A typeless lambda body can't infer the type parameter, so type inference failure excludes the alias from the candidate set and the tie resolves to the synchronous overload — exactly the outcome ORP produces — while a genuine async lambda binds the alias. Validated with a standalone net8.0/net472 probe coveringTask<T>(ref + value type),ValueTask<T>inference, typed parameters, and the null/throw disambiguation cases before wiring it into the generator.Task/ValueTaskmembers have no synchronous sibling to collide with, so their alias is now emitted unconditionally with no mechanism at all.TAsyncFactoryResultrather thanTResultto avoid CS0693 shadowing when the mocked method declares its ownTResult.Tests
Issue6495Testsdrops both#if NET9_0_OR_GREATERgates — the async-alias behavior tests (pending-task race, per-call invocation, ValueTask, typed parameters, null/throw disambiguation) now run on every TFM.Issue6515Testsadds the issue's consumer shape:Task.WhenAnytimeout race against a pending mocked call, plus the fast-path completion case.TUnit.Mocks.Tests: 1184/1184 on net10.0 (ORP path), 1168/1168 on net8.0 (generic path). Generator snapshots: 90/90, 8 snapshots updated.