Skip to content

Fix fix-all correctness and authoring conformance across the .NET analyzers - #55538

Merged
tannergooding merged 22 commits into
dotnet:mainfrom
tannergooding:tannergooding-analyzer-conformance-sweep
Aug 6, 2026
Merged

Fix fix-all correctness and authoring conformance across the .NET analyzers#55538
tannergooding merged 22 commits into
dotnet:mainfrom
tannergooding:tannergooding-analyzer-conformance-sweep

Conversation

@tannergooding

@tannergooding tannergooding commented Jul 30, 2026

Copy link
Copy Markdown
Member

A mechanical conformance sweep over the release-tracking rows and the code fixers, plus the repo-wide migration onto SyntaxEditorFixAllProvider introduced by #55533.


Fix-all. WellKnownFixAllProviders.BatchFixer computes each fix against the original document and text-merges the results, so overlapping edits silently lose one. 131 files referenced it, 17 still do. Five fixers were provably broken, each with a test that fails without the change:

Rule Symptom
CA1835 a nested ReadAsync argument re-emitted unfixed
CA1845 a second pass hidden by a hardcoded NumberOfFixAllIterations = 2
CA1854/CA1864 two ContainsKey guards both introducing value -- CS0136 / BC30288
CA1870 two extractions in one type both choosing s_myChars
CA2028 adjacent removals from one block, only one surviving

What stays on the batch fixer either crosses documents or unwraps a node from inside itself, neither of which one shared SyntaxEditor supports.


79 exported fixers that never register a fix, deleted. Roslyn constructs every exported type it finds, and the generated rule table reported CodeFix: True on the strength of one. docs/rules-with-no-code-fix.md records the affected rules so the intent to write them survives.

Release tracking. RS2000/RS2001 never ran -- the package was only a transitive dependency, and the build/ folder that activates it is imported for direct PackageReferences only. Nothing validated the Documentation column either: CA2266 had no link and CA1517 pointed at CA1516's page. 10 rows fixed, and the docs generator now fails on a mismatch.

Authoring conformance. [Shared] on 12 fixers missing it, well-known types through WellKnownTypeProvider rather than Compilation.GetTypeByMetadataName plus a ToDisplayString() comparison, SupportedDiagnostics/FixableDiagnosticIds cached rather than rebuilt per access, and CA2208/CA1421 reading arguments by parameter ordinal -- IOperation exposes them in evaluation order, which is syntactic in C#, so both emitted non-compiling code for a named-and-reordered call.


Out of scope. No rule IDs, severities or RuleLevel values change. Of the sweep's four checks, RuleLevel-versus-tracked-severity found no mismatches, and mutable analyzer state found one allocation nit rather than a correctness bug.

Note

Drafted with agent assistance.

Copilot AI review requested due to automatic review settings July 30, 2026 14:14
@tannergooding
tannergooding requested a review from a team as a code owner July 30, 2026 14:14

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot wasn't able to review this pull request because it exceeds the maximum number of files (300). Try reducing the number of changed files and requesting a review from Copilot again.

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 2 pipeline(s).
1 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

@jeffhandley jeffhandley left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯

tannergooding and others added 17 commits July 30, 2026 17:34
…zers

RS2000/RS2001 never ran: the package was only a transitive dependency, and the
build/ folder that activates it is imported for direct PackageReferences only.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
CA2266 had no link and CA1517 pointed at CA1516's page; the rest carried a
retired host or an uppercase slug segment. DiagnosticDescriptorHelper.Create
derives the canonical form as learn.microsoft.com plus id.ToLowerInvariant().

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
RS2000/RS2001 compare only a row's ID, category and severity, so a row
copy-pasted from its neighbour kept that rule's link. Absent links stay legal
and the page is not fetched, since a rule is documented after it ships.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Without it a MEF part is constructed per request, or fails composition
outright depending on the host.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Compilation.GetTypeByMetadataName bypasses the repo's per-compilation cache,
and comparing ContainingType.ToDisplayString() to a literal allocates a display
string per invocation.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…Fixer

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
DiagnosticAnalyzer instances are shared across compilations. The field is
assigned inline and only ever read, so this is an allocation fix rather than a
correctness one.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…ding them

An expression-bodied property runs ImmutableArray.Create on every access. The
declarations left expression-bodied build their array from a protected abstract
descriptor, so an initializer would dispatch to a derived override from the base
constructor.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Roslyn constructs every exported type it finds, and the generated rule table
reported CodeFix: True for CA2215 on the strength of one.
docs/rules-with-no-code-fix.md records the affected rules so the intent to write
them survives.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
IOperation exposes arguments in evaluation order, which is syntactic in C#, so
indexing by parameter ordinal reads the wrong argument for a named-and-reordered
call and both fixers emitted code that does not compile.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…ider

WellKnownFixAllProviders.BatchFixer merges independent text edits, so it produces
a wrong tree when diagnostics nest or overlap. What stays on it either crosses
documents or unwraps a node from inside itself, neither of which a shared
SyntaxEditor supports.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
An argument to Stream.ReadAsync can itself be a diagnosed ReadAsync call, so
rebuilding the outer one from the original argument syntax re-emitted the inner
one unfixed. The rewrite now reads its arguments and instance off the current
tree.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Two nested if statements each guarded by Regex.IsMatch remove adjacent
statements from the same block, so the batch fixer's text diffs overlap and only
one of the two removals survives a pass.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Two ContainsKey guards in one member both asked the pre-fix semantic model for
an unused name, so both introduced `value` -- CS0136 in C#, BC30288 in VB. The
fix-all state records the names already handed out, scoped to the enclosing
member so guards in different methods still both get `value`.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
An outer concatenation rebuilt string.Concat from the original operand syntax
and replaced the whole root, re-emitting a nested one in its pre-fix form. A
hardcoded NumberOfFixAllIterations = 2 had been hiding the second pass.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Two extractions in one type both chose `s_myChars`, because the name came from
the type's members as they stood before the pass. The System import separately
replaced the original root with a snapshot, discarding any fix applied after it.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@tannergooding
tannergooding force-pushed the tannergooding-analyzer-conformance-sweep branch from cc22566 to 001673d Compare July 31, 2026 00:34
tannergooding and others added 4 commits July 30, 2026 17:49
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Preserve the SyntaxEditor fix-all implementations while adapting them to the Roslyn 4.14 nullable API contracts.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@tannergooding

Copy link
Copy Markdown
Member Author

Resolved merge conflicts from the update to 4.14

@tannergooding
tannergooding merged commit 60d1899 into dotnet:main Aug 6, 2026
22 checks passed
@tannergooding
tannergooding deleted the tannergooding-analyzer-conformance-sweep branch August 6, 2026 17:17
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.

3 participants