Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 1 addition & 34 deletions src/TUnit.Mocks.SourceGenerator/Builders/MockImplBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1725,40 +1725,7 @@ private static string StripGlobalPrefix(string name)
=> name.StartsWith("global::") ? name.Substring("global::".Length) : name;

private static string SanitizeIdentifier(string name)
{
name = name.Replace("global::", "");

var sb = new System.Text.StringBuilder(name.Length);
var lastWasUnderscore = false;

foreach (var c in name)
{
if (c == ' ')
continue;

if (char.IsLetterOrDigit(c) || c == '_')
{
if (c == '_')
{
if (lastWasUnderscore)
continue;
lastWasUnderscore = true;
}
else
{
lastWasUnderscore = false;
}
sb.Append(c);
}
else if (!lastWasUnderscore)
{
sb.Append('_');
lastWasUnderscore = true;
}
}

return sb.ToString();
}
=> IdentifierEscaping.SanitizeIdentifier(name);

/// <summary>
/// Root namespace for fallback-mode mock emission, used when the original namespace
Expand Down
21 changes: 21 additions & 0 deletions src/TUnit.Mocks.SourceGenerator/Diagnostics.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
using Microsoft.CodeAnalysis;

namespace TUnit.Mocks.SourceGenerator;

/// <summary>
/// Diagnostics reported by the generator itself. Everything the analyzer can see at a call site
/// belongs in TUnit.Mocks.Analyzers (TM001-TM007); this file is for failures only the generator
/// can observe, which is currently just whole-compilation name collisions.
/// </summary>
internal static class Diagnostics
{
public static readonly DiagnosticDescriptor TM008_GeneratedNameCollision = new(
id: "TM008",

Check warning on line 13 in src/TUnit.Mocks.SourceGenerator/Diagnostics.cs

View workflow job for this annotation

GitHub Actions / modularpipeline (ubuntu-latest)

Enable analyzer release tracking for the analyzer project containing rule 'TM008' (https://github.com/dotnet/roslyn-analyzers/blob/main/src/Microsoft.CodeAnalysis.Analyzers/ReleaseTrackingAnalyzers.Help.md)

Check warning on line 13 in src/TUnit.Mocks.SourceGenerator/Diagnostics.cs

View workflow job for this annotation

GitHub Actions / modularpipeline (ubuntu-latest)

Enable analyzer release tracking for the analyzer project containing rule 'TM008' (https://github.com/dotnet/roslyn-analyzers/blob/main/src/Microsoft.CodeAnalysis.Analyzers/ReleaseTrackingAnalyzers.Help.md)

Check warning on line 13 in src/TUnit.Mocks.SourceGenerator/Diagnostics.cs

View workflow job for this annotation

GitHub Actions / modularpipeline (windows-latest)

Enable analyzer release tracking for the analyzer project containing rule 'TM008' (https://github.com/dotnet/roslyn-analyzers/blob/main/src/Microsoft.CodeAnalysis.Analyzers/ReleaseTrackingAnalyzers.Help.md)

Check warning on line 13 in src/TUnit.Mocks.SourceGenerator/Diagnostics.cs

View workflow job for this annotation

GitHub Actions / modularpipeline (windows-latest)

Enable analyzer release tracking for the analyzer project containing rule 'TM008' (https://github.com/dotnet/roslyn-analyzers/blob/main/src/Microsoft.CodeAnalysis.Analyzers/ReleaseTrackingAnalyzers.Help.md)

Check warning on line 13 in src/TUnit.Mocks.SourceGenerator/Diagnostics.cs

View workflow job for this annotation

GitHub Actions / modularpipeline (macos-latest)

Enable analyzer release tracking for the analyzer project containing rule 'TM008' (https://github.com/dotnet/roslyn-analyzers/blob/main/src/Microsoft.CodeAnalysis.Analyzers/ReleaseTrackingAnalyzers.Help.md)

Check warning on line 13 in src/TUnit.Mocks.SourceGenerator/Diagnostics.cs

View workflow job for this annotation

GitHub Actions / modularpipeline (macos-latest)

Enable analyzer release tracking for the analyzer project containing rule 'TM008' (https://github.com/dotnet/roslyn-analyzers/blob/main/src/Microsoft.CodeAnalysis.Analyzers/ReleaseTrackingAnalyzers.Help.md)
title: "Mocked types produce the same generated name",
messageFormat: "Cannot mock '{0}' because it produces the same generated name '{1}' as '{2}'. Rename one of the types or namespaces.",
category: "TUnit.Mocks",
defaultSeverity: DiagnosticSeverity.Error,
isEnabledByDefault: true,
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."
);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
using TUnit.Mocks.SourceGenerator.Builders;
using TUnit.Mocks.SourceGenerator.Models;

namespace TUnit.Mocks.SourceGenerator.Discovery;

/// <summary>
/// Flags mocked types whose generated names collide.
/// <para>
/// Generated type and <c>AddSource</c> hint names come from the mocked type's fully qualified name
/// with separators mapped to <c>_</c>. <see cref="IdentifierEscaping.SanitizeIdentifier"/> doubles
/// literal underscores so the realistic cases stay distinct, but no mapping onto
/// <c>[A-Za-z0-9_]</c> can be injective while a separator and an underscore both render as runs of
/// <c>_</c>: a run of three cannot say whether it was underscore-then-separator or the reverse, so
/// <c>A_.B.IFoo</c> and <c>A._B.IFoo</c> still meet at <c>A___B_IFoo</c>.
/// </para>
/// <para>
/// A duplicate hint name aborts the whole generator, which costs the user every mock in the
/// compilation and reports only a CS8785 warning pointing at the generator rather than at either
/// type. Skipping the colliding types and reporting TM008 keeps the rest of the compilation's
/// mocks and names both culprits. See issue #6505.
/// </para>
/// </summary>
internal static class GeneratedNameCollisionDetector
{
/// <summary>
/// Returns <paramref name="models"/> in input order, with <see cref="MockTypeModel.CollidesWith"/>
/// set on every model that shares its generated name with another.
/// </summary>
internal static List<MockTypeModel> Annotate(IEnumerable<MockTypeModel> models)
{
var ordered = models.ToList();

// The name alone is not the key: a multi-interface combo and the secondary setup surface
// for the same (primary, interface) pair intentionally share a composite name and are told
// apart by the hint-name suffix, so they must not be flagged.
var groups = new Dictionary<(bool IsSecondaryMemberSurface, string Name), List<MockTypeModel>>();

foreach (var model in ordered)
{
var key = (model.IsSecondaryMemberSurface, MockImplBuilder.GetCompositeSafeName(model));

if (!groups.TryGetValue(key, out var group))
{
groups[key] = group = new List<MockTypeModel>();
}

group.Add(model);
}

if (groups.Count == ordered.Count)
{
return ordered;
}

var annotated = new List<MockTypeModel>(ordered.Count);

foreach (var model in ordered)
{
var group = groups[(model.IsSecondaryMemberSurface, MockImplBuilder.GetCompositeSafeName(model))];

// Same target mocked in more than one mode (Mock.Of and Mock.Wrap of one type, say)
// reaches this point as separate models sharing an identity. Only distinct targets
// meeting at one name are a #6505 collision.
var others = group
.Where(other => Identity(other) != Identity(model))
.Select(other => other.FullyQualifiedName)
.Distinct()
.ToList();

annotated.Add(others.Count == 0
? model
: model with { CollidesWith = string.Join(", ", others) });
}

return annotated;
}

private static string Identity(MockTypeModel model)
=> model.AdditionalInterfaceNames.Length == 0
? model.FullyQualifiedName
: model.FullyQualifiedName + "|" + string.Join("|", model.AdditionalInterfaceNames);
Comment on lines +78 to +81

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 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.

}
Original file line number Diff line number Diff line change
Expand Up @@ -304,40 +304,5 @@ private static string StripGlobalPrefix(string name)
=> name.StartsWith("global::") ? name.Substring("global::".Length) : name;

private static string SanitizeIdentifier(string name)
{
name = name.Replace("global::", "");

var sb = new StringBuilder(name.Length);
var lastWasUnderscore = false;

foreach (var c in name)
{
if (c == ' ')
continue;

if (char.IsLetterOrDigit(c) || c == '_')
{
if (c == '_')
{
if (lastWasUnderscore)
continue;

lastWasUnderscore = true;
}
else
{
lastWasUnderscore = false;
}

sb.Append(c);
}
else if (!lastWasUnderscore)
{
sb.Append('_');
lastWasUnderscore = true;
}
}

return sb.ToString();
}
=> IdentifierEscaping.SanitizeIdentifier(name);
}
49 changes: 49 additions & 0 deletions src/TUnit.Mocks.SourceGenerator/IdentifierEscaping.cs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using System.Text;
using Microsoft.CodeAnalysis.CSharp;

namespace TUnit.Mocks.SourceGenerator;
Expand Down Expand Up @@ -26,4 +27,52 @@ internal static class IdentifierEscaping
// interface implementation matching against the source-declared member name.
internal static string EscapeIdentifier(string name) =>
SyntaxFacts.GetKeywordKind(name) != SyntaxKind.None ? "@" + name : name;

/// <summary>
/// Turns a type name (fully qualified, or a bare name with generic arguments) into a single
/// C# identifier, used for generated type names and <c>AddSource</c> hint names.
/// <para>
/// The mapping must be injective: a hint-name clash makes Roslyn drop the generated sources
/// for <em>both</em> types with no diagnostic, so the user only ever sees the downstream
/// CS1061/CS0117 from the missing surface. Every literal <c>_</c> is therefore doubled before
/// separators become <c>_</c>, so a single underscore in the result always came from a
/// separator. Without that, <c>A_B.IFoo</c> and <c>A.B.IFoo</c> both produced
/// <c>A_B_IFoo</c> — see issue #6505.
/// </para>
/// </summary>
internal static string SanitizeIdentifier(string name)
{
name = name.Replace("global::", "");

var sb = new StringBuilder(name.Length);
var lastWasSeparator = false;

foreach (var c in name)
{
if (c == ' ')
{
continue;
}

if (c == '_')
{
sb.Append("__");
lastWasSeparator = false;
}
else if (char.IsLetterOrDigit(c))
{
sb.Append(c);
lastWasSeparator = false;
}
else if (!lastWasSeparator)
{
// Runs of separators still collapse to one '_' ("IFoo<T>" -> "IFoo_T_"); only the
Comment on lines +57 to +69

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 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.

// separator/underscore distinction has to survive.
sb.Append('_');
lastWasSeparator = true;
}
}

return sb.ToString();
}
}
17 changes: 16 additions & 1 deletion src/TUnit.Mocks.SourceGenerator/MockGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,27 @@ namespace TUnit.Mocks.Generated;
foreach (var m in mockOfTypes) set.Add(m);
foreach (var m in attributeTypes) set.Add(m);
foreach (var m in extensionInvocations) set.Add(m);
return set;

// Flag types that would emit the same generated names before anything is written:
// duplicate hint names abort the generator and take every mock in the compilation
// with them. See issue #6505.
return GeneratedNameCollisionDetector.Annotate(set);
});

// Step 3: Generate source for each unique type
context.RegisterSourceOutput(distinctTypes, (spc, model) =>
{
if (model.CollidesWith is not null)
{
spc.ReportDiagnostic(Diagnostic.Create(
Diagnostics.TM008_GeneratedNameCollision,
Location.None,
model.FullyQualifiedName,
MockImplBuilder.GetCompositeSafeName(model),
model.CollidesWith));
return;
}

if (model.IsSecondaryMemberSurface)
{
// Pair model: the shared setup/verify surface for one additional interface of a
Expand Down
11 changes: 11 additions & 0 deletions src/TUnit.Mocks.SourceGenerator/Models/MockTypeModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,15 @@ internal sealed record MockTypeModel : IEquatable<MockTypeModel>
/// <summary>The C# visibility keyword to emit on generated wrapper/extension types.</summary>
public string Visibility => IsPublic ? "public" : "internal";

/// <summary>
/// Set by <see cref="Discovery.GeneratedNameCollisionDetector"/> when another mocked type in
/// the same compilation sanitizes to the same generated name (see issue #6505). Generation is
/// skipped and TM008 is reported, because emitting both would give Roslyn duplicate hint names
/// — which silently discards every mock in the compilation. Holds the other type's fully
/// qualified name (comma separated when more than one collides).
/// </summary>
public string? CollidesWith { get; init; }

/// <summary>
/// True for a class target that exposes no constructor the generated impl could chain to.
/// The impl derives from the target, so with every constructor private (or cross-assembly
Expand Down Expand Up @@ -98,6 +107,7 @@ public bool Equals(MockTypeModel? other)
&& Constructors.Equals(other.Constructors)
&& HasStaticAbstractMembers == other.HasStaticAbstractMembers
&& IsSecondaryMemberSurface == other.IsSecondaryMemberSurface
&& CollidesWith == other.CollidesWith
&& SecondaryMemberIdMaps.Equals(other.SecondaryMemberIdMaps);
}

Expand All @@ -120,6 +130,7 @@ public override int GetHashCode()
hash = hash * 31 + AdditionalInterfaceNames.GetHashCode();
hash = hash * 31 + HasStaticAbstractMembers.GetHashCode();
hash = hash * 31 + IsSecondaryMemberSurface.GetHashCode();
hash = hash * 31 + (CollidesWith?.GetHashCode() ?? 0);
hash = hash * 31 + SecondaryMemberIdMaps.GetHashCode();
return hash;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
<PackageId>TUnit.Mocks.SourceGenerator</PackageId>
<EnableTrimAnalyzer>false</EnableTrimAnalyzer>
<IsPackable>false</IsPackable>
<!-- Matches TUnit.Mocks.Analyzers: TM diagnostics are not release-tracked. -->
<NoWarn>RS2008</NoWarn>
</PropertyGroup>

<ItemGroup>
Expand Down
Loading
Loading