Skip to content

C#: Fix MaybeAutoFormat no-op when SDK consumed via NuGet PackageReference#7192

Merged
knutwannheden merged 8 commits intomainfrom
maybeautoformat-produces-no-indentation-in-local-rewritetest-non-rpc
Mar 29, 2026
Merged

C#: Fix MaybeAutoFormat no-op when SDK consumed via NuGet PackageReference#7192
knutwannheden merged 8 commits intomainfrom
maybeautoformat-produces-no-indentation-in-local-rewritetest-non-rpc

Conversation

@knutwannheden
Copy link
Copy Markdown
Contributor

@knutwannheden knutwannheden commented Mar 28, 2026

Motivation

Three issues with MaybeAutoFormat in the C# SDK:

  1. Empty constructor bodies with initializers — Roslyn's Formatter.Format does not expand Constructor() : base(){} to Allman-style multi-line braces (dotnet/roslyn#82974)
  2. SDK consumed via NuGet PackageReferenceAdhocWorkspace cannot discover Microsoft.CodeAnalysis.CSharp.Workspaces via Assembly.Load when consumed with PrivateAssets="all", making the Roslyn formatter a silent no-op
  3. Silent reconciler failures — When WhitespaceReconciler detects a structural mismatch (e.g., recipe uses Identifier("string") where the parser produces Primitive), it silently returns the original tree, making formatting bugs extremely hard to diagnose

Summary

  • Work around the Roslyn bug in MinimumViableSpacingVisitor by inserting newlines into empty constructor body blocks that have a constructor initializer and Space.Empty prefix/end
  • Fix AdhocWorkspace assembly discovery by explicitly providing already-loaded Roslyn assemblies to MefHostServices.Create
  • Mark test dependencies (xunit, coverlet, Microsoft.NET.Test.Sdk, etc.) as PrivateAssets="all" to prevent leaking to NuGet consumers
  • Add structural mismatch reporting to WhitespaceReconciler: when ThrowOnMismatchDefault is true (set by RewriteTest), collects up to 5 mismatches with property paths and throws WhitespaceReconcileMismatchException

Test plan

  • AutoFormatEmptyConstructorBodyWithInitializer — Allman-style braces for constructors with initializers
  • MaybeAutoFormatAtClassLevelWithSynthesizedConstructor — end-to-end recipe test
  • FormatSpansReconcilerAppliesFormattingToSynthesizedNodes — direct FormatSpans pipeline test
  • ReconcilerMismatchReportsPropertyPath — verifies exception contains property path on type mismatch
  • All 1791 tests pass

…lizers

Roslyn's Formatter.Format does not expand empty constructor bodies to
Allman style when a constructor initializer (`: base(...)`) is present
(dotnet/roslyn#82974). This means
MaybeAutoFormat produces `Foo() : base() { }` instead of multi-line
braces.

Work around this in MinimumViableSpacingVisitor by inserting newlines
into empty method body blocks that have a constructor initializer and
Space.Empty prefix/end, so the printed source has multi-line braces
that Roslyn can then indent correctly.
…rence

AdhocWorkspace relies on Assembly.Load to discover
Microsoft.CodeAnalysis.CSharp.Workspaces via the .NET dependency graph.
When the OpenRewrite SDK is consumed with PrivateAssets="all", the
dependency entries are absent from the consumer's .deps.json, so
Assembly.Load fails silently and MEF cannot discover the C# formatting
services — making Formatter.Format a no-op.

Fix by explicitly providing the already-loaded Roslyn assemblies to
MefHostServices.Create, bypassing the Assembly.Load discovery path.
…ces-no-indentation-in-local-rewritetest-non-rpc
…sumers

Test dependencies (xunit, coverlet, Microsoft.NET.Test.Sdk, etc.) were
flowing to NuGet package consumers because they lacked PrivateAssets="all".
This could interfere with assembly resolution in consuming projects.
@knutwannheden knutwannheden force-pushed the maybeautoformat-produces-no-indentation-in-local-rewritetest-non-rpc branch from bc8c5aa to 430ecc7 Compare March 29, 2026 07:05
@knutwannheden knutwannheden merged commit 25af52a into main Mar 29, 2026
1 check passed
@knutwannheden knutwannheden deleted the maybeautoformat-produces-no-indentation-in-local-rewritetest-non-rpc branch March 29, 2026 09:11
@github-project-automation github-project-automation bot moved this from In Progress to Done in OpenRewrite Mar 29, 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