Skip to content

feat(mocks): add Arg.AnyArgs() shortcut for setup/verify#5792

Merged
thomhurst merged 3 commits intomainfrom
feat/mocks-anyargs-shortcut
May 1, 2026
Merged

feat(mocks): add Arg.AnyArgs() shortcut for setup/verify#5792
thomhurst merged 3 commits intomainfrom
feat/mocks-anyargs-shortcut

Conversation

@thomhurst
Copy link
Copy Markdown
Owner

Summary

  • New Arg.AnyArgs() shortcut: instead of writing Any() once per parameter, callers can match every argument in one token, e.g. mock.Compute(AnyArgs()).Returns(42).
  • The shortcut is source-generated as an additional overload alongside the existing typed setup methods. It is only emitted when the method name is unique on the mocked type and the method has no out/ref/ref-struct parameters; for overloaded names the shortcut would be ambiguous, so callers fall back to the explicit per-parameter form.
  • Generated body uses AnyMatcher<T>.Instance directly (now public + EditorBrowsable(Never)) rather than Arg.Any<T>().Matcher — skips the CapturingMatcher<T> wrap, since AnyArgs callers have no Arg<T> handle to inspect captures anyway.

Before:

mock.Compute(Any(), Any(), Any(), Any(), Any()).Returns(42);

After:

mock.Compute(AnyArgs()).Returns(42);

Also extracts a small EmitReturnConstruction helper in MockMembersBuilder to deduplicate the wrapper/mock-call dispatch tail across the three setup-emission paths (existing + new).

Test plan

  • Runtime tests in TUnit.Mocks.Tests/AnyArgsTests.cs — 7 tests covering: setup returns, void+throws, verification with WasCalled, equivalence to all-Any() slots, two-param method, reflective check that overloaded names get no AnyArgs overload, counter-check that uniquely named methods do.
  • All 967 TUnit.Mocks.Tests pass on net10.0.
  • All 49 TUnit.Mocks.SourceGenerator.Tests snapshot tests pass; 6 .verified.txt snapshots updated to reflect the new emission.
  • Docs section added in docs/docs/writing-tests/mocking/argument-matchers.md.

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.
Copy link
Copy Markdown
Contributor

@claude claude Bot left a comment

Choose a reason for hiding this comment

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

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 : class

When 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 struct would let the JIT eliminate the allocation entirely, and Arg.AnyArgs() could just return 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 classpublic 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.

@codacy-production
Copy link
Copy Markdown

codacy-production Bot commented Apr 30, 2026

Not up to standards ⛔

🔴 Issues 1 medium · 7 minor

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

Results:
8 new issues

Category Results
CodeStyle 7 minor
Performance 1 medium

View in Codacy

🟢 Metrics 22 complexity

Metric Results
Complexity 22

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.

Copy link
Copy Markdown
Contributor

@claude claude Bot left a comment

Choose a reason for hiding this comment

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

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

  1. Generic-method suppression ✅ Fixed. if (method.IsGenericMethod) return; is now the first guard in EmitAnyArgsOverload. The FluentUI snapshot no longer contains the unusable UpdateDialogAsync<TData>(AnyArgs _) overload. The test for IGenericAnyArgsService.Pick is present.

  2. AnyArgs as a struct ✅ Fixed. public readonly struct AnyArgs with no fields, Arg.AnyArgs() returns default. Allocation concern is gone.

  3. < 2 parameter silent inconsistency ✅ Documented (docs note updated). Acceptable resolution — no overload is emitted and the docs explain why.

  4. Reflection test fragility ✅ Mitigated. SingleOrDefault + null-check means the test now returns false (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.

  5. 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.

@thomhurst thomhurst enabled auto-merge (squash) May 1, 2026 22:36
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.
Copy link
Copy Markdown
Contributor

@claude claude Bot left a comment

Choose a reason for hiding this comment

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

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.

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.

1 participant