feat(mocks): unified Mock<T> API — remove Setup/Verify/Raise surfaces

[thomhurst]

Summary

• Remove .Setup., .Verify., and .Raise. intermediate properties from Mock<T>. Methods are now called directly on the mock via source-generated extension methods, with chain methods (.Returns() vs .WasCalled()) disambiguating between setup and verification.
• Add unified return types (MockMethodCall<T>, VoidMockMethodCall, PropertyMockCall<T>, PropertySetterMockCall<T>) that expose both setup and verification surfaces on a single object.
• Replace MockSetupBuilder + MockVerifyBuilder with a single MockMembersBuilder that generates extension methods on Mock<T> and unified sealed classes per qualifying method.
• Void methods and property setters eagerly register their setups in the constructor, since they're commonly used without chaining (e.g., mock.Log(Arg.Any<string>()) to allow calls in strict mode).

Before

mock.Setup.Greet(Arg.Any<string>()).Returns("Hello");
mock.Verify.Greet(Arg.Any<string>()).WasCalled();
mock.Raise.OnMessage("hi");

After

mock.Greet(Arg.Any<string>()).Returns("Hello");
mock.Greet(Arg.Any<string>()).WasCalled();
mock.RaiseOnMessage("hi");

Test plan

• All 475 runtime tests pass (TUnit.Mocks.Tests on net10.0)
• All 10 source generator snapshot tests pass (TUnit.Mocks.SourceGenerator.Tests)
• Snapshot .verified.txt files updated to reflect new generated output
• Verify CI passes across all target frameworks (net8.0, net9.0, net10.0)

[claude]

Code Review: Unified Mock API — Remove Setup/Verify/Raise Surfaces

This is a well-motivated refactor. The before/after in the description shows a real ergonomic improvement, and the architecture is cleaner overall. A few issues worth discussing:

---

Issue 1 (Design): Thread-Safety Race in EnsureSetup() for Non-Void Methods

MockMethodCall.cs and all generated *_MockCall classes for non-void methods:

private MethodSetupBuilder<TReturn> EnsureSetup()
{
    if (_builder is null)   // Thread A and Thread B can both see null here
    {
        var setup = new MethodSetup(_memberId, _matchers, _memberName);
        _engine.AddSetup(setup);       // Called twice — double-registration
        _builder = new MethodSetupBuilder<TReturn>(setup);
    }
    return _builder;
}

This check-then-act pattern is a data race. Under concurrent setup calls, two threads can both pass the null check, both call _engine.AddSetup(), and both assign _builder. The result is a double-registered MethodSetup for the same mock member, with one orphaned in the engine.

A safer approach would be Lazy<T> initialization or Interlocked.CompareExchange. Since TUnit has ThreadSafetyTests.cs, this may already be tested — but in that case, the test should be validating the engine-side behavior as well.

Why this matters: Even if tests are sequential, the VoidMockMethodCall is the standard pattern for "allow in strict mode" where the same mock object might be shared across parallel test fixtures via [ClassDataSource].

---

Issue 2 (Design): Setup Registration Asymmetry Between Void and Non-Void

VoidMockMethodCall calls EnsureSetup() eagerly in the constructor, while MockMethodCall<TReturn> defers. The comment explains the intent, but it creates a subtle inconsistency:

// This DOES register a setup (void — eager):
mock.Log(Arg.Any<string>());

// This does NOT register a setup (non-void — lazy):
mock.GetValue(Arg.Any<string>());

A user who writes mock.GetValue(Arg.Any<string>()) without chaining .Returns() gets a no-op MockMethodCall<string> that's immediately garbage collected without side effects. That's actually the correct behavior in loose mode — but in strict mode it could produce a confusing "unexpected call" failure that doesn't match the mental model of "I called mock.GetValue to allow it."

Consider at minimum adding an XML doc note on MockMethodCall<TReturn> that explains this distinction. Or, if the intent is specifically to match void-style usage in strict mode, providing a .Allow() method that eagerly registers a permissive setup (similar to how VoidMockMethodCall works) would be more explicit than relying on the void/non-void asymmetry.

---

Issue 3 (Design): WasCalled() Creates a New Verification Object on Every Invocation

public void WasCalled() => _engine.CreateVerification(_memberId, _memberName, _matchers).WasCalled();
public void WasCalled(Times times) => _engine.CreateVerification(_memberId, _memberName, _matchers).WasCalled(times);
// ... repeated for every overload

Each WasCalled() call instantiates a fresh CallVerificationBuilder that's used once and discarded. This is a one-shot pattern and is functionally fine, but it means the matchers array is passed through the allocation on each call. Since the matchers are already stored on the MockMethodCall, consider caching the verification builder on first creation (similar to EnsureSetup()) rather than recreating it each time. This would also reduce object graph depth during verification failures.

---

Issue 4 (Maintenance): MockMemberNames Hardcoded List in MockMembersBuilder

private static readonly HashSet<string> MockMemberNames = new(System.StringComparer.Ordinal)
{
    "Object", "Engine", "Behavior", "Invocations", "DefaultValueProvider",
    "SetupAllProperties", "Reset", "VerifyAll", "VerifyNoOtherCalls",
    "GetAutoMock", "GetDiagnostics", "SetState", "InState",
    "GetHashCode", "GetType", "ToString", "Equals"
};

This is fragile — it must be manually kept in sync with Mock<T>'s public API. If a new method is added to Mock<T>, it won't be added here, silently generating a collision. A better approach would be to derive this list from the Mock<T> type at generator initialization time using symbol analysis (the generator already has access to the compilation, so typeof(Mock<T>) members can be enumerated at source-gen time). This would make it impossible for the list to go stale.

---

Issue 5 (Minor): PropertyMockCall<T>.Getter Creates a New Setup on Every Access

public IPropertySetup<TProperty> Getter
{
    get
    {
        var methodSetup = new MethodSetup(_getterMemberId, matchers, $"{_propertyName} (get)");
        _engine.AddSetup(methodSetup);   // New registration each access
        return new PropertySetupBuilder<TProperty>(methodSetup);
    }
}

This was the same behavior in PropertySetupAccessor<T>, so it's not a regression. But since PropertyMockCall<T> now also owns verification, the multiple-access concern is more visible. Worth a comment explaining the deliberate re-registration behavior (or raising it as a separate follow-up).

---

What's Good

• The API simplification is genuinely better. mock.Greet(Arg.Any<string>()).Returns("Hello") is more natural than mock.Setup.Greet(Arg.Any<string>()).Returns("Hello").
• Removing the IMockSetup<T> / IMockVerify<T> marker interfaces removes an entire layer of indirection including the unsafe downcasts ((ISomethingMockSetup)setup) that existed in every generated extension method.
• MockOfT.cs is notably simpler — removing Setup, Verify, and Raise properties reduces the public surface of Mock<T>.
• MockMembersBuilder.cs replacing two separate builders reduces generated file count (one _MockMembers.g.cs instead of _MockSetup.g.cs + _MockVerify.g.cs).
• The GetSafeMemberName escape hatch for name collisions is a necessary correctness fix.
• Snapshot tests are updated and committed — good hygiene per project rules.

[claude]

Code Review: feat(mocks): unified Mock API — remove Setup/Verify/Raise surfaces

Summary

This is a solid API simplification. Removing the .Setup., .Verify., and .Raise. intermediate properties reduces ceremony significantly and the fluent chaining approach is much more ergonomic. The overall design direction is correct. That said, there are several issues worth addressing.

---

Issues Found

1. PropertyMockCall<T>.Getter registers a new setup on every access — potential bug

In PropertyMockCall.cs, the Getter property creates and registers a brand-new MethodSetup on every single access:

public IPropertySetup<TProperty> Getter
{
    get
    {
        var methodSetup = new MethodSetup(_getterMemberId, matchers, $"{_propertyName} (get)");
        _engine.AddSetup(methodSetup);  // New setup registered every time!
        return new PropertySetupBuilder<TProperty>(methodSetup);
    }
}

This means:

mock.Name.Returns("Alice");  // Accesses Getter internally → registers setup #1
mock.Name.Returns("Bob");    // Accesses Getter again → registers setup #2 (!!)

It also means VerifyAll() tracks two separate setups and both must be invoked. The lazy EnsureSetup() pattern used in MockMethodCall<T> and VoidMockMethodCall is the right approach here too. The PropertyMockCall<T> should lazily cache its getter setup builder rather than registering fresh on each access.

2. Void verification has a hidden setup side effect — breaks VerifyAll() semantics

VoidMockMethodCall eagerly registers a setup in its constructor. This means calling a method for the purpose of verification also registers a setup:

// Old (verify only, no setup registered):
mock.Verify.Log(Arg.Any<string>()).WasCalled();

// New (verification + inadvertent setup registration):
mock.Log(Arg.Any<string>()).WasCalled();

After migrating to the new API, code that previously called VerifyAll() after doing verification-only calls will now see those verification calls counted as uninvoked setups and fail. The PR description mentions the void eager-registration is for strict mode compatibility (mock.Log(Arg.Any<string>()) as an "allow" statement), but the side effect on VerifyAll() is a behavioral regression. Consider either:

• Not eagerly registering until a setup method (.Callback(), .Throws()) is explicitly called
• Tracking setups vs. verifications separately in the engine

3. Allocation regression: readonly struct → sealed class

The previous *_TypedSetup types were readonly struct, meaning zero heap allocations for the setup-chain wrapper. All the new unified types (MockMethodCall<T>, VoidMockMethodCall, and every generated *_MockCall) are sealed class — one heap allocation per mock method invocation.

In tight loops or assertion-heavy tests, this creates measurable GC pressure. The CLAUDE.md "Performance First" principle flags this. The original structs were allocation-free for the common case; the new design trades that away for API simplicity. This may be acceptable, but it should be a deliberate decision rather than an implicit one.

If allocation matters here, consider a different split: keep the generated per-method wrapper types as readonly struct for the common path (setup+chain), and only allocate a verify object on-demand when .WasCalled() is called.

4. Member name mismatch between setup and verification in PropertyMockCall<T>

Setup registers with name $"{_propertyName} (get)" (e.g., "Name (get)"), but verification uses $"get_{_propertyName}" (e.g., "get_Name"). If the engine uses member names in error messages or to correlate setup/call records, these will be inconsistent:

// PropertyMockCall.cs — setup:
var methodSetup = new MethodSetup(_getterMemberId, matchers, $"{_propertyName} (get)");

// PropertyMockCall.cs — verify:
_engine.CreateVerification(_getterMemberId, $"get_{_propertyName}", ...).WasCalled(times);

These names should be unified. Since matching is probably done by _getterMemberId, this may only affect error messages, but even that inconsistency is confusing when debugging test failures.

5. Non-void standalone calls are silently a no-op

The asymmetry between void and non-void is non-obvious:

mock.Log("x");                 // void → setup is registered (strict mode compatible)
mock.GetUser(Arg.Any<int>()); // non-void → nothing happens (no setup, no verify)

There is no compiler warning or runtime diagnostic when mock.GetUser(Arg.Any<int>()) appears alone. This could silently mask developer mistakes (e.g., forgetting to chain .WasCalled() or .Returns()). A Roslyn analyzer that warns on unconsumed MockMethodCall<T> return values (similar to [MustUseReturnValue]) would significantly improve discoverability. It would be worth either adding that analyzer or documenting the asymmetry prominently.

---

Minor Notes

• The Raises method on ISetupChain<TReturn> takes a string event name ("ItemAdded"), which is stringly typed. The old typed raise extensions (mock.Raise.ItemAdded(e)) were compile-time safe. The new mock.RaisesItemAdded(e) generated overloads restore type safety for the Raises shortcut on the setup chain, which is good.

• The removal of IMockSetup<T> and IMockVerify<T> as separate extension targets simplifies the source generator considerably. The MockMembersBuilder replacing two separate builders is cleaner.

• MockOfT.cs constructor simplification (removing setup/verify/raise constructor parameters) is clean and correct.

• The snapshot .verified.txt files are appropriately updated and confirm the generator output is working correctly.

---

Overall

The API direction is right — unified call objects that support both setup and verification are more ergonomic. The Getter-side-effect bug and the VerifyAll() behavioral regression are the highest-priority issues to address before merging.

[thomhurst]

Addressed in f225a2c:

Issue 1 — Thread-safety race in EnsureSetup(): Fixed. Non-void MockMethodCall<TReturn> and generated non-void sealed classes now use Lazy<T> for thread-safe single initialization. Void methods and property setters were already safe (eager init in the constructor completes before the object is visible to other threads).

Issue 2 — Void vs non-void asymmetry: Added XML doc comment on MockMethodCall<TReturn> explaining the distinction and suggesting .Returns(default!) as the explicit escape hatch for non-void methods in strict mode.

Issue 3 — Verification object per call: Skipping. These MockMethodCall instances are ephemeral per-call objects, so the verification builder is equally short-lived. Caching adds complexity for no measurable benefit in test code.

Issue 4 — Hardcoded MockMemberNames: Acknowledged as a maintenance concern. Deriving from Mock<T> symbol analysis at generator time is doable but non-trivial (the generator runs against the user's compilation, not TUnit.Mocks itself). Tracking as a follow-up.

Issue 5 — Getter re-registration: Added clarifying comment explaining this is intentional — each access allows chaining different behaviors that stack as additional setups.

All 475 runtime tests and 10 snapshot tests pass.

[claude]

Code Review: Follow-up on feat(mocks): unified Mock API

This review follows the two previous Claude reviews submitted before commit f225a2c9. Good news: the thread-safety fix in that last commit (Lazy<T> for non-void setup registration) is the right approach and directly addresses the previous concern.

---

What Was Addressed Since Last Review

Thread safety (non-void path) — fixed correctly. MockMethodCall<T> and the generated *_MockCall types for methods with return values now use Lazy<MethodSetupBuilder<T>> for deferred, thread-safe setup creation. This is consistent and correct.

---

Remaining Concerns

1. PropertyMockCall.Setter has an uncontrolled multiple-registration bug (higher severity than Getter)

The Getter property registering a new setup on each access was flagged previously and has since been documented as intentional (supports sequential behavior chaining). That reasoning is defensible. However, the Setter property has the same pattern but with eager registration via VoidMockMethodCall, which makes it a genuine bug rather than a feature:

// In PropertyMockCall<T>:
public VoidMockMethodCall Setter
{
    get => new VoidMockMethodCall(_engine, _setterMemberId, $"{_propertyName} (set)",
               Array.Empty<IArgumentMatcher>());
    // ↑ Every access constructs a new VoidMockMethodCall, which eagerly calls EnsureSetup()
    // and registers a NEW MethodSetup in the engine
}

This means:

mock.Count.Setter.WasCalled(Times.Exactly(3));  // ← registers setup #1 as side effect
mock.Count.Setter.WasCalled(Times.Exactly(3));  // ← registers setup #2 as side effect

mock.VerifyAll();  // ← fails: setup #2 was never invoked

Unlike Getter, Setter eagerly registers, so accessing it once purely for verification still produces an uninvoked setup in the engine. The Getter case at least requires calling .Returns() before it registers.

The fix is the same as the Getter comment suggests for the lazy case: make Setter return a cached instance per PropertyMockCall, or defer registration until a setup chain method is explicitly called. Since PropertyMockCall<T> is a readonly struct recreated on each property access, the caching would need to live elsewhere (e.g., in the engine keyed by member ID, or via a different design for Setter).

2. Member name inconsistency between setup registration and verification in PropertyMockCall<T>

The setup side registers names with one convention and the verification side uses a different one:

// Setup (via Getter):
new MethodSetup(_getterMemberId, matchers, $"{_propertyName} (get)")  // → "Name (get)"

// Verification (via WasCalled):
_engine.CreateVerification(_getterMemberId, $"get_{_propertyName}", ...)  // → "get_Name"

// Setup (via PropertySetterMockCall):
new MethodSetup(_setterMemberId, matchers, $"{_propertyName} (set)")  // → "Name (set)"

// Verification (via PropertySetterMockCall.WasCalled):
_engine.CreateVerification(_setterMemberId, $"set_{_propertyName}", ...)  // → "set_Name"

The mock implementation itself uses "get_Name" / "set_Name" in call records (consistent with the C# convention). Error messages during setup failures will show "Name (get)" while error messages from verification failures will show "get_Name" — these two names appearing in the same diagnostic context when debugging a test failure is confusing. Unify to "get_{_propertyName}" / "set_{_propertyName}" throughout.

3. VoidMockMethodCall thread safety in EnsureSetup (minor)

The void path uses a manual null-check pattern:

private VoidMethodSetupBuilder EnsureSetup()
{
    if (_builder is null)  // ← not thread-safe (no Interlocked/volatile)
    {
        var setup = new MethodSetup(...);
        _engine.AddSetup(setup);
        _builder = new VoidMethodSetupBuilder(setup);
    }
    return _builder;
}

In practice this is harmless because the constructor calls EnsureSetup() eagerly and _builder is never null by the time any external caller invokes it. But it's inconsistent with the non-void path's Lazy<T> approach and the null-check will confuse readers who wonder if there's a valid uninitialized state. Consider replacing with Lazy<VoidMethodSetupBuilder> for consistency, or remove the null check and initialize _builder directly in the constructor.

---

Observations (no action required)

• C# 14 extension blocks for properties — using extension(Mock<T> mock) blocks to generate extension properties is clean and future-proof. Worth documenting that this requires LangVersion = preview / net10.0 for anyone reading the generated code.
• PropertyMockCall<T> stays as readonly struct — good. The struct is stack-allocated per access and the reference to the engine is stable. The allocation concern from the previous review applies mainly to MockMethodCall<T> and VoidMockMethodCall, which are now sealed class by design.
• MockOfT.cs constructor cleanup — the removal of setup/verify/raise parameters and the simplification to Mock(T mockObject, MockEngine<T> engine) is clean and correct.

---

Summary

The last commit (f225a2c9) correctly resolved the non-void thread-safety concern. The highest-priority remaining issue is the Setter property registering a new setup on every access — this is behaviorally different from the intentional Getter behavior because of eager registration, and will cause VerifyAll() failures for any test that accesses Setter more than once. The member name inconsistency is a secondary concern that will create confusing error messages in failure scenarios.

[thomhurst]

Addressed in f5f06f4 (responding to reviews 2 and 3):

Review 2 Issue 1 (Getter re-registration): This is pre-existing behavior from PropertySetupAccessor. Each access intentionally allows stacking setups (last wins). Already documented in prior commit.

Review 2 Issue 2 (Void eager + VerifyAll): The specific edge case is mock.Log(Arg.Any<string>()).WasNeverCalled() followed by mock.VerifyAll() — the eager registration creates a setup that was never invoked. In practice, if WasCalled() passes, the setup was also invoked, so VerifyAll() agrees. The WasNeverCalled() + VerifyAll() combo is the real concern, but no existing test hits this pattern (475/475 pass). A proper fix requires the engine to distinguish setup-registrations from verification-only objects — tracking as a follow-up.

Review 2 Issue 3 (Allocation regression): Deliberate. These are test-time ephemeral objects, not engine hot paths. The readonly struct → sealed class change was required for Lazy<T> and mutable builder fields.

Review 2 Issue 4 / Review 3 Issue 2 (Member name inconsistency): Fixed. Setup paths now use "get_Name" / "set_Name" matching the verification paths and the engine's call records.

Review 2 Issue 5 (Non-void silent no-op): Already documented. A [MustUseReturnValue]-style Roslyn analyzer is a good follow-up idea.

Review 3 Issue 1 (Setter re-registration): Fixed. PropertyMockCall.Setter now creates VoidMockMethodCall with eagerRegister: false. Accessing .Setter for verification no longer registers a setup. Setup methods (.Callback(), .Throws()) still trigger registration on demand.

Review 3 Issue 3 (VoidMockMethodCall consistency): Fixed. Now uses Lazy<T> matching the non-void pattern, with an eagerRegister parameter that forces evaluation in the constructor when true.

All 475 runtime tests and 10 snapshot tests pass.

[claude]

Code Review: Follow-up after f5f06f4

This is the fourth review pass on this PR. The previous three rounds of review (pre-f225a2c9 and pre-f5f06f4) surface-addressed thread safety and several design concerns. Here is where things stand now.

---

What Has Been Addressed

• Thread safety (non-void): MockMethodCall<TReturn> and generated non-void *_MockCall classes now use Lazy<T>, which is correct.
• Thread safety (void): VoidMockMethodCall uses Lazy<T> with eager initialization — also correct. The _lazyBuilder.Value call in the constructor ensures the setup is registered on construction before the object is visible to any other thread.
• PropertyMockCall<T>.Getter re-registration: Documented as intentional ("last wins" stacking). Acceptable given the prior-art in PropertySetupAccessor.
• Setter deferred registration: PropertyMockCall<T>.Setter now uses eagerRegister: false, and PropertySetterMockCall<T> handles its own eager registration. Consistent.

---

Remaining Issue 1: PropertySetterMockCall<T>.EnsureSetup() is inconsistent with the Lazy<T> pattern

VoidMockMethodCall and all non-void *_MockCall classes use Lazy<T> for safe lazy/eager initialization. PropertySetterMockCall<T> still uses a manual null-check:

private VoidMethodSetupBuilder EnsureSetup()
{
    if (_builder is null)
    {
        var matchers = new IArgumentMatcher[] { _matcher };
        var methodSetup = new MethodSetup(_setterMemberId, matchers, $"set_{_propertyName}");
        _engine.AddSetup(methodSetup);
        _builder = new VoidMethodSetupBuilder(methodSetup);
    }
    return _builder;
}

In practice, EnsureSetup() is called eagerly in the constructor so _builder is always non-null after construction. The null-check is unreachable in normal usage. But it's an inconsistency that could become a real race if someone subclasses or calls from a context where the object is visible before the constructor finishes. Recommend aligning with Lazy<T> for consistency:

private readonly Lazy<VoidMethodSetupBuilder> _lazyBuilder;

public PropertySetterMockCall(...) 
{
    ...
    _lazyBuilder = new Lazy<VoidMethodSetupBuilder>(() => {
        var matchers = new IArgumentMatcher[] { _matcher };
        var setup = new MethodSetup(_setterMemberId, matchers, $"set_{_propertyName}");
        _engine.AddSetup(setup);
        return new VoidMethodSetupBuilder(setup);
    });
    _ = _lazyBuilder.Value; // eager
}

---

Remaining Issue 2: VerifyAll + Void Eager Registration Semantic Conflict

This was raised in the previous review and thomhurst's response was truncated. The concern: when a user writes verification-only code for void methods:

mock.Log(Arg.Any<string>()).WasNeverCalled();

VoidMockMethodCall eagerly registers a MethodSetup before the user even calls .WasNeverCalled(). If the test then calls mock.VerifyAll(), the engine finds a registered setup for Log that was never invoked — and depending on how VerifyAll is implemented, this could produce a false failure ("Log was set up but never called").

The root issue is that eager void registration is a side effect of constructing the unified object, not of explicitly calling a setup method. Setup and verification now share a constructor, so it's impossible for the engine to know whether the user intends to set up or verify at construction time.

Suggested approach: Rather than eager registration at construction time, consider registering the setup only when a chain method with setup semantics is called, and verifying through CreateVerification for all WasCalled/WasNeverCalled paths. Strict mode compatibility (allowing the call without chaining) could be handled by a separate Allow() method:

mock.Log(Arg.Any<string>());           // lazy — does nothing yet
mock.Log(Arg.Any<string>()).Allow();    // explicit: "allow this call in strict mode"
mock.Log(Arg.Any<string>()).WasCalled(); // verify only — no setup registered

This is admittedly a larger API change, but it eliminates the ambiguity entirely. If that's too invasive, at minimum the documentation should warn about the VerifyAll interaction when using void methods for verification only.

---

Minor Issue 3: Lazy<T> Default Thread-Safety Mode

Lazy<T> without a constructor argument uses LazyThreadSafetyMode.ExecutionAndPublication, which acquires a lock on every value access until the value is initialized. Since most mock setups are single-threaded and the lock is held during _engine.AddSetup() (which may itself be thread-safe), this is fine but slightly heavier than necessary.

LazyThreadSafetyMode.PublicationOnly is a better fit here — it allows multiple threads to compute the value but only one registration "wins". However, since AddSetup has side effects (registering a MethodSetup), PublicationOnly could still cause double-registration races. The current ExecutionAndPublication is technically the safer choice. No change required — this is informational.

---

Minor Issue 4: Generated WasCalled Overloads — Repeated CreateVerification() Calls

In every generated *_MockCall class and in MockMethodCall<T> / VoidMockMethodCall, each WasCalled overload calls _engine.CreateVerification(...) independently:

public void WasCalled() => _engine.CreateVerification(_memberId, _memberName, _matchers).WasCalled();
public void WasCalled(Times times) => _engine.CreateVerification(_memberId, _memberName, _matchers).WasCalled(times);
public void WasCalled(Times times, string? message) => _engine.CreateVerification(_memberId, _memberName, _matchers).WasCalled(times, message);
public void WasCalled(string? message) => _engine.CreateVerification(_memberId, _memberName, _matchers).WasCalled(message);

Since only one of these is called per verification, this is not a runtime performance concern. But extracting a private ICallVerification BuildVerification() => _engine.CreateVerification(...) helper would reduce duplication and make it easier to spot if the arguments ever diverge. Low priority.

---

Overall Assessment

The API direction is sound — unified mock.Method().Returns() / mock.Method().WasCalled() is genuinely more ergonomic than mock.Setup.Method() / mock.Verify.Method(). The thread-safety work from previous review iterations is solid.

The two issues worth resolving before merge:

1. PropertySetterMockCall.EnsureSetup() alignment (minor, low risk but inconsistent with the rest of the codebase)
2. VerifyAll + void eager registration (needs confirmation that this is tested and documented if it's an accepted limitation)

The rest is polish.

[claude]

Code Review: Unified Mock API

This is a well-considered API simplification. Removing the three-surface model (Setup/Verify/Raise) in favor of direct extension methods on Mock<T> meaningfully reduces cognitive overhead and the amount of code users must write. The source generator consolidation from three builders into one MockMembersBuilder is a clear maintainability win. The test suite coverage is extensive and the snapshot updates look correct.

That said, there are a few design concerns worth discussing before merging.

---

Issue 1: Void method verification silently registers a setup (behavioral change)

This is the most important semantic difference from the old API. In the old API:

mock.Verify.Write(Arg.Any<string>()).WasCalled(); // no setup registered

In the new API:

mock.Write(Arg.Any<string>()).WasCalled(); // setup IS registered (eager in constructor)

The generated *_MockCall class for void methods with parameters calls EnsureSetup() eagerly in its constructor (MockMembersBuilder.cs:GenerateVoidUnifiedClass). This means every call to mock.Write(...) — including calls that are purely for verification — registers a new MethodSetup in the engine.

The consequence: in tests that first verify and then continue exercising the SUT, extra setups accumulate. This could cause hard-to-diagnose failures in strict mode (unexpected method-allowed errors) or mask genuine issues (a second setup silently matching a different call). It's a real footgun for users who adopt a verify-after pattern or copy-paste a verification call for reuse.

The PR description acknowledges this tradeoff, but consider whether the ergonomics gain is worth the hidden side-effect. An alternative:

// Opt-in to strict-mode allowance explicitly
mock.Write(Arg.Any<string>()).Allow(); // only way to register void setup
mock.Write(Arg.Any<string>()).WasCalled(); // verify only, no setup

This makes the side-effect explicit and keeps the verify path pure.

---

Issue 2: Thread-safety inconsistency in generated void wrapper classes

VoidMockMethodCall (the runtime base type) uses Lazy<VoidMethodSetupBuilder> — which is thread-safe by default. But the generated typed wrapper classes for void methods with parameters (e.g. IReadWriter_Write_M2_MockCall, MockMembersBuilder.cs:GenerateVoidUnifiedClass) use a manual null-check pattern:

private VoidMethodSetupBuilder? _builder;

private VoidMethodSetupBuilder EnsureSetup()
{
    if (_builder is null)  // not thread-safe
    {
        var setup = new MethodSetup(...);
        _engine.AddSetup(setup);
        _builder = new VoidMethodSetupBuilder(setup);
    }
    return _builder;
}

Currently this doesn't cause a race because EnsureSetup() is called eagerly in the constructor before the object is shared. But the code pattern is deceptive — if the eager-init is ever removed or the pattern is reused in a non-eager context, it becomes a latent data race. The return-type wrappers correctly use Lazy<T> throughout. Use Lazy<T> for the void wrappers too, or at minimum add a comment explaining why it's safe despite the pattern.

---

Issue 3: PropertyMockCall<T>.Getter registers a new setup on every access

From PropertyMockCall.cs:

public IPropertySetup<TProperty> Getter
{
    get
    {
        var methodSetup = new MethodSetup(_getterMemberId, matchers, $"get_{_propertyName}");
        _engine.AddSetup(methodSetup);  // NEW setup registered each access
        return new PropertySetupBuilder<TProperty>(methodSetup);
    }
}

The comment says this is intentional for chaining different behaviors. However, users might access .Getter for verification (checking what it returns), or store it in a variable and call it multiple times:

var name = mock.Name;
name.Getter.Returns("Alice"); // registers setup #1
// ... later
name.Getter.Returns("Bob");   // registers setup #2 (intentional)
// but:
name.WasCalled();             // verification — doesn't trigger Getter (fine)

This is actually consistent with the old PropertySetupAccessor behavior, so no regression here. But the documentation note is important. Consider whether PropertyMockCall<T> should use a lazy builder for .Getter similar to how MockMethodCall<T> works, registering only the most-recently configured setup.

---

Issue 4: Breaking change — no migration path documented

Mock<T>.Setup, Mock<T>.Verify, and Mock<T>.Raise are removed. InState(string, Action<IMockSetup<T>>) signature changed. These are breaking changes for any existing TUnit.Mocks users. The PR description explains the new API well, but there's no mention of a migration guide or version bump strategy. Even a brief "before/after" mapping in the docs would help.

---

Issue 5: MockMemberNames hardcoded set is a maintenance trap

MockMembersBuilder.cs:

private static readonly HashSet<string> MockMemberNames = new(...)
{
    "Object", "Engine", "Behavior", "Invocations", ...
};

This needs to be manually kept in sync with Mock<T> public members. If a new member is added to Mock<T>, an interface method with the same name would silently shadow or be shadowed by the extension method. Consider deriving this list from Mock<T>'s actual member names at source-gen time, or at minimum adding a comment linking back to MockOfT.cs.

---

Minor: struct → class for generated per-method wrappers

The old *_TypedSetup types were readonly struct. The new *_MockCall types are sealed class. This is necessary for the lazy/eager setup pattern (structs can't have mutable reference fields), so the change is correct. It does add one heap allocation per method call expression in test setup/verification code, but for test code this is inconsequential.

---

Summary

API ergonomics	Significant improvement
Source generator complexity	Reduced (3 builders → 1)
Void method verification side-effect	Design concern worth addressing
Thread safety in generated code	Inconsistent but not currently dangerous
Breaking change handling	Needs documentation

The architecture moves in the right direction. The void method eager-registration design decision is the one I'd most want to see reconsidered or at minimum explicitly documented as a deliberate trade-off in the API surface docs, since it represents a meaningful behavior change from the old model.

[claude]

Follow-up Review: Unified Mock API

Reviewing against my previous comments on this PR. Here's the status of each prior issue, plus new findings.

---

Prior Issues — Status Update

✅ Issue 1 (Thread Safety) — RESOLVED

The data race in EnsureSetup() has been fully fixed. Both MockMethodCall<TReturn> and VoidMockMethodCall now use Lazy<T> (which defaults to LazyThreadSafetyMode.ExecutionAndPublication), and the source generator emits the same pattern in the typed *_MockCall classes. This is correct.

✅ Issue 2 (Void/Non-Void Asymmetry) — Acknowledged and Documented

The eagerRegister parameter is now explicit and XML docs clearly explain the design intent. The public constructor of VoidMockMethodCall defaults to eagerRegister: true, and PropertyMockCall.Setter opts out with eagerRegister: false. The comment in MockMethodCall<TReturn> explaining "chain .Returns(default!)" is sufficient for the strict mode case. Acceptable.

❌ Issue 3 (WasCalled Allocates on Every Call) — Still Open

Each WasCalled() / WasNeverCalled() call in both concrete classes and all generated *_MockCall sealed classes still creates a fresh CallVerificationBuilder:

public void WasCalled() => _engine.CreateVerification(_memberId, _memberName, _matchers).WasCalled();
public void WasCalled(Times times) => _engine.CreateVerification(_memberId, _memberName, _matchers).WasCalled(times);
// ... repeated 6 times per class

Caching the verification builder (lazily, similar to the setup builder) or invoking verification directly on the engine without an intermediate builder object would reduce allocations in the happy path. Not blocking, but still worth addressing.

❌ Issue 4 (MockMemberNames Hardcoded List) — Still Open

MockMembersBuilder.cs still has a static HashSet<string> listing Mock<T> member names. If Mock<T> gains a new public member (e.g. a Record(...) method), the collision-avoidance logic silently misses it and generates an ambiguous extension. Deriving this list from the Mock<T> symbol in the compilation remains the correct long-term fix.

✅ Issue 5 (PropertyMockCall.Getter Creates New Setup on Each Access) — Documented

The comment now clearly explains the deliberate re-registration design for chaining. Acceptable.

---

New Issue: C# 14 Extension Block Syntax in Generated Code

This is the most important new concern. Property extensions are now generated using C# 14 extension(T receiver) syntax:

// Generated output — Interface_With_Properties.verified.txt
public static class IRepository_MockMemberExtensions
{
    extension(global::TUnit.Mocks.Mock<global::IRepository> mock)
    {
        public global::TUnit.Mocks.PropertyMockCall<string> Name
            => new(mock.Engine, 0, 1, "Name", true, true);
    }
}

Generated source files are compiled as part of the user's project. A user whose project has <LangVersion>13</LangVersion> (or lower — common on .NET 8/9 projects that haven't opted into preview) will get a compile error in the generated file the moment their mocked interface has any properties.

Meanwhile, method extensions are still generated using classic C# 8+ syntax (this Mock<T> mock), so mocks of interfaces with only methods continue to compile fine under older language versions. This creates an inconsistent experience where adding a property to a mocked interface suddenly breaks compilation in a confusing way.

Suggested alternatives:

Option A — Keep the same static extension class, expose a method returning PropertyMockCall<T>:

// Works with any C# version ≥ 8
public static PropertyMockCall<string> Name(this Mock<IRepository> mock)
    => new(mock.Engine, 0, 1, "Name", true, true);

The ergonomics become mock.Name() instead of mock.Name, which is a minor change but avoids the language version requirement.

Option B — Require C# 14 explicitly (document it as a minimum requirement for TUnit.Mocks). If TUnit.Mocks is already only targeting .NET 10+ consumers, this is more reasonable, but it should be a conscious, documented decision rather than an implicit side-effect.

Option A is preferable unless there's a strong reason to require C# 14. The getter-like access via extension property is nice ergonomics, but not worth imposing a language version floor on all consumers.

---

Minor Observations

• PropertyMockCall<TProperty> is a readonly struct while PropertySetterMockCall<TProperty> is a sealed class. The asymmetry in value vs. reference semantics within the same API surface is mildly surprising. Since PropertyMockCall only holds references, there's no performance benefit to the struct — consider aligning both to sealed class.

• The eagerRegister bool parameter on VoidMockMethodCall's internal constructor could become confusing as the type evolves. Replacing the bool with a static factory method (e.g., VoidMockMethodCall.Deferred(engine, ...)) would make intent clearer at each call site.

---

Summary

The thread-safety fix (Issue 1) was the most critical concern from the prior review and is now properly resolved with Lazy<T>. The overall direction of the API — removing the .Setup. / .Verify. indirection — is cleaner and the implementation is solid. The C# 14 extension block issue is the main thing I'd want addressed before merge, as it's a silent breaking change for projects targeting .NET 8/9 without preview LangVersion.

[claude]

Follow-up Review: Unified Mock API

Reviewing against my previous comments on this PR. Here is the status of each prior issue, plus new findings.

---

Prior Issues — Status Update

Issue 1 (Thread Safety) — RESOLVED

The data race in EnsureSetup() has been fully fixed. Both MockMethodCall<TReturn> and VoidMockMethodCall now use Lazy<T> (which defaults to LazyThreadSafetyMode.ExecutionAndPublication), and the source generator emits the same pattern in all typed *_MockCall classes. This is correct.

Issue 2 (Void/Non-Void Asymmetry) — Acknowledged and Documented

The eagerRegister parameter is now explicit and XML docs clearly explain the design intent. The public constructor of VoidMockMethodCall defaults to eagerRegister: true, and PropertyMockCall.Setter opts out with eagerRegister: false. The comment pointing to .Returns(default!) as the strict-mode escape hatch for non-void methods is sufficient. Acceptable.

Issue 3 (WasCalled Allocates on Every Call) — Still Open

Each WasCalled() / WasNeverCalled() call still creates a fresh CallVerificationBuilder in both the concrete classes and all generated *_MockCall sealed classes. Not blocking, but still worth a follow-up.

Issue 4 (MockMemberNames Hardcoded List) — Still Open

MockMembersBuilder.cs still has a static HashSet<string> listing Mock<T> member names. If Mock<T> gains a new public member, the collision-avoidance silently misses it and generates an ambiguous extension method. Deriving this list from the Mock<T> symbol at generator initialization remains the correct long-term fix.

Issue 5 (PropertyMockCall.Getter Creates New Setup on Each Access) — Documented

The comment now clearly explains the deliberate re-registration design for chaining behaviors. Acceptable.

---

New Issue: C# 14 Extension Block Syntax in Generated Code

This is the most important new concern. Property extensions are generated using C# 14 extension(T receiver) syntax:

// Actual generated output (Interface_With_Properties.verified.txt)
public static class IRepository_MockMemberExtensions
{
    extension(global::TUnit.Mocks.Mock<global::IRepository> mock)
    {
        public global::TUnit.Mocks.PropertyMockCall<string> Name
            => new(mock.Engine, 0, 1, "Name", true, true);
    }
}

Generated source files are compiled as part of the user's project, not TUnit's. A user whose project has <LangVersion>13</LangVersion> or lower — common on .NET 8 and .NET 9 projects that have not opted into preview — will receive a compile error in the generated file the moment their mocked interface has any properties.

Meanwhile, method extensions are still generated with classic this Mock<T> mock syntax (C# 8+), so mocks of interfaces with only methods continue to compile fine under older language versions. This creates an inconsistent experience: adding a property to a mocked interface suddenly breaks compilation with an obscure error about generated code, not a clear "upgrade your LangVersion" message.

Suggested alternatives:

Option A — Use a regular static extension method returning PropertyMockCall<T>:

// Works with any C# >= 8; same result, slightly different call-site syntax
public static PropertyMockCall<string> Name(this Mock<IRepository> mock)
    => new(mock.Engine, 0, 1, "Name", true, true);

Call-site becomes mock.Name() instead of mock.Name — a small ergonomics trade-off to avoid imposing a C# 14 floor on all consumers.

Option B — Document C# 14 / LangVersion preview as an explicit minimum requirement for TUnit.Mocks and emit a #error or analyzer diagnostic when it is not met. If .NET 10+ targeting is already assumed, this is a reasonable stance, but it should be a deliberate and documented decision rather than a silent side-effect.

Option A is preferable unless the property-access syntax (mock.Name vs mock.Name()) is considered important enough to impose the language version constraint.

---

Minor Observations

PropertyMockCall<T> struct vs PropertySetterMockCall<T> class asymmetry. PropertyMockCall<TProperty> is a readonly struct while PropertySetterMockCall<TProperty> is a sealed class. Since PropertyMockCall only holds references (IMockEngineAccess, string, int, bool), the struct gives no allocation benefit in practice — any passage through ICallVerification will box it anyway. Aligning both to sealed class would simplify the mental model without a meaningful performance cost.

eagerRegister bool parameter. The internal VoidMockMethodCall(..., bool eagerRegister) pattern works but will become hard to read as call sites accumulate. A static factory method (e.g., VoidMockMethodCall.Deferred(engine, ...)) would make intent explicit at each call site.

---

Summary

The thread-safety fix (Issue 1 from the prior review) was the most critical concern and is now properly resolved with Lazy<T> throughout both the concrete classes and generated wrappers. The overall direction — removing .Setup. / .Verify. indirection — is cleaner and the implementation is solid. The C# 14 extension block concern is the main item I would want addressed before merge, as it silently breaks compilation for any TUnit.Mocks consumer targeting .NET 8/9 without preview LangVersion whose mocked interface has properties.