feat: strongly-typed event raises and event subscription API

[thomhurst]

Summary

• Source generator now produces typed Raises{EventName}() methods on setup wrapper structs, replacing string-based Raises(nameof(...), args) with IntelliSense-friendly, compile-time-safe alternatives
• Wrapper structs implement both IMethodSetup<T> + ISetupChain<T> (or IVoidMethodSetup + IVoidSetupChain) with explicit interface implementations, and public self-returning methods that preserve the wrapper type through the entire fluent chain
• 0-parameter methods now also get typed wrappers when the mock type has events, so mock.Setup.DoSomething().RaisesStatusChanged("done") works
• New strongly-typed mock.Events surface for event subscription tracking — generated as a C# 14 extension property on Mock<T> only when the type has events (non-nullable, zero-allocation struct)
• Removed string-based OnSubscribe, OnUnsubscribe, GetEventSubscriberCount, WasEventSubscribed methods from Mock<T> in favor of the typed API

Event Raises — Before/After

// Before
mock.Setup.Process(Arg.Any<int>())
    .Returns(true)
    .Raises(nameof(IProcessService.StatusChanged), "completed");

// After
mock.Setup.Process(Arg.Any<int>())
    .Returns(true)
    .RaisesStatusChanged("completed");

Event Subscription — Before/After

// Before
mock.OnSubscribe("StatusChanged", () => subscribed = true);
var count = mock.GetEventSubscriberCount("StatusChanged");
var was = mock.WasEventSubscribed("StatusChanged");

// After
mock.Events.StatusChanged.OnSubscribe(() => subscribed = true);
var count = mock.Events.StatusChanged.SubscriberCount;
var was = mock.Events.StatusChanged.WasSubscribed;

Test plan

• Source generator builds cleanly (0 errors)
• All 10 snapshot tests pass with updated .verified.txt files
• All 5 AutoRaiseEventTests pass using the new typed API
• All 12 event subscription tests pass using the new typed API
• Full mock test suite passes (472/472 tests)

[claude]

Code Review: feat: strongly-typed event raises on setup chains

The core idea here is well-executed: replacing magic-string with IntelliSense-safe is a clear ergonomic win, and the test and snapshot coverage is thorough.

Potential Bug: Behavioral Inconsistency

The public and explicit-interface implementations of have divergent behavior:

// Public method — calls _inner.Then() but discards the return value
public IReadWriter_Write_M2_TypedSetup Then() { _inner.Then(); return this; }

// Explicit interface implementation — doesn't call _inner.Then() at all
global::TUnit.Mocks.Setup.IVoidMethodSetup global::TUnit.Mocks.Setup.IVoidSetupChain.Then() => this;

If _inner.Then() advances internal state (e.g., for sequential setups or per-call behavior), the explicit interface path silently skips that side effect. The behaviour then depends on whether the user holds the concrete wrapper type or a typed interface reference — which is the kind of subtle polymorphism bug that's hard to track down. If _inner.Then() is purely identity/no-op, this is harmless, but it's worth making the two paths consistent.

Generated Code Bloat — Even for Event-Free Interfaces

Every wrapper now unconditionally adds ~15 explicit interface implementations regardless of whether the interface has events:

// Generated for *every* wrapper, even IGreeter with no events
global::TUnit.Mocks.Setup.ISetupChain<string> global::TUnit.Mocks.Setup.IMethodSetup<string>.Returns(string value) => _inner.Returns(value);
global::TUnit.Mocks.Setup.ISetupChain<string> global::TUnit.Mocks.Setup.IMethodSetup<string>.Returns(global::System.Func<string> factory) => _inner.Returns(factory);
// ... 13 more lines ...

The dual-interface implementation (needed to preserve the wrapper type through the fluent chain so that RaisesX() is accessible) makes sense when there are events. But for interfaces with no events, there's no RaisesX() to expose, so the wrapper type doesn't need to be preserved past the first call — and all these explicit implementations add generated code with no user benefit. Conditioning the dual-interface + explicit-implementation block on events.Length > 0 (same guard already used for the RaisesX() methods themselves) would keep the generated output lean.

Missing Test: 0-parameter Methods

The PR notes that "0-parameter methods now also get typed wrappers when the mock type has events", but AutoRaiseEventTests only exercises 1-parameter methods (Process(int), Execute(string)). A test confirming that mock.Setup.SomeZeroParamMethod().RaisesStatusChanged("done") actually works and raises the event would close that gap.

Multi-parameter Event Arg Encoding

For events with multiple handler parameters, args are boxed into object?[]:

argsExpr = $", (object?)new object?[] {{ {argNames} }}";

Worth verifying that the underlying Raises(string eventName, object? args) implementation correctly detects and unpacks object?[] vs a plain value for multi-parameter event handlers. If it passes the array as a single args value, multi-param events would be raised incorrectly.

Minor: BOM Removal in Snapshots

All 8 snapshot files drop the UTF-8 BOM from the // <auto-generated/> header line. This looks like incidental tool/encoding noise rather than an intentional change — worth confirming it's expected.

What Works Well

• The generated RaisesX() methods are exactly the right granularity — per-event, strongly typed, composable in the same fluent chain.
• Keeping Raises(string, object?) for backwards compat is the right call.
• The removal of the dead isVoid branch in GenerateTypedCallbackOverload (both branches were identical before) is a clean simplification.
• Snapshot tests are updated and passing — good confidence in the generator output.
• The nameof() improvements to docs and tests are a low-cost correctness win.

[claude]

Code Review: feat: strongly-typed event raises on setup chains

The core idea here is well-executed: replacing magic-string .Raises("EventName", args) with IntelliSense-safe .RaisesEventName(args) is a clear ergonomic win, and the test and snapshot coverage is thorough.

Potential Bug: Then() Behavioral Inconsistency

The public and explicit-interface implementations of Then() have divergent behavior:

// Public method — calls _inner.Then() but discards the return value
public IReadWriter_Write_M2_TypedSetup Then() { _inner.Then(); return this; }

// Explicit interface implementation — doesn't call _inner.Then() at all
global::TUnit.Mocks.Setup.IVoidMethodSetup global::TUnit.Mocks.Setup.IVoidSetupChain.Then() => this;

If _inner.Then() advances internal state (e.g., for sequential setups or per-call behavior), the explicit interface path silently skips that side effect. The behaviour then depends on whether the user holds the concrete wrapper type or a typed interface reference — which is the kind of subtle polymorphism bug that's hard to track down. If _inner.Then() is purely identity/no-op, this is harmless, but it's worth making the two paths consistent.

Generated Code Bloat — Even for Event-Free Interfaces

Every wrapper now unconditionally adds ~15 explicit interface implementations regardless of whether the interface has events:

// Generated for *every* wrapper, even IGreeter with no events
global::TUnit.Mocks.Setup.ISetupChain<string> global::TUnit.Mocks.Setup.IMethodSetup<string>.Returns(string value) => _inner.Returns(value);
global::TUnit.Mocks.Setup.ISetupChain<string> global::TUnit.Mocks.Setup.IMethodSetup<string>.Returns(global::System.Func<string> factory) => _inner.Returns(factory);
// ... 13 more lines per wrapper ...

The dual-interface implementation (needed to preserve the wrapper type through the fluent chain so that RaisesX() is accessible after SetsOutParameter() etc.) makes sense when there are events. But for interfaces with no events, there's no RaisesX() to expose, so the wrapper type doesn't need to be preserved past the first call — and all these explicit implementations add generated code with no user benefit. Conditioning the dual-interface + explicit-implementation block on events.Length > 0 (the same guard already used for the RaisesX() methods) would keep the generated output lean for the common case.

Missing Test: 0-parameter Methods with Events

The PR notes that "0-parameter methods now also get typed wrappers when the mock type has events", but AutoRaiseEventTests only exercises 1-parameter methods (Process(int), Execute(string)). A test confirming that a zero-param method wrapper correctly generates and calls RaisesStatusChanged("done") would close that gap.

Multi-parameter Event Arg Encoding

For events with multiple handler parameters, args are boxed into an array:

argsExpr = $", (object?)new object?[] {{ {argNames} }}";

Worth verifying that the underlying Raises(string eventName, object? args) implementation correctly detects and unpacks object?[] vs a plain value for multi-parameter event handlers. If it passes the array as a single opaque args value, multi-param events would be raised incorrectly.

Minor: BOM Removal in Snapshots

All 8 snapshot files drop the UTF-8 BOM from the // <auto-generated/> header line. This looks like incidental encoding noise rather than an intentional change — worth confirming it's expected.

What Works Well

• The generated RaisesX() methods are exactly the right granularity: per-event, strongly typed, and composable in the same fluent chain.
• Keeping Raises(string, object?) for backwards compat is the right call.
• The removal of the dead isVoid branch in GenerateTypedCallbackOverload (both branches were identical) is a clean simplification.
• Snapshot tests are updated and passing — good confidence in the generator output.
• The nameof() improvements to docs and tests are a low-cost correctness win.

[claude]

Follow-up Code Review: feat: strongly-typed event raises on setup chains

Building on the previous review, here's a status check on those findings now that I've read the source directly.

---

Previous Issues: Status

1. Then() Behavioral Inconsistency — Low severity (harmless in practice)

The inconsistency flagged before is still present in MockSetupBuilder.cs:

// Explicit interface implementation — skips _inner.Then()
{setupInterface} {chainType}.Then() => this;

// Public method — calls _inner.Then(), discards the return value, returns this
public {wrapperName} Then() { _inner.Then(); return this; }

After reading VoidMethodSetupBuilder.cs and MethodSetupBuilder.cs, _inner.Then() is confirmed to be a pure identity method with no side effects (both builders implement it as return this). So the divergence is harmless today, but the public method is still doing unnecessary work — calling _inner.Then() and discarding the result before returning this. The more honest implementation would be symmetric:

// Option A: both paths just return this (since _inner.Then() is a no-op)
{setupInterface} {chainType}.Then() => this;
public {wrapperName} Then() => this;  // no need to call _inner

// Option B: both call _inner.Then() for future-proofing
{setupInterface} {chainType}.Then() { _inner.Then(); return this; }
public {wrapperName} Then() { _inner.Then(); return this; }

Either is fine. The current state is just slightly misleading code.

2. Generated Code Bloat for Event-Free Interfaces — Still present

The dual-interface + explicit implementations block is emitted unconditionally for every typed wrapper, regardless of events.Length. For a type with 10 methods and no events, that's ~150 extra generated lines providing no new capability to callers (no RaisesX() methods are added).

Wrapping the explicit interface implementations in if (events.Length > 0) — matching the guard already used for GenerateTypedEventRaises — would keep the generated files lean for the common case while preserving the feature for event-bearing types.

Note: After reading the code more carefully, the dual-interface approach does provide a secondary benefit for event-free types: it enables Callback(Action<T1,T2>) typed overloads to be accessible after calling other chain methods. If this chaining scenario is considered important even without events, then the bloat is justified and this issue can be closed.

3. Missing Test for 0-parameter Methods with Events — Still open

AutoRaiseEventTests only exercises methods with at least one parameter. The ShouldGenerateTypedWrapper change that returns true when nonOutParams.Count == 0 && hasEvents is a new code path with no test coverage. A simple test verifying a zero-param method produces a valid RaisesX() call would close this gap.

4. Multi-parameter Event Arg Encoding — Needs verification

The generated code for multi-param events:

argsExpr = $", (object?)new object?[] {{ {argNames} }}";

This boxes the args into an array before passing to _inner.Raises(string, object?). This is a concern if the underlying Raises implementation checks the type of args to unpack — if it treats any non-array value as a single arg and unpacks array values, this is correct. If it always passes args through as-is to the event handler, multi-param events would receive an object[] where they expect individual args. No multi-param event test exists to catch a regression here.

---

New Observation: Unnecessary _inner.Then() Call

Looking at the generated public Then() (both void and return variants):

public {wrapperName} Then() { _inner.Then(); return this; }

_inner.Then() returns an interface value that is immediately discarded. This is a unnecessary allocation-free call today, but it's cargo-cult code — the return value is thrown away, so it has no effect. Either remove the call (and document that Then() on the wrapper is just return this) or use the return value. This is purely a code clarity issue.

---

What Remains Strong

• The EventSubscriptionAccessor struct design is clean and focused.
• IMockEvents<T> following the same marker-interface + extension pattern as IMockSetup<T>, IMockVerify<T>, and IMockRaise<T> is architecturally consistent.
• Backwards compatibility with string-based Raises() is preserved correctly.
• The isVoid parameter removal from GenerateTypedCallbackOverload was a clean simplification.
• The overall feature — typed RaisesX() on setup chains — is well-scoped and the generated output matches the intent.

[claude]

Code Review: feat: strongly-typed event raises on setup chains (Follow-up #3)

This review covers the final state of the PR, which includes all 4 commits. The previous two reviews flagged several issues; this review checks their current status and adds new observations from the complete diff.

---

Previous Issues: Status After All Commits

1. Missing test for 0-parameter methods with events — Still open

ShouldGenerateTypedWrapper returns true when nonOutParams.Count == 0 && hasEvents, but AutoRaiseEventTests only exercises methods with at least one parameter. The new code path has no test. A simple case like:

public interface IEventSource
{
    event EventHandler Changed;
    void Trigger(); // 0 params, should still get RaisesChanged()
}

...would verify the nonOutParams.Count == 0 && hasEvents branch actually works end-to-end.

2. Multi-parameter event arg encoding — Still unverified

The generated code for events with multiple parameters boxes them into an object?[]:

argsExpr = $", (object?)new object?[] {{ {argNames} }}";

EventSubscriptionVerifyTests references ICustomEventService.OnMultiParamAction but only tests subscription tracking — never a setup-time RaisesOnMultiParamAction() call. Without a round-trip test, a regression in how the engine unpacks the array vs. a single value would be silent.

3. Then() discards _inner.Then() return value — Still present

public {wrapperName} Then() { _inner.Then(); return this; }

The return value of _inner.Then() is discarded. Since _inner.Then() is confirmed to be a return this no-op on both builders, this is harmless — but the call is misleading. The explicit interface counterpart is the honest version:

{setupInterface} {chainType}.Then() => this;  // just returns this, correctly

The public method should match, either by also just returning this directly or by using the _inner return value.

---

New Observations

4. Mock<T>.Events is nullable but tests use ! — API usability concern

Test code throughout EventSubscriptionSetupTests.cs uses the null-forgiving operator:

mock.Events!.DataReady.OnSubscribe(() => callbackFired = true);

For a mock of an interface with events, Events will never be null — the factory always sets it. The ! is there purely because the property is typed as IMockEvents<T>?. This puts burden on users: they either use ! everywhere (suppressing useful nullable checks) or null-check on every access.

The underlying tension is that Events must be nullable for mocks of event-free interfaces. One way to reduce the friction: a non-nullable extension helper or a convenience mock.Events<T>() that throws a descriptive error if null. Not blocking, but worth documenting clearly that Events is non-null whenever the mocked type has events.

5. Mock<T> constructor proliferation is growing

There are now 3 constructors:

• Mock(object, setup, verify, engine) — 4 params
• Mock(object, setup, verify, raise, engine) — 5 params
• Mock(object, setup, verify, raise, events, engine) — 6 params

All are [EditorBrowsable(Never)] (generator-only), but each new surface adds another overload. A parameter object (MockSurfaces) or primary constructor with optional fields would keep this from growing unboundedly as the system evolves. Not urgent, but the pattern will become unwieldy at 7–8 params.

6. Multi-interface mocks silently drop Events — worth documenting

In MockFactoryBuilder.BuildInterfaceFactory:

if (model.Events.Length > 0 && model.AdditionalInterfaceNames.Length == 0)
{
    // only emits events surface for single-interface mocks
}

Multi-interface mocks (e.g., Mock.Of<IFoo, IBar>() where IFoo has events) will have mock.Events == null even though the primary type has events. This is a deliberate trade-off but not documented in the generated code or user docs. A user hitting this will get a runtime null without a helpful error message.

---

What's Solid

• The IMockEvents<T> marker-interface + extension-properties pattern is architecturally consistent with IMockSetup<T>, IMockVerify<T>, and IMockRaise<T>. The uniformity is good.
• EventSubscriptionAccessor is clean, focused, and allocation-free on the hot path.
• The dual explicit-interface + public-self-returning method approach in the wrapper structs correctly solves the covariant return problem without breaking AOT or adding heap allocations (structs, no boxing in the normal fluent path).
• Backward compatibility with string-based .Raises(eventName, args) is preserved, including in the explicit interface implementations.
• Test coverage for subscription tracking (WasSubscribed, SubscriberCount, OnSubscribe, OnUnsubscribe) is thorough.
• The C# 14 extension(...) block usage for MockEventsExtensions is modern and consistent with how MockSetupExtensions uses extension members elsewhere.

---

Summary

The feature is well-designed and the core API is clean. Three of the four previously flagged issues remain open (the 0-param test gap, multi-param event encoding verification, and Then() no-op call). The new observations around nullable Events usability and multi-interface event surfacing are worth addressing before merging — particularly documenting the multi-interface limitation, since it's a silent null rather than an error.

[claude]

Code Review: feat: strongly-typed event raises on setup chains (Follow-up #4)

Reviewing the final state of the PR, including the last commit that switched to C# 14 extension blocks for mock.Events.

---

Previous Issues: Updated Status

1. mock.Events nullable concern — Resolved

The C# 14 extension(Mock<T> mock) block approach is a clean fix. Events now only exists as a property when the mocked type has events, so no null-forgiving operators, no runtime null, no marker interface needed. The test files no longer use !, confirming this works.

2. Mock<T> constructor proliferation — Resolved by design change

The extension-property approach sidesteps the parameter problem entirely. No new constructor parameter needed.

3. Multi-interface mocks silently dropping Events — Resolved by design change

The extension-only approach means multi-interface mocks simply don't have an Events property — which is a compile error, not a runtime null. This is the right trade-off.

---

Remaining Issues

4. Generated code bloat for event-free interfaces — Still present

This was flagged in reviews 2 and 3 and is still unresolved. The Simple_Interface_With_One_Method.verified.txt snapshot confirms it: IGreeter_Greet_M0_TypedSetup (no events on IGreeter) still receives the full ~15-line explicit interface implementation block:

global::TUnit.Mocks.Setup.ISetupChain<string> global::TUnit.Mocks.Setup.IMethodSetup<string>.Returns(string value) => _inner.Returns(value);
// ... 14 more lines ...
global::TUnit.Mocks.Setup.IMethodSetup<string> global::TUnit.Mocks.Setup.ISetupChain<string>.Then() => this;
// ... 3 more lines ...

These exist solely to preserve the concrete wrapper type through the chain so RaisesX() methods remain accessible. For event-free interfaces there are no RaisesX() methods, so this structural overhead provides no user-visible benefit. Conditioning the dual-interface + explicit-implementation block on events.Length > 0 — matching the existing guard for GenerateTypedEventRaises — would keep generated output lean for the common case without affecting any event-bearing interface.

5. Missing test for 0-parameter methods with events — Still open

ShouldGenerateTypedWrapper now returns true when nonOutParams.Count == 0 && hasEvents, but AutoRaiseEventTests still only tests methods with ≥ 1 parameter (Process(int), Execute(string)). The new branch has no end-to-end test:

public interface IEventSource
{
    event EventHandler Changed;
    void Trigger(); // 0 params — should get RaisesChanged()
}

A single test verifying this generates and invokes correctly would close the gap.

6. Multi-parameter event arg encoding — Still unverified

The generated code for events with multiple handler parameters boxes args into object?[]:

argsExpr = $", (object?)new object?[] {{ {argNames} }}";

EventSubscriptionVerifyTests references ICustomEventService.OnMultiParamAction but only for subscription tracking, never for a round-trip RaisesOnMultiParamAction() call. Without a test that sets up RaisesX() on a multi-param event and then verifies the handler actually receives the correct individual arguments, a regression in array unpacking vs. single-value passthrough would be silent.

7. Then() discards _inner.Then() return value — Still present, still harmless

public {wrapperName} Then() { _inner.Then(); return this; }

The return value of _inner.Then() is thrown away. Since both builders implement Then() as return this, this has no runtime effect — but it's misleading code. The explicit interface counterpart is the honest version (=> this). The public method should either match it or use the returned value.

---

What Improved Since Last Review

• The C# 14 extension-block approach for mock.Events is the right architectural choice. It eliminates nullable friction, removes a constructor parameter, and handles multi-interface mocks gracefully with a compile error rather than a runtime null.
• EventSubscriptionAccessor remains clean and allocation-free.
• Removing OnSubscribe, OnUnsubscribe, GetEventSubscriberCount, WasEventSubscribed from Mock<T> in favour of the typed surface is the right call — the API is cleaner.

---

Summary

Issues 4–7 remain open; #5 and #6 (test gaps) are the most actionable before merge since they leave new code paths untested. #4 (bloat) and #7 (Then no-op call) are lower priority but worth tracking.

[claude]

Code Review: feat: strongly-typed event raises and event subscription API (Follow-up #5)

This review covers the final commit — "fix: address code review feedback on setup wrappers" — which was added in response to the four open issues from the previous review.

---

Previous Issues: All Resolved ✅

#4 — Generated code bloat for event-free interfaces ✅

Confirmed resolved by snapshot diff. Interface_With_Async_Methods.verified.txt shows that event-free wrapper structs no longer carry interface inheritance or explicit implementations:

// Before (every event-free wrapper got ~15 extra lines)
public readonly struct IAsyncService_GetValueAsync_M0_TypedSetup : global::TUnit.Mocks.Setup.IMethodSetup<string>

// After (clean, no interface, no explicit impls)
public readonly struct IAsyncService_GetValueAsync_M0_TypedSetup

Methods now return the concrete wrapper type directly (self-returning pattern), which is exactly the right trade-off: the fluent chain preserves the type only when RaisesX() methods actually exist.

#7 — Then() discarded _inner.Then() return value ✅

Confirmed resolved by snapshot diff. Interface_With_Events.verified.txt shows the explicit interface implementation now calls _inner.Then():

// Before (skipped the side effect)
global::TUnit.Mocks.Setup.IVoidMethodSetup global::TUnit.Mocks.Setup.IVoidSetupChain.Then() => this;

// After (consistent with the public method)
global::TUnit.Mocks.Setup.IVoidMethodSetup global::TUnit.Mocks.Setup.IVoidSetupChain.Then() { _inner.Then(); return this; }

Both the explicit interface and public paths now behave identically — no more hidden divergence.

#5 — Missing test for 0-parameter methods with events ✅

Added per commit message. The new IMultiEventService interface (Trigger() 0-param void, Query() 0-param return) with Raises_Event_From_Zero_Param_Void_Method and Raises_Event_From_Zero_Param_Return_Method tests cover the nonOutParams.Count == 0 && hasEvents branch end-to-end.

#6 — Multi-parameter event arg encoding unverified ✅

Addressed. The Raises_Multi_Param_Event_Unpacks_Arguments test verifies the full round-trip: RaisesOnMultiParam("hello", 42) fires the handler with individual arguments correctly unpacked from the object?[] encoding. This closes the silent-regression risk.

---

Remaining Minor Observation

The dual-interface approach doubles the generated API surface

For event-bearing wrappers, every public method is now declared twice — once as an explicit interface implementation (returning the interface type) and once as a public method (returning the concrete wrapper type):

// Explicit impl for interface compatibility
global::TUnit.Mocks.Setup.IVoidSetupChain global::TUnit.Mocks.Setup.IVoidMethodSetup.Throws<TException>() => _inner.Throws<TException>();

// Public self-returning method for RaisesX() accessibility  
public INotifier_Notify_M0_TypedSetup Throws<TException>() where TException : global::System.Exception, new() { _inner.Throws<TException>(); return this; }

This is the correct solution to the covariant return problem and the trade-off is justified — but it means that any new method added to IMethodSetup<T> or IVoidMethodSetup in the future will require updating the generator in two places (public method + explicit impl) for event-bearing types. Consider adding a comment in MockSetupBuilder near the explicit-impl block noting this coupling, so future contributors don't accidentally add a public method without the matching explicit impl (which would break IMethodSetup<T> compatibility).

---

Overall Assessment

The feature is complete and all four previously flagged issues have been resolved. The C# 14 extension-block approach for mock.Events is architecturally clean, the event-free wrapper bloat is gone, Then() is now consistent across both dispatch paths, and the new code paths have end-to-end test coverage. This is ready to merge.