Skip to content

Commit 0dfaf9d

Browse files
authored
+semver:minor - fix(mocks): make generated identifier sanitization injective (#6507)
* fix(mocks): make generated identifier sanitization injective SanitizeIdentifier mapped every separator to '_' and collapsed runs, so `A_B.IFoo` and `A.B.IFoo` both produced `A_B_IFoo`. That string is the AddSource hint name, and a duplicate hint name makes Roslyn drop the generated sources for both types with no diagnostic — the user only sees the downstream CS1061/CS0117 from the missing setup surface. Double literal underscores before mapping separators, so a single '_' in the result always came from a separator. The two copies of the routine (MockImplBuilder and TypeSymbolExtensions) now share one implementation in IdentifierEscaping. Only names that already contain an underscore change. The three refreshed snapshots are multi-interface mocks whose composite hint name gains a doubled underscore at the join; their contents are unchanged, only the hint-name ordering of the emitted files. Fixes #6505 * fix(mocks): report TM008 when two mocked types share a generated name Review follow-up: doubling literal underscores is not enough on its own. No mapping onto [A-Za-z0-9_] can be injective while both a separator and an underscore render as runs of '_' — a run of three cannot say which order it came in, so `A_.B.IFoo` and `A._B.IFoo` still meet at `A___B_IFoo`. Detect that before anything is written: the dedupe step already sees every model, so group them by generated name and flag the ones that share. Colliding types are skipped with TM008 naming both culprits and the name they share, instead of duplicate hint names aborting the generator and taking every unrelated mock in the compilation with them. Grouping keys on the secondary-surface flag (a multi-interface combo and its pair surface share a composite name by design) and ignores models that are the same target mocked in different modes.
1 parent 6c5216f commit 0dfaf9d

13 files changed

Lines changed: 710 additions & 397 deletions

src/TUnit.Mocks.SourceGenerator/Builders/MockImplBuilder.cs

Lines changed: 1 addition & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1725,40 +1725,7 @@ private static string StripGlobalPrefix(string name)
17251725
=> name.StartsWith("global::") ? name.Substring("global::".Length) : name;
17261726

17271727
private static string SanitizeIdentifier(string name)
1728-
{
1729-
name = name.Replace("global::", "");
1730-
1731-
var sb = new System.Text.StringBuilder(name.Length);
1732-
var lastWasUnderscore = false;
1733-
1734-
foreach (var c in name)
1735-
{
1736-
if (c == ' ')
1737-
continue;
1738-
1739-
if (char.IsLetterOrDigit(c) || c == '_')
1740-
{
1741-
if (c == '_')
1742-
{
1743-
if (lastWasUnderscore)
1744-
continue;
1745-
lastWasUnderscore = true;
1746-
}
1747-
else
1748-
{
1749-
lastWasUnderscore = false;
1750-
}
1751-
sb.Append(c);
1752-
}
1753-
else if (!lastWasUnderscore)
1754-
{
1755-
sb.Append('_');
1756-
lastWasUnderscore = true;
1757-
}
1758-
}
1759-
1760-
return sb.ToString();
1761-
}
1728+
=> IdentifierEscaping.SanitizeIdentifier(name);
17621729

17631730
/// <summary>
17641731
/// Root namespace for fallback-mode mock emission, used when the original namespace
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
using Microsoft.CodeAnalysis;
2+
3+
namespace TUnit.Mocks.SourceGenerator;
4+
5+
/// <summary>
6+
/// Diagnostics reported by the generator itself. Everything the analyzer can see at a call site
7+
/// belongs in TUnit.Mocks.Analyzers (TM001-TM007); this file is for failures only the generator
8+
/// can observe, which is currently just whole-compilation name collisions.
9+
/// </summary>
10+
internal static class Diagnostics
11+
{
12+
public static readonly DiagnosticDescriptor TM008_GeneratedNameCollision = new(
13+
id: "TM008",
14+
title: "Mocked types produce the same generated name",
15+
messageFormat: "Cannot mock '{0}' because it produces the same generated name '{1}' as '{2}'. Rename one of the types or namespaces.",
16+
category: "TUnit.Mocks",
17+
defaultSeverity: DiagnosticSeverity.Error,
18+
isEnabledByDefault: true,
19+
description: "Generated type and file names are derived from the mocked type's fully qualified name with separators replaced by underscores. Two types can still map to the same name when their namespaces differ only in how underscores and dots are arranged (e.g. 'A_.B.IFoo' and 'A._B.IFoo'). Emitting both would give Roslyn duplicate hint names, which discards every mock in the compilation without saying why, so generation is skipped for the colliding types and reported here instead."
20+
);
21+
}
Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
using TUnit.Mocks.SourceGenerator.Builders;
2+
using TUnit.Mocks.SourceGenerator.Models;
3+
4+
namespace TUnit.Mocks.SourceGenerator.Discovery;
5+
6+
/// <summary>
7+
/// Flags mocked types whose generated names collide.
8+
/// <para>
9+
/// Generated type and <c>AddSource</c> hint names come from the mocked type's fully qualified name
10+
/// with separators mapped to <c>_</c>. <see cref="IdentifierEscaping.SanitizeIdentifier"/> doubles
11+
/// literal underscores so the realistic cases stay distinct, but no mapping onto
12+
/// <c>[A-Za-z0-9_]</c> can be injective while a separator and an underscore both render as runs of
13+
/// <c>_</c>: a run of three cannot say whether it was underscore-then-separator or the reverse, so
14+
/// <c>A_.B.IFoo</c> and <c>A._B.IFoo</c> still meet at <c>A___B_IFoo</c>.
15+
/// </para>
16+
/// <para>
17+
/// A duplicate hint name aborts the whole generator, which costs the user every mock in the
18+
/// compilation and reports only a CS8785 warning pointing at the generator rather than at either
19+
/// type. Skipping the colliding types and reporting TM008 keeps the rest of the compilation's
20+
/// mocks and names both culprits. See issue #6505.
21+
/// </para>
22+
/// </summary>
23+
internal static class GeneratedNameCollisionDetector
24+
{
25+
/// <summary>
26+
/// Returns <paramref name="models"/> in input order, with <see cref="MockTypeModel.CollidesWith"/>
27+
/// set on every model that shares its generated name with another.
28+
/// </summary>
29+
internal static List<MockTypeModel> Annotate(IEnumerable<MockTypeModel> models)
30+
{
31+
var ordered = models.ToList();
32+
33+
// The name alone is not the key: a multi-interface combo and the secondary setup surface
34+
// for the same (primary, interface) pair intentionally share a composite name and are told
35+
// apart by the hint-name suffix, so they must not be flagged.
36+
var groups = new Dictionary<(bool IsSecondaryMemberSurface, string Name), List<MockTypeModel>>();
37+
38+
foreach (var model in ordered)
39+
{
40+
var key = (model.IsSecondaryMemberSurface, MockImplBuilder.GetCompositeSafeName(model));
41+
42+
if (!groups.TryGetValue(key, out var group))
43+
{
44+
groups[key] = group = new List<MockTypeModel>();
45+
}
46+
47+
group.Add(model);
48+
}
49+
50+
if (groups.Count == ordered.Count)
51+
{
52+
return ordered;
53+
}
54+
55+
var annotated = new List<MockTypeModel>(ordered.Count);
56+
57+
foreach (var model in ordered)
58+
{
59+
var group = groups[(model.IsSecondaryMemberSurface, MockImplBuilder.GetCompositeSafeName(model))];
60+
61+
// Same target mocked in more than one mode (Mock.Of and Mock.Wrap of one type, say)
62+
// reaches this point as separate models sharing an identity. Only distinct targets
63+
// meeting at one name are a #6505 collision.
64+
var others = group
65+
.Where(other => Identity(other) != Identity(model))
66+
.Select(other => other.FullyQualifiedName)
67+
.Distinct()
68+
.ToList();
69+
70+
annotated.Add(others.Count == 0
71+
? model
72+
: model with { CollidesWith = string.Join(", ", others) });
73+
}
74+
75+
return annotated;
76+
}
77+
78+
private static string Identity(MockTypeModel model)
79+
=> model.AdditionalInterfaceNames.Length == 0
80+
? model.FullyQualifiedName
81+
: model.FullyQualifiedName + "|" + string.Join("|", model.AdditionalInterfaceNames);
82+
}

src/TUnit.Mocks.SourceGenerator/Extensions/TypeSymbolExtensions.cs

Lines changed: 1 addition & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -304,40 +304,5 @@ private static string StripGlobalPrefix(string name)
304304
=> name.StartsWith("global::") ? name.Substring("global::".Length) : name;
305305

306306
private static string SanitizeIdentifier(string name)
307-
{
308-
name = name.Replace("global::", "");
309-
310-
var sb = new StringBuilder(name.Length);
311-
var lastWasUnderscore = false;
312-
313-
foreach (var c in name)
314-
{
315-
if (c == ' ')
316-
continue;
317-
318-
if (char.IsLetterOrDigit(c) || c == '_')
319-
{
320-
if (c == '_')
321-
{
322-
if (lastWasUnderscore)
323-
continue;
324-
325-
lastWasUnderscore = true;
326-
}
327-
else
328-
{
329-
lastWasUnderscore = false;
330-
}
331-
332-
sb.Append(c);
333-
}
334-
else if (!lastWasUnderscore)
335-
{
336-
sb.Append('_');
337-
lastWasUnderscore = true;
338-
}
339-
}
340-
341-
return sb.ToString();
342-
}
307+
=> IdentifierEscaping.SanitizeIdentifier(name);
343308
}

src/TUnit.Mocks.SourceGenerator/IdentifierEscaping.cs

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
using System.Text;
12
using Microsoft.CodeAnalysis.CSharp;
23

34
namespace TUnit.Mocks.SourceGenerator;
@@ -26,4 +27,52 @@ internal static class IdentifierEscaping
2627
// interface implementation matching against the source-declared member name.
2728
internal static string EscapeIdentifier(string name) =>
2829
SyntaxFacts.GetKeywordKind(name) != SyntaxKind.None ? "@" + name : name;
30+
31+
/// <summary>
32+
/// Turns a type name (fully qualified, or a bare name with generic arguments) into a single
33+
/// C# identifier, used for generated type names and <c>AddSource</c> hint names.
34+
/// <para>
35+
/// The mapping must be injective: a hint-name clash makes Roslyn drop the generated sources
36+
/// for <em>both</em> types with no diagnostic, so the user only ever sees the downstream
37+
/// CS1061/CS0117 from the missing surface. Every literal <c>_</c> is therefore doubled before
38+
/// separators become <c>_</c>, so a single underscore in the result always came from a
39+
/// separator. Without that, <c>A_B.IFoo</c> and <c>A.B.IFoo</c> both produced
40+
/// <c>A_B_IFoo</c> — see issue #6505.
41+
/// </para>
42+
/// </summary>
43+
internal static string SanitizeIdentifier(string name)
44+
{
45+
name = name.Replace("global::", "");
46+
47+
var sb = new StringBuilder(name.Length);
48+
var lastWasSeparator = false;
49+
50+
foreach (var c in name)
51+
{
52+
if (c == ' ')
53+
{
54+
continue;
55+
}
56+
57+
if (c == '_')
58+
{
59+
sb.Append("__");
60+
lastWasSeparator = false;
61+
}
62+
else if (char.IsLetterOrDigit(c))
63+
{
64+
sb.Append(c);
65+
lastWasSeparator = false;
66+
}
67+
else if (!lastWasSeparator)
68+
{
69+
// Runs of separators still collapse to one '_' ("IFoo<T>" -> "IFoo_T_"); only the
70+
// separator/underscore distinction has to survive.
71+
sb.Append('_');
72+
lastWasSeparator = true;
73+
}
74+
}
75+
76+
return sb.ToString();
77+
}
2978
}

src/TUnit.Mocks.SourceGenerator/MockGenerator.cs

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,12 +57,27 @@ namespace TUnit.Mocks.Generated;
5757
foreach (var m in mockOfTypes) set.Add(m);
5858
foreach (var m in attributeTypes) set.Add(m);
5959
foreach (var m in extensionInvocations) set.Add(m);
60-
return set;
60+
61+
// Flag types that would emit the same generated names before anything is written:
62+
// duplicate hint names abort the generator and take every mock in the compilation
63+
// with them. See issue #6505.
64+
return GeneratedNameCollisionDetector.Annotate(set);
6165
});
6266

6367
// Step 3: Generate source for each unique type
6468
context.RegisterSourceOutput(distinctTypes, (spc, model) =>
6569
{
70+
if (model.CollidesWith is not null)
71+
{
72+
spc.ReportDiagnostic(Diagnostic.Create(
73+
Diagnostics.TM008_GeneratedNameCollision,
74+
Location.None,
75+
model.FullyQualifiedName,
76+
MockImplBuilder.GetCompositeSafeName(model),
77+
model.CollidesWith));
78+
return;
79+
}
80+
6681
if (model.IsSecondaryMemberSurface)
6782
{
6883
// Pair model: the shared setup/verify surface for one additional interface of a

src/TUnit.Mocks.SourceGenerator/Models/MockTypeModel.cs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,15 @@ internal sealed record MockTypeModel : IEquatable<MockTypeModel>
6363
/// <summary>The C# visibility keyword to emit on generated wrapper/extension types.</summary>
6464
public string Visibility => IsPublic ? "public" : "internal";
6565

66+
/// <summary>
67+
/// Set by <see cref="Discovery.GeneratedNameCollisionDetector"/> when another mocked type in
68+
/// the same compilation sanitizes to the same generated name (see issue #6505). Generation is
69+
/// skipped and TM008 is reported, because emitting both would give Roslyn duplicate hint names
70+
/// — which silently discards every mock in the compilation. Holds the other type's fully
71+
/// qualified name (comma separated when more than one collides).
72+
/// </summary>
73+
public string? CollidesWith { get; init; }
74+
6675
/// <summary>
6776
/// True for a class target that exposes no constructor the generated impl could chain to.
6877
/// The impl derives from the target, so with every constructor private (or cross-assembly
@@ -98,6 +107,7 @@ public bool Equals(MockTypeModel? other)
98107
&& Constructors.Equals(other.Constructors)
99108
&& HasStaticAbstractMembers == other.HasStaticAbstractMembers
100109
&& IsSecondaryMemberSurface == other.IsSecondaryMemberSurface
110+
&& CollidesWith == other.CollidesWith
101111
&& SecondaryMemberIdMaps.Equals(other.SecondaryMemberIdMaps);
102112
}
103113

@@ -120,6 +130,7 @@ public override int GetHashCode()
120130
hash = hash * 31 + AdditionalInterfaceNames.GetHashCode();
121131
hash = hash * 31 + HasStaticAbstractMembers.GetHashCode();
122132
hash = hash * 31 + IsSecondaryMemberSurface.GetHashCode();
133+
hash = hash * 31 + (CollidesWith?.GetHashCode() ?? 0);
123134
hash = hash * 31 + SecondaryMemberIdMaps.GetHashCode();
124135
return hash;
125136
}

src/TUnit.Mocks.SourceGenerator/TUnit.Mocks.SourceGenerator.csproj

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@
1010
<PackageId>TUnit.Mocks.SourceGenerator</PackageId>
1111
<EnableTrimAnalyzer>false</EnableTrimAnalyzer>
1212
<IsPackable>false</IsPackable>
13+
<!-- Matches TUnit.Mocks.Analyzers: TM diagnostics are not release-tracked. -->
14+
<NoWarn>RS2008</NoWarn>
1315
</PropertyGroup>
1416

1517
<ItemGroup>

0 commit comments

Comments
 (0)