fix: resolve CS0460/CS0122/CS0115 when mocking concrete classes from external assemblies

[thomhurst]

Summary

Fixes #5308 — TUnit.Mocks now correctly generates mock subclasses for concrete classes from external assemblies (like Azure SDK's TableClient, BlobClient).

Three bugs in the source generator caused compile errors:

• CS0460: GeneratePartialMethod and GenerateWrapMethod emitted generic constraint clauses on override methods — C# prohibits restating constraints on overrides
• CS0122: MemberDiscovery included internal virtual members from external assemblies that the consumer assembly can't access
• CS0115: Generated override for methods that aren't actually overridable from outside the assembly (internal accessibility or signatures using internal types)

Changes

File	Change
MockImplBuilder.cs	Remove constraint clauses from override method signatures
MemberDiscovery.cs	Add cross-assembly accessibility filtering using IAssemblySymbol.GivesAccessTo; filter members whose signatures reference inaccessible types; hoist per-level assembly checks out of per-member loop
MockTypeDiscovery.cs	Thread compilationAssembly through BuildSingleTypeModel, DiscoverTransitiveInterfaceTypes, and all callers
SnapshotTestBase.cs	Consolidate RunGenerator overloads; add CreateExternalAssemblyReference for external assembly simulation in tests
MockGeneratorTests.cs	5 new snapshot tests covering all three bug categories for both partial and wrap mock paths

Test plan

• 21/21 snapshot tests pass (16 existing + 5 new)
• 672/672 integration tests pass (zero regressions)
• Verify with actual Azure SDK packages (Azure.Data.Tables, Azure.Storage.Blobs)

[claude]

Code Review: fix CS0460/CS0122/CS0115 when mocking concrete classes from external assemblies

The three bugs are genuine and the fixes are correct. This is solid work overall. Some observations below.

---

Bug 1 — CS0460: Constraint clauses on override ✓

Correct fix. C# spec §13.2.6 explicitly prohibits re-stating where constraints on an override. Dropping GetConstraintClauses from GenerateWrapMethod and GeneratePartialMethod is the right change.

Nit: GetConstraintClauses is still correctly used in GenerateInterfaceMethod (interface implementations are not overrides). Since the two paths are easy to conflate in future edits, a brief comment on GetConstraintClauses noting "only valid for non-override declarations" would serve as a guard-rail.

---

Bug 2 — CS0122: Internal members from external assemblies ✓

IsMemberAccessibleFromExternal and IsMemberAccessible are both logically sound.

Observation — ProtectedOrInternal vs ProtectedAndInternal:
The filter correctly blocks ProtectedAndInternal (private protected) when hasInternalAccess is false, and correctly passes ProtectedOrInternal (protected internal) through since the protected part is unconditional for subclasses. The generated protected override string ProtectedInternalMethod() in the snapshot confirms this. Minor suggestion: the comment saying "internal virtual methods" could also mention private protected for the ProtectedAndInternal case for clarity.

---

Bug 3 — CS0115: Members with internal-type signatures ✓

AreMemberSignatureTypesAccessible walking parameters and return types via IsTypeAccessible (with recursive generic argument handling) is the correct approach.

Minor gap — pointer/function pointer types:
IsTypeAccessible handles ITypeParameterSymbol, IArrayTypeSymbol, and INamedTypeSymbol but not IPointerTypeSymbol or IFunctionPointerTypeSymbol. For unsafe methods with pointer-typed parameters, the method returns true (accessible), potentially including a non-overridable member. This is unlikely to matter in practice since virtual methods with pointer signatures are very rare, but worth a comment noting the assumption.

---

Architecture Concern — compilationAssembly propagation gap

Medium severity. BuildSingleTypeModel gets an optional compilationAssembly = null parameter. The null default creates a latent risk: any call-site that omits it silently skips the accessibility filter. One such gap already exists:

DiscoverTransitiveInterfaceTypes calls BuildSingleTypeModel(namedReturn, isPartialMock: false) without compilationAssembly. This is low-risk today because interfaces cannot have internal virtual members visible cross-assembly, but it leaves an inconsistency that could silently break as the code evolves.

Suggested approach: Remove the default value from the internal-facing BuildSingleTypeModel signature so the compiler enforces that all call-sites explicitly pass compilationAssembly. Thread it through DiscoverTransitiveInterfaceTypes too. The null default should only exist at the outermost public entry point (if needed). Making the parameter non-optional turns a silent runtime omission into a compile-time error — the right tradeoff here.

---

Two near-identical accessibility helpers

IsMemberAccessibleFromExternal and IsMemberAccessible implement overlapping logic. The comment explains the split (hoisting per-level assembly identity check out of the inner loop vs. full per-constructor check), and the performance rationale is sound. The naming is clear. This is fine.

---

Missing snapshot test: Wrap + external assembly

The Partial_Mock_Filters_Internal_Virtual_Members_From_External_Assembly test covers the partial-mock path. The wrap-mock path shares the same DiscoverMembers call, so it should behave identically — but there's no wrap-mock equivalent test. Worth adding to guard against regression if the paths are ever diverged.

---

Test infrastructure refactoring ✓

Making RunGeneratorAndFormat and VerifySnapshot private and exposing two VerifyGeneratorOutput overloads is a clean API shape.

Nit: CreateExternalAssemblyReference uses System.IO.MemoryStream with a fully-qualified prefix rather than a using System.IO; directive — inconsistent with the rest of the file, but purely cosmetic.

---

Summary

Severity	Item
Medium	DiscoverTransitiveInterfaceTypes calls BuildSingleTypeModel without compilationAssembly, silently skipping accessibility filter — recommend removing the null default to make omissions a compile error
Low	IsTypeAccessible does not handle IPointerTypeSymbol/IFunctionPointerTypeSymbol for unsafe method signatures
Low	No wrap-mock + external assembly snapshot test
Nit	GetConstraintClauses missing comment that it must not be used on override declarations
Nit	Inconsistent fully-qualified System.IO.MemoryStream in CreateExternalAssemblyReference

The core fixes are correct and the PR resolves the three reported compiler errors. The main item worth addressing before merge is making the compilationAssembly propagation in BuildSingleTypeModel non-optional so future call-sites cannot accidentally skip the filter.

[claude]

Code Review: fix CS0460/CS0122/CS0115 for external-assembly concrete class mocks

The three bugs are genuine and the root-cause analysis is correct. Overall this is solid, targeted work. Below is a detailed review organized by file and severity.

---

MockImplBuilder.cs — CS0460 fix (constraint clauses on override)

Correct. Removing GetConstraintClauses from GenerateWrapMethod and GeneratePartialMethod is exactly right — C# §13.2.6 prohibits repeating where clauses on override declarations. The comment // C# prohibits restating generic constraints on override methods (CS0460) at both call sites is a good guard-rail.

Observation — GetConstraintClauses name doesn't signal its restriction:
The method name alone doesn't signal the "only for non-override declarations" restriction. Consider renaming to GetConstraintClausesForNonOverride — it costs nothing and makes misuse obvious at the call site instead of requiring someone to read the comment.

Observation — no comment on property/indexer methods:
Properties and indexers go through GenerateWrapProperty/GeneratePartialProperty, neither of which emits constraint clauses (correct). A brief comment there mirroring the method case would prevent a future maintainer from thinking it was accidentally omitted.

---

MemberDiscovery.cs — CS0122 / CS0115 fix

Subtle correctness issue — compilationAssembly! null-forgiving operator

var isExternalType = compilationAssembly is not null
    && !SymbolEqualityComparer.Default.Equals(current.ContainingAssembly, compilationAssembly);
var hasInternalAccess = isExternalType
    && current.ContainingAssembly.GivesAccessTo(compilationAssembly!);

compilationAssembly! is safe only because isExternalType being false short-circuits it. This is fragile — if someone rewrites the condition slightly, you get a NullReferenceException at runtime. A more defensive form removes the null-forgiving operator entirely:

var hasInternalAccess = isExternalType
    && compilationAssembly is not null
    && current.ContainingAssembly.GivesAccessTo(compilationAssembly);

The extra guard is redundant today but eliminates the ! operator and makes the invariant self-documenting.

ProtectedOrInternal pass-through is undocumented

ProtectedOrInternal (protected internal) is correctly not blocked in IsMemberAccessibleFromExternal — subclass access is unconditional across assemblies. But without a comment, a future reader will assume it was accidentally omitted. A one-liner like // protected internal: subclass access crosses assembly boundaries would prevent confusion.

IsTypeAccessible — pointer/function-pointer handling is conservative but undocumented

Returning false for pointer types is pragmatically correct (you can't mock pointer-parameter methods anyway), but the same filter applies to same-assembly types, silently excluding public virtual unsafe void Foo(int* p) even within the same assembly. A comment noting "pointer types are intentionally excluded from mock generation in all cases" would prevent someone filing a future bug thinking same-assembly pointer methods should work.

DiscoverMembersFromMultipleTypes — missing compilationAssembly parameter (latent risk)

This method has no accessibility filtering. It's safe today because multi-type mocks only accept interfaces, and interfaces can't have internal virtual members visible cross-assembly. But if future work adds support for multi-type mocks with concrete types, this method silently skips the accessibility check — a potentially hard-to-debug regression.

Adding a compilationAssembly parameter now (even defaulting to null, wired through from the already-available call site value in MockTypeDiscovery) would make the gap explicit and trivially easy to fill later:

private static IEnumerable<IMemberModel> DiscoverMembersFromMultipleTypes(
    INamedTypeSymbol namedType, IAssemblySymbol? compilationAssembly = null)

---

MockTypeDiscovery.cs — threading compilationAssembly

The threading is clean and consistent. All three BuildSingleTypeModel call sites pass compilationAssembly explicitly (the null default was intentionally removed from the private method). DiscoverTransitiveInterfaceTypes correctly threads it through. No issues here.

---

SnapshotTestBase.cs — test infrastructure

The VerifyGeneratorOutput overload refactoring is the right shape. One issue worth flagging:

CreateExternalAssemblyReference — MemoryStream lifetime is non-obvious:

using var ms = new MemoryStream();
var emitResult = compilation.Emit(ms);
ms.Seek(0, SeekOrigin.Begin);
return MetadataReference.CreateFromStream(ms);

CreateFromStream copies bytes immediately, so disposing ms via using afterward is safe. But this is non-obvious from the API surface — a brief comment (// CreateFromStream copies bytes immediately; ms can be safely disposed after this call) would save a future reader from auditing the Roslyn source.

Also, System.IO.MemoryStream is fully-qualified here but the rest of the file uses unqualified type names. Trivial inconsistency.

---

MockGeneratorTests.cs — new snapshot tests

The four new tests cover the three bug categories well:

Test	Bug covered
Partial_Mock_With_Generic_Constrained_Virtual_Methods	CS0460
Partial_Mock_Filters_Internal_Virtual_Members_From_External_Assembly	CS0122
Partial_Mock_Filters_Members_With_Internal_Type_Signatures	CS0115
Wrap_Mock_With_Generic_Constrained_Virtual_Methods	CS0460 on wrap path

Missing test — wrap mock + external assembly accessibility:
The accessibility filter is exercised for partial mocks but not for wrap mocks. Both call MemberDiscovery.DiscoverMembers → ProcessClassMembers, so the behavior is shared today. But if the paths ever diverge, the wrap side has no regression guard. A Wrap_Mock_Filters_Internal_Virtual_Members_From_External_Assembly test would provide symmetry.

Missing test — private protected (ProtectedAndInternal) from external assembly:
The external assembly test includes a protected internal member and verifies it passes through. But there's no test that a private protected member from an external assembly is correctly excluded. This is the exact filter path that IsMemberAccessibleFromExternal handles — it should have direct test coverage.

---

Summary

Severity	Item
Medium	DiscoverMembersFromMultipleTypes lacks compilationAssembly parameter — silently skips accessibility filter; add it now before a future concrete-type multi-mock feature inherits the gap
Low	hasInternalAccess uses compilationAssembly! (null-forgiving) inside a short-circuit — redundant explicit null guard would remove the ! and make the invariant self-documenting
Low	No wrap-mock + external assembly accessibility snapshot test
Low	No private protected (ProtectedAndInternal) member in external assembly snapshot test to directly cover that filter path
Nit	GetConstraintClauses name doesn't signal override-incompatibility — consider GetConstraintClausesForNonOverride
Nit	ProtectedOrInternal pass-through in IsMemberAccessibleFromExternal deserves a comment explaining why
Nit	IsTypeAccessible pointer-type exclusion should note it applies to same-assembly types too (intentional)
Nit	CreateFromStream + using var ms pattern in SnapshotTestBase is correct but non-obvious — deserves a comment

The core fixes are correct and the PR resolves the three reported compiler errors. The medium item (DiscoverMembersFromMultipleTypes missing the filter) is the main thing worth addressing before merge; everything else is low/nit polish.

[claude]

Code Review

This is a clean, well-targeted bug fix for three distinct but related accessibility issues in the mock source generator. The changes are focused, well-tested, and don't introduce regressions.

Summary of bugs fixed

• CS0460: Constraint clauses on override methods — correctly removed in MockImplBuilder.cs
• CS0122: internal virtual members from external assemblies — correctly filtered in MemberDiscovery.cs
• CS0115: Non-overridable members — correctly filtered via the same accessibility machinery

---

What's good

CS0460 fix (MockImplBuilder.cs) is minimal and correct. C# explicitly disallows restating constraints on override methods. The retained comment on GetConstraintClauses clarifying it's only for non-override contexts is a nice touch.

Accessibility filtering (MemberDiscovery.cs) is well-structured. The decision to pre-compute isExternalType and hasInternalAccess per hierarchy level (rather than per-member) is a smart performance optimization:

// Pre-compute per-level: are internal members of this type accessible?
// Avoids repeated SymbolEqualityComparer + GivesAccessTo checks per member.
var isExternalType = ...
var hasInternalAccess = ...

The treatment of protected internal (ProtectedOrInternal) vs private protected (ProtectedAndInternal) is correct — since the generated impl subclasses the target type, the protected dimension grants access, so only private protected (which requires both inheritance and same-assembly) needs to be gated on hasInternalAccess.

Test infrastructure (SnapshotTestBase.cs): The CreateExternalAssemblyReference helper is well-designed and allows accurate simulation of cross-assembly scenarios. The comment about CreateFromStream copying bytes immediately (making ms safe to dispose) is accurate and appreciated.

Snapshot coverage is thorough — 5 new tests covering all three bug categories for both Partial and Wrap mock modes.

---

Observations

Minor: redundant null check in hasInternalAccess

var hasInternalAccess = isExternalType
    && compilationAssembly is not null                  // ← redundant
    && current.ContainingAssembly.GivesAccessTo(compilationAssembly);

isExternalType already requires compilationAssembly is not null (the first condition checks compilationAssembly is not null). The second is not null guard is dead code. Not a bug, but a minor clarity issue.

Minor: PR description says 4 new snapshot tests, but there are 5

Looking at the files added: Partial_Mock_With_Generic_Constrained_Virtual_Methods, Partial_Mock_Filters_Internal_Virtual_Members_From_External_Assembly, Partial_Mock_Filters_Members_With_Internal_Signature_Types, Wrap_Mock_Filters_Internal_Virtual_Members_From_External_Assembly, and Wrap_Mock_With_Generic_Constrained_Virtual_Methods. The PR body says "4 new snapshot tests" — just a documentation typo.

Consider: protected types used in method signatures from external assemblies

IsTypeAccessible handles Private, Internal, and ProtectedAndInternal accessibility for types in external assemblies, but doesn't explicitly block Protected nested types:

if (accessibility == Accessibility.Private) return false;
if (accessibility is Accessibility.Internal or Accessibility.ProtectedAndInternal)
{
    if (!typeAssembly.GivesAccessTo(compilationAssembly)) return false;
}
// Protected types from external assemblies fall through to `return true`

In practice, a protected nested type used as a parameter/return type of a public or protected member in an external class is a rare (arguably pathological) API design, and even if it were emitted, the generated subclass technically has access. But the extension methods (which are not subclasses) would reference this type, potentially causing a compile error. If this scenario is encountered in the wild, the filter would need to be extended. Worth keeping in mind for Azure SDK types.

DiscoverMembersFromMultipleTypes doesn't use compilationAssembly for interface discovery

The compilationAssembly parameter is accepted but only forwarded through to ProcessClassMembers. For multi-interface mocks, if an interface from an external assembly has methods referencing inaccessible types, those wouldn't be filtered. This matches the existing behavior (interfaces are typically all-public), but it's worth noting the asymmetry.

---

Verdict

The three core bugs are fixed correctly with good test coverage. The accessibility filtering logic is sound and the performance optimization (per-level pre-computation) is a good design choice. No blocking issues.