feat(mocks): add Arg.AnyArgs() shortcut for setup/verify#5792
feat(mocks): add Arg.AnyArgs() shortcut for setup/verify#5792
Conversation
Mock setup with many parameters required Any() per slot, e.g. mock.Compute(Any(), Any(), Any(), Any(), Any()).Returns(42). The new Arg.AnyArgs() shortcut fills every parameter with AnyMatcher<T>.Instance in one token, so that becomes mock.Compute(AnyArgs()).Returns(42). The generator emits the overload only when the method name is unique on the mocked type and there are no out / ref / ref-struct parameters; for overloaded names the shortcut would be ambiguous, so callers fall back to the explicit form. Generated body uses AnyMatcher<T>.Instance directly rather than Arg.Any<T>().Matcher to skip the CapturingMatcher wrap, since AnyArgs callers have no Arg<T> handle to inspect captured values anyway.
There was a problem hiding this comment.
Code Review: feat(mocks) — Arg.AnyArgs() shortcut
Overall this is a well-scoped, thoughtful feature. The EmitReturnConstruction refactor is a clean improvement on its own, the test coverage is solid, and the snapshot-based verification of generator output is the right approach. A few design points worth considering:
1. Generic methods require explicit type arguments — ergonomics gap
From the snapshot in Interface_FluentUI_Shape_Nullable_Warnings.verified.txt:
public static ... UpdateDialogAsync<TData>(this Mock<IDialogService> mock, AnyArgs _) where TData : classWhen a mocked method has its own type parameters (<TData>, <TDialog>, etc.), the AnyArgs overload cannot infer them because the single AnyArgs _ parameter carries no type information. Callers must write:
mock.UpdateDialogAsync<SomeConcreteType>(AnyArgs()); // not mock.UpdateDialogAsync(AnyArgs())This is arguably a worse experience than the regular overload, where the typed Arg<T> parameters guide inference. The PR description doesn't mention this limitation, and the docs section doesn't either. At minimum the docs should call this out. Ideally, the generator could suppress the AnyArgs shortcut for methods that carry their own type parameters (just as it already suppresses it for out/ref params), since the shortcut provides little ergonomic benefit when the caller still has to supply <TData> explicitly.
2. AnyArgs should be a struct (or the public visibility is over-broad)
AnyArgs is a pure sentinel — it carries no state and its only purpose is method dispatch. As a sealed class with a private constructor and an internal static Instance, it works, but:
- It allocates a heap object (even if it's a singleton, the reference semantics add indirection).
- Making it a
readonly structwould let the JIT eliminate the allocation entirely, andArg.AnyArgs()could justreturn default;— no factory method, no singleton required.
Alternatively, keeping it a class is fine, but then the internal static readonly AnyArgs Instance doesn't need to be readonly to enforce anything since the constructor is private — minor nit.
3. The < 2 parameter guard creates a silent inconsistency
The guard:
if (method.Parameters.Length < 2) return;means AnyArgs() silently doesn't exist for single-parameter methods. A user writing mock.GetById(AnyArgs()) gets a compile error with no actionable message — they have to read the docs to discover the constraint. The docs do mention this, but it would be more discoverable if the generator emitted a comment or if the Arg class had an overloaded AnyArgs() that accepted a count (future concern). Not a blocker, but worth a sentence in the warning note in the docs.
4. The reflection-based test encodes a naming-convention contract
var extensionsType = typeof(AnyArgsTests).Assembly.GetTypes()
.Single(t => t.Name == extensionsTypeName); // "TUnit_Mocks_Tests_IOverloadedSumService_MockMemberExtensions"This test is valuable — it verifies the generator correctly omits the overload for ambiguous names — but it's fragile because the class name is a string literal that will silently diverge if the naming convention changes. A minor improvement would be to resolve the type via a known generated method signature rather than by class name, or add a comment here flagging that the literal must be kept in sync with the generator naming convention.
5. AnyMatcher<T> visibility change is a minor API surface expansion
internal sealed class → public sealed class with [EditorBrowsable(Never)] and private constructor is the right approach here — the generated code needs to reference it by fully-qualified name across assembly boundaries. The private constructor correctly prevents external instantiation. No action needed, just worth noting as a technically non-reversible public API addition.
Summary
The core design is sound and the implementation is clean. The one thing I'd recommend acting on before merging is documenting or suppressing the generic-method limitation (#1), as it's likely to cause confusion for users of the FluentUI-style interfaces in the existing snapshots. Everything else is minor polish.
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| CodeStyle | 7 minor |
| Performance | 1 medium |
🟢 Metrics 22 complexity
Metric Results Complexity 22
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.
There was a problem hiding this comment.
Code Review — Round 2: feat(mocks) — Arg.AnyArgs() shortcut
The follow-up commit (cd3b873) addressed all five issues from the previous round. Here is what I found when reviewing the updated diff:
Previous issues — status
-
Generic-method suppression ✅ Fixed.
if (method.IsGenericMethod) return;is now the first guard inEmitAnyArgsOverload. The FluentUI snapshot no longer contains the unusableUpdateDialogAsync<TData>(AnyArgs _)overload. The test forIGenericAnyArgsService.Pickis present. -
AnyArgs as a struct ✅ Fixed.
public readonly struct AnyArgswith no fields,Arg.AnyArgs()returnsdefault. Allocation concern is gone. -
< 2 parameter silent inconsistency ✅ Documented (docs note updated). Acceptable resolution — no overload is emitted and the docs explain why.
-
Reflection test fragility ✅ Mitigated.
SingleOrDefault+ null-check means the test now returnsfalse(and fails) rather than throws if the naming convention diverges. The comment flags the coupling. This is a reasonable middle ground without a full redesign. -
AnyMatcher visibility ✅ No further action needed — previously marked informational.
New / residual concerns in the updated diff
a. Overload-name uniqueness check may not account for explicit interface members correctly
In EmitAnyArgsOverload:
foreach (var m in model.Methods)
{
if (m.ExplicitInterfaceName is not null && !m.IsStaticAbstract) continue;
if (m.Name == method.Name) sameNameCount++;
}This skips non-static explicit-interface methods entirely. Consider a class that both explicitly implements IFoo.Compute(int, int) and has a public Compute(int, int). The explicit-interface version is skipped, so sameNameCount would be 1, and the AnyArgs overload would be emitted even though a second Compute shape exists. This is a minor edge case, but worth a quick check to confirm the guard logic is consistent with whatever GenerateMemberMethod uses for extension-method emission.
b. AnyArgs_Verify_Catches_Any_Call test is missing an await
[Test]
public async Task AnyArgs_Verify_Catches_Any_Call()
{
...
mock.Compute(AnyArgs()).WasCalled(Times.Exactly(2)); // no await
}If WasCalled is synchronous this is fine, but if it can throw asynchronously or returns a Task, the failure would be silently swallowed. Please confirm WasCalled is purely synchronous, or add await for consistency with the rest of the test class.
c. Minor: guard-check ordering in EmitAnyArgsOverload
The early-return < 2 length check currently falls after the ref/out loop. Since a method with 0 or 1 parameter can never have matchable params, the length check could move to the top to make the guard logic read top-to-bottom:
if (method.Parameters.Length < 2) return; // move before the loop
foreach (var p in method.Parameters) { ... }This avoids the minor reader confusion of "what about single-param methods with out params?" before hitting the length guard.
d. Nullable type annotation in snapshot (pre-existing pattern)
The snapshot for Interface_With_Nullable_Reference_Type_Parameters emits AnyMatcher<string?>.Instance. At runtime AnyMatcher<string?> and AnyMatcher<string> are separate closed generic types (NRT annotations are erased). Since AnyMatcher<T>.Matches already returns true unconditionally, this is functionally harmless, but it creates a separate singleton per nullability annotation. Worth checking if the broader mocking engine uses string? vs string consistently — if so, keeping it here is fine for uniformity.
Summary
All five original review concerns have been addressed — the implementation is in solid shape. Items (a) and (b) above are the ones worth confirming before merging; (c) is cosmetic and (d) is a pre-existing pattern question not introduced by this PR.
Ready to merge once (a) and (b) are confirmed or addressed.
PublishTrimmed strips metadata for the generated *_MockMemberExtensions extension classes, so Assembly.GetTypes()/GetMethods() returns nothing and the IsTrue assertions in AnyArgs_Emitted_For_Unique_Method_Names fail (with the IsFalse counterparts trivially passing). The source generator output is identical between JIT and AOT, so skip the three reflection-only tests when dynamic code is unsupported.
There was a problem hiding this comment.
Code Review — Round 3: feat(mocks) — Arg.AnyArgs() shortcut
The third commit (518fb67) adds SkipIfNotDynamicCodeSupportedAttribute to gate the reflection-based emission tests under AOT — that is exactly the right fix, and the reason documented in the commit message is clear and accurate.
Outstanding items from Round 2 — status
a. Explicit-interface uniqueness check — Still present as-is:
foreach (var m in model.Methods)
{
if (m.ExplicitInterfaceName is not null && !m.IsStaticAbstract) continue;
if (m.Name == method.Name) sameNameCount++;
}For a method that is itself an explicit-interface member (ExplicitInterfaceName != null), the loop skips it, so sameNameCount stays 0. The condition sameNameCount > 1 is false, so the overload is not suppressed. In practice this is safe because explicit-interface methods aren't reachable via the same extension-method receiver type, but the guard expresses "no overloads" rather than the real intent of "no ambiguous overloads visible in this extension context." A comment clarifying why explicit-interface methods are excluded from the count would make the invariant explicit and easier to verify in future.
b. Missing await on WasCalled — ✅ Confirmed not a bug. WasCalled(Times) is void — there is nothing to await. The concern from round 2 was unfounded.
Summary
The PR is in good shape. The AOT fix is correct and well-motivated. Item (a) is a minor clarity gap in the guard logic — it won't cause a runtime bug given how extension-method emission works, but a short comment explaining why explicit-interface methods are filtered out of the uniqueness count would prevent future maintainers from wondering whether the continue is accidental. Everything else from the prior rounds has been addressed.
Ready to merge once (a) is either commented or acknowledged as intentional.
Summary
Arg.AnyArgs()shortcut: instead of writingAny()once per parameter, callers can match every argument in one token, e.g.mock.Compute(AnyArgs()).Returns(42).out/ref/ref-struct parameters; for overloaded names the shortcut would be ambiguous, so callers fall back to the explicit per-parameter form.AnyMatcher<T>.Instancedirectly (nowpublic+EditorBrowsable(Never)) rather thanArg.Any<T>().Matcher— skips theCapturingMatcher<T>wrap, sinceAnyArgscallers have noArg<T>handle to inspect captures anyway.Before:
After:
Also extracts a small
EmitReturnConstructionhelper inMockMembersBuilderto deduplicate the wrapper/mock-call dispatch tail across the three setup-emission paths (existing + new).Test plan
TUnit.Mocks.Tests/AnyArgsTests.cs— 7 tests covering: setup returns, void+throws, verification withWasCalled, equivalence to all-Any()slots, two-param method, reflective check that overloaded names get noAnyArgsoverload, counter-check that uniquely named methods do.TUnit.Mocks.Testspass on net10.0.TUnit.Mocks.SourceGenerator.Testssnapshot tests pass; 6.verified.txtsnapshots updated to reflect the new emission.docs/docs/writing-tests/mocking/argument-matchers.md.