Generated mocks live in the same namespace as the mocked type

[thomhurst]

Summary

• Generated mock types ({Type}Mock, {Type}Mockable, {Type}MockFactory, {Type}_MockStaticExtension, {Type}_MockMemberExtensions, plus events extensions) now emit into the same namespace as the mocked type instead of TUnit.Mocks.Generated.{Namespace}.
• When a user-declared type in the target namespace would collide with one of the generator's emitted names, generation falls back automatically to TUnit.Mocks.Generated[.{ns}] — the existing scheme — so mocks never overwrite user code.
• Drops a long-standing inconsistency where MockEventsBuilder and MockMembersBuilder always emitted into TUnit.Mocks.Generated, regardless of the mocked type's namespace.
• Adds MockNamespaceConflictDetector which walks the consumer's compilation symbol table during the semantic transform phase. Cost is negligible — runs once per MockTypeModel, not per syntax node.
• Cross-assembly contract: types from referenced assemblies are checked against the consumer's namespace contents, not the foreign assembly's, so collisions in the user's project are correctly detected.

Breaking change

Code that imports a generated sub-namespace such as using TUnit.Mocks.Generated.MyApp.Services; will no longer compile, because mocks now live in MyApp.Services directly. The fix is to drop the qualified using (the file's own namespace usually already imports it). The post-init namespace TUnit.Mocks.Generated; safety stub is preserved, so plain using TUnit.Mocks.Generated; continues to compile.

If a future framework version introduces a type whose name collides with one of the generator's suffixes, the source generator automatically detects the new collision on the next build and flips the affected mocks back to the TUnit.Mocks.Generated.* namespace — no manual migration required.

Test plan

• TUnit.Mocks.SourceGenerator.Tests — 174/174 pass across net8.0, net9.0, net10.0 (snapshots refreshed for the new namespace placement).
• TUnit.Mocks.Tests — 968/968 pass on net10.0 (one stale using TUnit.Mocks.Generated.TUnit.Mocks.Tests; removed).
• TUnit.Mocks.Analyzers.Tests — 30/30 pass on net10.0.
• New Global_Namespace_Mock_Does_Not_Emit_Empty_Namespace_Prefix regression test asserts the generator does not emit invalid global::.TypeName syntax for globally-namespaced mocks.
• New Conflict_With_Existing_Type_Falls_Back_To_Generated_Namespace snapshot exercises the fallback path.

Notes

• No engine/runtime changes; source-gen-only behaviour. AOT and dual-mode rules satisfied.
• Mocks for external-assembly types (e.g. System.IO.Stream) emit into the original namespace (StreamMock lives in System.IO). Legal C# and uniform with user-authored types; conflict detection on the next build automatically reroutes if a collision later appears.

[codacy-production]

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 30 complexity

Metric	Results
Complexity	30

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 is a well-structured PR. The core design — capturing UseFallbackNamespace at transform time and carrying it through the model — is the right approach for an incremental source generator. The new abstractions (OptionalNamespaceBlock, GetGlobalMockNamespacePrefix, MockNamespaceConflictDetector) are clean and focused. The global-namespace regression guard test is particularly thoughtful given Roslyn's inability to parse C# 14 extension(...) blocks.

---

Issue 1: Duplicate conflict detection in TypeSymbolExtensions.GetGeneratedMockNamespace (architecture concern)

GetGeneratedMockNamespace recomputes the conflict independently of the UseFallbackNamespace flag already on the model:

// TypeSymbolExtensions.cs — recomputes from scratch
public static string GetGeneratedMockNamespace(this INamedTypeSymbol type, Compilation compilation)
{
    var hasEvents = type.GetMembers().OfType<IEventSymbol>().Any(); // all events
    var fallback = MockNamespaceConflictDetector.HasConflict(compilation, type, hasEvents);
    ...
}

This diverges from the model's decision in BuildSingleTypeModel:

UseFallbackNamespace = MockNamespaceConflictDetector.HasConflict(
    compilation, namedType, hasEvents: events.Length > 0) // filtered events

The hasEvents inputs differ: the extension method uses raw symbol members (all events), while the model uses filtered/discovered events (e.g. inaccessible events from external assemblies could be excluded). In edge cases — mocking an external type that has internal events — these two paths could reach different conclusions and produce mismatched FQNs.

Better approach: GetAutoMockFactoryMethod is called while building member models, which occurs before the model is finalized, so passing UseFallbackNamespace directly isn't possible there. However, the method could accept the fallback bool as a parameter derived from the same events.Length > 0 computation, eliminating the re-derivation:

// MemberDiscovery.cs
private static string? GetAutoMockFactoryMethod(ITypeSymbol returnType, Compilation compilation, bool useFallback)

This keeps the computation in one place and makes the dependency explicit.

---

Issue 2: Framework namespace pollution (design concern)

From the PR description:

Mocks for external-assembly types (e.g. System.IO.Stream) now emit into the original namespace (StreamMock lives in System.IO). Legal C# and uniform with user-authored types.

While legal, placing generated types into System.IO (or any BCL namespace the user doesn't own) is surprising. IDE intellisense will show System.IO.StreamMock alongside System.IO.Stream; some analyzers and tools treat System.* namespaces specially. If a future .NET release adds a System.IO.StreamMock type, consumers get a compile error with no obvious cause.

Consider always using the fallback namespace for types from external assemblies not under a user-controlled root. This could be a simple guard in MockNamespaceConflictDetector.HasConflict or a separate check in BuildSingleTypeModel:

// Always fall back for types from referenced assemblies
if (!SymbolEqualityComparer.Default.Equals(namedType.ContainingAssembly, compilationAssembly))
    return true; // use fallback

This is also more conservative: if a user explicitly wants to mock IDisposable, the generated IDisposableMock living in System is confusing rather than helpful.

---

Issue 3: Breaking change scope — sub-namespace using directives

The safety stub preserves TUnit.Mocks.Generated so using TUnit.Mocks.Generated; keeps compiling. But users with using TUnit.Mocks.Generated.MyApp.Services; will get a compile error after this change, because those mocks now live in MyApp.Services instead.

This is a real breaking change for any consumer who had a qualified using for a generated namespace. It should be called out more explicitly in the PR description and ideally surfaced in migration documentation or a changelog entry.

---

Minor: ResolveNamespaceInCompilation linear scan per segment

The namespace resolution walks GetNamespaceMembers() with a linear scan per dotted segment. For deep namespaces with many siblings this is O(depth × breadth). This is unlikely to be a bottleneck in practice, but compilation.GetTypeByMetadataName can't help here; the existing approach is fine for now — just noting it for future profiling if conflict detection shows up in generator timing.

---

What works well

• OptionalNamespaceBlock with the no-op disposable is a clean abstraction that removes the if (ns != "") scatter from every builder.
• GetGlobalMockNamespacePrefix correctly consolidates the guard that caused the global::.TypeName bug.
• MockNamespaceConflictDetector is isolated, well-documented, and uses the consumer compilation for cross-assembly checks — the remarks section correctly explains why.
• UseFallbackNamespace is included in Equals/GetHashCode, which is required for Roslyn's incremental generator caching to work correctly.
• Test coverage is solid: global namespace, nested namespace, generics, events-conditional flag, and cross-assembly absence all covered.
• The safety stub backward-compat preservation is a considerate touch.

[claude]

Follow-up Review (round 2)

This review checks whether the three issues raised in round 1 have been addressed.

---

Issue 1: Duplicate conflict detection — still open

The root concern from round 1 remains unchanged. TypeSymbolExtensions.GetGeneratedMockNamespace (called from MemberDiscovery.GetAutoMockFactoryMethod at line 813) independently re-derives the fallback decision using type.GetMembers() to find events:

// TypeSymbolExtensions.cs L90-104
foreach (var member in type.GetMembers())   // all symbol members
{
    if (member is IEventSymbol) { hasEvents = true; break; }
}
var fallback = MockNamespaceConflictDetector.HasConflict(compilation, type, hasEvents);

This diverges from the model path in BuildSingleTypeModel:

// MockTypeDiscovery.cs L387-388
UseFallbackNamespace = MockNamespaceConflictDetector.HasConflict(
    compilation, namedType, hasEvents: events.Length > 0)   // filtered/discovered events

The GetMembers() path returns all raw symbol members, including inaccessible events from external assemblies (e.g. internal events not visible to the consumer assembly) and events on inherited interfaces. The events.Length path uses the result of MemberDiscovery.DiscoverMembers, which applies accessibility and deduplication filters. For a type from a referenced assembly that has internal events, these two paths can reach different conclusions, causing GetAutoMockFactoryMethod to emit an AutoMockFactoryMethod reference in one namespace while the factory itself is generated in the other.

Suggested fix (as proposed in round 1): accept useFallback as a parameter instead of recomputing it:

// MemberDiscovery.cs
private static string? GetAutoMockFactoryMethod(
    ITypeSymbol returnType, Compilation compilation, bool useFallback)
{
    // ... existing guards ...
    var factoryNamespace = MockImplBuilder.SelectMockNamespace(namespaceName, useFallback);
    var globalPrefix = MockImplBuilder.ToGlobalPrefix(factoryNamespace);
    // ...
}

Then GetGeneratedMockNamespace in TypeSymbolExtensions could be removed entirely — it is only called from GetAutoMockFactoryMethod, so eliminating the re-derivation also eliminates the divergence surface.

---

Issue 2: Framework namespace pollution — still open

The PR description states: "Mocks for external-assembly types (e.g. System.IO.Stream) now emit into the original namespace (StreamMock lives in System.IO)."

The IsFrameworkNamespace guard prevents transitive auto-mocking of BCL interfaces but it does not affect direct mocking. A user calling Mock.Of<System.IO.Stream>() still gets StreamMock placed in System.IO. This is surprising from both a discoverability standpoint (IDE completion in System.IO now shows StreamMock) and a future-compatibility standpoint (a future System.IO.StreamMock in the BCL would cause a compile error with no obvious cause).

The most conservative and correct fix is to always use the fallback namespace when the target type comes from an assembly that is not the consumer's own compilation assembly. This is a one-liner in BuildSingleTypeModel and BuildDelegateTypeModel:

UseFallbackNamespace = !SymbolEqualityComparer.Default.Equals(
    namedType.ContainingAssembly, compilationAssembly)
    || MockNamespaceConflictDetector.HasConflict(compilation, namedType, hasEvents: events.Length > 0)

This is also more consistent with what IsFrameworkNamespace does for transitive types — if a BCL return type is not worth auto-mocking into the target namespace, it is also not worth polluting System.* with user-generated mocks.

---

Issue 3: Breaking change for sub-namespace using directives

The safety stub preserves TUnit.Mocks.Generated (the root), so using TUnit.Mocks.Generated; still compiles. But any consumer that had using TUnit.Mocks.Generated.MyApp.Services; will get a compile error after this change, because mocks that previously lived in TUnit.Mocks.Generated.MyApp.Services now live in MyApp.Services.

This breaking change was noted in round 1 but is still not called out in the PR description or in any migration notes. For a library that auto-generates user-facing code, this is meaningful: it will break CI for anyone who upgrades without auditing their using directives.

Minimum ask: add a note in the PR description / changelog entry documenting the breaking change and what affected consumers need to do (remove the TUnit.Mocks.Generated.{namespace} using directives and replace with the natural namespace).

---

What's good

The core implementation is solid. The snapshot tests are comprehensive, the OptionalNamespaceBlock abstraction is clean, the global-namespace regression test is well-designed, and UseFallbackNamespace correctly participates in Equals/GetHashCode for incremental caching. No regressions were introduced.

The three issues above are the same ones from round 1 and represent genuine correctness risk (issue 1), surprising user-visible behaviour (issue 2), and an undocumented break (issue 3). Issue 3 is documentation-only; issues 1 and 2 are code changes.

[thomhurst]

Thanks for the review @claude. Addressed point-by-point:

1. Divergent conflict detection — fixed in f76553729. The two call sites passed hasEvents differently (MockTypeDiscovery used filtered events from MemberDiscovery, GetGeneratedMockNamespace used raw symbol members), so the edge case you flagged would genuinely produce mismatched FQNs. I dropped the hasEvents parameter from HasConflict entirely — the detector now always checks all seven emitted suffixes (Mock, Mockable, MockFactory, _MockStaticExtension, _MockMemberExtensions, _MockEvents, _MockEventsExtensions). Slightly more conservative (a colliding T_MockEvents flips fallback even when the target has no events), but eliminates the divergence and falling back is always safe. Updated EventsExtensionsCollision_AlwaysFlagged test.

2. Framework namespace pollution — keeping current behaviour. The risk is real but the source generator re-evaluates conflict detection on every build, so if .NET (or any reference) later adds a colliding type the next compile automatically flips the affected mocks to the fallback namespace. The "no obvious cause" failure mode the reviewer worries about would manifest as a normal compile error pointing at the colliding type — same UX as any other namespace conflict. The user explicitly chose this trade-off when designing the change; the auto-recovery property makes the choice safe rather than fragile.

3. Breaking-change scope — accurate but acceptable for the same reason as (2): existing using TUnit.Mocks.Generated.MyApp.Services; directives will fail to compile, the user removes them, and the next compile resolves cleanly via the new namespace placement (or implicit usings if the file already shares the namespace). I'll add a note to the PR description.

Minor — namespace resolution scan: agreed, not a hot path. Leaving as-is.

[claude]

Claude finished @thomhurst's task in 1m 22s —— View job

---

Round 3 Review

Tasks

• Read updated MockNamespaceConflictDetector.cs to verify hasEvents removal
• Check TypeSymbolExtensions.cs for remaining divergence
• Check MemberDiscovery.cs / GetAutoMockFactoryMethod call path
• Check MockTypeDiscovery.cs for updated HasConflict call
• Verify updated conflict tests
• Provide final review verdict

---

Issue 1: Duplicate conflict detection — FIXED ✓

The fix is clean and correct. HasConflict now has a single signature (Compilation compilation, INamedTypeSymbol target) with no hasEvents parameter. Both call sites are now identical:

• MockTypeDiscovery.BuildSingleTypeModel (line 386): MockNamespaceConflictDetector.HasConflict(compilation, namedType)
• TypeSymbolExtensions.GetGeneratedMockNamespace (line 93): MockNamespaceConflictDetector.HasConflict(compilation, type)

Since MemberDiscovery.GetAutoMockFactoryMethod calls GetGeneratedMockNamespace, it now reaches the same result as BuildSingleTypeModel for any given (compilation, namedType) pair — the divergence is fully eliminated.

The choice to unconditionally check all seven suffixes (including _MockEvents / _MockEventsExtensions even when the target has no events) is the right trade-off: slightly more conservative but guarantees symmetry and simplifies the invariant. The EventsExtensionsCollision_AlwaysFlagged test at MockNamespaceConflictTests.cs:59 correctly documents the reason:

"Keeping the rule symmetric ensures both call sites — model construction and auto-mock factory references — reach the same fallback decision without re-deriving event lists."

---

Issue 2: Framework namespace pollution — accepted as designed

The auto-recovery argument holds: the detector re-runs on every incremental build, so any future BCL collision auto-corrects on next compile with a normal, diagnosable error. This is a reasonable trade-off and I accept the author's reasoning.

---

Issue 3: Breaking change scope — note promised

The breaking change (removal of TUnit.Mocks.Generated.{namespace} sub-namespaces) is acknowledged. The author noted they'll add a note to the PR description — that should be done before merge.

---

Verdict

All three issues from rounds 1 and 2 are resolved: Issue 1 by a code fix, Issues 2 and 3 by deliberate design choices with clear reasoning. The implementation is correct. Pending the PR description note for the breaking change, this is ready to merge.

[claude]

Round 4 Review — Final

All three issues from rounds 1–3 have been resolved. Here is the final assessment.

---

Issue 1: Duplicate conflict detection — FIXED ✓

The hasEvents parameter was removed from HasConflict entirely. The detector now unconditionally checks all seven emitted suffixes, giving both call sites — BuildSingleTypeModel and TypeSymbolExtensions.GetGeneratedMockNamespace — identical inputs and outputs for any (compilation, namedType) pair. The divergence that could cause mismatched FQNs is fully eliminated. The EventsExtensionsCollision_AlwaysFlagged test correctly documents the deliberate conservatism.

---

Issue 2: Framework namespace pollution — accepted as designed ✓

The auto-recovery argument is sound: HasConflict runs at every incremental build, so any future BCL collision auto-corrects on next compile with a diagnosable error pointing at the colliding type. This is a reasonable trade-off. The Notes section in the PR description now makes this explicit.

---

Issue 3: Breaking change for sub-namespace using directives — DOCUMENTED ✓

The PR description now has a dedicated Breaking change section that:

• Names the exact pattern that breaks (using TUnit.Mocks.Generated.MyApp.Services;)
• Explains why (mocks now live in the natural namespace)
• Gives the fix (drop the qualified using)
• Clarifies that the root using TUnit.Mocks.Generated; is preserved
• Explains the auto-recovery behaviour for future collisions

This is exactly what was requested in round 2.

---

Overall verdict

All issues are resolved — one by code fix (Issue 1), one by design acceptance with documented reasoning (Issue 2), and one by PR description update (Issue 3). The implementation is correct, the incremental-generator model is sound (UseFallbackNamespace participates in Equals/GetHashCode), test coverage is comprehensive, and the breaking change is properly called out. Ready to merge.