fix(mocks): mocking a method with more params than Func/Action arity (#6254)

[thomhurst]

Fixes #6254

Problem

Mocking an interface method with more parameters than the BCL System.Func<>/System.Action<> arity limit (16 inputs) failed to compile:

error CS0305: Using the generic type 'Func<T1,...,T16,TResult>' requires 17 type arguments
error CS0305: Using the generic type 'Action<T1,...,T16>' requires 15 type arguments

The generator's typed Returns/ReturnsAsync/Callback/Throws convenience overloads build Func<...>/Action<...> from the parameter list, but were emitted whenever a method had >= 1 matchable parameter — no upper bound. Async methods always get a typed wrapper (for the generated ReturnsAsync surface), so a 20-param Task method overflowed the BCL delegate arity. (Non-async methods past MaxTypedParams already fell back to the untyped surface, so they were unaffected.)

Fix

Gate the typed parameter overloads on a new MaxDelegateParams = 16 constant. Methods past it still get the wrapper and its untyped Returns/ReturnsAsync/Throws/Callback/verification members — only the parameter-typed convenience overloads are omitted.

Chose the BCL arity (16) rather than the existing MaxTypedParams (8) deliberately: 9–16 param methods currently emit working typed overloads (their bodies already route through the untyped args => ... delegate path), so gating at 8 would have dropped working API.

Tests

• Generator snapshot test (Method_With_More_Params_Than_Func_Action_Arity) — confirms no oversized Func/Action is emitted and the wrapper retains the untyped surface.
• Runtime coverage (LongMethodSignatureTests): 20-param Task, 20-param Task<int>, and a 10-param method asserting typed overloads survive above the old cap of 8.

All 71 snapshot + 1125 runtime mocks tests pass.

[codacy-production]

Not up to standards ⛔

🔴 Issues 1 medium · 1 minor

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

Results:
2 new issues

Category	Results
CodeStyle	1 minor
Performance	1 medium

View in Codacy

🟢 Metrics 28 complexity

Metric	Results
Complexity	28

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: fix(mocks): mocking a method with more params than Func/Action arity

Summary

The fix is correct and targeted. The core logic is sound: async methods with >16 params now correctly omit the typed convenience overloads while retaining the full untyped setup surface. Tests are appropriate.

---

Finding 1 — Implicit relationship between two arity constants (design concern)

The most significant maintainability concern is that the relationship between two constants is implicit and could confuse future readers:

• MaxTypedParams = 8 — gates whether a typed wrapper class is generated at all (in ShouldGenerateTypedWrapper, line ~285). Non-async methods with >8 params skip the wrapper entirely and are never affected by MaxDelegateParams.
• MaxDelegateParams = 16 — gates typed overload emission within a wrapper. This guard is only ever reached for async methods, because async methods bypass the MaxTypedParams gate at line 268.

The net effect is correct, but the reason MaxDelegateParams only matters for async methods is not documented where the constants are defined or where ShouldGenerateTypedWrapper makes the async short-circuit. A future maintainer who changes one constant without understanding the other could introduce a regression.

Suggestion: Add a comment at the async short-circuit in ShouldGenerateTypedWrapper linking the two:

// Async methods bypass the MaxTypedParams gate and are instead gated by MaxDelegateParams
// at the typed-overload emission sites (BCL Func<>/Action<> support at most 16 type args).
// For non-async methods, MaxTypedParams is the effective ceiling.
return matchableParams.Count <= MaxTypedParams;

This makes the two-level gating strategy self-documenting.

---

Finding 2 — Missing snapshot for the returning-async (Task<T>) code path

The snapshot test Method_With_More_Params_Than_Func_Action_Arity covers the Task (void-async) variant, which routes through GenerateVoidUnifiedClass. The Task<int> (returning-async) variant routes through GenerateReturnUnifiedClass, which also has the new MaxDelegateParams guard (line ~475) but is not covered by a snapshot test.

The runtime test LongMethodSignatureTests does exercise the Task<int> 20-param case at the behavior level, so this is not a blocking gap. But a generator snapshot for the returning-value path would give additional confidence that the emitted source is clean (no oversized Func<> with >16 type args).

---

Finding 3 — Boundary case (minor observation, not a defect)

The fix triggers at >16 params; the tests use 20 and 10. An async method with exactly 17 params is the precise boundary case for the new guard. This is a minor observation — 20 clearly demonstrates the condition — but a 17-param snapshot would be the sharpest regression guard.

---

Verdict

Approve with suggestions. The fix is correct, well-scoped, and adequately tested at the behavior level. The suggestions above are aimed at long-term maintainability rather than correctness: documenting the two-level gating strategy and optionally adding a snapshot for the Task<T> returning-async path. Neither is a blocker.

[claude]

Code Review: fix(mocks): mocking a method with more params than Func/Action arity (#6254)

This is a clean, minimal, well-understood fix. The root cause analysis in the PR description is accurate and thorough.

What the fix does

The generator's typed Returns/ReturnsAsync/Callback/Throws overloads build Func<T1...Tn, TResult>/Action<T1...Tn> from the parameter list. BCL caps those at 16 input type arguments. Non-async methods already had an implicit guard via MaxTypedParams = 8, but async methods always got a typed wrapper regardless of parameter count — causing CS0305 at 17+ params.

The fix adds MaxDelegateParams = 16 and gates typed overloads with nonOutParams.Count <= MaxDelegateParams, applied symmetrically to both GenerateReturnUnifiedClass and GenerateVoidUnifiedClass. That's exactly the right scope.

Strengths

• Minimal blast radius. Only 16 changed lines in production code; the constants are clearly named and commented.
• Boundary handling is correct. Gating at the BCL limit (16) rather than MaxTypedParams (8) avoids silently dropping typed overloads for 9–16 param methods that currently work.
• Test coverage is exceptional. LongMethodSignatureTests covers the regression case; ManyParameterMockTests is comprehensive — it tests exactly-at-limit (16), just-over-limit (17), extreme (32), mixed types, params keyword, nullable types, void/sync/async/ValueTask shapes, per-slot matching, and verification. The fact that the interface definitions compile at all is itself the regression assertion, which the comments correctly call out.
• The snapshot is correct. It shows the generated class retains the untyped Returns()/ReturnsAsync(Task)/ReturnsAsync(Func<Task>)/Throws/Callback(Action) surface for a 20-param async method — the right fallback.

One mild design note (not blocking)

There are now two related but distinct constants: MaxTypedParams = 8 and MaxDelegateParams = 16. The former controls the happy-path typed overload threshold for non-async methods; the latter is the hard BCL ceiling for all typed delegate generation. Their interplay is implicit — someone reading MockMembersBuilder.cs cold might not immediately see that MaxTypedParams also implicitly upholds the MaxDelegateParams contract for non-async paths. A cross-reference comment near MaxTypedParams pointing to MaxDelegateParams (e.g. "// Must also be <= MaxDelegateParams") would make the relationship explicit and prevent a future bump of MaxTypedParams past 16 from silently re-introducing the bug. That said, given both are in the same file with nearby comment blocks, this is a very minor observation.

No issues found

The change is correct and complete as submitted. The two-constant relationship above is worth noting for long-term maintainability but not a blocking concern — the fix is sound as-is.

[claude]

Code Review: Updated assessment after refactor commit (e2ba5fc)

This review checks the current state of the PR against the two previous reviews (comments at 2c6508b and bb801f6).

Previous concerns and whether they're addressed

Finding 1 — Implicit relationship between MaxTypedParams and MaxDelegateParams
Both prior reviews flagged this. The refactor commit extracted the guard into CanEmitTypedParamOverloads, which improves the 'single place for the limit' concern. However, the cross-reference between the two constants is still missing: MaxTypedParams = 8 and MaxDelegateParams = 16 sit next to each other with no comment explaining that MaxTypedParams must stay ≤ MaxDelegateParams, or that MaxTypedParams is the non-async ceiling and MaxDelegateParams is the hard BCL ceiling. A future bump of MaxTypedParams past 16 would silently reintroduce CS0305 for non-async methods.

Suggestion (not blocking): Add one comment near the constants:

private const int MaxTypedParams = 8;       // for non-async methods; must stay <= MaxDelegateParams
private const int MaxDelegateParams = 16;   // BCL Func<>/Action<> arity ceiling

Finding 2 — Missing Task snapshot
Still not present. The runtime test MixedTypes_Twelve_TypedCallback_CastsEachParameterCorrectly covers the typed-delegate path for a 12-param Task, and the IBoundarySeventeen tests cover Task at 17 params. This is still a mild gap but not blocking given runtime coverage.

Finding 3 — 17-param boundary in tests
Fully addressed: IBoundarySeventeen (exactly 17 params), IBoundarySixteen (exactly 16), and SeventeenParams_UntypedSurface_Works give precise boundary coverage. Excellent.

Assessment of the refactor commit itself

The refactor is clean and correct:

• CanEmitTypedParamOverloads eliminates the duplicated condition and its comment — the right abstraction for a two-site check.
• Consolidating LongMethodSignatureTests into ManyParameterMockTests removes duplication without losing coverage; the issue-6254 repro shape is preserved.
• The MixedTypes_Twelve fix (asserting all 12 parameters, not just 8) is a genuine correctness improvement to the test — it was previously only proving half of what it claimed.
• Loop for ManyParams_Verification_CountsAndNever is a nice readability cleanup.

Verdict

Approve. The fix is correct, minimal, and now well-structured. The one remaining nit (cross-reference comment between the two constants) is genuinely minor — the code is correct as-is. Everything raised in previous reviews is either resolved or has adequate runtime coverage as a substitute.

[claude]

Code Review: Final assessment after docs/snapshot commit (e905038)

This review covers the final commit which directly addresses the two remaining findings from the previous review at e2ba5fc.

Previous findings — all resolved

Finding 1 — Implicit relationship between MaxTypedParams and MaxDelegateParams
Fully addressed. The new comment at the MaxTypedParams declaration is precise:

Must stay <= MaxDelegateParams so the typed overloads it permits never exceed the BCL delegate arity.

And the expanded comment inside ShouldGenerateTypedWrapper at the async short-circuit explains the two-level gating strategy clearly, linking MaxTypedParams (the non-async ceiling) to MaxDelegateParams (the BCL Func<>/Action<> hard ceiling) and pointing to CanEmitTypedParamOverloads as the single enforcement site. A future maintainer bumping either constant will now see exactly why the constraint exists.

Finding 2 — Missing Task returning-async snapshot
Fully addressed. The new Returning_Async_Method_With_More_Params_Than_Func_Action_Arity snapshot test uses 17 params (exactly one past the BCL limit) on a Task<int> method — the sharpest possible regression guard for the GenerateReturnUnifiedClass emit site. The snapshot confirms the typed parameter overloads are absent while Returns(int), ReturnsAsync(Task<int>), ReturnsAsync(Func<Task<int>>), and the untyped Throws/Callback surface are all present. That's precisely what the fix promises.

Overall assessment

The PR is complete and correct:

• Root cause: BCL Func<>/Action<> arity limit not enforced for async methods.
• Fix: Minimal, two-site change behind CanEmitTypedParamOverloads, gated at MaxDelegateParams = 16.
• Tests: Exhaustive — boundary-exact (16 and 17), extreme (32), mixed types, nullable, params keyword, all method shapes, and per-slot matcher verification.
• Documentation: Constants are now self-evidently safe to maintain; the two-level gating strategy is explained where it matters.

Approve.