fix(mocks): generate mock for qualified-name X.Mock() calls (#6298)

[thomhurst]

Fixes #6298.

Problem

Invoking .Mock() through a fully-qualified type reference didn't generate the mock:

var mock = Namespace.ITestInterface.Mock();
mock.Method().Returns("value"); // error CS1061: Mock<ITestInterface> does not contain a definition for 'Method'

A simple-name reference (ITestInterface.Mock()) or a using alias worked — and once anything triggered generation, the qualified-name call worked too.

Root cause

MockTypeDiscovery.IsMockExtensionInvocation gated the left-hand side of X.Mock() on syntax node kind, accepting IdentifierNameSyntax, GenericNameSyntax, QualifiedNameSyntax, and AliasQualifiedNameSyntax.

In expression position a dotted name like Namespace.ITestInterface parses as a MemberAccessExpressionSyntax, not a QualifiedNameSyntax (that node only appears in type/using contexts). So the qualified form fell through the gate and the mock surface was never generated.

Fix

Drive discovery off the resolved symbol rather than the syntactic shape. The predicate now matches any <expr>.Mock(), and the existing semantic transform (TransformMockExtensionInvocation) already bails unless the LHS resolves to an INamedTypeSymbol — which naturally filters out variables, fields, and namespaces. Loosening the cheap syntax gate is therefore safe.

Tests

• Snapshot: MockGeneratorTests.Static_Extension_Discovery_Via_Qualified_Name
• Runtime regression: TUnit.Mocks.Tests/Issue6298Tests.cs (mirrors the issue repro)

All snapshot (69) and TUnit.Mocks.Tests (1158) tests pass locally on net10.0.

[claude]

Code Review

Summary

Clean, well-motivated fix for a genuine Roslyn quirk. The root cause analysis is accurate and the approach is sound. A few observations worth considering.

---

The Core Fix — MockTypeDiscovery.IsMockExtensionInvocation

What changed: The syntax predicate went from a tight whitelist of LHS syntax shapes to a pure name-gate (any <expr>.Mock() where the callee is literally "Mock"), deferring all symbol-level filtering to TransformMockExtensionInvocation.

Why this is correct: In C#'s syntax tree, a dotted name in expression position (Namespace.IFoo.Mock()) parses as nested MemberAccessExpressionSyntax nodes, not QualifiedNameSyntax (which only appears in type/using contexts). The old LHS whitelist couldn't reach this form. Driving discovery off the resolved symbol — which TransformMockExtensionInvocation already does — is the right call.

Tradeoff to be aware of: The predicate now passes any <expr>.Mock() through to the semantic phase, including:

• someVariable.Mock() (instance method named Mock)
• myMockerFactory.Mock() (non-type LHS)
• Third-party types that happen to have a Mock() method

The semantic transform correctly rejects all of these (non-INamedTypeSymbol, sealed, value type, etc.), so correctness is not at risk. The cost is that more nodes flow into the semantic model lookup, which is heavier than a pure syntax check. In large test suites with many .Mock() calls on variables this could add up, but it is a realistic tradeoff and the XML <remarks> block explains it well. If perf ever becomes a concern, one possible middle ground would be to also gate on the argument list being empty or containing a single MockBehavior argument at the syntax level — but that is an optimisation, not a correctness concern, and would add complexity now.

---

Tests

Snapshot test (Static_Extension_Discovery_Via_Qualified_Name) — this is the definitive regression test. No using directive, so the only thing that can trigger generation is the MyNamespace.ITestInterface.Mock() call. Solid.

Runtime test (Issue6298Tests.cs) — works correctly but has a minor clarity issue: using Issue6298Namespace; is imported at the top of the file, even though it is never used unqualified. The using does not trigger generation (only .Mock() calls do), and the test exclusively uses the qualified form, so the test is valid. But a reader might wonder "does the using here cause the old code to accidentally pass?" A comment clarifying that the using is intentionally unused (or removing it if not needed for compilation) would reduce that confusion.

Missing edge case: IFoo<T>.Mock() with a qualified namespace prefix — MyNamespace.IFoo<T>.Mock(). The new predicate matches Name: IdentifierNameSyntax { Identifier.ValueText: "Mock" }, so the outer member access still has Name = IdentifierNameSyntax("Mock") with a generic type as the expression. This should work, but there is no snapshot test covering the qualified-generic form. Worth adding Static_Extension_Discovery_Via_Qualified_Generic_Name to lock it in.

---

Minor Nits

• The old inline comment // Static extension calls use a type name on the left — never a variable or member access. was correctly removed since it no longer reflects the behaviour.
• The <remarks> block in the XML doc accurately captures the reasoning. Good.
• The verified snapshot file does not end with a newline (No newline at end of file) — might be intentional given other snapshots in this project, but worth a consistency check.

---

Verdict

The fix is correct and the approach is well-reasoned. The main actionable items are:

1. (Optional) Add a test for the qualified-generic form MyNamespace.IFoo<T>.Mock() to lock in that case.
2. (Optional) Clarify or remove the unused using Issue6298Namespace; in Issue6298Tests.cs.

Neither blocks merging — this is ready to go.

[codacy-production]

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 0 complexity

Metric	Results
Complexity	0

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

This PR fixes issue #6298 by driving mock type discovery off the resolved symbol rather than the syntactic shape of the LHS in T.Mock() calls. The root cause was well-diagnosed: a fully-qualified reference like Namespace.IFoo.Mock() parses as MemberAccessExpressionSyntax in expression position — not QualifiedNameSyntax — so it fell through the old syntax gate. The fix is correct, well-documented, and includes both a snapshot test and a runtime regression test.

---

Finding 1 — Runtime regression test does not exercise the fix (CONFIRMED)

TUnit.Mocks.Tests/Issue6298Tests.cs, line 1 / line 23

using Issue6298Namespace;   // ← this line weakens the test

var mock = Issue6298Namespace.IIssue6298Interface.Mock();  // line 23

The using Issue6298Namespace; directive at the top of the file means IIssue6298Interface is also discoverable via simple name. If the fix were reverted, the old predicate would still find the type through the using-imported simple name and generate the mock — the qualified-name call on line 23 would compile regardless. The runtime test would pass even without the fix, making it a false regression guard.

The real guard is the snapshot test in MockGeneratorTests.cs (Static_Extension_Discovery_Via_Qualified_Name), whose source string has using TUnit.Mocks; but no using MyNamespace;, correctly forcing discovery to happen only through the qualified-name path.

Suggested fix: Remove the using Issue6298Namespace; directive from Issue6298Tests.cs and rely on the fully-qualified form throughout. Alternatively, add a comment noting that the snapshot test is the authoritative regression guard.

---

Finding 2 — Mock(behavior) overload not covered by the regression test (PLAUSIBLE)

TUnit.Mocks.Tests/Issue6298Tests.cs, line 23

The runtime test only calls the zero-argument form IIssue6298Interface.Mock(). The generator emits both Mock() and Mock(MockBehavior behavior) overloads. Adding one call like:

var strictMock = Issue6298Namespace.IIssue6298Interface.Mock(MockBehavior.Strict);

would ensure both overloads are reachable in the regression scenario. Low priority given the snapshot test covers both, but would make the runtime test more complete.

---

Finding 3 — Broadened syntax predicate increases semantic analysis work (PLAUSIBLE)

TUnit.Mocks.SourceGenerator/Discovery/MockTypeDiscovery.cs, ~line 559

IsMockExtensionInvocation is the CreateSyntaxProvider predicate — it runs on every syntax node on every keystroke in the IDE. With the old guard, someVar.Mock() was rejected in ~5 ns with no allocations. Now every x.Mock() call (including Moq/NSubstitute/domain method calls on instances) passes through to TransformMockExtensionInvocation, which calls SemanticModel.GetSymbolInfo. The transform's early-exit guard (leftSymbol.Symbol is not INamedTypeSymbol) limits actual work for non-type expressions, and Roslyn caches semantic queries within a pass — so the impact is mitigated. This is plausible rather than confirmed, and for most codebases Mock is not a ubiquitous method name. Worth monitoring if performance complaints arise in large solutions.

---

Summary

The fix is architecturally sound and the approach of delegating type-vs-non-type discrimination to semantic analysis is the correct one — it's immune to future syntax-shape edge cases. The main actionable item is Finding 1: the runtime regression test inadvertently doesn't guard the thing it's supposed to guard. The snapshot test is the real protection; the runtime test needs its using removed to pull its weight.

[thomhurst]

Rebased onto latest main (clean) and investigated the review finding about the using Issue6298Namespace; directive in Issue6298Tests.cs.

Removing the using does not compile — the generated mock's setup/proxy members are emitted into the interface's own namespace, so the using is genuinely required for the test to build (CS1061 otherwise). Per the reviewer's fallback suggestion, kept the using and added a comment documenting that it's intentional/required, clarifying that the thing actually under test is the fully-qualified Issue6298Namespace.IIssue6298Interface.Mock() call form. The snapshot test Static_Extension_Discovery_Via_Qualified_Name (no using) remains the authoritative regression guard.

All green on net8/9/10.

[claude]

Follow-up Review

This is a follow-up to my two previous reviews on this PR.

Previous Findings — Status

Finding 1 (runtime test weakened by using) — RESOLVED

The author has added a detailed header comment to Issue6298Tests.cs explaining why using Issue6298Namespace; is present and why it does not weaken the regression. That explanation is correct: the using directive is a semantic construct — it affects name resolution, not the syntax tree. The call Issue6298Namespace.IIssue6298Interface.Mock() still parses as a nested MemberAccessExpressionSyntax regardless of what namespaces are imported. The old predicate would still reject it, compilation of the test would fail (no extension method found), and the regression would be caught. The using is needed purely so that the generated MockMethodCall extension — emitted into Issue6298Namespace — is accessible for mock.Method().Returns(...). The comment makes this clear; good resolution.

Finding 2 (Mock(behavior) overload not covered in runtime test) — STILL OPEN, low priority

Still only the zero-argument Mock() overload is exercised at runtime. The snapshot test covers both overloads in the generated output, so this is non-critical.

Finding 3 (broader syntax predicate increases semantic work) — ACCEPTED TRADEOFF

The <remarks> block documents this explicitly. No further action needed.

Snapshot file trailing newline — STILL OPEN, minor

The new .verified.txt still ends without a trailing newline (namespace TUnit.Mocks.Generated; with no EOL). If other snapshots in the project end with a newline, this is an inconsistency. Not a blocker.

---

Overall

The fix is correct, the explanation added in response to the previous review is accurate, and the regression is properly guarded by the snapshot test. Ready to merge.