feat(mocks): IFoo.Mock() discovery with generic fallback and ORP resolution

[thomhurst]

Summary

• IFoo.Mock() auto-discovery: The source generator now detects IFoo.Mock() invocations and generates specialized mock types automatically — no Mock.Of<T>() or [GenerateMock] required
• Generic fallback for IntelliSense: Added T.Mock() extension with [OverloadResolutionPriority(-1)] so IntelliSense works immediately; generated specific extensions use [OverloadResolutionPriority(1)] to win resolution
• Namespace fix for ORP scope: Generated static extensions are placed in TUnit.Mocks (not TUnit.Mocks.Generated) so they compete at the same scope level as the fallback — C# resolves enclosing namespaces before imported ones, which would bypass ORP
• Code quality fixes: GetHashCode consistency for DeclaringInterfaceName, deduplication of GetArgPassList, proper declaringInterfaceName in multi-type discovery
• Docs: Updated mocking docs with the IFoo.Mock() pattern

Test plan

• All 712 TUnit.Mocks.Tests pass (net10.0)
• All 23 source generator snapshot tests pass
• New Extension_Only_Discovery_Works_Without_Mock_Of integration test validates IFoo.Mock()-only discovery
• New Static_Extension_Discovery_Without_Mock_Of snapshot test validates generated output
• Verified ORP works: IGreeter.Mock() returns IGreeter_Mock wrapper (not Mock<IGreeter>)

[claude]

Code Review: feat(mocks): generate wrapper type and C# 14 static extension for interface mocks

Great feature overall — solving the implicit conversion limitation cleanly by making the mock object itself implement the interface is the right approach architecturally. The snapshot tests are updated and the design is sound. A few things worth addressing:

---

⚠️ Architectural concern: duplicated mock creation logic

** duplicates the factory creation chain verbatim:**

// In MockStaticExtensionBuilder.Build():
var engine = new global::TUnit.Mocks.MockEngine<{mockableType}>(behavior);
var impl = new {safeName}_MockImpl(engine);
engine.Raisable = impl;
return new {safeName}_Mock(impl, engine);

// Identical to MockFactoryBuilder.cs (now updated to use the wrapper)

Since MockFactoryBuilder already creates {safeName}_Mock, the static extension can simply delegate:

public static {safeName}_Mock Mock(MockBehavior behavior = MockBehavior.Loose)
    => (IGreeter_Mock){safeName}_MockFactory.Create(behavior);

Or — since the factory now returns the wrapper — just cast the return. Why this matters: if the factory creation chain ever changes (e.g., adding telemetry, cleanup hooks, DI registration), the static extension won't get the update automatically. The factory is already the canonical creation point; the extension should delegate to it rather than reinventing it.

---

🐛 Test doesn't verify the actual fix

Wrapper_Can_Be_Passed_As_Interface_Parameter tests:

var result = AcceptGreeter((IGreeter)mock);  // explicit cast — this worked BEFORE the fix too

The whole point of this PR is that no cast is needed. The test should be:

var result = AcceptGreeter(mock);  // ✅ this is what the PR enables — and what should be tested

Similarly, Wrapper_Can_Be_Cast_To_Interface uses (IGreeter)mock — valid, but not the motivating scenario. A test that assigns/passes mock without a cast would actually verify the compile-time guarantee this feature provides.

---

ℹ️ C# 14 consumer requirement (worth documenting)

The extension(T) block is a C# 14 feature. Since the project uses LangVersion = preview, the source generator compiles fine. But consumers using the generated IFoo.Mock() syntax need C# 14 / .NET 10 as well, or they'll get a compile error on that specific syntax (the wrapper type itself will still work fine). Worth a note in the changelog/docs so users aren't surprised.

---

💭 Minor: exclusion paths lack integration test coverage

There are no tests verifying that multi-interface mocks (Mock.Of<T1, T2>()) and static-abstract interfaces do not generate a wrapper. Given those are explicit exclusion cases, a snapshot test for each edge case would be a safety net against future regressions in the guard logic.

---

What's working well

• Explicit interface implementation forwarding in the wrapper is correct — using global:: fully qualified names throughout
• ref/out/in parameter direction handling in GetArgPassList is properly carried through
• EditorBrowsable(Never) on the constructor is the right call to discourage direct instantiation without hiding it entirely
• The DeclaringInterfaceName approach for correct explicit interface dispatch across inherited interfaces is clean
• Snapshot tests are updated and comprehensive for the happy path

The core design is solid. The main asks are: delegate the static extension to the factory (avoid duplicate creation logic), and fix the test to actually exercise the no-cast scenario that motivated the feature.

[codacy-production]

Not up to standards ⛔

🔴 Issues 13 minor . 1 medium

Alerts:
⚠ 14 issues (≤ 0 issues of at least minor severity)

Results:
14 new issues

Category	Results
BestPractice	1 medium
CodeStyle	13 minor

View in Codacy

🟢 Metrics 54 complexity

Metric	Results
Complexity	54

View in Codacy

_{TIP This summary will be updated as you push new changes. Give us feedback}

[claude]

Code Review: feat(mocks): generate wrapper type and C# 14 static extension for interface mocks

Great feature overall — solving the implicit conversion limitation cleanly by making the mock object itself implement the interface is the right approach architecturally. The snapshot tests are updated and the design is sound. A few things worth addressing:

---

Architectural concern: duplicated mock creation logic

MockStaticExtensionBuilder.cs duplicates the factory creation chain verbatim:

// In MockStaticExtensionBuilder.Build():
var engine = new global::TUnit.Mocks.MockEngine<{mockableType}>(behavior);
var impl = new {safeName}_MockImpl(engine);
engine.Raisable = impl;
return new {safeName}_Mock(impl, engine);

// Identical to what MockFactoryBuilder.cs now does too

Since MockFactoryBuilder already creates {safeName}_Mock, the static extension could delegate instead:

public static IGreeter_Mock Mock(MockBehavior behavior = MockBehavior.Loose)
    => (IGreeter_Mock)IGreeter_MockFactory.Create(behavior);

Why this matters: if the factory creation chain ever changes (e.g., adding telemetry, cleanup hooks, DI registration), the static extension won't get the update automatically. The factory is already the canonical creation point; the extension should delegate to it rather than reinventing it.

---

Test doesn't verify the actual fix

Wrapper_Can_Be_Passed_As_Interface_Parameter tests:

var result = AcceptGreeter((IGreeter)mock);  // explicit cast — this worked BEFORE the fix too

The whole point of this PR is that no cast is needed. The test should be:

var result = AcceptGreeter(mock);  // this is what the PR enables — and what should be tested

Similarly, Wrapper_Can_Be_Cast_To_Interface uses (IGreeter)mock — valid, but not the motivating scenario. A test that passes mock directly without any cast would actually verify the compile-time guarantee this feature provides.

---

C# 14 consumer requirement (worth documenting)

The extension(T) block is a C# 14 feature. Since the project uses LangVersion = preview, the source generator compiles fine. But consumers using the generated IFoo.Mock() syntax need C# 14 / .NET 10 as well, or they'll get a compile error on that specific call site (the wrapper type itself still works). Worth a note in the changelog/docs so users aren't surprised.

---

Minor: exclusion paths lack integration test coverage

There are no tests verifying that multi-interface mocks (Mock.Of<T1, T2>()) and static-abstract interfaces do not generate a wrapper. Given those are explicit exclusion cases, a snapshot test for each would be a safety net against future regressions in the guard logic.

---

What's working well

• Explicit interface implementation forwarding is correct — fully qualified global:: names throughout
• ref/out/in parameter direction handling in GetArgPassList is properly carried through
• EditorBrowsable(Never) on the constructor is the right call to discourage direct instantiation without hiding it entirely
• The DeclaringInterfaceName approach for correct explicit interface dispatch across inherited interfaces (e.g., IReadWriter inheriting IReader and IWriter) is clean
• Snapshot tests are updated and comprehensive for the happy path

The core design is solid. The main asks are: delegate the static extension to the factory (avoid duplicate creation logic), and fix the test to actually exercise the no-cast scenario that motivated the feature.

[claude]

Code Review: feat(mocks) — wrapper type and C# 14 static extension for interface mocks

This is a well-structured feature that elegantly solves the implicit conversion limitation. The approach of generating a typed subclass that both extends Mock<T> and implements the interface is the right architectural choice — it preserves the full setup/verify API while enabling direct assignment. The snapshot coverage is solid and the new tests are meaningful. A few issues worth addressing before merging:

---

Bug: Wrapper generated but unused for multi-interface mocks

MockWrapperTypeBuilder.Build() only guards against HasStaticAbstractMembers, not AdditionalInterfaceNames.Length > 0:

// MockWrapperTypeBuilder.cs — guards
if (model.HasStaticAbstractMembers)
    return string.Empty;
// ⚠️ No guard for AdditionalInterfaceNames.Length > 0

But MockFactoryBuilder.cs correctly falls back to the raw Mock<T> for multi-interface mocks:

if (model.AdditionalInterfaceNames.Length > 0 || model.HasStaticAbstractMembers)
    writer.AppendLine($"var mock = new global::TUnit.Mocks.Mock<{mockableType}>(impl, engine);");
else
    writer.AppendLine($"var mock = new {safeName}_Mock(impl, engine);");

Result: for every Mock.Of<T1, T2>() type, a wrapper class IS generated (even including both interface names via model.AdditionalInterfaceNames) but the factory never constructs it. This is dead generated code. The fix is to add the same guard to MockWrapperTypeBuilder:

if (model.HasStaticAbstractMembers || model.AdditionalInterfaceNames.Length > 0)
    return string.Empty;

---

Design concern: Factory logic duplicated in two generated files per interface

Both MockStaticExtensionBuilder and MockFactoryBuilder independently emit the same 4-line construction pattern:

var engine = new global::TUnit.Mocks.MockEngine<T>(behavior);
var impl = new ...MockImpl(engine);
engine.Raisable = impl;
return new ...Mock(impl, engine);

This means every interface gets this logic generated twice across two separate files. One cleaner approach would be to have the static extension delegate to the factory (or vice versa):

// In MockStaticExtensionBuilder — call the factory instead
public static global::TUnit.Mocks.Generated.IGreeter_Mock Mock(MockBehavior behavior = MockBehavior.Loose)
    => (global::TUnit.Mocks.Generated.IGreeter_Mock)global::TUnit.Mocks.Generated.IGreeter_MockFactory.Create(behavior, []);

This keeps the construction logic in one place (the factory) and removes the duplication. The cast is safe because, for single-interface non-static-abstract mocks, the factory now returns the wrapper type.

---

Concern: IsMockExtensionInvocation is too broad for a syntax predicate

The predicate matches any X.Mock(...) member access — including calls from Moq, NSubstitute, custom mocking libs, etc. Since syntax predicates are called on every node in every file, this fires far more often than intended:

public static bool IsMockExtensionInvocation(SyntaxNode node, CancellationToken ct)
{
    return node is InvocationExpressionSyntax
    {
        Expression: MemberAccessExpressionSyntax
        {
            Name: IdentifierNameSyntax { Identifier.ValueText: "Mock" }
        }
    };
}

The transform handles false positives correctly (by checking the LHS resolves to an interface type), but the semantic step is still invoked unnecessarily. A tighter predicate could first verify the expression starts with an uppercase identifier (common interface naming convention) or check for zero arguments, though neither is bulletproof. At minimum, a comment explaining why the broad match is intentional and acceptable would help.

---

Minor: Attribute placed outside the using block visually in MockStaticExtensionBuilder

using (writer.Block($"extension({model.FullyQualifiedName})"))
{
    writer.AppendLine("[global::System.Runtime.CompilerServices.OverloadResolutionPriority(1)]");
using (writer.Block($"public static ..."))   // ← not indented inside the outer using
    {

The code works because CodeWriter tracks indent state internally, but the C# source indentation is misleading — the inner using looks like it opens at the outer block's indentation level. This is a readability issue that could confuse future maintainers.

---

Observation: ExplicitInterfaceName missing from MockMemberModel.GetHashCode() (pre-existing)

This PR correctly adds DeclaringInterfaceName to GetHashCode(), but the pre-existing ExplicitInterfaceName remains absent from the hash despite being in Equals(). This is a latent hash collision risk. Not introduced by this PR, but worth fixing while you're editing these models.

---

What's good

• The explicit interface forwarding approach is exactly right — avoids the ambiguity of implicit conversion while preserving the full Mock<T> API surface.
• [EditorBrowsable(Never)] on the wrapper constructor keeps IntelliSense clean.
• Using OverloadResolutionPriority to make generated specializations win over the generic fallback is clean and future-proof.
• IsMockExtensionInvocation + TransformMockExtensionInvocation correctly handles the standalone IFoo.Mock() discovery path without requiring Mock.Of<T>() to co-exist.
• Snapshot tests are updated consistently and the new Static_Extension_Discovery_Without_Mock_Of test covers the important standalone discovery path.
• Docs are updated with clear before/after examples.

The multi-interface wrapper leak and factory duplication are the primary things I'd want resolved before merging.

[claude]

Code Review: feat(mocks): IFoo.Mock() discovery with generic fallback and ORP resolution

This is a well-designed PR that adds a genuinely ergonomic API improvement. The overall architecture is sound, but I have several observations worth addressing.

---

Architecture: Duplication of mock creation logic

The most significant issue is that the mock creation logic (create engine → create impl → set Raisable → construct wrapper) is duplicated in three places:

1. MockFactoryBuilder.cs — factory's Create() method
2. MockStaticExtensionBuilder.cs — generated IFoo.Mock() static extension body
3. MockWrapperTypeBuilder.cs does not duplicate this, but the static extension and the factory are doing the same thing independently

Why this matters: If the mock construction sequence ever needs to change (e.g., lifecycle hooks, tracing, additional initialization), it must be updated in both the factory builder and the static extension builder, and there is no test ensuring they stay in sync.

A better approach would be to have the generated static extension delegate to the factory rather than reconstruct the engine inline:

// Instead of the current approach in MockStaticExtensionBuilder.cs:
public static IGreeter_Mock Mock(MockBehavior behavior = MockBehavior.Loose)
{
    // duplicated creation logic...
}

// Consider having it call the factory:
public static IGreeter_Mock Mock(MockBehavior behavior = MockBehavior.Loose)
    => (IGreeter_Mock)IGreeter_MockFactory.Create(behavior, []);

This makes the factory the single source of truth, and the static extension just becomes a typed re-cast of it. The factory already returns Mock<T> and creates IGreeter_Mock — the cast would be safe since the factory now instantiates {safeName}_Mock not the base Mock<T>.

---

Inconsistency: MockFactoryBuilder condition asymmetry with MockStaticExtensionBuilder

In MockFactoryBuilder.BuildInterfaceFactory():

if (model.AdditionalInterfaceNames.Length > 0 || model.HasStaticAbstractMembers)
{
    writer.AppendLine($"var mock = new global::TUnit.Mocks.Mock<{mockableType}>(impl, engine);");
}
else
{
    writer.AppendLine($"var mock = new {safeName}_Mock(impl, engine);");
}

In MockStaticExtensionBuilder.Build():

if (model.AdditionalInterfaceNames.Length > 0 || model.HasStaticAbstractMembers)
{
    return string.Empty;
}

And in MockWrapperTypeBuilder.Build():

if (model.HasStaticAbstractMembers)
{
    return string.Empty;
}

The guard conditions differ between builders: MockWrapperTypeBuilder allows multi-interface mocks (only guards HasStaticAbstractMembers), but MockStaticExtensionBuilder guards both. This means a multi-interface mock gets a wrapper type but no static extension, which is intentional. However, the factory generates new {safeName}_Mock(...) for multi-interface models where AdditionalInterfaceNames.Length > 0 is false, which is the correct path.

The conditions are correct but the asymmetry between the three builders is fragile and not documented. A shared helper or a well-named property on MockTypeModel like bool CanHaveTypedWrapper => !HasStaticAbstractMembers and bool CanHaveStaticExtension => !HasStaticAbstractMembers && AdditionalInterfaceNames.Length == 0 would make the intent explicit and keep the three builders in sync.

---

Indentation bug in MockStaticExtensionBuilder.cs

Line 29–30 has a misaligned using block:

// line 29 — AppendLine has 16 spaces of indentation (inside extension block)
                    writer.AppendLine("[global::System.Runtime.CompilerServices.OverloadResolutionPriority(1)]");
                using (writer.Block($"public static ..."))  // line 30 — same 16 spaces but starts a new using()

This is a minor formatting inconsistency in the source generator source code itself — the [OverloadResolutionPriority] attribute line is indented at the extension block level, but the using (writer.Block(...)) that opens the method starts at the same indentation level. The generated output looks correct because CodeWriter tracks its own indent, but the source is misleading to read. Both should be at the same indentation level.

---

Semantic detection: false-positive risk for IsMockExtensionInvocation

The predicate matches any X.Mock() where the LHS is a type name (identifier, qualified name, alias-qualified name):

return lhs is IdentifierNameSyntax or QualifiedNameSyntax or AliasQualifiedNameSyntax;

This will fire for any method named Mock called on a type — e.g., MyClass.Mock() where MyClass is a concrete class with a static method named Mock. The semantic transform (TransformMockExtensionInvocation) then correctly filters to only TypeKind.Interface, so no incorrect code is generated. However, the predicate runs on every node in every syntax tree, including user code that has nothing to do with TUnit mocks. Adding the TypeKind.Interface check to the semantic pass is sufficient (already done), but it is worth noting that this hot path will trigger unnecessary semantic lookups for every .Mock() call on a non-interface type across the entire compilation.

If performance matters (large codebases), a reasonable heuristic to add to the syntax predicate would be to check that the identifier starts with "I" (matching the convention for interface names), reducing false positives without requiring semantic analysis. This is an optional optimization.

---

IsMockExtensionInvocation excludes GenericNameSyntax LHS

The predicate explicitly excludes GenericNameSyntax on the left-hand side:

return lhs is IdentifierNameSyntax or QualifiedNameSyntax or AliasQualifiedNameSyntax;

This means IFoo<int>.Mock() would not be detected by this path even if IFoo<int> is a valid closed generic interface. The Mock.Of<IFoo<int>>() path still works, but the new ergonomic syntax would silently fall back to the generic fallback extension. This limitation is not documented and could confuse users who try IRepository<User>.Mock() and get Mock<IRepository<User>> instead of IRepository_User__Mock. Consider either:

1. Adding GenericNameSyntax to the predicate and handling it in the transform, or
2. Documenting this limitation explicitly (it's mentioned vaguely in the note in the docs but not specifically for generic interfaces).

---

WrapMock doesn't get a typed wrapper

The MockWrapperTypeBuilder and MockStaticExtensionBuilder are only called in GenerateSingleTypeMock, so wrap mocks never receive a typed wrapper type. This is consistent behavior, but a comment explaining why would help future contributors (wrap mocks wrap a real class instance, so the implicit conversion on Mock<T> already satisfies most use cases).

---

Positive notes

• The ORP namespace fix (TUnit.Mocks instead of TUnit.Mocks.Generated) is correct and well-reasoned — the C# scoping rules mean ORP can only compete between extensions at the same namespace level.
• The [EditorBrowsable(Never)] on the _Mock constructor prevents the generated wrapper's internal constructor from polluting IntelliSense.
• The DeclaringInterfaceName fix in MemberDiscovery.cs for multi-type discovery is a legitimate correctness fix — the bug existed prior to this PR.
• Deduplication of GetArgPassList by making it internal and sharing it with MockWrapperTypeBuilder is exactly right.
• The incremental generator design is correct: the "skip if already resolved" check in TransformMockExtensionInvocation prevents double-generation on subsequent compilation passes.
• Test coverage with both a snapshot test (Static_Extension_Discovery_Without_Mock_Of) and an integration test (Extension_Only_Discovery_Works_Without_Mock_Of) is thorough.

---

Summary

The PR is generally well-implemented. The most important concern is the duplicated creation logic between the factory and the static extension body — this is a latent maintenance hazard. The inconsistent guard conditions across the three new builders and the missing documentation around generic interfaces are secondary concerns. The indentation issue in MockStaticExtensionBuilder.cs is cosmetic.

[claude]

Code Review: PR #5327 — IFoo.Mock() auto-discovery with generic fallback and ORP resolution

Overview

This PR adds three interlocking features: IFoo.Mock() call-site auto-discovery, a typed wrapper class (IFoo_Mock) that implements the interface directly, and an ORP-based fallback mechanism so both generated and generic extensions coexist. The core design decisions are sound, and the ORP namespace placement fix is particularly well-reasoned. However, there are a few issues that need attention before merging.

---

Critical Bug: Interfaces with Indexers or Ref-Struct Properties Will Fail to Compile

MockWrapperTypeBuilder silently skips indexers and ref-struct-returning properties:

// MockWrapperTypeBuilder.cs
if (prop.IsIndexer) continue;
if (prop.IsRefStructReturn) continue;

Since the wrapper declares IFoo_Mock : Mock<IFoo>, IFoo, any skipped interface member becomes an unimplemented interface member, producing CS0535/CS0738 compile errors for users calling .Mock() on such an interface.

Fix: Add guards analogous to HasStaticAbstractMembers — either bail out of wrapper generation entirely and fall back to Mock.Of<T>() (the safer choice), or emit a throw new NotSupportedException(...) stub for the problematic members. MockImplBuilder handles indexers correctly; the same logic should be reflected here.

---

Moderate Concern: Syntax Predicate Will Fire on Non-TUnit .Mock() Calls

IsMockExtensionInvocation matches any call of the form <type>.Mock() without checking whether TUnit.Mocks is even referenced in the compilation:

// Will match calls from any library that has a .Mock() method
SomeMockingLibrary.ISomeInterface.Mock()

The semantic filter correctly bails out for non-TUnit types, but it incurs a full semantic model lookup per call site. In large codebases with other mocking frameworks, this adds unnecessary incremental generator overhead.

Better approach: Add an early exit in TransformMockExtensionInvocation checking whether the compilation references TUnit.Mocks (e.g. context.Compilation.GetTypeByMetadataName("TUnit.Mocks.MockEngine1") != null), or check that the method being called resolves to the TUnit.Mocksnamespace before callingBuildModelWithTransitiveDependencies`.

---

Design Issue: Dead Code in MockWrapperTypeBuilder

// Line 26 — AdditionalInterfaceNames is always empty here due to the early return above
var interfaceList = string.Join(", ", new[] { model.FullyQualifiedName }.Concat(model.AdditionalInterfaceNames));

The bail-out for AdditionalInterfaceNames.Length > 0 means AdditionalInterfaceNames is always empty by the time we reach the interfaceList build. The Concat is dead code that creates a misleading impression of multi-interface support in the wrapper.

---

Design Issue: Three-Way Combine Nesting Is Fragile

// MockGenerator.cs
var distinctTypes = mockTypes
    .Collect()
    .Combine(attributeTypes.Collect())
    .Combine(extensionTypes.Collect())
    .SelectMany((pair, _) => pair.Left.Left.AddRange(pair.Left.Right).AddRange(pair.Right).Distinct());

pair.Left.Left is fragile — swapping any of the three sources or adding a fourth would require updating this unpacking logic. Consider destructuring with named locals:

.SelectMany((pair, _) => {
    var (attributeAndMock, extensionInvocations) = pair;
    var (mockTypes, attributeTypes) = attributeAndMock;
    return mockTypes.AddRange(attributeTypes).AddRange(extensionInvocations).Distinct();
})

---

Minor: GetHashCode in MockMemberModel Is Still Substantially Incomplete

The PR correctly adds ExplicitInterfaceName and DeclaringInterfaceName to GetHashCode, but roughly 15 fields still participate in Equals but not in hashing (IsVoid, IsAsync, IsProperty, IsIndexer, etc.). This degrades Distinct() performance when many members share the same name/ID/return type. Pre-existing debt, but this PR is a good opportunity to fix it fully.

---

Minor: Constructor Visibility on IFoo_Mock

The constructor is public but hidden from IntelliSense with [EditorBrowsable(Never)]. Since the only valid construction path is through the factory, making it internal would be safer — preventing direct instantiation even for determined developers who look past the attribute.

---

What's Done Well

• ORP namespace fix is correct and subtle. Placing generated extensions in TUnit.Mocks rather than TUnit.Mocks.Generated is the right call — C# resolves extensions from the enclosing namespace before imported ones, which would have caused the TUnit.Mocks fallback to shadow generated types regardless of ORP priority.

• BuildModelWithTransitiveDependencies extraction is clean. Removes duplication between the Mock.Of<T>() path and the new IFoo.Mock() path.

• DeclaringInterfaceName threading is complete. All four CreateXxxModel helpers now receive and store the declaring interface FQN, fixing a real multi-interface discovery bug.

• Typed wrapper is a genuine UX win. Having IGreeter.Mock() return something that IS-A IGreeter without a cast removes real friction. The forwarding precedence chain (ExplicitInterfaceName ?? DeclaringInterfaceName ?? FullyQualifiedName) is correct.

• Good test coverage. Snapshot test for new discovery, MockWrapperTypeTests with dedicated scenarios, and IExtensionOnlyService validating generation without Mock.Of<T>() usage.

---

Summary

The indexer/ref-struct guard in MockWrapperTypeBuilder is the only issue likely to cause user-facing compile errors and should be addressed before merging. The other items are robustness and maintainability improvements. Overall this is a well-structured and ergonomically valuable addition.

[claude]

Code Review: PR #5327 — auto-discovery with ORP resolution (follow-up)

This iteration has addressed all major concerns from previous reviews. Here's a summary of what was resolved and what remains.

---

Addressed from previous reviews ✅

• Duplicated factory creation logic — now delegates to rather than reconstructing the engine chain. The factory is the canonical creation path.
• Test verifies the no-cast scenario — now calls directly without a cast. This is exactly the compile-time guarantee the PR provides.
• Indexer/ref-struct guard — correctly bails out for interfaces with indexers or ref-struct-returning properties, preventing errors for consumers.
• Asymmetric guard conditions — The shared helper consolidates exclusion logic in one place and is called from both and (indirectly via caller checks in) .
• Three-way fragility — Named destructuring is now used, making the merge logic readable.
• ** consistency** — Both and are now included in for both and .
• Generic interface handling — is now accepted in , so will be detected.

---

Remaining concern: has a partial/inconsistent guard

The current guard at the top of :

This is a subset of — it doesn't check for indexers or ref-struct properties. In practice this is safe because only calls after confirming is non-empty (which means already returned true). But the guard is now misleading: it implies the extension can be skipped independently, when really the wrapper being skipped is the effective gate.

A cleaner fix would be to either:

1. Replace the guard entirely with — making the dependency explicit and keeping the builders in sync, or
2. Remove the guard from entirely with a comment noting it's only called when the wrapper already passed, relying on the call site in as the sole gate.

Why it matters: If new exclusion criteria are added to in the future (e.g., interfaces with members, or -scoped types), the extension guard will silently diverge unless someone knows to update both places.

---

Minor: No test for generic interface discovery ()

is now accepted in the predicate, enabling detection. But there's no snapshot test or integration test validating that a closed generic interface (e.g., ) correctly generates a wrapper via the extension path. This is worth a test since the naming logic () would need to produce a valid C# identifier for the generic type.

---

Minor: breadth (carry-over from previous review)

The predicate still fires on any call regardless of whether TUnit.Mocks is even referenced. The semantic transform correctly filters, but in large codebases with other mocking frameworks (Moq, NSubstitute) this incurs unnecessary semantic lookups. Adding an early exit in checking before calling would eliminate the false-positive overhead. Low priority but worth noting for performance at scale.

---

Overall

This is a well-executed follow-up. The core design is solid, the previous critical concerns (indexer guard, factory duplication, test coverage for the no-cast scenario) are all fixed. The helper is a clean way to share guard logic. The remaining items are maintenance improvements rather than correctness bugs — the guard inconsistency in particular could silently bite a future contributor, but it won't cause any user-visible problem today.

Ready to merge with the guard alignment improvement as a nice-to-have.

[claude]

Code Review: PR #5327 — IFoo.Mock() auto-discovery with ORP resolution (follow-up)

This iteration has addressed all major concerns from previous reviews. Here's a summary of what was resolved and what remains.

---

Addressed from previous reviews ✅

• Duplicated factory creation logic — MockStaticExtensionBuilder now delegates to {safeName}_MockFactory.Create() rather than reconstructing the engine chain. The factory is the canonical creation path.
• Test verifies the no-cast scenario — Wrapper_Can_Be_Passed_As_Interface_Parameter now calls AcceptGreeter(mock) directly without a cast. This is exactly the compile-time guarantee the PR provides.
• Indexer/ref-struct guard — CanGenerateWrapper() correctly bails out for interfaces with indexers or ref-struct-returning properties, preventing CS0535 errors for consumers.
• Asymmetric guard conditions — The shared CanGenerateWrapper() helper consolidates exclusion logic in one place and is called from both MockWrapperTypeBuilder and (indirectly via caller checks in) MockGenerator.
• Three-way Combine fragility — Named destructuring is now used, making the merge logic readable.
• GetHashCode consistency — Both ExplicitInterfaceName and DeclaringInterfaceName are now included in GetHashCode for both MockMemberModel and MockEventModel.
• Generic interface handling — GenericNameSyntax is now accepted in IsMockExtensionInvocation, so IRepository<User>.Mock() will be detected.

---

Remaining concern: MockStaticExtensionBuilder.Build() has a partial/inconsistent guard

The current guard at the top of MockStaticExtensionBuilder.Build():

if (model.AdditionalInterfaceNames.Length > 0 || model.HasStaticAbstractMembers)
    return string.Empty;

This is a subset of CanGenerateWrapper() — it doesn't check for indexers or ref-struct properties. In practice this is safe because MockGenerator.cs only calls MockStaticExtensionBuilder.Build() after confirming wrapperSource is non-empty (which means CanGenerateWrapper already returned true). But the guard is now misleading: it implies the extension can be skipped independently, when really the wrapper being skipped is the effective gate.

A cleaner fix would be to either:

1. Replace the guard with if (!MockWrapperTypeBuilder.CanGenerateWrapper(model)) return string.Empty; — making the dependency explicit and keeping the builders in sync, or
2. Remove the guard from MockStaticExtensionBuilder.Build() entirely with a comment noting it's only called when the wrapper already passed.

Why it matters: If new exclusion criteria are added to CanGenerateWrapper in the future (e.g., interfaces with required members or file-scoped types), the extension guard will silently diverge unless someone knows to update both places.

---

Minor: No test for generic interface discovery

GenericNameSyntax is now accepted in the predicate, enabling IFoo<T>.Mock() detection. But there's no snapshot or integration test validating that a closed generic interface (e.g., IRepository<User>) correctly generates a wrapper via the extension path — particularly the naming logic in GetCompositeSafeName which must produce a valid C# identifier for the generic type. Worth a test to lock in this behavior.

---

Minor: IsMockExtensionInvocation breadth (carry-over)

The predicate still fires on any TypeName.Mock() call regardless of whether TUnit.Mocks is referenced. The semantic transform correctly filters, but in large codebases with other mocking frameworks (Moq, NSubstitute) this incurs unnecessary semantic lookups. Adding an early exit in TransformMockExtensionInvocation checking context.SemanticModel.Compilation.GetTypeByMetadataName("TUnit.Mocks.MockEngine1") != nullbefore callingBuildModelWithTransitiveDependencies` would eliminate false-positive overhead. Low priority but worth noting.

---

Overall

This is a well-executed follow-up that resolves the previous critical concerns. The CanGenerateWrapper helper is a clean approach to sharing guard logic, the static extension now correctly delegates to the factory, and the no-cast test actually validates the feature. The guard inconsistency in MockStaticExtensionBuilder is a latent maintenance risk rather than a correctness bug today. Ready to merge with the guard alignment as a nice-to-have.