Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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
91 changes: 55 additions & 36 deletions src/TUnit.Mocks.Analyzers/InaccessibleConstructorMockAnalyzer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ private static void AnalyzeInvocation(SyntaxNodeAnalysisContext context)
return;
}

var target = ResolveMockTarget(context, invocation);
var target = ResolveMockTarget(context, invocation, out var isWrapMock);

if (target is not { TypeKind: TypeKind.Class } namedType)
{
Expand All @@ -46,7 +46,10 @@ private static void AnalyzeInvocation(SyntaxNodeAnalysisContext context)
return;
}

if (HasAccessibleConstructor(namedType, context.Compilation.Assembly))
if (HasAccessibleConstructor(
namedType,
context.Compilation,
requiresFactoryAccessibleParameterTypes: !isWrapMock))
{
return;
}
Expand All @@ -64,8 +67,12 @@ private static void AnalyzeInvocation(SyntaxNodeAnalysisContext context)
/// Resolves the mocked type from either entry point: the generic <c>Mock.Of&lt;T&gt;()</c> /
/// <c>Mock.Wrap&lt;T&gt;()</c> form, or the generated <c>T.Mock()</c> static extension.
/// </summary>
private static INamedTypeSymbol? ResolveMockTarget(SyntaxNodeAnalysisContext context, InvocationExpressionSyntax invocation)
private static INamedTypeSymbol? ResolveMockTarget(
SyntaxNodeAnalysisContext context,
InvocationExpressionSyntax invocation,
out bool isWrapMock)
{
isWrapMock = false;
var symbolInfo = context.SemanticModel.GetSymbolInfo(invocation, context.CancellationToken);

if (symbolInfo.Symbol is not IMethodSymbol methodSymbol)
Expand All @@ -75,6 +82,8 @@ private static void AnalyzeInvocation(SyntaxNodeAnalysisContext context)

if (IsMockEntryPointMethod(methodSymbol))
{
isWrapMock = methodSymbol.Name == "Wrap";

// The multi-type overloads — Of<T1, T2>() through Of<T1, T2, T3, T4>() — return
// Mock<T1>: T1 is the type the impl subclasses, T2..T4 are interfaces layered on it,
// and MockTypeDiscovery reuses T1's constructors for the multi-type model. So the
Expand Down Expand Up @@ -132,21 +141,26 @@ private static bool IsMockEntryPointMethod(IMethodSymbol method)
}

/// <summary>
/// Mirrors the generator's constructor discovery — <c>MemberDiscovery.DiscoverConstructors</c>
/// → <c>IsMemberAccessible</c> → <c>AreMemberSignatureTypesAccessible</c>. A generated subclass
/// can chain to a constructor only when the constructor itself is reachable AND every one of
/// its parameter types is: the generator drops a constructor whose signature mentions an
/// inaccessible type, and a target left with none is exactly the case this rule reports.
/// Keep the two in step; they live in separate assemblies with no shared project.
/// Protected (and protected internal) constructors are reachable precisely because the
/// generated impl derives from the target.
/// Mirrors <c>MemberDiscovery.DiscoverConstructors</c>. The constructor must be reachable from
/// the generated subclass, while every parameter type must also be nameable by its non-derived
/// factory for partial mocks. Wrap factories accept an existing instance, so only subclass
/// accessibility applies there. A target left with no such constructor is exactly the case
/// this rule reports. Keep both implementations in step; they live in separate assemblies
/// with no shared project.
/// </summary>
private static bool HasAccessibleConstructor(INamedTypeSymbol type, IAssemblySymbol compilationAssembly)
private static bool HasAccessibleConstructor(
INamedTypeSymbol type,
Compilation compilation,
bool requiresFactoryAccessibleParameterTypes)
{
return type.InstanceConstructors.Any(ctor => IsChainable(ctor, compilationAssembly));
return type.InstanceConstructors.Any(
ctor => IsChainable(ctor, compilation, requiresFactoryAccessibleParameterTypes));
}

private static bool IsChainable(IMethodSymbol ctor, IAssemblySymbol compilationAssembly)
private static bool IsChainable(
IMethodSymbol ctor,
Compilation compilation,
bool requiresFactoryAccessibleParameterTypes)
{
// DiscoverConstructors rejects private constructors outright, same-assembly or not —
// a subclass can never chain to one.
Expand All @@ -155,8 +169,9 @@ private static bool IsChainable(IMethodSymbol ctor, IAssemblySymbol compilationA
return false;
}

return IsAssemblyReachable(ctor.DeclaredAccessibility, ctor.ContainingAssembly, compilationAssembly)
&& ctor.Parameters.All(p => IsTypeAccessible(p.Type, compilationAssembly));
return IsAssemblyReachable(ctor.DeclaredAccessibility, ctor.ContainingAssembly, compilation.Assembly)
&& (!requiresFactoryAccessibleParameterTypes
|| ctor.Parameters.All(p => IsTypeAccessibleFromAssembly(p.Type, compilation)));
Comment on lines +173 to +174

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Keep rejecting pointer-only wrap constructors

When Mock.Wrap targets an external unsafe class whose only chainable constructor takes a pointer or function-pointer parameter, this bypass treats the constructor as usable and suppresses TM006. The generator still rejects that constructor in MemberDiscovery.IsMemberAccessible because IsTypeAccessible returns false for pointer signatures, leaving an empty constructor model; it consequently emits only the unconstructable stub and never registers a wrap factory, so Mock.Wrap(instance) throws “No wrap mock factory registered” at runtime without the intended diagnostic. Wrap should skip only the assembly-nameability portion of parameter validation, not the intrinsic pointer/function-pointer rejection.

Useful? React with 👍 / 👎.

}

/// <summary>
Expand All @@ -181,31 +196,35 @@ private static bool IsAssemblyReachable(Accessibility accessibility, IAssemblySy
|| declaringAssembly.GivesAccessTo(compilationAssembly);
}

private static bool IsTypeAccessible(ITypeSymbol type, IAssemblySymbol compilationAssembly)
private static bool IsTypeAccessibleFromAssembly(ITypeSymbol type, Compilation compilation)
{
// Type parameters are always accessible.
if (type is ITypeParameterSymbol)
switch (type)
{
return true;
}
case ITypeParameterSymbol:
return true;

// Pointer types can't appear in a generated override signature, even same-assembly.
if (type is IPointerTypeSymbol or IFunctionPointerTypeSymbol)
{
return false;
}
case IPointerTypeSymbol or IFunctionPointerTypeSymbol:
return false;

if (type is IArrayTypeSymbol arrayType)
{
return IsTypeAccessible(arrayType.ElementType, compilationAssembly);
}
case IArrayTypeSymbol array:
return IsTypeAccessibleFromAssembly(array.ElementType, compilation);

if (!IsAssemblyReachable(type.DeclaredAccessibility, type.ContainingAssembly, compilationAssembly))
{
return false;
}
case INamedTypeSymbol named:
if (!compilation.IsSymbolAccessibleWithin(named, compilation.Assembly))
{
return false;
}

return type is not INamedTypeSymbol namedType
|| namedType.TypeArguments.All(arg => IsTypeAccessible(arg, compilationAssembly));
if (named.ContainingType is not null
&& !IsTypeAccessibleFromAssembly(named.ContainingType, compilation))
{
return false;
}

return named.TypeArguments.All(arg => IsTypeAccessibleFromAssembly(arg, compilation));

default:
return true;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -65,15 +65,16 @@ public static string BuildForPartialMock(MockTypeModel model)

var argList = string.Join(", ",
ctor.Parameters.Select(p => p.Name));
var overloadVisibility = ctor.HasPubliclyAccessibleParameterTypes ? visibility : "internal";

using (writer.Block($"{visibility} static global::TUnit.Mocks.Mock<{mockableType}> Mock({parameterListWithoutBehavior})"))
using (writer.Block($"{overloadVisibility} static global::TUnit.Mocks.Mock<{mockableType}> Mock({parameterListWithoutBehavior})"))
{
writer.AppendLine($"return global::TUnit.Mocks.Mock.Of<{mockableType}>({argList});");
}

writer.AppendLine();

using (writer.Block($"{visibility} static global::TUnit.Mocks.Mock<{mockableType}> Mock({parameterListWithBehavior})"))
using (writer.Block($"{overloadVisibility} static global::TUnit.Mocks.Mock<{mockableType}> Mock({parameterListWithBehavior})"))
{
writer.AppendLine($"return global::TUnit.Mocks.Mock.Of<{mockableType}>(behavior, {argList});");
}
Expand Down
27 changes: 23 additions & 4 deletions src/TUnit.Mocks.SourceGenerator/Discovery/MemberDiscovery.cs
Original file line number Diff line number Diff line change
Expand Up @@ -846,19 +846,38 @@ private static string GetAccessorObsoleteAttributeSyntax(string propertyObsolete
}

/// <summary>
/// Discovers all accessible constructors of a class type for partial mock generation.
/// Discovers constructors usable by the generated subclass. Partial-mock factories also need
/// to name every parameter type; wrap factories only accept an existing instance.
/// </summary>
public static EquatableArray<MockConstructorModel> DiscoverConstructors(INamedTypeSymbol typeSymbol, IAssemblySymbol? compilationAssembly = null)
public static EquatableArray<MockConstructorModel> DiscoverConstructors(
INamedTypeSymbol typeSymbol,
Compilation compilation,
bool requiresFactoryAccessibleParameterTypes)
{
var constructors = new List<MockConstructorModel>();

foreach (var ctor in typeSymbol.InstanceConstructors)
{
if (ctor.DeclaredAccessibility == Accessibility.Private) continue;
if (!IsMemberAccessible(ctor, compilationAssembly)) continue;
if (ctor.DeclaredAccessibility == Accessibility.Private)
{
continue;
}

if (!IsMemberAccessible(ctor, compilation.Assembly))
{
continue;
}

if (requiresFactoryAccessibleParameterTypes
&& !ctor.Parameters.All(p => TypeAccessibility.IsAccessibleFromAssembly(p.Type, compilation)))
Comment on lines +874 to +875

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Report rejected constructors on GenerateMock attributes

When [assembly: GenerateMock(typeof(T))] targets a class whose constructors are all removed by this parameter-accessibility filter, the model reaches LacksAccessibleConstructor and emits only an unregistered stub. TM006 cannot explain the failure because its analyzer visits only invocation expressions, so attribute-only generation succeeds silently and later runtime-driven or generic Mock.Of<T> calls fail with “No mock factory registered.” Report TM006 at the stored attribute location instead of silently accepting this request.

Useful? React with 👍 / 👎.

{
continue;
}

constructors.Add(new MockConstructorModel
{
HasPubliclyAccessibleParameterTypes = ctor.Parameters.All(
p => TypeAccessibility.IsEffectivelyPublic(p.Type)),
Parameters = new EquatableArray<MockParameterModel>(
ctor.Parameters.Select(p => new MockParameterModel
{
Expand Down
68 changes: 20 additions & 48 deletions src/TUnit.Mocks.SourceGenerator/Discovery/MockTypeDiscovery.cs
Original file line number Diff line number Diff line change
Expand Up @@ -106,13 +106,15 @@ public static ImmutableArray<MockTypeModel> TransformToModels(GeneratorSyntaxCon
if (namedType.TypeKind != TypeKind.Class || namedType.IsSealed || namedType.IsValueType)
return ImmutableArray<MockTypeModel>.Empty;

// Wrap uses the same model as partial mock but with IsWrapMock flag
var wrapModel = BuildSingleTypeModel(namedType, isPartialMock: true, compilationAssembly, compilation);
var wrapModel = BuildSingleTypeModel(
namedType,
isPartialMock: true,
compilationAssembly,
compilation,
isWrapMock: true);
if (wrapModel is null)
return ImmutableArray<MockTypeModel>.Empty;

// Set IsWrapMock flag
wrapModel = wrapModel with { IsWrapMock = true };
return ImmutableArray.Create(wrapModel);
}

Expand Down Expand Up @@ -193,7 +195,7 @@ public static ImmutableArray<MockTypeModel> TransformToModels(GeneratorSyntaxCon
HasStaticAbstractMembers = methods.Any(m => m.IsStaticAbstract) || properties.Any(p => p.IsStaticAbstract) || events.Any(e => e.IsStaticAbstract),
// The secondary setup extensions surface additional-interface types in public
// signatures, so the whole multi model must drop to internal if ANY type is.
IsPublic = IsEffectivelyPublic(namedType) && additionalTypes.All(IsEffectivelyPublic),
IsPublic = TypeAccessibility.IsEffectivelyPublic(namedType) && additionalTypes.All(TypeAccessibility.IsEffectivelyPublic),
UseFallbackNamespace = singleTypeModel.UseFallbackNamespace
};

Expand Down Expand Up @@ -390,7 +392,7 @@ private static bool HasStaticAbstractMembers(INamedTypeSymbol interfaceType)
Properties = EquatableArray<MockMemberModel>.Empty,
Events = EquatableArray<MockEventModel>.Empty,
AllInterfaces = EquatableArray<string>.Empty,
IsPublic = IsEffectivelyPublic(delegateType),
IsPublic = TypeAccessibility.IsEffectivelyPublic(delegateType),
UseFallbackNamespace = MockNamespaceConflictDetector.HasConflict(compilation, delegateType),
};
}
Expand All @@ -414,7 +416,12 @@ private static ImmutableArray<MockTypeModel> BuildModelWithTransitiveDependencie
return builder.MoveToImmutable();
}

private static MockTypeModel? BuildSingleTypeModel(INamedTypeSymbol namedType, bool isPartialMock, IAssemblySymbol? compilationAssembly, Compilation compilation)
private static MockTypeModel? BuildSingleTypeModel(
INamedTypeSymbol namedType,
bool isPartialMock,
IAssemblySymbol? compilationAssembly,
Compilation compilation,
bool isWrapMock = false)
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
{
// An interface with abstract members this compilation can't access (e.g. `internal`
// members declared in another assembly) cannot be implemented by any type we could emit,
Expand All @@ -429,7 +436,10 @@ private static ImmutableArray<MockTypeModel> BuildModelWithTransitiveDependencie

// Discover constructors for partial mocks of classes
var constructors = isPartialMock && namedType.TypeKind == TypeKind.Class
? MemberDiscovery.DiscoverConstructors(namedType, compilationAssembly)
? MemberDiscovery.DiscoverConstructors(
namedType,
compilation,
requiresFactoryAccessibleParameterTypes: !isWrapMock)
: EquatableArray<MockConstructorModel>.Empty;

return new MockTypeModel
Expand All @@ -441,6 +451,7 @@ private static ImmutableArray<MockTypeModel> BuildModelWithTransitiveDependencie
IsInterface = namedType.TypeKind == TypeKind.Interface,
IsAbstract = namedType.IsAbstract,
IsPartialMock = isPartialMock,
IsWrapMock = isWrapMock,
TypeParameters = new EquatableArray<MockTypeParameterModel>(GetTypeParameterModels(namedType)),
Methods = methods,
Properties = properties,
Expand All @@ -452,7 +463,7 @@ private static ImmutableArray<MockTypeModel> BuildModelWithTransitiveDependencie
),
Constructors = constructors,
HasStaticAbstractMembers = methods.Any(m => m.IsStaticAbstract) || properties.Any(p => p.IsStaticAbstract) || events.Any(e => e.IsStaticAbstract),
IsPublic = IsEffectivelyPublic(namedType),
IsPublic = TypeAccessibility.IsEffectivelyPublic(namedType),
UseFallbackNamespace = MockNamespaceConflictDetector.HasConflict(compilation, namedType)
};
}
Expand Down Expand Up @@ -516,45 +527,6 @@ private static bool ContainsTypeParameters(ITypeSymbol type)
};
}

/// <summary>
/// True if every part of <paramref name="type"/>'s signature is publicly accessible: the
/// type itself, every enclosing type, and (recursively) every generic type argument and
/// array element. Mock wrappers built for types that are not effectively public must
/// themselves be emitted as <c>internal</c> to avoid CS9338 / CS0051 — including the
/// case where a public generic interface is closed over an internal type argument
/// (e.g. <c>ILogger&lt;InternalClass&gt;</c>). See issues #5426 and #5453.
/// </summary>
private static bool IsEffectivelyPublic(ITypeSymbol type)
{
switch (type)
{
case ITypeParameterSymbol:
// Bound at use site by the consumer; not the discovery point's concern.
return true;

case IArrayTypeSymbol array:
return IsEffectivelyPublic(array.ElementType);

case INamedTypeSymbol named:
for (INamedTypeSymbol? t = named; t is not null; t = t.ContainingType)
{
if (t.DeclaredAccessibility != Accessibility.Public)
return false;
}
foreach (var typeArg in named.TypeArguments)
{
if (!IsEffectivelyPublic(typeArg))
return false;
}
return true;

default:
// Pointers, function pointers, dynamic, error types — not expected in
// mockable signatures.
return true;
}
}

// ─── T.Mock() static extension discovery ─────────────────────────

/// <summary>
Expand Down
Loading
Loading