perf(mocks): memoize generator discovery per compilation and decouple emitted source from call-site locations

[thomhurst]

Summary

Makes TUnit.Mocks.SourceGenerator cheaper per edit. Mocks are now modelled once per compilation rather than once per call site. Moving a call site (for example, adding a line above it) no longer regenerates that type's source. Generated output is unchanged: all existing snapshots pass without updates.

Changes

1. Per-compilation memoization (Discovery/MockDiscoveryCache.cs, MockTypeDiscovery.cs). A ConditionalWeakTable<Compilation, ...> holds three ConcurrentDictionary memos:

   • BuildSingleTypeModel, keyed by (symbol with SymbolEqualityComparer.IncludeNullability, isPartialMock, isWrapMock)
   • BuildModelWithTransitiveDependencies, keyed by (symbol, isPartialMock)
   • the whole Mock.Of<T1, T2, ...>() result, keyed by the type-argument list

   Everything that depends on the consumer (accessibility and InternalsVisibleTo through compilation.Assembly, MockNamespaceConflictDetector, InterfaceImplementability) is a function of the compilation, which is the outer key. Every site and every transitive walk that reaches the same type shares one model instance. A cancelled computation throws before anything is cached.

2. Generated source no longer depends on location (MockGenerator.cs, Models/MockEmitResult.cs). Distinct requests are projected to their models (DistinctModels) before emitting. The emit step is now a Select, and its equatable MockEmitResult holds the generated files plus any failure. A RegisterSourceOutput adds those files and reports TM008. TM009 is reported in a separate output that combines the failed results with the location-bearing distinct requests, so it still points at the same call site or attribute as before. TM006 for attributes was already its own output and is unchanged. The internal test hook now takes a MockSourceSink instead of a SourceProductionContext. Files added before a failure are still emitted, as the existing diagnostic test expects. Trade-off: the pipeline now holds the generated text for each model.

3. Dedup hashing (MockTypeModel.cs, EquatableArray.cs). MockTypeModel.GetHashCode is now shallow: identity fields, flags, AdditionalInterfaceNames and array lengths, all of which Equals also compares. It no longer walks every member and parameter. Equals gets a ReferenceEquals fast path, and EquatableArray.Equals gets a same-backing-array fast path. With memoization, duplicate models are usually the same instance. I did not cache the hash lazily: with expressions copy fields, so a cached hash would carry over to modified copies (CollidesWith, EmitsSharedMemberSurface).

4. Cheaper transform checks (MockTypeDiscovery.cs):

   • UnwrapAsyncType now matches System.Threading.Tasks.Task<TResult> / ValueTask<TResult> by name, arity and namespace chain instead of calling ConstructedFrom.ToDisplayString(). The match is equivalent, including the TResult parameter name and not being nested.
   • The framework-namespace filter walks to the root namespace instead of formatting it.
   • The TUnit.Mocks namespace checks for the invocation and the attribute compare namespace segments.
   • The visited check now runs before HasStaticAbstractMembers. A type rejected by that scan is rejected every time, so marking it visited first never changes the result.

5. T.Mock() binding check (TransformMockExtensionInvocation). A generator never sees its own output, so a .Mock() can only already bind to a *_MockStaticExtension that comes from a referenced assembly or hand-written source. Once per compilation, the generator looks for such a type in any namespace. For source it asks the declaration table (ContainsSymbolsWithName). For references it walks every namespace, but only in assemblies that are or reference TUnit.Mocks, because an extension that returns TUnit mocks must reference it. GetSymbolInfo(invocation) now runs only when one exists; otherwise the old check could never have matched, so behaviour is unchanged.

6. Tracking names and incrementality tests. Added MockTrackingNames and WithTrackingName on the pipeline steps. The new MockGeneratorIncrementalityTests live in tests/TUnit.Mocks.SourceGenerator.Tests, which already has the Mocks references and test infrastructure. TUnit.SourceGenerator.IncrementalTests is an xunit project wired to the Core/Assertions generators. The tests cover:

   • an unrelated edit, after which models are Cached/Unchanged and emit steps are Cached
   • a blank line above the call sites, after which requests are Modified (locations moved) but models are Unchanged, emit steps are Cached and the output is identical
   • changing the mocked interface, after which emit steps are Modified and the new member appears in the output

Skipped

• Caching across compilations for metadata types. Skipped because it would not be correct as it stands. UseFallbackNamespace (MockNamespaceConflictDetector) looks at the consumer's own source declarations in the target's namespace, so it changes with ordinary edits. Member and constructor accessibility and auto-mock factory resolution also depend on the consuming compilation. Keying on MetadataReference plus assembly identity would miss those changes. Within one compilation the memo already removes the per-site repetition.
• A test for "T.Mock() already bound by a referenced assembly's extension". The test compilations use Roslyn 4.12, which cannot compile extension(...) blocks, so that referenced assembly cannot be built in-process. The gate only skips a check that could not match anyway (see item 5).

Tests run

• tests/TUnit.Mocks.SourceGenerator.Tests on net8.0, net9.0 and net10.0: all pass (159 / 161 / 166), snapshots unchanged, no .received.txt
• tests/TUnit.Mocks.Tests (net10.0): 1336 passed
• tests/TUnit.Mocks.Http.Tests (net10.0): 58 passed
• tests/TUnit.Mocks.Logging.Tests (net10.0): 31 passed
• tests/TUnit.Mocks.InternalsAccess.Tests (net10.0): 33 passed

Summary by CodeRabbit

• Performance

  • Mock generation now reuses cached results across compilations, helping avoid unnecessary regeneration after unrelated code edits or when mock call sites move.

• Bug Fixes

  • Changes to a mocked interface are reflected in its generated mock.
  • Improved discovery of mock-related types and async return types across namespaces.
  • Generation failures now preserve any sources produced before the failure and provide clearer error details.

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-28T18:44:31.040337Z	e79b4f4	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Warning

Review limit reached

Next included review available in 26 seconds.

Check out review usage here.

View limit details

Limit details: You’ve used all 8 included reviews currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: cf8da057-a7b0-41a6-93db-78aecaf2430b

📥 Commits

Reviewing files that changed from the base of the PR and between e79b4f4 and 865f3a1.

📒 Files selected for processing (3)

• src/TUnit.Mocks.SourceGenerator/Discovery/MockDiscoveryCache.cs
• src/TUnit.Mocks.SourceGenerator/Discovery/MockTypeDiscovery.cs
• tests/TUnit.Mocks.SourceGenerator.Tests/MockDiscoveryCacheTests.cs

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 0929e419-c958-4611-878f-f27823f6bc84

📥 Commits

Reviewing files that changed from the base of the PR and between 77e5c8b and e79b4f4.

📒 Files selected for processing (9)

• src/TUnit.Mocks.SourceGenerator/Discovery/MockDiscoveryCache.cs
• src/TUnit.Mocks.SourceGenerator/Discovery/MockTypeDiscovery.cs
• src/TUnit.Mocks.SourceGenerator/MockGenerator.cs
• src/TUnit.Mocks.SourceGenerator/MockTrackingNames.cs
• src/TUnit.Mocks.SourceGenerator/Models/EquatableArray.cs
• src/TUnit.Mocks.SourceGenerator/Models/MockEmitResult.cs
• src/TUnit.Mocks.SourceGenerator/Models/MockTypeModel.cs
• tests/TUnit.Mocks.SourceGenerator.Tests/MockGeneratorDiagnosticTests.cs
• tests/TUnit.Mocks.SourceGenerator.Tests/MockGeneratorIncrementalityTests.cs

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 6 remain after this review.

---

📝 Walkthrough

Walkthrough

The mock source generator now caches discovery models per compilation and uses symbol-based namespace checks. Its incremental pipeline tracks equatable requests, models, and emission results. A source sink collects generated files and retains partial output when generation encounters a non-cancellation exception. New tests check cache reuse and regeneration.

Changes

Mock generator pipeline

Layer / File(s)	Summary
Discovery and per-compilation model caching src/TUnit.Mocks.SourceGenerator/Discovery/MockDiscoveryCache.cs, src/TUnit.Mocks.SourceGenerator/Discovery/MockTypeDiscovery.cs	Discovery caches single-type, transitive, and multi-type models per compilation. Namespace and framework checks use namespace symbols.
Equatable requests and tracked pipeline stages src/TUnit.Mocks.SourceGenerator/MockGenerator.cs, src/TUnit.Mocks.SourceGenerator/MockTrackingNames.cs, src/TUnit.Mocks.SourceGenerator/Models/EquatableArray.cs, src/TUnit.Mocks.SourceGenerator/Models/MockTypeModel.cs, tests/TUnit.Mocks.SourceGenerator.Tests/MockGeneratorIncrementalityTests.cs	The generator tracks equatable requests and models. Equality and hash behavior changes support comparison of pipeline values. Tests cover unrelated edits, shifted call sites, and changes to a mocked interface.
Source collection and generation diagnostics src/TUnit.Mocks.SourceGenerator/MockGenerator.cs, src/TUnit.Mocks.SourceGenerator/Models/MockEmitResult.cs, tests/TUnit.Mocks.SourceGenerator.Tests/MockGeneratorDiagnosticTests.cs	Generation uses a source sink to collect files and records failure details in emission results. The output stage adds collected sources and reports collision and generation diagnostics.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Refactor

Sequence Diagram(s)

sequenceDiagram
  participant MockGenerator
  participant MockSourceSink
  participant CompilationOutput
  participant TM009Reporter
  MockGenerator->>MockSourceSink: Collect generated source files
  MockSourceSink-->>MockGenerator: Return accumulated sources and failure details
  MockGenerator->>CompilationOutput: Add collected sources
  MockGenerator->>TM009Reporter: Report failed result with request location

Loading

Merge Risk: ⚪ Minimal · up to e79b4

No actionable merge-blocking issue is established by the supplied evidence; the change is mergeable after normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to e79b4

The changes affect how mock sources are reused and emitted, but the reviewed paths keep cached state within a compilation and leave source output under the generator’s control. No new privilege or cross-service access was identified. Cancellation behavior and some security coverage remain uncertain.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The directly affected output is generated source for a consumer compilation. The cache’s compilation key limits reuse between distinct compilation objects, while source added by the output callback can affect that compilation’s build.

Trust Boundaries and Controls

• observed — Source-text production receives a model and a source-collection sink rather than the source-output context; the registered callback retains authority to add generated source and report diagnostics.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 15.79% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 57 functions across 9 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately identifies the main changes: compilation-scoped discovery caching and decoupling emitted source from call-site locations. It is specific and concise.

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{A rabbit checks each mock cache,
Then watches tracked requests pass.
A source sink gathers files in rows,
And keeps what generation throws.
New interface members join the view,
The rabbit hops through tests anew.}

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

Review

I read the diff and found no blocking issues.

Memoization (MockDiscoveryCache)

• The cache is keyed on the Compilation in a ConditionalWeakTable, so it can't outlive a compilation or leak across edits.
• SingleTypeKey covers every input that changes the result (symbol with nullability, partial mode, wrap mode).
• MultiTypeModels is keyed on the type-argument list only. That is safe because isPartialMock is derived from typeArguments[0], and wrap mocks return before that path.
• A cancelled computation throws before GetOrAdd stores anything, so partial results are never cached.
• GetOrAdd(key, value) can compute the same value twice under a race. That is harmless because the computation is pure.

Pipeline restructuring

• Projecting to DistinctModels before emit keeps locations out of the cached source. Moving a call site should no longer regenerate that type.
• Pairing failures with locations in a separate TM009 output is a clean way to keep diagnostics precise.
• ReportGenerationFailures finds each request with a linear scan and Model.Equals. That's fine for the expected number of failures. If it ever matters, a dictionary keyed by model would replace the nested loop.

Micro-optimisations

• The pattern named.Name is "Task" or "ValueTask" binds as a pattern or, so it behaves as intended, but it's easy to misread next to the surrounding &&. A short comment or extra parentheses would help.
• Skipping GetSymbolInfo when no *_MockStaticExtension is visible is sound, since a generator never sees its own output.

Tests: the PR says existing snapshots pass unchanged, and the tracking names make incrementality tests possible. I'd like to see a test that asserts EmitResults is Cached after inserting a line above a call site, if one isn't already included.

Overall this is a good, well-reasoned change.

[greptile-apps]

Confidence Score: 5/5

[High risk] Refactors the source generator pipeline for performance and incrementality.

The PR appears safe to merge; no outstanding findings remain.

Summary

This PR memoizes mock discovery within a compilation and separates generated source from call-site locations so moving a request need not regenerate its mock. It also adds tracking and regression tests for discovery and incremental emission.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[Mock calls and attributes] --> B[Per-compilation discovery cache]
  B --> C[Distinct requests]
  C --> D[Location-free models]
  D --> E[Emitted source results]
  E --> F[Generated files]
  E --> G[Failures]
  C --> G
  G --> H[TM009 at request location]

Loading

_{Reviews (3) · Last reviewed commit: "test(mocks): cover the static-extension ..."}

[github-actions]

Review of #6913

I read the diff and did not run the tests, so this covers the design and logic only. I found no blocking issues.

What looks good

• Per-compilation memoization through ConditionalWeakTable<Compilation, …> has no cross-compilation state to invalidate. Cancellation throws before anything is cached, so a cancelled run cannot poison the memo.
• Dropping the location before emitting (DistinctModels → Emit) is the right fix. Moving a call site no longer invalidates generated source, and TM009 still gets its location through a separate output.
• Keeping the hash shallow while Equals stays deep is valid. Everything the hash reads is also compared by Equals.
• Not caching the hash because of with copies is the right call.
• Moving the visited.Add check ahead of the static-abstract scan does not change what is generated. A type that fails the scan fails it every time.
• The UnwrapAsyncType pattern ... && named.Name is "Task" or "ValueTask" && ... parses as intended, because or binds inside the pattern.

Minor observations (non-blocking)

1. TryGetValue followed by GetOrAdd(key, value) computes eagerly. Under parallel transforms, two threads can both build the same model, and the loser's instance is discarded. This is harmless because the result is deterministic. It does weaken the "one shared instance" claim in the doc comment, which then relies on Equals rather than ReferenceEquals. Documenting that would be enough. Lazy<T> values would enforce a single build but cost an allocation per entry.
2. MayReferenceGeneratedStaticExtensions is a heuristic. It walks only assemblies that reference TUnit.Mocks, and it assumes a generator never sees its own output. That holds for Roslyn today. A short test with a referenced assembly containing a *_MockStaticExtension type would lock it in, if one does not already exist.
3. ReportGenerationFailures matches failed models to requests with a linear scan (failures × requests). It only runs on failure, so this is fine.
4. As the description says, the pipeline now retains the generated text per model. This trades memory for less rework, and I think it is acceptable.

The change is sound and the existing snapshots are unchanged, which supports the claim that output is identical. Approving from my side.

[github-actions]

Review of #6913

I reviewed the diff by reading it. I did not build the PR or run its tests. The code-review skill failed to start, so this is a manual review.

Overall the design is sound. Per-compilation memoization keyed on the Compilation instance is the right scope, because it never outlives the compilation and has nothing to invalidate. Dropping the source location before the emit step is what makes call-site moves cacheable. Splitting TM009 into its own output keeps the diagnostic location without invalidating emitted source.

Notes, none blocking:

1. Redundant caching layers. BuildModelWithTransitiveDependencies and the multi-type cache both sit on top of the BuildSingleTypeModel memo. That is fine while the wrapper does real work such as the transitive walk. If profiling shows the outer layers rarely hit, dropping one would leave fewer places to reason about.
2. TM009 lookup. ReportGenerationFailures scans input.Requests linearly for each failure, using Model.Equals. That is O(failures × requests) with a deep model equality. It only runs on the failure path, so this is acceptable. A dictionary keyed by model would be tidier if failures ever become common.
3. MayReferenceGeneratedStaticExtensions heuristic. The check assumes a type named *_MockStaticExtension can only come from a source declaration or from an assembly referencing TUnit.Mocks. That holds today. A short test with a referenced assembly containing such a type would protect this shortcut, and I couldn't see one in the diff.
4. Cache-race comment. The doc comment already says racing misses may compute twice and discard one. Nothing depends on reference identity, so I agree that's fine.
5. Reordered visited.Add. Moving visited.Add before HasStaticAbstractMembers is correct. It only avoids repeated scans, and the generated set doesn't change.

The PR reports that the existing snapshots pass unchanged. Together with the new tracking names (MockTrackingNames), that suggests the incrementality claim is testable. Please make sure a test asserts the EmitResults step reports Unchanged or Cached after a line is inserted above a call site.

LGTM.

[thomhurst]

Thanks for the review. Replies by point:

1. Redundant caching layers. Keeping all three for now. The outer layers are not only memos. ModelsWithTransitiveDependencies caches the transitive auto-mock walk, which is the expensive part for a type mocked at many sites. The multi-type cache holds the combined models for Mock.Of<T1, T2, ...>(). Neither can be rebuilt cheaply from the single-type memo alone. If profiling ever shows one of them rarely hits, I'll drop it.
2. TM009 lookup. Agreed that it's O(failures × requests). It only runs when an emit throws, and in practice that's zero or one failure per run, so I'm leaving it linear rather than building a dictionary on every pass.
3. MayReferenceGeneratedStaticExtensions heuristic. This is already covered in MockDiscoveryCacheTests, added in 865f3a1. That commit landed just before this review, which is probably why you didn't see it. Referenced_Static_Extension_In_Other_Namespace_Is_Detected compiles a separate assembly that references TUnit.Mocks and declares IExternalNotifier_MockStaticExtension in a non-TUnit namespace, then checks that the gate returns true. Referenced_Assembly_Without_TUnit_Mocks_Reference_Is_Not_Walked pins the other side of the assumption. Source_Declared_Static_Extension_In_Other_Namespace_Is_Detected and No_Static_Extension_Anywhere_Skips_Binding_Check cover source declarations and the fast path.
4. / 5. Thanks, no action.

Call-site move test. MockGeneratorIncrementalityTests.Moving_A_Call_Site_Leaves_Emitted_Source_Cached inserts a blank line above the call sites. It first checks that DistinctRequests reports Modified, so the test can't pass vacuously. It then checks that DistinctModels is Cached or Unchanged, that every EmitResults output is Cached (stricter than Cached-or-Unchanged), and that the generated sources are identical.

I ran the TUnit.Mocks.SourceGenerator tests locally (snapshot and incrementality) at 865f3a1. All passed: 170/170 on net10.0 and 163/163 on net8.0. No new commit was needed for this round.