refactor(mocks): move Mock<T> control members to static Mock helpers

[thomhurst]

Summary

• Moved 11 public instance members off Mock<T> onto the Mock static class (VerifyAll, Reset, GetInvocations, GetBehavior, SetupAllProperties, GetDiagnostics, SetState, InState, SetDefaultValueProvider, etc.)
• Mock<T> now has exactly one public member: Object — Engine moved behind explicit IMockEngineAccess<T> interface, all control methods are static
• IMock methods implemented explicitly so MockRepository batch operations continue working unchanged
• Added Mock.Get<T>(T obj) — retrieves the Mock<T> wrapper for any mock object, replacing GetAutoMock (no magic strings, no redundant type params)
• Source generator reserved names reduced from 16 to 5 (Object + 4 inherited System.Object members) — user interfaces with members like Reset, Invocations, VerifyAll, Engine now generate clean extension method names without the _ suffix
• Updated all 19 test files, 3 documentation files, and all 11 snapshots

Motivation

Source-generated extension methods on Mock<T> can collide with TUnit's own instance members. When a user's interface has a method named Reset(), the generator had to emit mock.Reset_() with a trailing underscore — ugly and confusing. By moving control operations to static helpers and hiding the engine behind an explicit interface, the collision surface drops from 16 names to just 5 (the unavoidable Object + System.Object inherited members).

New API

// Control operations — static helpers
Mock.VerifyAll(mock)
Mock.Reset(mock)
Mock.GetInvocations(mock)
Mock.GetBehavior(mock)
Mock.SetState(mock, "s")
Mock.InState(mock, "s", m => ...)
Mock.GetDiagnostics(mock)
Mock.SetupAllProperties(mock)
Mock.SetDefaultValueProvider(mock, provider)

// Auto-mock access — Mock.Get replaces GetAutoMock
var serviceB = mock.Object.GetServiceB();  // auto-mocked
var autoMock = Mock.Get(serviceB);          // get the wrapper
autoMock.GetValue().Returns(42);

// Engine access for generated code (hidden from IntelliSense)
Mock.GetEngine(mock)  // [EditorBrowsable(Never)]

Consistent with the existing static pattern: Mock.Of<T>(), Mock.VerifyInOrder(...).

Test plan

• TUnit.Mocks library builds on all TFMs (netstandard2.0, net8.0, net9.0, net10.0)
• All 489 runtime tests pass (net10.0)
• All 11 source generator snapshot tests pass
• Documentation examples updated

[claude]

Code Review: refactor(mocks): move Mock control members to static Mock helpers

The core motivation here is solid — eliminating the _ suffix on source-generated extension methods when a user interface has a member named Reset, Invocations, etc. is a real ergonomics win. The implementation is clean and the tests/docs are comprehensively updated. That said, here are some concerns worth addressing:

---

⚠️ Breaking Change Without a Deprecation Path

This PR removes 11 public instance members from Mock<T> in a single step. Any user currently calling mock.VerifyAll(), mock.Reset(), mock.Invocations, mock.Behavior, etc. will get compile errors with no warning ahead of time.

Suggested approach: Add [Obsolete("Use Mock.VerifyAll(mock) instead. This member will be removed in a future version.", error: false)] wrappers temporarily, allowing users to migrate. This is especially important since the TUnit.Mocks library is still relatively new but likely has early adopters.

---

🐛 Mock.Invocations(mock) Called Multiple Times in Loops

In the updated test (AdditionalCoverageTests.cs):

for (int i = 1; i < Mock.Invocations(mock).Count; i++)
{
    await Assert.That(Mock.Invocations(mock)[i].SequenceNumber)
        .IsGreaterThan(Mock.Invocations(mock)[i - 1].SequenceNumber);
}

Mock.Invocations(mock) calls mock.Engine.GetAllCalls() on every iteration. If GetAllCalls() returns a new snapshot each time, this is wasteful and potentially inconsistent. Even if it returns the same backing list, this is a pattern that teaches users bad habits.

The previous mock.Invocations property had the same behavior but was at least a single assignment point. Now users are more likely to call this in a loop without realizing it. Consider caching in the test to set a better example:

var invocations = Mock.Invocations(mock);
for (int i = 1; i < invocations.Count; i++) { ... }

---

🔍 GetAutoMock Requires Both Type Parameters — Ergonomics Regression

Before:

var autoMock = mock.GetAutoMock<IServiceB>("GetServiceB");

After:

var autoMock = Mock.GetAutoMock<IServiceA, IServiceB>(mock, "GetServiceB");

C# cannot do partial type inference, so users must now specify both IServiceA and IServiceB. The parent type T is redundant information since mock is already typed as Mock<IServiceA>.

A cleaner alternative: keep it as an extension method on Mock<T> since it doesn't conflict with any user interface member (it's not a name a typical interface would have):

// Extension method — no collision risk with user interfaces
public static Mock<TChild> GetAutoMock<T, TChild>(this Mock<T> mock, string memberName) ...

Or at minimum, add an overload that takes the mock as a typed parameter so T can still be inferred:

// At call site, only TChild needs to be specified
var autoMock = Mock.GetAutoMock<IServiceB>(mock, "GetServiceB");

This requires restructuring the generic constraints but would restore the original ergonomics.

---

📝 Naming Inconsistency in the Static API

The new static methods have inconsistent naming conventions:

Pattern	Method
No Get prefix	Mock.Invocations(mock)
Get prefix	Mock.GetBehavior(mock), Mock.GetDiagnostics(mock), Mock.GetAutoMock(mock, ...)
Set prefix	Mock.SetState(mock, ...), Mock.SetDefaultValueProvider(mock, ...)
Action name	Mock.Reset(mock), Mock.SetupAllProperties(mock), Mock.VerifyAll(mock)

Why is Invocations not GetInvocations? Consider making this consistent — either all property-style accessors get a Get prefix, or none do.

---

💭 Minor: Indirect Cast in Reset Static Helper

// Mock.cs
public static void Reset<T>(Mock<T> mock) where T : class
    => ((IMock)mock).Reset();  // routes through explicit interface implementation

// MockOfT.cs  
void IMock.Reset() => Engine.Reset();

The ((IMock)mock).Reset() approach works but adds a boxing-like indirection for no benefit. The static helper could call mock.Engine.Reset() directly since Engine is accessible (just [EditorBrowsable(Never)]). Same applies to VerifyAll and VerifyNoOtherCalls, though those have real logic rather than just a delegation.

This is minor, but VerifyAll and VerifyNoOtherCalls in particular duplicate logic rather than delegate — consider factoring the common verification logic into a shared internal method so the explicit interface implementations and future overloads don't drift.

---

✅ What's Done Well

• The primary motivation (reducing reserved name collisions from 16 to 6) is achieved cleanly. The snapshot diff (Reset_ → Reset) is exactly right.
• The explicit IMock interface implementations keep MockRepository batch operations working with zero changes — good use of the existing interface design.
• Comprehensive test and documentation updates across all 19 test files and 3 doc files.
• The approach is consistent with the existing static pattern already established by Mock.Of<T>() and Mock.VerifyInOrder(...).

The biggest ask before merging is the deprecation path for existing users. Everything else is either a naming polish or an ergonomics improvement.

[claude]

Code Review

Summary

The motivation here is solid: name collisions between instance members and source-generated extension methods (e.g., user interfaces with forcing ) are a real UX problem, and the static-helper pattern is consistent with the existing / style. The overall direction is good.

---

Breaking Change Without a Migration Strategy

Moving 11 public instance members off Mock<T> is a breaking change for any existing consumer. The PR description documents what moved but not how existing users migrate. Consider:

• Adding [Obsolete("Use Mock.Reset(mock) instead.", error: false)] shims on the removed members for one version cycle, then removing them in the next major bump.
• Alternatively, document this as a semver-major change in the changelog so downstream adopters know to search-and-replace.

Without either, adopters upgrading between patch/minor versions will hit unexpected compile errors with no guidance in the error message itself.

---

Inconsistent GetInvocations Caching in Tests

In several tests, Mock.GetInvocations(mock) is called once per assertion on the same snapshot:

// AdditionalCoverageTests.cs — 5 separate calls, each allocates a new list
await Assert.That(Mock.GetInvocations(mock)).HasCount().EqualTo(4);
await Assert.That(Mock.GetInvocations(mock)[0].MemberName).IsEqualTo("Add");
await Assert.That(Mock.GetInvocations(mock)[1].MemberName).IsEqualTo("Log");
await Assert.That(Mock.GetInvocations(mock)[2].MemberName).IsEqualTo("GetName");
await Assert.That(Mock.GetInvocations(mock)[3].MemberName).IsEqualTo("Add");

The PR already uses the correct pattern once (the sequence-number loop), but doesn't apply it consistently. The method name GetInvocations looks like a cached property accessor; in practice it calls GetAllCalls() on every invocation. Caching locally makes the allocation cost visible and the intent clearer:

var invocations = Mock.GetInvocations(mock);
await Assert.That(invocations).HasCount().EqualTo(4);
await Assert.That(invocations[0].MemberName).IsEqualTo("Add");
// ...

This is a test-code concern, but the inconsistency could mislead future authors into thinking the method is cheap/cached.

---

IMockEngineAccess<T> Is a Public Leaky Abstraction

IMockEngineAccess<T> is marked [EditorBrowsable(Never)] but remains public. Any external assembly can:

var engine = ((IMockEngineAccess<IFoo>)mock).Engine; // bypasses GetEngine

Since MockEngine<T> is the real internal surface, exposing it through a public (if hidden) interface couples external code to the engine's concrete type. If MockEngine<T> ever needs to be replaced or renamed, this becomes a breaking change on a path you didn't intend to support.

Better approach: Make IMockEngineAccess<T> internal. Mock.GetEngine is the intended gateway for generated code — keep the interface as a private implementation detail of the Mock<T> class. The existing [EditorBrowsable(Never)] hint works for IDE discoverability but doesn't protect API surface in compiled assemblies.

---

Positive Notes

• Mock.Get<T>(T mockedObject) is a genuinely better API than GetAutoMock<T>(string methodName). Requiring callers to have the actual object rather than remembering a string name is more type-safe and less error-prone.
• ConditionalWeakTable for the object→wrapper registry is the right choice — mock objects are GC-eligible normally and the mapping evicts automatically.
• The netstandard2.0 compatibility path for AddOrUpdate is handled cleanly with the #if NET7_0_OR_GREATER guard.
• Shrinking the reserved-name set from 16 to 6 names is the concrete win that motivated all this work — the snapshot diff showing Reset_ → Reset is the proof it works.
• All 11 snapshot files are updated, and the test counts confirm no regressions.

[claude]

Code Review

Summary

The motivation here is solid: name collisions between Mock<T> instance members and source-generated extension methods (e.g., user interfaces with Reset() forcing Reset_()) are a real UX problem, and the static-helper pattern is consistent with the existing Mock.Of<T>() / Mock.VerifyInOrder() style. The overall direction is good.

---

Breaking Change Without a Migration Strategy

Moving 11 public instance members off Mock<T> is a breaking change for any existing consumer. The PR description documents what moved but not how existing users migrate. Consider:

• Adding [Obsolete("Use Mock.Reset(mock) instead.", error: false)] shims on the removed members for one version cycle, then removing them in the next major bump.
• Alternatively, document this as a semver-major change in the changelog so downstream adopters know to search-and-replace.

Without either, adopters upgrading between patch/minor versions will hit unexpected compile errors with no guidance in the error message itself.

---

Inconsistent GetInvocations Caching in Tests

In several tests, Mock.GetInvocations(mock) is called once per assertion on the same snapshot:

// AdditionalCoverageTests.cs — 5 separate calls, each allocates a new list
await Assert.That(Mock.GetInvocations(mock)).HasCount().EqualTo(4);
await Assert.That(Mock.GetInvocations(mock)[0].MemberName).IsEqualTo("Add");
await Assert.That(Mock.GetInvocations(mock)[1].MemberName).IsEqualTo("Log");
await Assert.That(Mock.GetInvocations(mock)[2].MemberName).IsEqualTo("GetName");
await Assert.That(Mock.GetInvocations(mock)[3].MemberName).IsEqualTo("Add");

The PR already uses the correct pattern once (the sequence-number loop), but doesn't apply it consistently. The method name GetInvocations looks like a cached property accessor; in practice it calls GetAllCalls() on every invocation. Caching locally makes the allocation cost visible and the intent clearer:

var invocations = Mock.GetInvocations(mock);
await Assert.That(invocations).HasCount().EqualTo(4);
await Assert.That(invocations[0].MemberName).IsEqualTo("Add");
// ...

This is a test-code concern, but the inconsistency could mislead future authors into thinking the method is cheap/cached.

---

IMockEngineAccess<T> Is a Public Leaky Abstraction

IMockEngineAccess<T> is marked [EditorBrowsable(Never)] but remains public. Any external assembly can:

var engine = ((IMockEngineAccess<IFoo>)mock).Engine; // bypasses GetEngine

Since MockEngine<T> is the real internal surface, exposing it through a public (if hidden) interface couples external code to the engine's concrete type. If MockEngine<T> ever needs to be replaced or renamed, this becomes a breaking change on a path you didn't intend to support.

Better approach: Make IMockEngineAccess<T> internal. Mock.GetEngine is the intended gateway for generated code — keep the interface as a private implementation detail of the Mock<T> class. The existing [EditorBrowsable(Never)] hint works for IDE discoverability but doesn't protect API surface in compiled assemblies.

---

Positive Notes

• Mock.Get<T>(T mockedObject) is a genuinely better API than GetAutoMock<T>(string methodName). Requiring callers to have the actual object rather than remembering a string name is more type-safe and less error-prone.
• ConditionalWeakTable for the object-to-wrapper registry is the right choice — mock objects are GC-eligible normally and the mapping evicts automatically.
• The netstandard2.0 compatibility path for AddOrUpdate is handled cleanly with the #if NET7_0_OR_GREATER guard.
• Shrinking the reserved-name set from 16 to 6 names is the concrete win that motivated all this work — the snapshot diff showing Reset_ to Reset is the proof it works.
• All 11 snapshot files are updated, and the test counts confirm no regressions.

[claude]

Code Review

Great refactor! The core motivation is solid: reducing the reservation set from 16 to 5 eliminates a class of ugly -suffixed extension method collisions. The static-helper pattern is internally consistent with and , and the approach (replacing magic-string ) is a clean improvement.

What's good

• API symmetry — all control operations now live on the static class, matching the existing / pattern. Users have one mental model.
• **** — retrieving a wrapper from the concrete object is far better UX than requiring a magic string + redundant type parameter. The keyed on the mock object is the right data structure (correct GC semantics, no memory leaks).
• Explicit interface implementations — as explicit implementations keeps batch operations working while removing the methods from the public surface.
• Snapshot and doc coverage — all 11 snapshots and 3 doc files updated in the same PR.

---

Issues

1. throws on type mismatch — misleading error

If a user has a registered and calls , they get an with a runtime message pointing at an internal cast rather than their code. They might not understand why it failed. A type-safe check is cheap and gives a much better error:

This is architecturally better because it distinguishes "not a mock at all" from "mock of the wrong type", producing actionable errors in both cases.

---

2. Pre-.NET 7 fallback is logically inconsistent

The comment correctly states "each mock object is unique so Add should not throw." But if that invariant holds, the catch block is dead code. If it can throw (e.g., someone calls with the same object twice), then the + is a race condition — another thread could insert between those two calls and the second would throw again.

Pick one consistent approach: if the invariant is guaranteed, remove the try/catch entirely. If re-registration is a real scenario, use a lock or restructure. The current code gives false confidence of thread safety.

---

3. should be , not

is only called from the constructor, which lives in the same assembly (). It does not need to be . Making it actually enforces the encapsulation rather than relying on IntelliSense suppression, which is easily bypassed.

, by contrast, legitimately needs to be because it's called from source-generated extension methods that live in the user's compilation — that's correct.

---

4. Minor: still calls twice on consecutive lines

The last commit explicitly fixed the loop case, but still makes two allocating calls:

Given the rename to was specifically chosen to signal "this call has cost," it would be consistent to cache the result here too. Minor, but the commit message set the expectation.

---

Nit

The new file is named while the interface name is . C# convention usually uses the backtick form in the file name () or the same verbatim name ( with a suffix to distinguish from the non-generic sibling). The suffix is a workable alternative but it's inconsistent with how (non-generic) is named. Not a blocker.

---

Summary

The architectural direction is right and the implementation is clean. The main issues to address before merging are the misleading from (issue 1) and the logically confused pre-.NET 7 fallback (issue 2). Issue 3 ( visibility) is a meaningful encapsulation concern. Issue 4 is a minor consistency fix.

[claude]

Code Review

Great refactor! The core motivation is solid: reducing the MockMemberNames reservation set from 16 to 5 eliminates a class of ugly _-suffixed extension method collisions. The static-helper pattern is internally consistent with Mock.Of<T>() and Mock.VerifyInOrder(...), and Mock.Get<T> (replacing magic-string GetAutoMock) is a clean improvement.

What's good

• API symmetry — all control operations now live on the static Mock class, matching the existing Mock.Of<T>() / Mock.VerifyInOrder(...) pattern. Users have one mental model.
• Mock.Get<T> — retrieving a wrapper from the concrete object is far better UX than requiring a magic string + redundant type parameter. The ConditionalWeakTable keyed on the mock object is the right data structure (correct GC semantics, no memory leaks).
• Explicit interface implementations — IMock.VerifyAll/VerifyNoOtherCalls/Reset as explicit implementations keeps MockRepository batch operations working while removing the methods from the public surface.
• Snapshot and doc coverage — all 11 snapshots and 3 doc files updated in the same PR.

---

Issues

1. Mock.Get<T> throws InvalidCastException on type mismatch — misleading error

public static Mock<T> Get<T>(T mockedObject) where T : class
{
    if (_objectToMock.TryGetValue(mockedObject, out var mock))
    {
        return (Mock<T>)mock;  // InvalidCastException if T is wrong
    }
    throw new InvalidOperationException("...");
}

If a user has a Mock<IServiceB> registered and calls Mock.Get<IServiceA>(serviceB), they get an InvalidCastException with a runtime message pointing at an internal cast rather than their code. A type-safe check is cheap and gives a much better error:

if (_objectToMock.TryGetValue(mockedObject, out var mock))
{
    if (mock is Mock<T> typed)
        return typed;
    throw new InvalidOperationException(
        $"The object is a mock of '{mock.GetType().GenericTypeArguments[0].Name}', not '{typeof(T).Name}'.");
}

This is architecturally better because it distinguishes "not a mock at all" from "mock of the wrong type", producing actionable errors in both cases.

---

2. Pre-.NET 7 Register fallback is logically inconsistent

// Each mock object is unique so Add should not throw, but be safe.
try { _objectToMock.Add(mockObject, mockWrapper); }
catch (ArgumentException) { _objectToMock.Remove(mockObject); _objectToMock.Add(mockObject, mockWrapper); }

The comment correctly states "each mock object is unique so Add should not throw." But if that invariant holds, the catch block is dead code. If it can throw (e.g., someone calls Register with the same object twice), then Remove + Add is a TOCTOU race condition — another thread could insert between those two calls and the second Add would throw again.

Pick one consistent approach: if the invariant is guaranteed (objects are always unique), remove the try/catch entirely. If re-registration is a real scenario, use a lock or restructure. The current code gives false confidence of thread safety.

---

3. Mock.Register should be internal, not public [EditorBrowsable(Never)]

Register is only called from the Mock<T> constructor, which lives in the same assembly (TUnit.Mocks). It does not need to be public. Making it internal actually enforces the encapsulation rather than relying on IntelliSense suppression, which is easily bypassed.

GetEngine, by contrast, legitimately needs to be public because it is called from source-generated extension methods that live in the user's compilation — that's correct.

---

4. Minor: InvocationsTests.cs still calls GetInvocations twice consecutively

The last commit explicitly fixed the loop case, but Invocations_Contains_Correct_Method_Names still makes two allocating calls:

await Assert.That(Mock.GetInvocations(mock)[0].MemberName).IsEqualTo("GetValue");
await Assert.That(Mock.GetInvocations(mock)[1].MemberName).IsEqualTo("Process");

Given the rename to GetInvocations was specifically chosen to signal "this call has cost", caching in a local variable here would be consistent with that reasoning. Minor, but the commit message set the expectation.

---

Nit

The new file is named IMockEngineAccessOfT.cs while the interface name is IMockEngineAccess<T>. C# convention usually uses the verbatim type name as the file name. The OfT suffix is workable but mildly inconsistent with how the non-generic sibling IMockEngineAccess.cs is named. Not a blocker.

---

Summary

The architectural direction is right and the implementation is clean. Issues to address before merging:

1. Issue 1 (recommended): Catch the wrong-type scenario in Mock.Get<T> with a meaningful error instead of an opaque InvalidCastException.
2. Issue 2: Resolve the logical contradiction in the pre-.NET 7 Register fallback.
3. Issue 3: Make Register internal — it has no reason to be public.
4. Issue 4: Nit — cache GetInvocations in the remaining test.

[claude]

Code Review: refactor(mocks): move Mock control members to static Mock helpers (Follow-up)

This is a follow-up to the previous review. The refactoring is solid and the majority of earlier concerns have been addressed. Here's the updated assessment:

---

✅ Previous Issues — Addressed

Naming inconsistency: Fixed. Invocations → GetInvocations, giving all read accessors a consistent Get prefix.

Loop anti-pattern in tests: Fixed. Mock.GetInvocations(mock) is now captured before the loop, not called per-iteration.

GetAutoMock ergonomics: Replaced entirely with Mock.Get<T>(T mockedObject) — a significantly better API that eliminates both the string name and the redundant parent type parameter. No casting or guessing required.

Indirect cast in Reset: Addressed. Mock.Reset(mock) now calls GetEngine(mock).Reset() directly. VerifyAll/VerifyNoOtherCalls correctly still route through ((IMock)mock) since the logic lives there.

---

⚠️ Remaining Concern: Breaking Change Without Deprecation

The prior review raised this and it remains unaddressed. All 11 removed public members (mock.VerifyAll(), mock.Reset(), mock.Invocations, mock.Behavior, mock.DefaultValueProvider, etc.) disappear without any [Obsolete] bridge. Early adopters will get compile errors with no guidance.

Recommended approach — even a single release with warnings would help:

[Obsolete("Use Mock.VerifyAll(mock) instead. Will be removed in the next version.")]
public void VerifyAll() => Mock.VerifyAll(this);

If the library is intentionally pre-1.0 and breaking changes are acceptable without notice, this can be skipped — but it should be called out in release notes.

---

🐛 Mock.Get<T> Limitation With Multi-Interface Mocks

The _objectToMock table registers a proxy object against a single IMock (the first type parameter). Consider:

var mock = Mock.Of<IMultiLogger, IMultiDisposable>();
IMultiDisposable disposable = mock.Object;  // proxy implements both interfaces

// This will throw: "The object is a mock of 'IMultiLogger', not 'IMultiDisposable'"
var wrapper = Mock.Get<IMultiDisposable>(disposable);

The proxy implements IMultiDisposable correctly, but Mock.Get<IMultiDisposable> fails because the table holds Mock<IMultiLogger>. The error message is misleading in this case — it says "not a mock of IMultiDisposable" when the object truly does mock that interface.

This is a real footgun if a user receives a multi-interface mock object typed as the second interface and tries to retrieve its wrapper. Two options:

1. Covariant lookup: If IMultiDisposable is a known secondary interface, return the same Mock<IMultiLogger> wrapper (it controls both).
2. Better error message: "This object is a multi-interface mock registered under IMultiLogger. Call Mock.Get<IMultiLogger>(obj) instead." — requires storing the registered type name.

---

💡 DefaultValueProvider Setter Loses Property-Style Ergonomics

Previously:

mock.DefaultValueProvider ??= new MyProvider();

Now requires:

if (Mock.GetDefaultValueProvider(mock) is null)
    Mock.SetDefaultValueProvider(mock, new MyProvider());

This is a minor ergonomic regression. Since DefaultValueProvider won't conflict with user interface names (it's not a common interface method name), an extension property on Mock<T> would preserve the nice syntax without polluting the reserved-name list significantly. That said, this is a low-priority polish item.

---

✅ What's Well Done

• The core motivation is sound and the snapshot diff proves it works: Reset_() → Reset() in Multi_Method_Interface.verified.txt is the concrete win.
• ConditionalWeakTable for _objectToMock is GC-friendly and correct. The .NET < 7 fallback using Add (safe because each proxy is new()) is well-reasoned.
• IMockEngineAccess<T> with [EditorBrowsable(Never)] correctly hides the engine from IntelliSense while keeping it accessible to generated code.
• All 19 test files, 11 snapshots, and 3 doc files are consistently updated — no orphaned old API usage left behind.
• Mock.GetEngine being [EditorBrowsable(Never)] but public is the right choice for source-generator consumption without polluting user IntelliSense.

---

Summary: The refactoring achieves its stated goal cleanly. The breaking change concern is the most actionable item before merging. The multi-interface Mock.Get limitation is worth documenting at minimum, and fixing if multi-interface auto-mocking is a supported use case.

[claude]

Code Review: refactor(mocks): move Mock control members to static Mock helpers (Follow-up)

This is a follow-up to the previous review. The refactoring is solid and the majority of earlier concerns have been addressed. Here's the updated assessment:

---

Previous Issues — Addressed

Naming inconsistency: Fixed. Invocations => GetInvocations, giving all read accessors a consistent Get prefix.

Loop anti-pattern in tests: Fixed. Mock.GetInvocations(mock) is now captured before the loop, not called per-iteration.

GetAutoMock ergonomics: Replaced entirely with Mock.Get<T>(T mockedObject) — a significantly better API that eliminates both the string name and the redundant parent type parameter. No casting or guessing required.

Indirect cast in Reset: Addressed. Mock.Reset(mock) now calls GetEngine(mock).Reset() directly. VerifyAll/VerifyNoOtherCalls correctly still route through ((IMock)mock) since the logic lives there.

---

Remaining Concern: Breaking Change Without Deprecation

The prior review raised this and it remains unaddressed. All 11 removed public members (mock.VerifyAll(), mock.Reset(), mock.Invocations, mock.Behavior, mock.DefaultValueProvider, etc.) disappear without any [Obsolete] bridge. Early adopters will get compile errors with no guidance.

Recommended approach — even a single release with warnings would help:

[Obsolete("Use Mock.VerifyAll(mock) instead. Will be removed in the next version.")]
public void VerifyAll() => Mock.VerifyAll(this);

If the library is intentionally pre-1.0 and breaking changes are acceptable without notice, this can be skipped — but it should be called out in release notes.

---

Mock.Get<T> Limitation With Multi-Interface Mocks

The _objectToMock table registers a proxy object against a single IMock (the first type parameter). Consider:

var mock = Mock.Of<IMultiLogger, IMultiDisposable>();
IMultiDisposable disposable = mock.Object;  // proxy implements both

// This throws: "The object is a mock of 'IMultiLogger', not 'IMultiDisposable'"
var wrapper = Mock.Get<IMultiDisposable>(disposable);

The proxy implements IMultiDisposable correctly, but Mock.Get<IMultiDisposable> fails because the table holds Mock<IMultiLogger>. The error message is misleading — it says "not a mock of IMultiDisposable" when the object does mock that interface.

Two options:

1. Covariant lookup: Return the same Mock<IMultiLogger> wrapper when the registered type is a known secondary interface.
2. Better error message: "This object is a multi-interface mock registered under IMultiLogger. Call Mock.Get<IMultiLogger>(obj) instead."

---

DefaultValueProvider Setter Loses Property-Style Ergonomics

Previously: mock.DefaultValueProvider ??= new MyProvider();

Now requires:

if (Mock.GetDefaultValueProvider(mock) is null)
    Mock.SetDefaultValueProvider(mock, new MyProvider());

Since DefaultValueProvider is unlikely to collide with user interface names, an extension property on Mock<T> could preserve the nice syntax. Low-priority polish item.

---

What's Well Done

• The core motivation is sound and the snapshot diff proves it: Reset_() => Reset() in Multi_Method_Interface.verified.txt is the concrete win this whole PR is built around.
• ConditionalWeakTable for _objectToMock is GC-friendly and correct. The pre-.NET 7 fallback using Add (safe because each proxy is unique by construction) is well-reasoned.
• IMockEngineAccess<T> with [EditorBrowsable(Never)] correctly hides the engine from IntelliSense while keeping it accessible to generated code.
• All 19 test files, 11 snapshots, and 3 doc files are consistently updated — no orphaned old API usage left behind.

---

Summary: The refactoring achieves its stated goal cleanly. The breaking change concern is the most actionable item before merging. The Mock.Get multi-interface limitation is worth at least documenting, and fixing if multi-interface auto-mocking is a supported use case.