Add a shared fix-all provider that fixes a document through one editor - #55533
Merged
tannergooding merged 4 commits intoJul 30, 2026
Merged
Conversation
|
Azure Pipelines: Successfully started running 1 pipeline(s). 2 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
Member
Author
|
Namely this more closely unifies on what Roslyn does internally so that it is easier to migrate all the other analyzers to the "better" pattern. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds a shared “single-editor” fix-all implementation for NetAnalyzers code fixes, aiming to avoid WellKnownFixAllProviders.BatchFixer’s per-diagnostic document forking/merging behavior and to ensure fixes within a document compose through one SyntaxEditor. It also migrates CA1516’s intrinsics fixer to the new base/provider and adds targeted regression tests for ordering and scale.
Changes:
- Add
SyntaxEditorFixAllProviderto apply all diagnostics for a document through a singleSyntaxEditor, plus ordering logic for nested spans. - Add
SyntaxEditorBasedCodeFixProviderto share the single-fix and fix-all pipelines via the same implementation. - Migrate
UseCrossPlatformIntrinsicsFixeroff the removedOrderedCodeFixProvider, and add fix-all regression tests for deep nesting and higher diagnostic counts.
Show a summary per file
| File | Description |
|---|---|
| src/Microsoft.CodeAnalysis.NetAnalyzers/src/Microsoft.CodeAnalysis.NetAnalyzers/SyntaxEditorFixAllProvider.cs | New shared fix-all provider built around a single SyntaxEditor and diagnostic ordering. |
| src/Microsoft.CodeAnalysis.NetAnalyzers/src/Microsoft.CodeAnalysis.NetAnalyzers/SyntaxEditorBasedCodeFixProvider.cs | New base CodeFixProvider that routes single-fix and fix-all through the same editor-based pipeline. |
| src/Microsoft.CodeAnalysis.NetAnalyzers/src/Microsoft.CodeAnalysis.NetAnalyzers/OrderedCodeFixProvider.cs | Removed the previous per-fixer ordered DocumentEditor fix-all base. |
| src/Microsoft.CodeAnalysis.NetAnalyzers/src/Microsoft.CodeAnalysis.NetAnalyzers/Microsoft.CodeQuality.Analyzers/Maintainability/UseCrossPlatformIntrinsicsFixer.cs | Migrated to the new editor-based base class/provider and updated fix application pattern. |
| src/Microsoft.CodeAnalysis.NetAnalyzers/tests/Microsoft.CodeAnalysis.NetAnalyzers.UnitTests/SyntaxEditorFixAllProviderTests.cs | New unit tests validating ordering rules and semantic-model binding behavior. |
| src/Microsoft.CodeAnalysis.NetAnalyzers/tests/Microsoft.CodeAnalysis.NetAnalyzers.UnitTests/Microsoft.CodeQuality.Analyzers/Maintainability/CSharpUseCrossPlatformIntrinsicsTests.cs | Added fix-all regression tests for deep nesting and higher diagnostic counts. |
Copilot's findings
- Files reviewed: 6/6 changed files
- Comments generated: 4
This was referenced Jul 30, 2026
Closed
WellKnownFixAllProviders.BatchFixer computes a separate fixed document per diagnostic, each against the original, then diffs and merges them. That is a document fork, a semantic model, and a text diff per diagnostic, and where two of those text changes overlap the merge silently drops one, so the user has to invoke fix-all again. SyntaxEditorFixAllProvider records every one of a document's fixes into a single SyntaxEditor instead. It has no consumer yet. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
CA1516 was the only fixer with a shared base of its own, hand-rolling a fix-all loop over a DocumentEditor and pinning a descending order. Route it through SyntaxEditorFixAllProvider and name the base after Roslyn's own SyntaxEditorBasedCodeFixProvider, which it now mirrors. The order it pinned is no longer declared: Order sorts ascending by span start, as Roslyn does, and only goes innermost-first when the spans actually nest. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The remarks said nested spans order innermost-first, which reads as though only the nested pair moves. One nested pair reverses the whole document, as the existing test already asserts. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The add-net-analyzer skill and netcore-getting-started.md both link OrderedCodeFixProvider, which this branch removes. Retarget them at SyntaxEditorBasedCodeFixProvider, and record what the shared provider now handles for the author: registration, ordering, and the equivalence-key filtering DocumentBasedFixAllProvider does not do. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
tannergooding
force-pushed
the
tannergooding-shared-fix-all-provider
branch
from
July 30, 2026 21:57
6d1964c to
f2542b0
Compare
tannergooding
enabled auto-merge
July 30, 2026 22:13
jeffhandley
approved these changes
Jul 30, 2026
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.
WellKnownFixAllProviders.BatchFixercomputes a separate fixed document per diagnostic, eachagainst the original, then diffs and merges them — a document fork, a semantic model bind, and a
text diff per diagnostic. Where two of those text changes overlap the merge silently drops one, and
the user has to invoke fix-all again.
SyntaxEditorFixAllProviderrecords every one of a document's fixes into a singleSyntaxEditorinstead. It mirrors Roslyn's own
SyntaxEditorBasedCodeFixProvider— same name, same shape, same pipeline:
Distinct(), source order, one editor per document, and thesingle-fix and fix-all paths sharing one code path so they cannot diverge.
CA1516's
OrderedCodeFixProvideris the only caller moved, because it already hand-rolled this exactloop — over a
DocumentEditor, with a hardcoded descending order. It now takes the shared one and isrenamed to match Roslyn.
The order it pinned is no longer declared:
Ordersorts ascending by span start, as Roslyn does, andgoes innermost-first only when the spans actually nest — read off the spans rather than declared by
the fixer.
SyntaxEditorrather thanDocumentEditor, becauseDocumentEditor.CreateAsyncawaits
GetSemanticModelAsyncunconditionally, whether or not the fix reads one — and CA1516 neverdoes. Measured on Roslyn 3.11 over 40 fresh 400-member documents per arm, run in both orders to rule
out an ordering artifact: 1.27 / 1.20 ms per document versus 0.029 / 0.025 — 44x / 48x. That is
the cold case, which is also the fix-all case: Project and Solution scope walk unopened documents.
Out of scope. The repo-wide migration off the batch fixer is a large diff with its own regression
risk, and does not need reviewing in the same sitting as the thing it migrates onto.
ContainingMember/ContainingTypefix-all scopes need the Roslyn 4.4DocumentBasedFixAllProvider(ImmutableArray<FixAllScope>)constructor, sinceGetSupportedFixAllScopesis sealed at every version. This PR is scope-neutral — CA1516 advertises the same three before and
after — and leaves that as one constructor call.
No
ForkingSyntaxEditorBasedCodeFixProvider<T>counterpart. It has 6 consumers in Roslyn against~120 on the plain base, and the nesting shape it exists for is covered here two cheaper ways: the
innermost-first ordering, and expressing an enclosing fix through
ReplaceNode(SyntaxNode, Func<SyntaxNode, SyntaxGenerator, SyntaxNode>). It is purely additive if afixer ever needs a re-bound
SemanticModelmid-batch, which is forking's one irreplaceablecapability.
Builds 0 errors / 0 warnings on Roslyn 3.11;
Microsoft.CodeAnalysis.NetAnalyzers.UnitTestsat17,374 / 0,
main's 17,364 plus the 10 added here.Note
Drafted with agent assistance.