fix(mocks): report TM006 instead of CS1729 for unsubclassable classes

[thomhurst]

Fixes #6493

Problem

T.Mock() on a class whose constructors are all inaccessible produced:

QueueRuntimeProperties_MockImplFactory.g.cs(15,18): error CS1729:
'QueueRuntimeProperties' does not contain a constructor that takes 0 arguments

MemberDiscovery.DiscoverConstructors only collects accessible constructors, so the model ended up with an empty Constructors array and MockFactoryBuilder.GenerateConstructorDispatch fell through to new XMockImpl(engine). The impl derives from the target, and its implicit base() call has nothing to bind to. The error pointed into generated code with no link back to the call site.

Such a type genuinely cannot be mocked: every subclass constructor must chain to a base constructor, so with all of them private (or internal in another assembly) no subclass can be declared at all. The right outcome is a clear diagnostic, not generated code that cannot compile.

Fix

• MockTypeModel.LacksAccessibleConstructor — true for a class target with no chainable constructor.
• MockGenerator skips impl/factory/members for those targets and emits only the static Mock() entry point, so the call site still binds and the diagnostic below is the single error the user sees.
• New TM006 + InaccessibleConstructorMockAnalyzer, reported at the call site for Mock.Of<T>(), Mock.Wrap<T>() and the generated T.Mock() form. The message points at the library's own factory (e.g. Azure's ServiceBusModelFactory) as the way to build such a value.

Protected and protected-internal constructors remain chainable — the generated impl derives from the target — so abstract classes are unaffected. Sealed and value types are left to TM001/TM002 rather than double-reported.

Tests

tests/TUnit.Mocks.Analyzers.Tests/InaccessibleConstructorMockAnalyzerTests.cs — 9 cases: private-only ctor via all three entry points, plus non-reporting cases for public / implicit parameterless / protected / same-assembly internal constructors, interfaces, and sealed types.

Analyzer tests: 39 passed. TUnit.Mocks.Tests: 1160 passed. Generator snapshots: 80 passed, unchanged.

[greptile-apps]

Greptile Summary

This PR replaces generated-code constructor failures with a call-site diagnostic for classes that cannot be subclassed.

• Adds TM006 and analyzes Mock.Of, Mock.Wrap, and generated static Mock entry points.
• Skips implementation and factory generation when no chainable constructor exists while retaining the static entry-point stub.
• Adds analyzer coverage for constructor accessibility and updates the cross-assembly analyzer verifier documentation.

Confidence Score: 3/5

The PR is not yet safe to merge because generated Mock entry-point matching can still apply TM006 to unrelated consumer code.

The analyzer continues to identify generated entry points through a spoofable namespace and containing-type suffix, then treats the invocation receiver as a mock target and can emit a compilation-blocking diagnostic.

Files Needing Attention: src/TUnit.Mocks.Analyzers/InaccessibleConstructorMockAnalyzer.cs

Important Files Changed

Filename	Overview
src/TUnit.Mocks.Analyzers/InaccessibleConstructorMockAnalyzer.cs	Adds TM006 target resolution and constructor eligibility checks; the previously reported generated-entry identity issue remains unresolved.
src/TUnit.Mocks.Analyzers/Rules.cs	Defines the TM006 error descriptor and guidance.
src/TUnit.Mocks.SourceGenerator/MockGenerator.cs	Emits only the static mock entry-point stub for classes lacking an accessible constructor.
src/TUnit.Mocks.SourceGenerator/Models/MockTypeModel.cs	Adds the model predicate used to identify class targets without discovered constructors.
tests/TUnit.Mocks.Analyzers.Tests/InaccessibleConstructorMockAnalyzerTests.cs	Covers inaccessible and chainable constructor cases across the supported mock entry points.
tests/TUnit.Mocks.Analyzers.Tests/Verifiers/CSharpAnalyzerVerifier`1.cs	Clarifies cross-assembly accessibility coverage in the verifier documentation.

_{Reviews (4): Last reviewed commit: "fix(mocks): report TM006 for the multi-t..." | Re-trigger Greptile}

[thomhurst]

@claude @codex could we use UnsafeAccessor to allow mocking these? It would be good if we could still actually mock them - unsafe accessory gives us reflection-like access, but should also work for aot?

[chatgpt-codex-connector]

To use Codex here, create an environment for this repo.

[claude]

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

---

Tasks

• Read the trigger comment and understand the question
• Review current mock architecture (MockGenerator, MockTypeModel.LacksAccessibleConstructor, InaccessibleConstructorMockAnalyzer)
• Check whether Mock.Wrap<T>() also relies on subclassing
• Post final analysis

Answer: UnsafeAccessor doesn't fix this specific case

I looked at how the generated mock is built (MockImplBuilder.cs:44, :104): both Mock.Of<T>() and Mock.Wrap<T>() emit a file sealed class XMockImpl : T — a genuine subclass of the target, so overridden virtual members actually intercept calls. That's the crux of why this doesn't extend to UnsafeAccessor.

UnsafeAccessor lets your own code call an otherwise-inaccessible member (private ctor/method/field) via a compiler/runtime-generated stub — it's a member-access bypass, and it works great for constructing a standalone instance of a type whose constructors are all private.

But that's not what's needed here. The problem isn't "call the private constructor" — it's that XMockImpl's own constructor is itself a constructor of a type deriving from T, and every derived-type constructor must chain to a T constructor as part of its own definition (base(...)). That chaining call is checked for accessibility by the CLR the same way any other call is (this isn't just a C# compiler rule — it's enforced by the type system, because constructing a Derived requires initializing the Base slice in-place as part of the same object). UnsafeAccessor generates a separate stub method that returns a new, standalone T instance — it has no way to serve as the implicit base-chain call inside XMockImpl's constructor. There's no C# (or verifiable IL) syntax that says "construct my base part via this arbitrary accessor instead of an accessible base constructor."

So: if a class truly has zero constructor a derived type could reach (all private, or internal from another assembly), no subclass of it can exist, full stop — not via reflection, not via UnsafeAccessor, not by hand-writing the IL. The only way around that is unverifiable IL emission (Reflection.Emit with skip-visibility trust), which isn't AOT-compatible and isn't something we'd want to ship anyway.

Where UnsafeAccessor could help is a fundamentally different feature: constructing a real, non-overridable instance of such a type (bypassing ctor visibility) instead of a mock — useful for a "just give me a working instance to pass around" case, but that's not mocking (no virtual-member interception), so it doesn't replace the diagnostic here. It'd be a separate, additive feature if there's appetite for it (e.g. Mock.Wrap already requires a real instance to be passed in — for types like QueueRuntimeProperties where even that can't be constructed by user code, an UnsafeAccessor-based helper could produce the real instance to wrap, if wrapping without overriding is still useful). That's out of scope for this PR but could be a good follow-up issue if you want the "still get a real object, just can't override its methods" experience for these specific unmockable types.

For this PR, TM006 (reporting at the call site instead of a broken CS1729 in generated code) remains the right fix for the actual bug.

[claude]

Review

Good fix overall — replacing a confusing generated-code CS1729 with a targeted diagnostic at the call site is the right shape for this problem, and the entry-point coverage (Mock.Of<T>(), Mock.Wrap<T>(), generated T.Mock()) plus the sealed/struct double-report guard is well thought through.

One correctness gap and one coverage gap worth addressing before merge:

1. Analyzer's chainability check diverges from the generator's — reintroduces the same failure mode for a narrower case

InaccessibleConstructorMockAnalyzer.IsChainable (src/TUnit.Mocks.Analyzers/InaccessibleConstructorMockAnalyzer.cs:124-139) only checks a constructor's own DeclaredAccessibility + assembly visibility. But the generator's real notion of "usable constructor" — MemberDiscovery.DiscoverConstructors → IsMemberAccessible → AreMemberSignatureTypesAccessible / IsTypeAccessible (src/TUnit.Mocks.SourceGenerator/Discovery/MemberDiscovery.cs:848-876, 553-634) — also requires every constructor parameter type to be accessible from the compilation assembly.

Concretely: a class with a single public constructor whose parameter type is internal in another assembly (a real pattern — some libraries use an internal "capability" parameter to gate public-looking constructors) will:

• Generator side: DiscoverConstructors filters that ctor out (param type inaccessible) → Constructors.Length == 0 → LacksAccessibleConstructor == true → only the static stub is emitted, which forwards to Mock.Of<T>().
• Analyzer side: IsChainable sees a public ctor and returns true → no TM006 is reported.
• Runtime: Mock.Of<T>() finds no registered factory (MockRegistry.TryGetFactory fails, src/TUnit.Mocks/Mock.cs:70-77) and throws InvalidOperationException: No mock factory registered for type ....

So for this case the user gets exactly the kind of unclear, disconnected-from-the-call-site failure this PR sets out to eliminate — just moved from a compile-time CS1729 to a runtime exception, with no analyzer warning in between.

This is a structural risk, not just a missed edge case: the analyzer and generator now each hand-maintain their own copy of "is this constructor chainable," and they've already drifted apart on day one. Rather than porting AreMemberSignatureTypesAccessible/IsTypeAccessible into the analyzer as a second copy, it'd be worth extracting the shared predicate (constructor accessibility + parameter-type accessibility) into one place both projects reference — e.g. a small shared source file/project linked into both TUnit.Mocks.Analyzers and TUnit.Mocks.SourceGenerator (both are already netstandard2.0 Roslyn components, so linking a shared .cs file via <Compile Include> costs nothing). That guarantees the two can't silently diverge again the next time either side's logic changes.

2. The primary target scenario (T.Mock()) isn't actually tested against the real generated shape

Generated_Static_Mock_Entry_Point_Reports_TM006 (tests/TUnit.Mocks.Analyzers.Tests/InaccessibleConstructorMockAnalyzerTests.cs:274-306) says in its own comment that the real generated entry point is a C# 14 extension(...) block (confirmed in MockStaticExtensionBuilder.BuildCore, src/TUnit.Mocks.SourceGenerator/Builders/MockStaticExtensionBuilder.cs:100-106), but the test harness's Roslyn version doesn't support extension blocks yet, so the test substitutes a plain static method with a matching name/namespace/staticness instead.

That's a reasonable stopgap, but it means ResolveMockTarget's namespace-based matching (chosen specifically to cope with the "compiler-synthesised nested extension type" shape, per its own comment) is unverified against the actual symbol shape Roslyn produces for extension members — which can differ from a plain static method (synthesized containing/marker types, ReducedFrom, etc.). Since this is the exact path from the original bug report (T.Mock() → CS1729), it'd be worth locking it in with real coverage — e.g. an end-to-end test that runs the actual source generator output through the analyzer, or bumping/adding a second test harness on a newer Roslyn/LangVersion that supports extension syntax — rather than relying on manual verification (per the PR description) staying correct across future refactors.

Nothing else stood out — Rules.cs addition, the sealed/value-type exclusion to avoid double-reporting with TM001/TM002, and the LacksAccessibleConstructor model property all look correct and well-documented.

[thomhurst]

Thanks — finding 1 is a real divergence and is fixed in 2985b13. One correction on the worked example, and a push-back on the extraction.

1. Divergence fixed; the specific repro isn't constructible though. IsChainable now applies the same signature-type rule as AreMemberSignatureTypesAccessible / IsTypeAccessible (arrays, generic type arguments and pointers included), so the analyzer and DiscoverConstructors can no longer disagree.

I did try to pin it with the scenario you described — a public constructor whose parameter type is internal in another assembly — and it can't exist: the declaring assembly won't compile it.

Library.cs(9,16): error CS0051: Inconsistent accessibility: parameter type 'Capability' is less
accessible than method 'GatedService.GatedService(Capability)'

C# requires a constructor to be no more accessible than its parameter types, so for constructors the parameter-type gate can only fire where the constructor itself is already gated (internal, private protected) and therefore already reported. I kept the check anyway — it costs nothing, removes the divergence, and the rule is shared with member signatures where it does bite — but I couldn't write a failing test for it, so the test I added instead is the cross-assembly internal constructor: the actual Azure QueueRuntimeProperties shape, which the previous same-assembly-private tests could only approximate. Plus a public-constructor-with-public-parameter case guarding the new check against false positives.

2. On the real T.Mock() extension shape. Agreed it's the weakest point, and I'd take real coverage if it were available. It isn't within this PR: the analyzer test harness resolves Microsoft.CodeAnalysis.CSharp.Workspaces 4.12.0 / Microsoft.CodeAnalysis.Common 4.7.0 from Directory.Packages.props, which is repo-wide and also the floor the shipped analyzer and generator compile against. Bumping it to a Roslyn that parses extension(...) blocks raises the minimum Roslyn for consumers, which I don't think belongs in a bug-fix PR. Running generator output through the analyzer in TUnit.Mocks.SourceGenerator.Tests hits the same pinned Roslyn, and the only real-SDK project (TUnit.Mocks.Tests) can't host a TM006 trigger because TM006 is an error and would break the build.

What I do have is a real-SDK verification: with the branch built into TUnit.Mocks.Tests, both entry points report and the CS1729 is gone —

Issue6493Repro.cs(17,20): error TM006: Cannot mock 'NoAccessibleCtor' ...   // NoAccessibleCtor.Mock()
Issue6493Repro.cs(18,21): error TM006: Cannot mock 'NoAccessibleCtor' ...   // Mock.Of<NoAccessibleCtor>()

That's also what drove the namespace-based matching in the first place: matching on ContainingType.Name.EndsWith("_MockStaticExtension") did not fire against the real extension-block symbol, only the namespace check did. Happy to open a follow-up issue for a newer-Roslyn analyzer test harness if you'd like that tracked.

Analyzer tests 41 passed, TUnit.Mocks.Tests 1160, generator snapshots 80.

[thomhurst]

could we use UnsafeAccessor to allow mocking these? ... unsafe accessor gives us reflection-like access, but should also work for aot?

I tested both halves of this against the real Azure package. Short answer: it can't unlock mocking, but it can unlock a genuinely useful stub/value-builder — and yes, it's AOT-safe.

It doesn't help mocking, for two independent reasons.

1. The blocker is subclassing, not construction. TUnit.Mocks intercepts by emitting a subclass, and every derived constructor must chain to an accessible base constructor. UnsafeAccessorKind.Constructor produces a factory-shaped method (it newobjs and returns an instance) — it can't stand in for a base(...) initializer, and the generator emits C#, which always emits that call. So the impl still won't compile:

Sub.cs(7,12): error CS1729: 'QueueRuntimeProperties' does not contain a constructor that takes 0 arguments

2. Even with a subclass there'd be nothing to intercept. Reading the metadata for QueueRuntimeProperties:

TYPE QueueRuntimeProperties attrs=Public, BeforeFieldInit
  methods: virtual=0 nonvirtual=23
  properties: Name:Assembly, TotalMessageCount:Assembly, ActiveMessageCount:Assembly, ... (all internal setters)

Zero virtual members. A subclass could override nothing, so .Returns(...) would have no slot to attach to regardless of how we got an instance.

Where it does pay off. What people actually want from these types is a value to hand back from a mocked GetQueueRuntimePropertiesAsync. UnsafeAccessor does that cleanly — internal constructor plus internal setters, no backing-field name guessing:

[UnsafeAccessor(UnsafeAccessorKind.Constructor)]
static extern QueueRuntimeProperties NewProps(string name);

[UnsafeAccessor(UnsafeAccessorKind.Method, Name = "set_ActiveMessageCount")]
static extern void SetActive(QueueRuntimeProperties props, long value);

var p = NewProps("my-queue");
SetActive(p, 42);
// prints: OK Name=my-queue ActiveMessageCount=42

That ran clean against Azure.Messaging.ServiceBus 7.20.2. It's AOT-safe — the runtime/ILC binds these at link time, which is exactly why it's the sanctioned replacement for private reflection.

Caveats worth pricing in before committing to it: it needs #if NET8_0_OR_GREATER (TUnit.Mocks also targets netstandard2.0, and consumer-side TFM guards matter here — see the net8.0 issue on #6503); it deliberately bypasses accessibility, so a library renaming an internal member breaks consumers at runtime rather than compile time; and the result is a stub, not a mock — no verification, no setups.

So I'd frame it as a separate feature — something like T.Stub() returning a configurable instance of an otherwise-unconstructible type — rather than something that changes TM006. TM006 is still the right answer for T.Mock() on these types, because mocking them genuinely cannot work. Happy to open an issue for the stub feature with the above as the feasibility evidence if you want to pursue it.

[claude]

Review

Re-reviewed at c5b3c2c (latest commit). Both findings from the previous review round are properly addressed: the analyzer's IsChainable now mirrors the generator's parameter-type-accessibility rule (2985b13), and the namespace-suffix walk-up in IsGeneratedStaticEntryPoint correctly matches the real generated container name ({name}_MockStaticExtension, per MockStaticExtensionBuilder.BuildCore) rather than a plain static-method shape. Good pushback on the "public ctor with internal parameter type" scenario too — that's correctly unconstructible in C#.

One new gap worth fixing before merge:

TM006 doesn't cover the multi-type Mock.Of<T1, T2>() family

ResolveMockTarget only resolves a target when the invoked method has exactly one type argument:

return methodSymbol.TypeArguments.Length == 1
    ? methodSymbol.TypeArguments[0] as INamedTypeSymbol
    : null;

Mock.Of<T1, T2>(), Of<T1, T2, T3>() and Of<T1, T2, T3, T4>() are real public overloads (src/TUnit.Mocks/Mock.cs:123-181), and T1 — the first type argument — is always the concrete/primary type being subclassed; T2..T4 are interfaces layered on top (MockTypeDiscovery validates this and reuses singleTypeModel.Constructors for the multi-type model). So when T1 lacks an accessible constructor, the generator correctly skips codegen (same LacksAccessibleConstructor branch), but the analyzer silently returns null for these calls and reports nothing. At runtime, Mock.Of<T1,T2>() → OfMulti<T1> finds no registered factory and throws InvalidOperationException: No mock factory registered for type ... — which is exactly the "failure disconnected from the call site" problem this PR sets out to eliminate, just shifted from a compile-time CS1729 to a runtime exception for this one entry point.

Fix is small: use methodSymbol.TypeArguments[0] whenever there's at least one type argument, instead of requiring exactly one. Worth a test case alongside the existing 9 (Mock.Of<RuntimeProperties, ISomeInterface>() reporting TM006).

Minor (non-blocking): naming-convention-based generated-code detection

IsGeneratedStaticEntryPoint identifies "did the generator emit this Mock()" by string-matching a namespace + _MockStaticExtension suffix convention. It's correctly aligned with today's generator output, but nothing enforces that the two stay in sync — a future rename of the suffix in MockStaticExtensionBuilder silently reopens the false-negative gap Greptile flagged, and (in the other direction) a consumer type that happens to match the convention still spoofs a diagnostic. Since the generator and analyzer are both authored in this repo, a compiler-emitted marker (e.g. an internal marker attribute or [EditorBrowsable(Never)]-style sentinel stamped on the generated extension class) would let the analyzer check "is this symbol actually ours" definitively instead of by convention — removing the coupling-by-string-literal between the two projects. Not blocking; today's approach is a reasonable pragmatic tradeoff given the Roslyn-version constraint already discussed in this thread.

Nothing else stood out — LacksAccessibleConstructor gating (including its interaction with IsWrapMock, which also sets IsPartialMock = true and hits the same branch first) is correctly ordered, and the sealed/value-type double-report guard is unchanged and correct.

[thomhurst]

Fixed in 9c17356. The multi-type gap is real — verified end to end rather than taken on the review's word:

• Mock.Of<T1, T2>() … Of<T1, T2, T3, T4>() all return Mock<T1> and route to OfMulti<T1> (src/TUnit.Mocks/Mock.cs:123-181).
• MockTypeDiscovery builds the multi-type model with Constructors = singleTypeModel.Constructors (Discovery/MockTypeDiscovery.cs:192), i.e. T1's constructors verbatim — so LacksAccessibleConstructor fires on exactly the same condition as the single-type model and generation is skipped.
• The analyzer's TypeArguments.Length == 1 gate therefore let precisely those calls through with no diagnostic, converting a compile-time error into a runtime InvalidOperationException.

ResolveMockTarget now takes TypeArguments[0] whenever there's at least one type argument, with a comment recording why the first argument is the constructor-bearing target for every overload. Added Multi_Type_Of_With_Inaccessible_Primary_Constructor_Reports_TM006 covering Mock.Of<RuntimeProperties, ISomeInterface>(); the suite is 12/12 green.

On the marker-attribute suggestion for IsGeneratedStaticEntryPoint: agreed in principle, but it's a cross-project contract change (the generator would have to stamp a marker TUnit.Mocks declares, and the analyzer resolve it by symbol) that touches every generated static extension and its snapshots. That's a wider change than this fix warrants — leaving the convention match, which is exercised by Unrelated_Static_Mock_Method_Does_Not_Report.

Also rebased onto main to clear the conflict (Rules.cs — TM006 and TM007 landed in the same slot; both descriptors kept).

[claude]

Review

Re-reviewed at 9c17356 (latest commit). This addresses the only outstanding gap from the previous round.

Multi-type Mock.Of<T1, T2>() family now reports TM006. ResolveMockTarget (src/TUnit.Mocks.Analyzers/InaccessibleConstructorMockAnalyzer.cs:75-82) now accepts TypeArguments.Length >= 1 instead of requiring exactly one, taking TypeArguments[0] as the target. Verified this is correct against the actual overload set in src/TUnit.Mocks/Mock.cs:123-181: every Of<T1, T2>() … Of<T1, T2, T3, T4>() overload returns Mock<T1>, and MockTypeDiscovery reuses T1's constructors for the multi-type model — so T1 is the right (and only) constructor-bearing target regardless of arity. IsMockEntryPointMethod still scopes this to Mock/MockRepository.Of/Wrap generic methods in the TUnit.Mocks namespace, so widening == 1 to >= 1 doesn't risk false positives on unrelated generic calls.

Ran the new test (Multi_Type_Of_With_Inaccessible_Primary_Constructor_Reports_TM006) locally across net8.0/net9.0/net10.0 — all pass.

No new issues found. The remaining architectural note from the previous round (naming-convention-based detection in IsGeneratedStaticEntryPoint, coupling the analyzer to the generator's _MockStaticExtension suffix by string) was already flagged as non-blocking and still stands as a reasonable tradeoff given the pinned-Roslyn constraint discussed earlier in this thread — no action needed for this PR.

Good to merge from my side.