fix(mocks): make generated identifier sanitization injective

[thomhurst]

Fixes #6505

Problem

SanitizeIdentifier mapped every non-alphanumeric character to _ and collapsed runs, so it was not injective: A_B.IFoo and A.B.IFoo both sanitized to A_B_IFoo.

That string is the AddSource hint name (via GetSafeFileName → GetCompositeSafeName), so mocking both types in one compilation produced duplicate hint names and Roslyn dropped the generated sources for both — with no diagnostic. The user only saw the downstream symptom (CS1061 on every setup, or CS0117 on T.Mock()), with nothing pointing at the cause.

Fix

Double literal underscores before mapping separators (_ → __, separators → _), so a single _ in the result always came from a separator:

• A.B.IFoo → A_B_IFoo
• A_B.IFoo → A__B_IFoo

Runs of separators still collapse (IFoo<T> → IFoo_T_); only the separator/underscore distinction has to survive.

The two byte-identical copies of the routine (MockImplBuilder and TypeSymbolExtensions) now delegate to one implementation in IdentifierEscaping, so they can't drift.

Snapshot churn

Only names that already contain an underscore change. Three snapshots were refreshed — all multi-interface mocks, whose composite hint name is built by joining with _ before sanitizing and so now gains a doubled underscore at the join. The file contents are identical; only the hint-name sort order of the emitted files moved. (Keeping the join pre-sanitize is deliberate: joining already-sanitized parts with a plain _ would let A.B + additional interface C collide with the type A.B.C.)

Tests

New Issue6505Tests — verified to fail on main and pass here:

• both colliding types generate their member surfaces
• mocking both drops no generated file (count equals the sum of the individual runs, minus the shared post-init stub)

Full runs, all green: TUnit.Mocks.SourceGenerator.Tests (87), TUnit.Mocks.Tests (1182), TUnit.Mocks.Http.Tests (54), TUnit.Mocks.Logging.Tests (31), TUnit.Mocks.Analyzers.Tests (56).

[greptile-apps]

Adjacent tokens still collide

When a compilation mocks types such as A_.B.IFoo and A._B.IFoo, the literal underscore and namespace separator are emitted in opposite orders but both sanitize to A___B_IFoo, causing generated identifiers and AddSource hint names to collide and the mock sources to be dropped.

[greptile-apps]

Greptile Summary

The PR centralizes generated-identifier sanitization, preserves literal underscores by doubling them, and adds collision diagnostics that skip conflicting mock models while allowing unrelated mocks to generate.

Confidence Score: 3/5

The PR is not yet safe to merge because combining Mock.Of() and Mock.Wrap(...) for one constructable class can still emit duplicate hint names and lose generated mock sources.

The new detector deliberately equates same-target models without considering their generation mode, while the wrap and ordinary branches both emit identical implementation-factory and member hint names.

Files Needing Attention: src/TUnit.Mocks.SourceGenerator/Discovery/GeneratedNameCollisionDetector.cs, src/TUnit.Mocks.SourceGenerator/MockGenerator.cs

Important Files Changed

Filename	Overview
src/TUnit.Mocks.SourceGenerator/IdentifierEscaping.cs	Centralizes sanitization and distinguishes literal underscores from separators by doubling underscores.
src/TUnit.Mocks.SourceGenerator/Discovery/GeneratedNameCollisionDetector.cs	Detects remaining sanitized-name collisions, but its mode-agnostic identity lets same-target models emit duplicate hint names.
src/TUnit.Mocks.SourceGenerator/MockGenerator.cs	Integrates collision annotation and TM008 reporting before generation, while retaining overlapping emission paths for distinct same-target modes.
tests/TUnit.Mocks.SourceGenerator.Tests/Issue6505Tests.cs	Covers underscore/separator collisions, diagnostics, and preservation of unrelated output.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[Collect mock models] --> B[Deduplicate models]
  B --> C[Annotate generated-name collisions]
  C --> D{CollidesWith set?}
  D -- Yes --> E[Report TM008 and skip model]
  D -- No --> F[Generate mock sources]

Loading

_{Reviews (2): Last reviewed commit: "fix(mocks): report TM008 when two mocked..." | Re-trigger Greptile}

[claude]

Code review

Nice, focused fix — good catch tracing the silent AddSource drop back to a hint-name collision, and good call consolidating the two byte-identical SanitizeIdentifier copies into IdentifierEscaping.

However, I traced the new algorithm's state machine by hand (and verified with a brute-force search over a small alphabet) and the mapping is still not injective, which is the exact property this PR sets out to guarantee.

The residual collision

In IdentifierEscaping.SanitizeIdentifier, lastWasSeparator is reset to false on every literal _ (the c == '_' branch), so a literal underscore immediately adjacent to a separator produces the same output regardless of order:

• A_.B (namespace A_, type B): A → _ doubles to __ → . sees lastWasSeparator == false so it emits another _ → B. Result: A___B
• A._B (namespace A, type _B): A → . emits _ (lastWasSeparator now true) → _ doubles to __ regardless of lastWasSeparator → B. Result: A___B

Both are valid, real fully-qualified type names, and both collapse to A___B, reproducing issue #6505's exact failure mode (Roslyn silently drops both generated sources; the user only sees the downstream CS1061/CS0117). The same pattern applies to any separator: IFoo_<T> and IFoo<_T> both → IFoo___T_.

The doc comment's claim — "a single underscore in the result always came from a separator" — is true but insufficient: a run of 3+ underscores is still ambiguous about where the separator/underscore boundary falls, so the output can't be uniquely decoded. Doubling only fixes collisions where the underscore and separator aren't adjacent (which happens to cover the A_B.IFoo / A.B.IFoo case from #6505, but not the general case).

Why this matters architecturally: any escaping scheme that relies on "run length distinguishes X from Y" needs to handle the boundary between runs, not just within a single run. A more robust fix that sidesteps this whole class of problem: append a short stable hash (e.g. 8 hex chars of FNV-1a or similar) of the original fully-qualified name to the sanitized identifier used for the AddSource hint name. That makes the hint name collision-free regardless of how the human-readable prefix is escaped, and also covers a separate pre-existing gap the current approach doesn't touch: all separator characters collapse to the same _, so N.IFoo<A.B, C> and N.IFoo<A, B.C> still collide today (N_IFoo_A_B_C_) even after this fix. A hash suffix is also simpler to reason about/prove correct than extending the escaping grammar further.

It'd be worth adding a regression test for the A_.B / A._B case (or whatever the actual fix ends up being) alongside Issue6505Tests, since neither new test covers it.

Related, lower-severity (pre-existing, not introduced by this diff)

• MockImplBuilder.GetCompositeSafeName joins additional interface names with a literal _ before calling GetSafeName/SanitizeIdentifier. Since the join separator becomes indistinguishable from a real underscore by the time it's sanitized, Mock.Of<X, Y_Z>() and Mock.Of<X_Y, Z>() build the identical raw string "X_Y_Z" pre-sanitization and therefore the identical hint name. This is the same bug class this PR fixes, just one call site earlier, and it's directly in the multi-interface-mock path the PR's own snapshots touch — worth a follow-up (or covering with the hash-suffix approach above, which would fix this too).
• MockNamespaceConflictDetector.IsCollidingName compares candidate types against the raw, unsanitized target.Name (typeName + "Mock", etc.), which only agreed with the actual generated names because the old sanitizer left simple underscored identifiers untouched. Now that literal underscores are doubled, an interface named e.g. I_Foo generates I__FooMock, but the conflict detector still looks for I_FooMock — so a real conflict on I__FooMock would go undetected (missed fallback → possible CS0101), while an unrelated I_FooMock would trigger an unnecessary fallback. This file isn't touched by the diff, but its correctness assumption was silently invalidated by this change.

Neither of these two is something this PR needs to fix, but they're close enough to the change (same bug family, one directly downstream of SanitizeIdentifier's new behavior) that they seemed worth flagging while in this code.

No CLAUDE.md violations found — snapshot files are correctly committed as .verified.txt with no stray .received.txt, and the dual-mode/AOT/performance rules don't apply to this compile-time-only source-generator code.

[thomhurst]

Confirmed — A_.B.IFoo and A._B.IFoo both sanitize to A___B_IFoo. Good catch.

That case can't be fixed by a better escape, though: the output alphabet is [A-Za-z0-9_], and if both a separator and a literal underscore render as runs of _, a run only carries a sum, not an order — three underscores is __ + _ or _ + __ either way. Making it injective means escaping separators too (A.B.IFoo → A__B__IFoo), which uglifies every generated file and extension-class name for a namespace shape nobody writes.

So instead the residual case is now detected and reported rather than dropped (pushed in f44a8fa):

• The dedupe step already sees every model, so it groups them by generated name and flags collisions before anything is written.
• Colliding types are skipped and TM008 is reported, naming both types and the name they share.
• Unrelated mocks in the same compilation keep generating — the old failure mode was a duplicate hint name aborting the generator, which killed every mock and reported only a CS8785 warning against the generator.

Grouping keys on the secondary-surface flag (a multi-interface combo and its pair setup surface deliberately share a composite name and are told apart by the hint-name suffix) and ignores models that are the same target mocked in different modes, so neither is a false positive.

Three new tests cover it: the sanitizer's separator/underscore distinction, TM008 on the adjacent-token case, and an unrelated mock surviving alongside a collision. Full suites still green — SourceGenerator 90, Mocks 1182, Http 54, Logging 31, Analyzers 56.

[greptile-apps]

Same-target modes still collide

When a constructable class is used with both Mock.Of<T>() and Mock.Wrap<T>(...), the models survive deduplication but this identity treats them as the same target and suppresses collision annotation. Both generation branches then emit the same _MockImplFactory.g.cs and _MockMembers.g.cs hint names, causing the generator to lose those mock sources instead of reporting TM008.

[claude]

Code review

Re-reviewed after commit f44a8fa, which responds to the non-injectivity concern raised in the earlier review on this PR.

The core concern is resolved, via a design pivot rather than a direct fix — and it's the right call.

The original approach tried to make SanitizeIdentifier fully injective by doubling literal underscores. As flagged previously, that's provably impossible: once both a namespace separator and a literal _ render as runs of _, a run of three can't say which order they occurred in (A_.B.IFoo vs A._B.IFoo both hash to A___B_IFoo). Trying to extend the escaping scheme further (e.g. reserving alnum marker sequences for separators) runs into the same wall — any marker built from the output alphabet can itself be reproduced by ordinary identifier content, so the ambiguity just moves rather than disappears.

Rather than keep chasing an unreachable "fully injective" encoding, the new GeneratedNameCollisionDetector (src/TUnit.Mocks.SourceGenerator/Discovery/GeneratedNameCollisionDetector.cs) detects any actual collision after the fact by comparing each model's derived name against its true identity (fully-qualified name + interface list), and reports TM008 for the colliding types instead of silently discarding them. This is a better architecture than trying to fix the encoding: it doesn't matter why two models produced the same generated name — only that they did — so it's naturally exhaustive rather than tied to one specific ambiguity.

That exhaustiveness also happens to close a second gap from the earlier review: GetCompositeSafeName still joins raw type names with a bare _ before sanitizing (so Mock.Of<X, Y_Z>() and Mock.Of<X_Y, Z>() can still produce the same composite string). That's still true today, but it no longer matters — since the detector compares against Identity() (raw FQNs joined with |, which can't collide the way _-joined names can), any such collision is now caught and reported as TM008 too, rather than silently dropping one of the mocks.

Verified in the diff/tests:

• IdentifierEscaping.SanitizeIdentifier correctly separates the originally-reported case (A_B.IFoo → A_B_IFoo, A.B.IFoo's doubled sibling A__B_IFoo) — issue [TUnit.Mocks] Two mocked types whose sanitized names collide silently generate nothing (e.g. A_B.IFoo and A.B.IFoo) #6505's reported scenario is fixed for real, not just detected.
• The residual ambiguity is caught and reported (TM008) rather than silently dropped, and unrelated mocks in the same compilation still generate (Issue6505Tests.A_Collision_Does_Not_Stop_Other_Mocks_In_The_Compilation).
• MockNamespaceConflictDetector.IsCollidingName (unsanitized-name comparison) is pre-existing and untouched by this PR — out of scope here, but worth a follow-up issue since it's a related class of bug.
• Snapshot diffs are hint-name sort-order only, consistent with the PR description, and no .received.txt files are present.

No blocking issues. Nice use of a compile-time diagnostic to convert a silent, whole-compilation generator failure into a scoped, actionable error.