fix: share interface members with the class members implementing them - #854
Merged
Conversation
A combination mock re-lists the additional interface in its base list and emits an explicit implementation for every one of its members. That re-mapping is what makes a member the class implements non-virtually configurable through the interface slot, but applied to a member the mock already overrides it forked one member into two: separate member ids, separate registry keys, separate recorded interactions. `mock.Mock.Setup.Multiply(...)` then had no effect on `((ICalculator)mock).Multiply(...)`, and neither surface saw the other's interactions. When the mocked class already implements the additional interface, rebase the interface members it implements overridably onto the class member - same containing type, so both surfaces resolve to one registry key and one member id - and stop re-implementing them, letting interface mapping resolve the slot to the mock's own override. Members the class implements non-virtually or explicitly keep their own containing type and stay re-implemented. Matching requires the return/member type to agree on top of the containing-type- independent comparers, so a member hidden via `new` with a different signature never rebases. Indexers are excluded: they are keyed by their parameter signature rather than by the declaring type and already shared storage. Interface mocks are excluded as well - an inherited interface member already carries its declaring interface on both surfaces, and a hidden member is meant to stay separate.
There was a problem hiding this comment.
Pull request overview
Fixes combination mocks so overridable class/interface members share registry IDs and interactions.
Changes:
- Adds member aliasing and rebasing.
- Avoids duplicate interface implementations for aliased members.
- Adds generator and runtime coverage.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
Tests/Mockolate.Tests/MockTests.ImplementedInterfaceTests.cs |
Tests shared runtime behavior. |
Tests/Mockolate.SourceGenerators.Tests/MockTests.CombinationTests.cs |
Tests generated combination mocks. |
Source/Mockolate.SourceGenerators/Sources/Sources.MockDelegate.cs |
Initializes alias-aware IDs. |
Source/Mockolate.SourceGenerators/Sources/Sources.MockCombination.cs |
Rebases implemented interfaces. |
Source/Mockolate.SourceGenerators/Sources/Sources.MockClass.cs |
Skips aliased implementations. |
Source/Mockolate.SourceGenerators/Sources/Sources.MemberIds.cs |
Resolves aliases to shared IDs. |
Source/Mockolate.SourceGenerators/Entities/Property.cs |
Allows containing-type rebasing. |
Source/Mockolate.SourceGenerators/Entities/MockClass.cs |
Tracks implemented interfaces. |
Source/Mockolate.SourceGenerators/Entities/Method.cs |
Allows containing-type rebasing. |
Source/Mockolate.SourceGenerators/Entities/MemberAliases.cs |
Defines alias mappings. |
Source/Mockolate.SourceGenerators/Entities/Event.cs |
Allows containing-type rebasing. |
Source/Mockolate.SourceGenerators/Entities/Class.cs |
Implements member rebasing. |
Suppressed comments (3)
Source/Mockolate.SourceGenerators/Entities/Class.cs:284
- A same-named virtual property can coexist with an explicit implementation of this interface property. In that case this heuristic aliases to the unrelated public property and changes interface dispatch instead of preserving the explicit implementation. Use the compiler's actual interface-member mapping and rebase only when its mapped property is overridable.
Property? target = implementor.AllProperties().FirstOrDefault(candidate
=> candidate is { IsIndexer: false, ExplicitImplementation: null, IsStatic: false, } &&
candidate.Type == property.Type &&
Property.ContainingTypeIndependentEqualityComparer.Equals(candidate, property));
Source/Mockolate.SourceGenerators/Entities/Class.cs:305
- This can alias an interface event to an unrelated virtual event when the class explicitly implements the interface event and also declares a public event with the same name and type. Suppressing re-implementation then changes which event the interface slot addresses. Resolve the actual interface implementation symbol before creating the alias.
Event? target = implementor.AllEvents().FirstOrDefault(candidate
=> candidate is { ExplicitImplementation: null, IsStatic: false, } &&
candidate.Type == @event.Type &&
Event.ContainingTypeIndependentEqualityComparer.Equals(candidate, @event));
Source/Mockolate.SourceGenerators/Entities/Class.cs:263
- Generic interface implementations are missed when corresponding type parameters use different names.
IFoo.Echo<T>(T)can be implemented byEcho<U>(U), butName, parameterFullname, andReturnTypecompare the literalT/Unames, so the member is not rebased and its setup/interaction state remains forked. Compare generic signatures by type-parameter ordinal rather than identifier.
candidate.ReturnType == method.ReturnType &&
Method.ContainingTypeIndependentEqualityComparer.Equals(candidate, method));
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
🚀 Benchmark ResultsDetails
Details
Details
Details
Details
Details
|
…dexers Follow-up to sharing interface members with the class members implementing them. Which class member implements which interface member was derived by matching signatures, which missed members differing only in a nullability annotation or in the name of a generic type parameter, and skipped indexers entirely. MockClass now asks Roslyn (FindImplementationForInterfaceMember) which class member fills each interface slot and carries the pairings in the equatable model. RebaseOnto looks that implementation up in the mock's own surface, so an implementation the mock cannot override - non-virtual, sealed, explicit, or with an inaccessible signature - keeps its own containing type and stays re-implemented, which is the only way to reach that slot. This makes the guarantee local instead of resting on the member filter in Class' ctor, and replaces the coarse ImplementedInterfaces name check. Indexers now rebase too. They had been left forked on the grounds that their setups already match by access, but their value storage is keyed by a signature index derived from the containing type, so `mock[1] = 5` followed by `((I)mock)[1]` read two different slots.
…members Rebasing rewrote the member's only containing type, so generated setup, verify and raise documentation of a shared member named the class member even on the interface surface. Members now keep a DeclaredContainingType that survives rebasing and feeds all documentation, while registry keys, member ids and backing-field names stay on the rebased ContainingType. Since DeclaredContainingType participates in record equality, a rebased member can no longer equal its target, making the equal-skip guard in MemberAliases.Add dead code: removed, with the invariant documented. Also build the FindImplementation lookups eagerly in the MockClass constructor instead of lazily, and cover raising a shared event through the interface surface, a get-only interface property implemented with a setter, and two interfaces implemented by the same class member.
|
Comment on lines
+255
to
+256
| new EquatableArray<Property>(Properties.AsArray() | ||
| .Select(property => Rebase(property, implementor, aliases)).ToArray()), |
github-actions Bot
added a commit
that referenced
this pull request
Aug 31, 2026
…lass members implementing them (#854) by Valentin Breuß
github-actions Bot
added a commit
that referenced
this pull request
Aug 31, 2026
…lass members implementing them (#854) by Valentin Breuß
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A combination mock re-lists the additional interface in its base list and emits an explicit implementation for every one of its members. That re-mapping is what makes a member the class implements non-virtually configurable through the interface slot, but applied to a member the mock already overrides it forked one member into two: separate member ids, separate registry keys, separate recorded interactions.
mock.Mock.Setup.Multiply(...)then had no effect on((ICalculator)mock).Multiply(...), and neither surface saw the other's interactions.When the mocked class already implements the additional interface, rebase the interface members it implements overridably onto the class member - same containing type, so both surfaces resolve to one registry key and one member id
Matching requires the return/member type to agree on top of the containing-type- independent comparers, so a member hidden via
newwith a different signature never rebases. Indexers are excluded: they are keyed by their parameter signature rather than by the declaring type and already shared storage. Interface mocks are excluded as well - an inherited interface member already carries its declaring interface on both surfaces, and a hidden member is meant to stay separate.