Skip to content

Fix default mock argument matching - #7676

Merged
thomhurst merged 1 commit into
mainfrom
issue-7441-default-arg
Nov 5, 2026
Merged

thomhurst merged 1 commit into
mainfrom
issue-7441-default-arg

Conversation

@thomhurst

@thomhurst thomhurst commented Nov 5, 2026 •

Copy link
Copy Markdown
Owner

A default-initialized Arg<T> had no matcher, so setup and verification could fail for methods with five parameters, where predicate overloads are not generated. Default arguments now use a cached exact matcher for default(T).

The fallback is stateless to avoid sharing captures between unrelated default arguments. Default capture accessors return empty values; explicitly constructed arguments keep their existing capture behavior. Regression tests cover null, zero, nullable values, mismatches, and verification through a five-parameter generated method.

Validation: dotnet test --project tests/TUnit.Mocks.Tests --framework net10.0 --no-restore passed all 1,481 tests. No generator output or public API signatures changed.

Closes #7441

Summary by CodeRabbit

  • Bug Fixes
    • Default-initialized argument matchers now match the type’s default value, including zero for numeric types, without capturing values.
    • When no values have been captured, the captured-values list is empty and the latest value is the type’s default.
    • Matching now correctly checks all configured arguments across nullable and non-nullable parameters.

@coderabbitai

coderabbitai Bot commented Nov 5, 2026 •

Copy link
Copy Markdown

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

Default-initialized Arg<T> instances now match default(T) without capturing values. The change also makes Values and Latest safe when no capture matcher exists. Tests cover default matching for a five-parameter setup and a default-initialized integer argument.

Changes: Default argument matching

Layer / File(s) Summary
Default matcher and regression tests
src/TUnit.Mocks/Arguments/ArgOfT.cs, tests/TUnit.Mocks.Tests/NullLiteralMatcherTests.cs
Arg<T>.Matcher falls back to an exact matcher for default(T). Values returns an empty list and Latest returns default when no capture matcher exists. Tests cover a five-parameter setup and confirm that a default Arg<int> matches zero but not one.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Low

Possibly related PRs

  • thomhurst/TUnit#7415: Maps null predicate arguments to Arg.IsNull<T>() in generated overloads, addressing a connected argument-matching path.
Merge Risk: Minimal · up to a2c00

Default arguments match without capturing values. No merge-blocking issue was established.

Pre-merge checks: Passed 5

Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title "Fix default mock argument matching" matches the main change. The PR changes default-initialized Arg so it matches default(T) and returns empty captures, which addresses the linked issue …
Linked Issues check Passed Issue #7441 requires that default argument matchers are safe on setup and verification paths, and that regression coverage exists for methods without generated predicate overloads (five parameters). A…
Out of Scope Changes check Passed The whole-PR diff against the merge base 6bc28cd touches two files. src/TUnit.Mocks/Arguments/ArgOfT.cs changes the default-Arg matcher, capture accessors, and XML documentation that describes the de…
Docstring Coverage Passed No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Full details: Title check

Explanation

The title "Fix default mock argument matching" matches the main change. The PR changes default-initialized Arg<T> so it matches default(T) and returns empty captures, which addresses the linked issue about default arguments not matching null. The title is short, specific, and states the primary fix.

Full details: Linked Issues check

Explanation

Issue #7441 requires that default argument matchers are safe on setup and verification paths, and that regression coverage exists for methods without generated predicate overloads (five parameters). At the reviewed head, Arg&lt;T&gt;.Matcher returns _capturingMatcher ?? DefaultMatcher.Instance (src/TUnit.Mocks/Arguments/ArgOfT.cs). DefaultMatcher.Instance is new ExactMatcher&lt;T&gt;(default). ExactMatcher&lt;T&gt;.Matches uses EqualityComparer&lt;T&gt;.Default, so a default string? matches null, a default int matches 0, and a default int? matches null. The fallback is stateless, which avoids shared captures. Values and Latest also handle the missing capture matcher without throwing. Regression tests in tests/TUnit.Mocks.Tests/NullLiteralMatcherTests.cs add the five-parameter INullLiteralService.Five method. They cover null, zero, nullable, and mismatch cases, and verify call counts through the generated method. The default-Arg capture-state test checks empty Values, Latest equal to 0, and matcher behavior. The source change addresses the issue diagnosis. I did not execute the test suite. The test result rests on the PR description and on source inspection.

Full details: Out of Scope Changes check

Explanation

The whole-PR diff against the merge base 6bc28cd touches two files. src/TUnit.Mocks/Arguments/ArgOfT.cs changes the default-Arg matcher, capture accessors, and XML documentation that describes the default behavior. tests/TUnit.Mocks.Tests/NullLiteralMatcherTests.cs adds the five-parameter test interface member and regression tests. Each change maps directly to #7441. The PR states that generator output and public API signatures are unchanged. The diff shows no unrelated changes.

Full details: Docstring Coverage

Explanation

No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 2 files.

Finishing Touches

Generate docstrings

  • Commit to this branch
  • Create a new PR
Generate unit tests (beta)

  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

A rabbit checks a default value with care
No capture waits, yet matching works there
Zero is found, while one is not
Five arguments follow the values set
A quiet test confirms the lot

Recent review info

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: ec570a38-7d3f-4fab-8b1b-220a65ed6100
Commits

Reviewing files that changed from the base of the PR and between 6bc28cd and a2c008f.

Files selected for processing: 2

  • src/TUnit.Mocks/Arguments/ArgOfT.cs
  • tests/TUnit.Mocks.Tests/NullLiteralMatcherTests.cs

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 1 remain after this review.

@thomhurst

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 5, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-05T20:07:29.334199Z a2c008f Draft marked ready
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions

github-actions Bot commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

Review

No blocking issues. The change is small and correct.

  • Arg<T> now falls back to a cached, stateless ExactMatcher<T>(default) when _capturingMatcher is null. That fixes setup and verification for five-parameter methods, which have no predicate overloads.
  • The shared matcher is held in a nested static class inside the generic struct, so there is one instance per T. It carries no capture state, so unrelated default arguments cannot interfere with each other.
  • Values and Latest now return empty results on a default Arg<T> instead of throwing a NullReferenceException.
  • The ExactMatcher constructor's Arg<>-boxing guard does not fire for default, since the value is null or default(T).
  • The tests cover null, zero, nullable values, mismatches, and verification through a five-parameter method, plus the empty capture state.

Minor, non-blocking: a default Arg<T> silently exposes empty Values, so a user who writes Arg<int> a = default; and later reads a.Values gets an empty list with no hint about why. The XML doc now says this, which is enough.

Approve from my side.

I did not run the tests. I only read the diff and ExactMatcher.

@greptile-apps

greptile-apps Bot commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] This PR appears safe to merge.

Summary

Default-initialized Arg<T> values now match default(T) through a cached matcher without capturing values.

  • Default arguments match only their type’s default value.

Reviews (1) · Last reviewed commit: "Fix default mock argument matching (#744..." · Reviewed by Greptile

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. More of your lovely PRs please.

Reviewed commit: a2c008f1f6

ℹ️ 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".

@thomhurst
thomhurst marked this pull request as ready for review November 5, 2026 20:06
@thomhurst

Copy link
Copy Markdown
Owner Author

Marked ready at unchanged head a2c008f after positive current-head Claude and Codex verdicts; all review surfaces are clear. Claude ready-for-review run 42100184523 is queued. Auto-merge must wait until that run passes and its new comment is read, per repository merge guidance. CI also starts on promotion.

@github-actions

github-actions Bot commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

Review

The fix is small and correct. A default Arg<T> now falls back to a cached ExactMatcher<T>(default). Its Values and Latest return empty or default values instead of throwing a NullReferenceException.

  • The nested static class holds one matcher per closed T. It has no mutable state, so default args can't share captures. The ExactMatcher constructor's boxed-Arg guard doesn't trigger for default.
  • The tests cover null, zero, nullable, mismatch and verification through a five-parameter method, plus the empty capture state.

Optional points:

  • A user who reads .Values on a default Arg gets a silently empty list, which could hide a mistake. The XML doc now says this, which is enough.
  • Array.Empty<T?>() is allocation-free. Good.

No blocking issues. I haven't run the tests myself.

This branch was previously deployed

1 inactive deployment
Pull Requests — a2c008f1 Deployed Nov 5, 2026 by thomhurst via modularpipeline (ubuntu-latest) #21510
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.

Default argument matcher has no matcher for generated mock methods with five parameters

1 participant