feat(mocks): add ReturnsAsync typed factory overload with method parameters#5367
feat(mocks): add ReturnsAsync typed factory overload with method parameters#5367
Conversation
…meters Closes #5361. Generates a typed ReturnsAsync overload for async methods with parameters, enabling e.g. `.ReturnsAsync((string key) => Task.FromResult(key))`. Also refactors the typed overload emission into a shared EmitTypedOverloads helper and adds tests for typed Callback/Throws on async methods.
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 17 |
TIP This summary will be updated as you push new changes. Give us feedback
There was a problem hiding this comment.
Code Review
This is a clean, well-scoped feature addition. The implementation is consistent with existing patterns and the test coverage is thorough.
What's Good
Consistency with existing ReturnsAsync pattern
The new GenerateTypedReturnsAsyncOverload generates EnsureSetup().ReturnsRaw(args => (object?)factory(castArgs)), which is exactly consistent with the parameterless factory overload at line 893:
// Existing zero-arg ReturnsAsync (line 893)
EnsureSetup().ReturnsRaw(() => (object?)taskFactory())
// New typed-param ReturnsAsync (this PR)
EnsureSetup().ReturnsRaw(args => (object?)factory(castArgs))This symmetry is the right design — the Task<T>/ValueTask<T> object is returned as-is (not awaited), which is correct since the mock method's return type is the task.
EmitTypedOverloads refactor
The extraction into a shared helper reduces ~15 lines of duplication cleanly and correctly threads isAsync/fullReturnType through both the NET9+ and fallback branches. The resulting call sites at lines 214–222 are much easier to read.
Test coverage
Seven tests covering Task, ValueTask, chaining with .Then(), Callback, and Throws on async methods — exactly the coverage matrix the feature needs.
One Concern: Null-forgiving on nullable reference types in cast args
In the generated code for Interface_With_Nullable_Reference_Type_Parameters:
EnsureSetup().ReturnsRaw(args => (object?)factory((string?)args[0]!));The ! null-forgiving operator is applied to a cast to string? (a nullable type). This is pre-existing in Callback and Returns overloads (same BuildCastArgs helper), so this PR doesn't introduce it. But it's technically misleading: a caller passing null for a string? parameter will have args[0] == null, and the ! tells the compiler "not null" on something that can legitimately be null. At runtime it's harmless (the cast to string? accepts null), but it muddies intent.
This is pre-existing technical debt in BuildCastArgs rather than a problem with this PR, but it's worth tracking if the null-handling story for casts is ever cleaned up.
Minor Observation
The fullReturnType parameter passed into EmitTypedOverloads is the full task type (e.g. global::System.Threading.Tasks.Task<string>). For the #if NET9_0_OR_GREATER branch and the #else branch, the same fullReturnType is passed — this is correct since the return type doesn't change across TFMs, only the parameter list differs. Verified in context.
Overall: Solid implementation. The feature fills a real gap (avoiding .Result blocking in mock factories) and the implementation is consistent with existing infrastructure. Good to merge.
BuildCastArgs was generating `(string?)args[0]!` where the `!` claims non-null on a value that can legitimately be null. Now only applies `!` for non-nullable target types.
There was a problem hiding this comment.
Follow-up Review
The previous review noted one concern about the misleading null-forgiving ! operator on nullable cast args — the follow-up commit 051a8aec ("fix: remove misleading null-forgiving operator on nullable cast args") directly addresses it.
Concern Addressed
The CastArg helper now correctly distinguishes:
private static string CastArg(MockParameterModel p, int index)
{
// For nullable types, skip the null-forgiving operator since the value can legitimately be null.
// For non-nullable types, ! suppresses the object? -> T conversion warning.
var bang = p.FullyQualifiedType.EndsWith("?") ? "" : "!";
return $"({p.FullyQualifiedType})args[{index}]{bang}";
}This is the right fix. Using EndsWith("?") to detect nullable reference types is a reasonable heuristic at codegen time — it correctly handles string?, object?, T? etc. without needing full Roslyn type analysis.
The snapshot update confirms the generated output is now correct: (string?)args[0] (no bang) vs (string)args[0]! (with bang).
One Minor Note
EndsWith("?") will also match nullable value types like int? — but that's fine since nullable value types also accept null legitimately, so omitting ! is equally correct there.
Overall: Both commits are clean, the concern from the first review has been properly resolved. This PR is ready to merge.
Summary
Closes #5361
The
Returnsmethod generates typed factory overloads that provide access to the actual method parameters (e.g.Returns((string key) => ...)), butReturnsAsyncwas missing equivalent overloads. This meant users had to use.Returnswith a blocking.Resultcall to access parameters in async methods.Changes
ReturnsAsync(Func<T1, ..., Task<TReturn>> factory)overload for async mock methods that have parameters (bothTask<T>andValueTask<T>)ComputedRawReturnWithArgsBehaviorandReturnsRaw(Func<object?[], object?>)in the runtime library to support the generated codeEmitTypedOverloadshelperReturnsAsync,Callback, andThrowsoverloads on asyncTask<T>andValueTask<T>methodsBefore
After
Test plan