feat: migrate whole document in a single pass via custom FixAllProvider#22
Merged
Conversation
The previous BatchFixer-based pipeline dropped per-diagnostic rewrites whose text changes overlapped on the using-directive line, and once the first using swap landed the analyzer no longer fired on remaining TestableIO bindings — leaving files partially migrated and uncompilable. Each per-pattern Apply method is split into a pure helper that operates on (CompilationUnitSyntax, SyntaxNode) without touching usings. A new SystemIOAbstractionsFixAllProvider annotates every diagnostic target, dispatches the pure helpers sequentially with annotation-relocation and re-acquired semantic models, then applies a single using-directive change at the end. The per-diagnostic CodeAction re-runs the analyzer to find sibling diagnostics in the same document and routes through the same provider, so one click in the IDE migrates the whole file (Mockolate-style).
There was a problem hiding this comment.
Pull request overview
Replaces the default BatchFixer-based fix-all pipeline (which dropped overlapping using-line edits and lost subsequent diagnostics after the first using swap) with a custom SystemIOAbstractionsFixAllProvider. Each per-pattern rewriter is refactored into a pure helper operating on (CompilationUnitSyntax, SyntaxNode); the new provider annotates targets, dispatches the pure helpers sequentially with re-acquired semantic models, and applies a single using-directive change at the end. Per-diagnostic CodeActions also reroute through this provider so one click migrates the whole file.
Changes:
- Introduce
SystemIOAbstractionsFixAllProvider.MigrateDocumentAsyncto annotate, sequentially rewrite, and apply a single using-directive change. - Refactor every per-pattern
ApplyXxxRewriteAsyncinto pureApplyXxxPurehelpers; centralize using-directive policy (UsingChange,GetUsingChange,ApplyUsingChange) and addApplySinglePatternAsyncthat re-runs the analyzer to expand single fixes to whole-file fixes. - Add
FixAllTestscovering multi-diagnostic, mixed-pattern, and same-receiver scenarios.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
| Source/Testably.Abstractions.Migration.Analyzers.CodeFixers/SystemIOAbstractionsFixAllProvider.cs | New DocumentBasedFixAllProvider orchestrating the annotation-and-rewrite pass. |
| Source/Testably.Abstractions.Migration.Analyzers.CodeFixers/SystemIOAbstractionsCodeFixProvider.cs | Refactors per-pattern rewrites into pure helpers; adds dispatch, using-change policy, and single-diagnostic→whole-file routing. |
| Tests/Testably.Abstractions.Migration.Tests/SystemIOAbstractionsCodeFixProviderTests.FixAllTests.cs | End-to-end tests for multi-diagnostic fix-all scenarios. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Extract CollectAndAnnotate + ApplyRewritesAsync helpers so each method stays under Sonar's cognitive-complexity ceiling of 15 (the original MigrateDocumentAsync hit 23). Hoist maxUsingChange computation from the work-item intent before the rewrite loop, so a block-level rewrite that absorbs a sibling annotated node still applies the absorbed item's using-directive change. Guard the final swap with anyRewriteSucceeded so a no-op pass doesn't add a Testably using to a still-TestingHelpers-bound file. Skip the Document round-trip for purely syntactic patterns (most of them) and only sync through a fresh SemanticModel for AccessorAddFile, FilesCtor*, and AddFilesFromEmbeddedNamespace. When a sync is necessary, re-locate the annotated target in the round-tripped tree — Roslyn's WithSyntaxRoot may produce a tree whose nodes are not reference-equal to the input, which would make SemanticModel.GetSymbolInfo reject them.
|
|
This is addressed in release v0.2.0. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The previous BatchFixer-based pipeline dropped per-diagnostic rewrites whose text changes overlapped on the using-directive line, and once the first using swap landed the analyzer no longer fired on remaining TestableIO bindings — leaving files partially migrated and uncompilable.
Each per-pattern Apply method is split into a pure helper that operates on (CompilationUnitSyntax, SyntaxNode) without touching usings. A new SystemIOAbstractionsFixAllProvider annotates every diagnostic target, dispatches the pure helpers sequentially with annotation-relocation and re-acquired semantic models, then applies a single using-directive change at the end. The per-diagnostic CodeAction re-runs the analyzer to find sibling diagnostics in the same document and routes through the same provider, so one click in the IDE migrates the whole file (Mockolate-style).