Skip to content

C#: Preserve MinimumViableSpacing fixes when Roslyn formatting is a no-op#7182

Merged
knutwannheden merged 1 commit intomainfrom
fix-makefieldreadonly-readonly-keyword-missing-trailing-space
Mar 27, 2026
Merged

C#: Preserve MinimumViableSpacing fixes when Roslyn formatting is a no-op#7182
knutwannheden merged 1 commit intomainfrom
fix-makefieldreadonly-readonly-keyword-missing-trailing-space

Conversation

@knutwannheden
Copy link
Copy Markdown
Contributor

Motivation

The MakeFieldReadOnly recipe in recipes-csharp produces readonlyList<T> instead of readonly List<T> when adding the readonly modifier to fields. The readonly keyword is missing a trailing space before the type name.

The root cause is in RoslynFormatter: when MinimumViableSpacingVisitor (MVS) correctly adds a space between the modifier and the type expression, but Roslyn formatting makes no additional changes, the formatter short-circuits at source == formattedSource and returns the original CU — discarding the MVS fix.

The flow was:

  1. MVS adds space to type expression prefix → mvsCu (correct: readonly List<int>)
  2. Print mvsCu → correct output
  3. Roslyn formats → no change (already correct)
  4. source == formattedSource → returns originalCu (no space!) → readonlyList<int>

Summary

  • When Roslyn is a no-op but MVS made changes, reconcile the MVS changes within the target subtrees instead of discarding them
  • Fixed in both Format() (single subtree) and FormatSpans() (multi-target batch)
  • Added integration tests simulating MakeFieldReadOnly recipe behavior (adding modifier + MaybeAutoFormat)
  • Added MVS unit tests confirming the visitor correctly handles added modifiers

Test plan

  • New AutoFormatAfterAddingReadonlyModifierToGenericField — adds readonly to List<int> field via visitor + MaybeAutoFormat, verifies readonly List<int> output
  • New AutoFormatAfterAddingReadonlyModifierToSimpleField — same for int field
  • New MVS unit tests (AddedModifierToFieldWithGenericType, AddedModifierToFieldWithSimpleType)
  • Full C# test suite (1771 tests pass)

…o-op

When a recipe adds a modifier to a VariableDeclarations (e.g., MakeFieldReadOnly
adding `readonly`), the type expression often has an empty prefix because the
indentation lives on the VarDecl prefix. MinimumViableSpacingVisitor correctly
adds a space between the modifier and the type, but RoslynFormatter discarded
this fix when Roslyn formatting made no additional changes — the short-circuit
`source == formattedSource` returned the original CU without MVS changes.

Now, when Roslyn is a no-op but MVS made changes, we reconcile the MVS changes
within the target subtrees instead of discarding them.
@github-project-automation github-project-automation bot moved this to In Progress in OpenRewrite Mar 27, 2026
@knutwannheden knutwannheden changed the title C#: Preserve MinimumViableSpacing fixes when Roslyn formatting is a no-op C#: Preserve MinimumViableSpacing fixes when Roslyn formatting is a no-op Mar 27, 2026
@knutwannheden knutwannheden merged commit 31788cb into main Mar 27, 2026
1 check passed
@knutwannheden knutwannheden deleted the fix-makefieldreadonly-readonly-keyword-missing-trailing-space branch March 27, 2026 20:09
@github-project-automation github-project-automation bot moved this from In Progress to Done in OpenRewrite Mar 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant