Skip to content

fix(mocks): emit the setup surface into the globally-imported namespace - #6504

Merged
thomhurst merged 3 commits into
mainfrom
fix/6494-member-extensions-namespace
Jul 28, 2026
Merged

fix(mocks): emit the setup surface into the globally-imported namespace#6504
thomhurst merged 3 commits into
mainfrom
fix/6494-member-extensions-namespace

Conversation

@thomhurst

Copy link
Copy Markdown
Owner

Fixes #6494

Problem

error CS1061: 'IClusterMock' does not contain a definition for 'ConnectAsync', and no accessible
extension method 'ConnectAsync' accepting a first argument of type 'IClusterMock' could be found

The report attributed this to a name collision between Cassandra.ICluster and Couchbase.ICluster. It isn't — it reproduces with a single mock and no collision:

namespace Some.Library { public interface ICluster { string Bucket { get; } } }

// no `using Some.Library;`
var b = Some.Library.ICluster.Mock();   // binds — Mock() lives in namespace TUnit.Mocks
b.Bucket.Returns("bucket");             // CS1061

The _MockMemberExtensions / _MockEventsExtensions classes were emitted beside the mocked type. Extension members are only found when their containing namespace is imported, and the Mock() entry point is in TUnit.Mocks (a global using) so the call site compiles — leaving a mock whose entire setup/verify surface is invisible. The resulting error reads as "this member isn't supported", which is what cost the reporter debugging time.

Fix

Emit the extension classes, the call wrappers they return, and the Events surface into TUnit.Mocks.Generated — added as a global using by TUnit.Mocks.targets. Those type names are already derived from the fully qualified type name, so two mocked types sharing a short name across namespaces remain distinct in the flat namespace.

Unchanged: the impl, factory, bridge and IFooMock wrapper types keep their current placement beside the mocked type, as do the out/ref setter delegates (named from the short name and referenced by the impl through GetGlobalMockNamespacePrefix), which now emit in their own namespace block.

Docs note added for projects that set TUnitMockImplicitUsings=disable.

Tests

tests/TUnit.Mocks.Tests/Issue6494Tests.cs — member setup, verification and event raising against interfaces in namespaces the test deliberately does not import, including the reported two-ICluster shape.

TUnit.Mocks.Tests 1164 passed. Generator snapshots regenerated (59 files — namespace wrapper and re-indentation only) and passing on net8.0/net9.0/net10.0. Analyzers 30, Http 54, Logging 31.

// The setup/verify surface goes in the globally-imported namespace rather than beside the
// mocked type — an extension member is invisible unless its namespace is imported, which
// silently hid every setup for types from a namespace the test hadn't `using`'d (#6494).
using (writer.OptionalNamespaceBlock(MockImplBuilder.MemberSurfaceNamespace))

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Flat namespace exposes name collisions

When two mocked types have fully qualified names that sanitize identically, such as A_B.IFoo and A.B.IFoo, both surfaces are now emitted into TUnit.Mocks.Generated with the same generated class and wrapper names, causing duplicate-type compilation errors. The original namespaces previously kept these declarations separate, so the shared namespace needs a collision-resistant type identity.

@greptile-apps

greptile-apps Bot commented Jul 28, 2026

Copy link
Copy Markdown

Greptile Summary

The PR makes generated mock setup, verification, and event extensions globally discoverable.

  • Moves member extension classes and their call wrappers into TUnit.Mocks.Generated.
  • Moves generated event accessors into the same globally imported namespace.
  • Keeps specialized out/ref setter delegates beside the mocked type.
  • Adds regression coverage, refreshed generator snapshots, and documentation for disabled implicit usings.

Confidence Score: 3/5

The PR is not yet safe to merge because collision-equivalent fully qualified type names can still make generated mock surfaces fail compilation.

The setup and event surfaces now share one namespace, while their declaration names still use a lossy sanitizer that maps distinct qualified names such as A_B.IFoo and A.B.IFoo to the same identifier, producing duplicate generated declarations when both types are mocked.

Files Needing Attention: src/TUnit.Mocks.SourceGenerator/Builders/MockImplBuilder.cs, src/TUnit.Mocks.SourceGenerator/Builders/MockMembersBuilder.cs, src/TUnit.Mocks.SourceGenerator/Builders/MockEventsBuilder.cs

Important Files Changed

Filename Overview
src/TUnit.Mocks.SourceGenerator/Builders/MockMembersBuilder.cs Relocates setup and verification extensions and wrappers into a shared namespace, where the previously reported collision-prone generated names remain unchanged.
src/TUnit.Mocks.SourceGenerator/Builders/MockEventsBuilder.cs Relocates event accessors into the shared generated namespace using the same non-injective sanitized type identity.
src/TUnit.Mocks.SourceGenerator/Builders/MockImplBuilder.cs Introduces the common member-surface namespace but does not add collision-resistant generated identities.
tests/TUnit.Mocks.Tests/Issue6494Tests.cs Covers globally discoverable setup, verification, and events for ordinary same-short-name interfaces, but not fully qualified names that sanitize identically.
docs/docs/writing-tests/mocking/index.md Documents the generated namespace and the explicit using required when mock implicit usings are disabled.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
    A[Mocked type] --> B[Source generator]
    B --> C[Implementation and factory beside mocked type]
    B --> D[Setup and verification surface]
    B --> E[Event surface]
    D --> F[TUnit.Mocks.Generated]
    E --> F
    F --> G[Globally imported at call site]
Loading

Reviews (3): Last reviewed commit: "test(mocks): refresh snapshots after reb..." | Re-trigger Greptile

@claude

claude Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Code review

Reviewed the source generator changes (MockImplBuilder.cs, MockMembersBuilder.cs, MockEventsBuilder.cs), the new Issue6494Tests.cs, and the regenerated snapshots.

Verified correctness of the core fix:

  • The setup/verify extension classes and their call-wrapper types now emit into the flat TUnit.Mocks.Generated namespace (MemberSurfaceNamespace) instead of beside the mocked type, and TUnit.Mocks.targets confirms TUnit.Mocks.Generated is already a global using (enabled by default via TUnit.Mocks.props), so this actually fixes the invisible-extension-member bug described in [TUnit.Mocks] Generated mock wrapper type names collide across namespaces (e.g. two different ICluster interfaces) - one silently loses its members #6494.
  • Name collisions across namespaces are avoided because GetCompositeSafeName derives the generated type name from the fully qualified type name, not the short name — confirmed by reading the implementation.
  • The out/ref non-span ref-struct setter delegates correctly stay in the per-type mockNamespace (via the new EmitOutRefSetterDelegateNamespace), matching the doc comment's rationale that the impl references them through GetGlobalMockNamespacePrefix. The extracted WrappedMethods filter and the pre-check in EmitOutRefSetterDelegateNamespace are logically equivalent to the original inline guard in EmitNonSpanRefStructSetterDelegates, so this is a safe, behavior-preserving refactor.
  • Snapshot diffs (e.g. Conflict_With_Existing_Type_Falls_Back_To_Generated_Namespace, Interface_With_Out_Ref_Parameters) match the intended change exactly — namespace wrapping only, no logic drift.
  • The other builders (impl, factory, bridge, wrapper) were correctly left untouched, consistent with the PR description.

No high-signal bugs or CLAUDE.md violations found. The regression tests in Issue6494Tests.cs directly exercise the reported scenario (single mock, no namespace collision) plus verification and events, which is good coverage for a bug that was easy to miss silently.

Nit (non-blocking): the non-span ref-struct out/ref delegate emission path (EmitOutRefSetterDelegateNamespace/EmitNonSpanRefStructSetterDelegates) doesn't appear to be covered by any existing snapshot test — none of the snapshots contain a delegate keyword. This is a pre-existing gap (the function predates this PR), not something introduced here, so not blocking.

@thomhurst

Copy link
Copy Markdown
Owner Author

Addressed the snapshot-coverage nit in 1fbc04d. Since the delegate emission is the exact path this PR split out of the members loop, it was worth closing rather than deferring: Interface_With_NonSpan_RefStruct_Out_Ref_Parameters pins both halves of the split — the OutSetter/RefSetter delegates staying in the mocked type's namespace (where the impl references them by short name) and the setup surface landing in TUnit.Mocks.Generated. Snapshots now 81, green on net8.0/net9.0/net10.0.

@claude

claude Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Code review (follow-up)

Reviewed the changes since my previous review — commit 1fbc04d, which addresses the snapshot-coverage nit I flagged.

Verified:

  • Interface_With_NonSpan_RefStruct_Out_Ref_Parameters in MockGeneratorTests.cs exercises exactly the construct that was previously uncovered: an interface with out/ref parameters of a non-span ref struct type.
  • The new snapshot (Interface_With_NonSpan_RefStruct_Out_Ref_Parameters.verified.txt) confirms both halves of the split emit correctly:
  • No .received.txt files were committed, consistent with the snapshot-testing rule in CLAUDE.md.
  • No test name collisions or other stray changes in the diff.

No new issues found — this closes out the nit from my earlier review. Nice fix.

@thomhurst

Copy link
Copy Markdown
Owner Author

On the flat-namespace collision finding (A_B.IFoo vs A.B.IFoo): the collision is real but pre-existing, and not a regression from this PR.

SanitizeIdentifier isn't injective, and its output is used for the AddSource hint names — so the two types collide at the hint-name level before either surface is emitted. Generation dies the same way on main as on this branch, and it dies silently:

PROBE: generator produced 1 files, no exception       // only the post-init TUnit.Mocks.Generated file

Mocking either type alone yields 5 files. Same result on main and on this branch, so moving the surface into TUnit.Mocks.Generated doesn't change it — the duplicate-type error the finding predicts can't happen, because the second file is never added.

Filed as #6505 rather than folded in here: the fix is to make the identifier mapping injective (escape _ before replacing separators), which renames generated types and files across the board and needs a full snapshot refresh — not something to bury in a namespace-placement fix. The silent-failure aspect is the more interesting half of that issue.

Setup, verify and event extensions were generated beside the mocked type.
An extension member is only visible when its namespace is imported, so
mocking a type whose namespace the test had not `using`'d compiled the
Mock() call — that entry point already lives in TUnit.Mocks — but hid
every member setup behind CS1061, which reads as "this member isn't
supported". Reported as a name collision between two same-named ICluster
interfaces; it reproduces with a single mock and no collision involved.

Emit the extension classes and the call wrappers they return into
TUnit.Mocks.Generated, which TUnit.Mocks.targets adds as a global using.
Their names are derived from the fully qualified type name, so types
sharing a short name across namespaces stay distinct. The impl, factory
and wrapper types keep their current placement, as do the out/ref setter
delegates the impl references by short name.

Fixes #6494
Review nit: no snapshot covered the one construct that emits a
namespace-scoped delegate, which is precisely the path this PR split out
of the members loop. The new fixture pins both halves — the delegates stay
in the mocked type's namespace (the impl references them by short name)
while the setup surface moves to TUnit.Mocks.Generated.
…factories

Rebase conflict resolution: #6503 added the Func<Task<T>> Returns
overloads to the same generated call-wrapper types this branch moved
into TUnit.Mocks.Generated. Regenerated rather than hand-merged.
@thomhurst
thomhurst force-pushed the fix/6494-member-extensions-namespace branch from 1fbc04d to 426c1b1 Compare July 28, 2026 11:08
@thomhurst

Copy link
Copy Markdown
Owner Author

Rebased onto main to clear the conflict. All 8 conflicts were generated-code snapshots: #6503 (async Returns factories) added overloads to the same call-wrapper types this branch moves into TUnit.Mocks.Generated, so the two edits collided inside the same generated blocks.

Regenerated rather than hand-merged them (426c1b1) — the diff against the pre-rebase snapshots is additions-only and entirely #6503's Func<Task<T>>/Func<ValueTask<T>> Returns overloads, now emitted inside the relocated types. Snapshot suite 85/85 and TUnit.Mocks.Tests 1182/1182 green locally on net10.0.

@claude

claude Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Code review (follow-up)

Reviewed the changes since my last review — commit 426c1b1, rebasing onto main to pick up #6503's async Returns overloads.

Verified:

  • The rebase/regeneration commit is additions-only: git show 426c1b152 --stat shows 8 snapshot files with only + lines (135 insertions, 0 deletions). Spot-checked Interface_With_Async_Methods.verified.txt — the new Func<Task<T>>/Func<ValueTask<T>> Returns overloads from feat(mocks): accept an async factory in Returns() on async members #6503 land correctly inside the call-wrapper types this PR relocated into TUnit.Mocks.Generated, gated behind #if NET9_0_OR_GREATER as expected. No logic drift from the namespace-relocation fix.
  • No .received.txt files committed, consistent with the CLAUDE.md snapshot-testing rule.
  • Greptile's collision finding (A_B.IFoo vs A.B.IFoo sanitizing to the same identifier) was investigated and correctly scoped out: it's a pre-existing, silent generator failure on main too (confirmed via the PR thread's probe showing generation dies the same way before either surface is emitted), not a regression from this PR. Filed separately as [TUnit.Mocks] Two mocked types whose sanitized names collide silently generate nothing (e.g. A_B.IFoo and A.B.IFoo) #6505 rather than folded in here, which is the right call — fixing it requires an injective identifier mapping that renames generated types/files repo-wide and needs its own full snapshot refresh.
  • Previously flagged nit (missing snapshot coverage for the non-span ref-struct out/ref delegate path) was addressed in 1fbc04d and verified in my prior comment.

No new issues found. This PR is in good shape.

This was referenced Jul 28, 2026
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.

[TUnit.Mocks] Generated mock wrapper type names collide across namespaces (e.g. two different ICluster interfaces) - one silently loses its members

1 participant