Skip to content

Compile the text produced by the fixer in the test harness - #1342

Merged
meziantou merged 1 commit into
mainfrom
feature/meziantou-analyzer-1330-1196b1
Aug 26, 2026
Merged

meziantou merged 1 commit into
mainfrom
feature/meziantou-analyzer-1330-1196b1

Conversation

@meziantou

Copy link
Copy Markdown
Owner

Fixes #1330

Problem

ApplyFix called compilation.Emit(...) on the solution produced directly by the CodeAction, so it validated the syntax tree built by the fixer. The text compared against the expected fixed code is produced only afterwards, by Simplifier.ReduceAsync and Formatter.Format in GetStringFromDocument — and that text was never compiled.

Any fixer that builds a structurally valid tree whose serialization re-parses differently therefore passed. Operator-precedence and parenthesization bugs are the most common way a Roslyn fixer corrupts user code, and the harness was structurally blind to all of them across the 109 fixers.

Change

ApplyFix now runs every document the code action changed or added through GetStringFromDocument and puts that text back into the solution with WithDocumentText before compiling. The trees that get compiled are the ones parsed from the text of the fix — the text the user actually gets. The failure message prints the document text rather than the raw syntax root.

A self-test for the harness is added in ProjectBuilderValidationTests, with a test-only analyzer on is expressions and two test-only fixers:

  • NegateWithoutParenthesesFixer builds ! applied to the is-expression — a valid tree whose text is !o is string, which re-parses as (!o) is string and does not compile. The test asserts the harness now fails with The fixed code doesn't compile.
  • NegateWithParenthesesFixer is the correct version, and the test asserts a valid fix still passes.

I checked that the first test fails when ApplyFix is reverted to its previous behavior, so it genuinely guards the change.

Notes for the reviewer

The issue expected this to surface more existing fixer bugs. It does not: all 18575 tests pass across the five Roslyn versions (4.8, 4.14, 5.0, 5.6, 5.9). The MA0073 fixer bug used as the demonstration in the issue is not covered by any existing test case — no test compares an is expression with a bool constant — so it stays a live bug for its own issue rather than turning red here.

dotnet run --project src/DocumentationGenerator produced no markdown changes, as expected for a test-only change.

ApplyFix compiled the solution produced by the CodeAction, so it validated
the syntax tree built by the fixer instead of its serialization. The text
compared against the expected fixed code is only produced afterwards, by
Simplifier.ReduceAsync and Formatter.Format, and was never compiled. A fixer
building a structurally valid tree whose text re-parses differently (a
missing set of parentheses, for instance) therefore passed, even though the
text is what the user gets.

ApplyFix now runs every changed or added document through GetStringFromDocument
and puts that text back into the solution before compiling it, so the trees
that are compiled are the ones parsed from the text of the fix.

Add a self-test for the harness with a test-only analyzer and two test-only
fixers: one that negates an is-expression without parenthesizing it, whose
text re-parses as '(!o) is string' and must be rejected, and the correct one,
which must still pass.
@meziantou
meziantou enabled auto-merge (squash) August 26, 2026 18:37
@meziantou
meziantou merged commit 4235ac0 into main Aug 26, 2026
13 checks passed
@meziantou
meziantou deleted the feature/meziantou-analyzer-1330-1196b1 branch August 26, 2026 18:38
This was referenced Aug 26, 2026
This was referenced Sep 21, 2026
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.

Test harness: code-fix verification compiles the syntax tree, not the text it writes

1 participant