feat(mocks): setup/verify on secondary interfaces of multi-type mocks

[thomhurst]

Closes the gap reported in discussion #4981 (comment 17280965): Mock.Of<DbContext, IInfrastructure<IServiceProvider>>() had no way to configure the secondary interface, and casting the object to it executed the real getter.

What users get

var dbContext = Mock.Of<DbContext, IInfrastructure<IServiceProvider>>();
dbContext.Instance.Returns(serviceProvider);   // secondary member, directly on the mock
dbContext.Instance.WasCalled();

((IInfrastructure<IServiceProvider>)dbContext.Object).Instance; // => serviceProvider

• Secondary members surface as extensions on Mock<T1> — setup, verify, raise, VerifyInOrder, unified Invocations, strict mode and Reset all work through the existing single engine.
• Name collisions across interfaces get a short interface prefix (mock.IBar_Tag); signature-identical members dedupe onto the primary surface and intercept calls through every cast.
• Misuse throws: using a secondary extension on a mock not created with that interface raises InvalidOperationException with the correct Mock.Of<...> hint, instead of a silently-never-matching setup.
• Class primaries (the DbContext case) now work end-to-end: previously the multi factory was never registered (Mock.Of<Class, IExtra>() threw) and the impl never implemented the secondary interface. Explicit/non-virtual class implementations are intercepted via explicit re-implementation. Ctor args supported: Mock.Of<T1, T2>(params object[]) up to T4.

Design notes

The same (T1, Tn) pair can appear in multiple combos (Mock.Of<T1,T2>() and Mock.Of<T1,T2,T3>()), where the member gets different union member IDs. The extension surface is therefore generated once per pair (deduped pipeline model) with local ordinals, and each combo factory registers a typeof(Tn) + localId -> unionId map on the engine (MockEngine.RegisterSecondaryInterface/TryGetSecondaryMemberId — new uniquely-named methods, no overload changes). This keeps one engine (no split Invocations/Reset semantics) while making the shared surface correct for every combo layout.

The single- and multi-type member walks were unified into one collector, making the primary-members-get-identical-IDs invariant structural instead of two hand-synced copies (which had already diverged for class primaries).

Testing

• 17 new runtime tests (MultipleInterfaceSecondarySetupTests): secondary setup/verify/reset/strict, cross-combo pair reuse, dedup, collision prefixing, wrong-mock guard, class primary with explicit impl / non-virtual blocker / ctor args, generic secondary interface.
• 3 new generator snapshot tests (interface-primary, class-primary with explicit impl, name collision).
• Full suites green: TUnit.Mocks.Tests 1106 (net10) / 1105 (net9) / 1090 (net8), generator snapshots 69/69, Mocks.Http 54, Mocks.Logging 31. Roslyn 4.14/4.4/4.7 generator variants build clean.

Docs: multi-interface section in docs/writing-tests/mocking/setup.md updated.

[claude]

This is a well-designed and thoroughly tested implementation of a genuinely tricky feature. The design notes in the PR description show careful thinking about the hard cases. A few observations:

Design: the indirection layer is solid

The core design — one engine per mock, pair models carrying local ordinals, a factory-registered map translating local→union IDs at runtime — is the right choice. The alternative of duplicating member IDs per-combo would have broken the dedup invariant across combos, and the alternative of splitting engines would have fragmented Invocations/Reset/strict mode. The approach chosen threads the needle well.

The DiscoveryState refactor (replacing the parallel local variables in DiscoverMembers and DiscoverMembersFromMultipleTypes with a single shared object) is a clean win and eliminates the hand-sync drift the PR description notes had already occurred.

Potential issue: ComputeMemberIdMap silently leaves setter slots unmapped when the primary has no setter

In SecondarySurfaceFactory.ComputeMemberIdMap, when a standalone property p has a setter but the matched union property up does not, the setter's slot in map stays at -1:

if (p.HasSetter && up.HasSetter)
{
    map[p.SetterMemberId] = up.SetterMemberId;
}

That means TryGetSecondaryMemberId returns false for the setter ID, and __Id throws InvalidOperationException with the unhelpful message "This mock was not created with '...' as a secondary interface" — even though it was, the setter just doesn't exist on the primary. This scenario (secondary interface has a read-write property that the primary only exposes read-only) is unlikely but worth a comment at minimum, or an explicit guard in TryGetSecondaryMemberId to give a cleaner error.

Minor: HasSecondaryInterface is not used in the generated code

MockEngine.SecondaryInterfaces.cs adds HasSecondaryInterface, but scanning the generated snapshots and the __Id resolver emitted by EmitSecondaryMemberIdResolver shows it's not called anywhere in the PR. If it's intended as a future API hook, a note would help; if it's dead code from an earlier draft, it could be removed to keep surface area minimal.

Minor: name-collision prefix breaks ComputeMemberIdMap lookup

In BuildPairModel, a secondary member whose name collides with the primary is renamed (m with { Name = $"{shortName}_{m.Name}" }). However, the renamed model still carries the original MemberId (the standalone ordinal), and ComputeMemberIdMap matches by GetMethodSignatureKey / GetPropertySignatureKey which use the original name — so the map computation is correct. But the renamed model's Name field is now inconsistent with the signature key used for matching, which could confuse future readers. A comment linking the two places would be worthwhile.

Minor: new int[] { ... } in generated code

The snapshot in Multi_Interface_Mock_With_Secondary_Setup_Surface.verified.txt shows:

engine.RegisterSecondaryInterface(typeof(global::IMultiDisposable), new int[] { 2, 3 });

Since the project targets net8+, this could use collection expression syntax ([2, 3]), matching the style used elsewhere in the generated code. Not a correctness issue.

Test coverage

The 17 runtime tests in MultipleInterfaceSecondarySetupTests cover the important cases thoroughly: the DbContext-shaped explicit impl, non-virtual blocker, constructor args, generic secondary interfaces, cross-combo pair reuse, collision prefixing, wrong-mock guard, and the dedup/order invariant. The snapshot tests validate three representative generator outputs. The coverage looks solid for an initial implementation.

Overall this is high-quality work that closes a real gap. The main thing worth addressing before merge is the setter-slot silent failure in ComputeMemberIdMap.

[codacy-production]

Not up to standards ⛔

🔴 Issues 4 high · 22 minor

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

Results:
26 new issues

Category	Results
UnusedCode	1 minor
BestPractice	1 minor
ErrorProne	4 high
CodeStyle	20 minor

View in Codacy

🟢 Metrics 116 complexity

Metric	Results
Complexity	116

View in Codacy

_{NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.}

[claude]

Follow-up review — checking previous findings

Comparing against my prior review to track what was addressed.

✅ Addressed: HasSecondaryInterface removed

The unused HasSecondaryInterface method is no longer in the diff. Clean.

⚠️ Still open: misleading error for unmapped setter slots

The ComputeMemberIdMap setter-slot issue remains. When a secondary interface has a read-write property but the union deduplicated it onto a primary getter-only property, map[setterMemberId] stays -1. TryGetSecondaryMemberId now correctly returns false for -1 entries (the bounds+sign guard added is correct), but the caller — __Id in every generated secondary surface file — still throws:

throw new InvalidOperationException(
  "This mock was not created with 'IFoo' as a secondary interface. Create it with Mock.Of<T1, IFoo>()...");

The mock was created with that interface; the setter just doesn't exist on the primary. This is the wrong error for the situation. The fix would be distinguishing the two failure modes in TryGetSecondaryMemberId (or its caller):

// Option A: two-result enum returned from TryGetSecondaryMemberId
public SecondaryMemberLookupResult TryGetSecondaryMemberId(Type interfaceType, int localMemberId, out int memberId)
// -> NotRegistered | MemberUnmapped | Ok

// Option B: separate HasSecondaryInterface check before throwing
if (!engine.HasSecondaryInterface(typeof(IFoo)))
    throw new InvalidOperationException("Create it with Mock.Of<T1, IFoo>()...");
else
    throw new InvalidOperationException("IFoo is registered but member X has no setup surface (e.g. setter not available on primary)");

This is still worth fixing before merge — the scenario is possible when a class primary exposes a read-only property that the secondary interface declares as read-write.

Minor: new int[] { ... } in generator still unaddressed

MockFactoryBuilder.EmitSecondaryInterfaceMapFields (line ~1079 in the diff) still emits new int[] { 2, 3 }. This projects to net8+ so collection expressions ([2, 3]) are fine. Not blocking, but was called out previously and the snapshots would need to be re-accepted after the change — easy to batch with the setter fix.

Minor: name-collision rename / signature-key comment still missing

The observation from the prior review stands: BuildPairModel renames colliding members (m with { Name = $"{shortName}_{m.Name}" }) but ComputeMemberIdMap matches by original signature key, so the renamed Name field is inconsistent with the lookup key. A one-line comment at the rename site saying "Name only; MemberId and signature key remain on the original for ComputeMemberIdMap" would save a future reader the confusion. Still minor, not blocking.

Overall

The core design is solid and the critical removal (HasSecondaryInterface) was done. The main open item before merge is the setter-slot error message — it's rare but when it fires it will actively mislead the developer. The two minor items (collection expressions, rename comment) can be done in a follow-up if preferred.

[thomhurst]

Addressed the follow-up review in 32d921e:

Setter-slot / unmapped-member error — fixed via your Option B. HasSecondaryInterface is back (and now actually called by generated code): __Id distinguishes "mock lacks the interface" (keeps the Mock.Of<T1, Tn>() hint) from "interface registered but slot unmapped" (honest "no setup mapping on this mock instance" message). Worth noting the unmapped path is currently unreachable through generated extensions — signature-identical members (including the read-write-on-getter-only case, since the property key is name+type) are excluded from the pair surface and configured through the primary extension instead — but the split keeps the error honest if emission ever changes.

Rename/signature-key comment — added at the rename site in BuildPairModel: rename touches Name only; ComputeMemberIdMap correlates by the original signature key, so the map is unaffected.

Collection expressions in generated code — declining this one: generated code compiles inside consumer projects whose LangVersion we don't control, and [2, 3] requires C# 12. Every other emission site in the generator sticks to new T[] { ... } / Array.Empty<T>() for the same reason, so this also keeps the emitted style uniform.

[claude]

Follow-up review — all previous findings resolved

Checking against my prior review.

Setter-slot / unmapped-member error — fixed

The Option B fix is correctly implemented. HasSecondaryInterface is re-added to MockEngine.SecondaryInterfaces.cs with a focused contract (checking the flat _secondaryMemberIdMaps pair array by reference equality), and the generated __Id resolver in the snapshots now calls it to branch between the two failure modes:

throw new InvalidOperationException(engine.HasSecondaryInterface(typeof(IMultiDisposable))
    ? "Member #" + localId + " of 'IMultiDisposable' has no setup mapping on this mock instance..."
    : "This mock was not created with 'IMultiDisposable' as a secondary interface. Create it with Mock.Of<...>()...");

The snapshot at Multi_Interface_Mock_With_Secondary_Setup_Surface.verified.txt line 90 confirms the generated code reflects this. Good — this path is currently unreachable through the generated extensions (the comment in the PR response is accurate), but if emission ever changes the error is now honest.

Rename/signature-key comment — added

The clarifying comment at the rename site in SecondarySurfaceFactory.cs ("Rename changes the extension's Name only; MemberId stays the standalone ordinal and ComputeMemberIdMap correlates by the ORIGINAL signature key") is in place. Clear and sufficient.

Collection expressions — declined (accepted)

The reasoning is sound: generated code compiles in consumer projects with unknown LangVersion, so staying with new T[] { ... } / Array.Empty<T>() keeps the emitted style safe and consistent. No issue.

Overall

All findings from the previous two reviews are resolved. The implementation is solid: the core design is correct, the error surfaces are honest, test coverage is thorough, and the snapshots are updated. This is ready to merge.