Skip to content

Make ReusesCachedOutputOnRerun detect a non-incremental generator - #22

Merged
matt-edmondson merged 1 commit into
mainfrom
fix/incremental-check-uses-fresh-compilation
Sep 26, 2026
Merged

matt-edmondson merged 1 commit into
mainfrom
fix/incremental-check-uses-fresh-compilation

Conversation

@matt-edmondson

Copy link
Copy Markdown
Contributor

Fixes #19

What was wrong

GeneratorHarness.ReusesCachedOutputOnRerun ran the driver twice against the same CSharpCompilation instance. Roslyn compares incremental inputs by reference, so every step, CompilationProvider included, came back Cached/Unchanged, and the check returned true for any generator. A generator that recomputes on every keystroke passed. That also meant TheGeneratorReusesItsOutputWhenNothingChanged proved nothing.

Change

  • The second run now uses compilation.AddSyntaxTrees(CSharpSyntaxTree.ParseText(string.Empty)). That is a new, semantically equivalent Compilation, which is what an edit hands a generator.
  • A generator with no tracked output steps now returns false instead of passing by default.
  • The doc comment and the README API table describe the new behaviour.

Design note: the AdditionalText instances are kept, not rebuilt. The issue listed rebuilding them as optional. On a keystroke the IDE keeps the same instances for unchanged additional files, so reusing them models the scenario the check is for. Rebuilding them would be stricter than the IDE. It would also fail GeneratorBase itself, because MetadataFile has reference equality, and that is a separate question from this bug.

Release note: downstream tests that call this check may now fail. Each such failure is a generator that really was not incremental, as the triage comment anticipated.

Tests

  • TheRerunCheckCatchesAGeneratorThatRecomputesOnEveryEdit: the CompilationProvider.Select(_ => new object()) generator from the issue must fail the check.
  • TheRerunCheckDoesNotPassAGeneratorWithNoOutputSteps
  • The existing positive test (ThingsGenerator) still passes under the stricter check, so it now proves something.
  • With the harness fix reverted, both new tests fail. With it applied, the full suite passes (45/45).
  • A local Sonar build reports no findings.

This PR is independent of #21 (duplicate diagnostic numbers); the two merge cleanly in either order.

🤖 Generated with Claude Code

https://claude.ai/code/session_019StKk4VdpfAZ5cdW5X685e


Generated by Claude Code

The check reran the driver against the same Compilation instance.
Roslyn compares incremental inputs by reference, so every step came
back Cached and the check passed for any generator, including one that
recomputes on every keystroke. The second run now gets a new but
equivalent compilation, which is what an edit hands a generator. A
generator with no tracked output steps now reports false instead of
passing by default.

Downstream tests that call this check may now fail. Each such failure
is a generator that really was not incremental.

Fixes #19

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019StKk4VdpfAZ5cdW5X685e
@sonarqubecloud

Copy link
Copy Markdown

@matt-edmondson
matt-edmondson merged commit da12ba2 into main Sep 26, 2026
12 checks passed
@matt-edmondson
matt-edmondson deleted the fix/incremental-check-uses-fresh-compilation branch September 26, 2026 11:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ReusesCachedOutputOnRerun always passes, even for a non-incremental generator, because both runs use the same Compilation

2 participants