fix(mocks): let one type be mocked regularly and wrapped in one compilation - #6835
Conversation
…lation
`Mock.Of<T>()` and `Mock.Wrap(instance)` produce two models for the same type
that differ only in `IsWrapMock`. Model equality includes that flag, so both
survive dedup — correctly, since each needs its own impl and factory — but both
emission paths then added the same hint names. A duplicate hint name aborts the
generator, so every mock in the compilation disappeared and the build failed
with a cascade of CS1061 errors, the cause visible only in a CS8785 warning.
The wrap impl and factory are already types of their own and are file-scoped, so
they just need a hint name of their own: they now emit as
`{name}_WrapMockImplFactory.g.cs`.
The setup and verification surface is not per mode. It describes the mocked
type, is byte-identical between the two models, and lives in an extension class
that is not file-scoped, so it has to be emitted exactly once. A new
`SharedMemberSurfaceResolver` step assigns that ownership across the collected
requests: the regular model emits it, since it also emits the static `Mock()`
entry point, and a wrap model emits it only when it is the type's only model.
The identity used to recognise one target reached in several modes is shared
with `GeneratedNameCollisionDetector`, which already had to tell that case apart
from a genuine #6505 name collision.
Four wrap snapshots are re-recorded: renaming the wrap hint changes where that
file sorts in the concatenated snapshot. The generated code is unchanged.
Fixes #6834
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
GitHub Actions
Failure
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (12)
Included review availability: Your plan provides up to 8 included reviews per hour; 5 remain after this review. 📝 WalkthroughWalkthroughThe source generator now supports regular and wrap mocks for the same type. It assigns distinct implementation and factory hint names, emits shared member and event surfaces once, and adds generator and runtime regression coverage. ChangesDual-mode mock generation
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Bug fix · Severity of issue fixed: Medium Sequence Diagram(s)sequenceDiagram
participant TestSource
participant MockGenerator
participant MockRegistry
participant MockEngine
TestSource->>MockGenerator: Generate regular and wrap mocks
MockGenerator->>MockRegistry: Register regular factory
MockGenerator->>MockRegistry: Register wrap factory
TestSource->>MockEngine: Configure shared setup
MockEngine-->>TestSource: Handle regular and wrapped calls
Merge Risk: ⚪ Minimal · up to The dual-mode mock generation change has no identified actionable risk and is ready to merge. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 9.09% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 22 functions across 7 files. (5 skipped: 5 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. A rabbit checks the mock files bright Comment |
Code reviewReviewed the fix for #6834 (duplicate hint name when a type is reached through both Summary of the change
This is a clean, well-targeted fix. It correctly recognizes that the wrap impl/factory are already One maintainability nit (not a live bug today):
|
 |
This branch was successfully deployed
1 participant
Fixes #6834.
Problem
Reaching one type through both a regular mock and
Mock.Wrapin the same compilation aborts the generator:Because the generator aborts, every mock in the compilation disappears and the build fails with a cascade of CS1061 "does not contain a definition for ..." errors on unrelated mocks; the real cause is visible only in that one warning.
Mock.Of<T>()andMock.Wrap(instance)produce twoMockTypeModels differing only inIsWrapMock. Model equality includes that flag, so both survive dedup — correctly, since each needs its own impl and factory — butGenerateWrapMockandGenerateSingleTypeMockboth routed throughGenerateImplFactoryMembersAndEvents, adding the same_MockImplFactory.g.cs,_MockMembers.g.csand_MockEvents.g.cshint names.GeneratedNameCollisionDetectoralready recognised this situation — its own comment names "Mock.Of and Mock.Wrap of one type" as models sharing an identity — and deliberately does not report TM008 for it, but nothing downstream prevented the duplicate emission.Fix
The wrap impl and factory are already distinct types (
{name}WrapMockImpl/{name}WrapMockFactory, bothfile-scoped), so they only needed a hint name of their own:{name}_WrapMockImplFactory.g.cs.The setup and verification surface is a different matter. It describes the mocked type rather than the construction mode, is byte-identical between the two models, and its extension class is not
file-scoped — so it has to be emitted exactly once. A newSharedMemberSurfaceResolverstep runs over the collected requests and assigns ownership: the regular model emits it (it also emits the staticMock()entry point), and a wrap model emits it only when it is the type's only model.The identity used to recognise one target reached in several modes is now shared with
GeneratedNameCollisionDetector, which already had to tell that case apart from a genuine #6505 collision.Tests
tests/TUnit.Mocks.Tests/Issue6834Tests.cs— one type used both ways in one compilation: the regular mock, the wrap mock falling through to the real instance and then being configured, one setup surface serving both, and raise extensions emitted once and working for both.tests/TUnit.Mocks.SourceGenerator.Tests/Issue6834Tests.cs— a snapshot of the dual-mode output plus assertions that hint names are unique, that both impl/factory files are present, that_MockMembers.g.csis emitted exactly once, and that wrapping without a regular mock still emits the member surface.Four existing wrap snapshots are re-recorded. Renaming the wrap hint changes where that file sorts in the concatenated snapshot; the generated code is unchanged (verified by comparing the sorted contents).
Suites on net10.0: TUnit.Mocks.Tests 1309/1309, TUnit.Mocks.SourceGenerator.Tests 156/156, Analyzers 63/63, Http 58/58, Logging 31/31, InternalsAccess 29/29.
Note
#6833 works around this bug by using two separate types in its tests (
VirtualInitOnlyProperty/WrappableInitOnlyProperty). Once this merges that split is no longer needed and the comment there can go.Summary by CodeRabbit
Bug Fixes
Tests