Add a skill for authoring .NET analyzers by tannergooding · Pull Request #55510 · dotnet/sdk

tannergooding · 2026-07-28T23:20:16Z

Adds .github/skills/add-net-analyzer, covering the parts of adding a CA#### rule that are easy to get wrong: allocating a diagnostic ID, choosing a RuleLevel, wiring the .resx/.xlf and release-tracking entries, and deciding what belongs in the analyzer versus the fixer.

Most of it is general Roslyn guidance rather than repo trivia -- references/authoring-patterns.md has no external links and no dependency on this repo's layout, so it applies just as well to someone writing an analyzer for their own code. The repo-specific parts are confined to one section there and to SKILL.md.

scripts/NextDiagnosticId.cs is a file-based app that allocates the next free ID for a category, checking DiagnosticCategoryAndIdRanges.txt, the working tree (including untracked files), and open PRs. It caught a live collision while I was writing this -- two in-flight branches had both picked CA1878.

Also de-stales src/Microsoft.CodeAnalysis.NetAnalyzers/docs, which came over from dotnet/roslyn-analyzers largely untouched:

Deleted FxCopPort/ (a port that finished years ago), docs/rules/RS10{22,38,41}.md (three Microsoft.CodeAnalysis.Analyzers rules that are documented upstream), performance.md (measured against a Roslyn tree that no longer exists), documenting-your-analyzers.md and analyzer-reference-page-template.md (a docs pipeline that moved to dotnet/docs), and two orphaned screenshots.
Rewrote netcore-getting-started.md -- the build/test/debug commands, paths, and validation steps all predate the migration.
Retargeted 39 links in writing-dataflow-analysis-based-analyzers.md that pointed into dotnet/roslyn-analyzers.

One find worth calling out separately: VS ships an IAnalyzerAssemblyRedirector that redirects Microsoft.CodeAnalysis.NetAnalyzers.dll out of the SDK to its own copy (documentation/general/analyzer-redirecting.md). It matches on path suffix, so it silently defeats both local-validation routes at design time, and the symptom looks like a stale build. netcore-getting-started.md now says so and points at DOTNET_ANALYZER_REDIRECTING=0.

Out of scope:

Enabling RS0030 for this tree. Microsoft.CodeAnalysis.BannedApiAnalyzers is not in the dependency flow yet, so the two existing #pragma warning disable RS0030 sites stay dead for now.
An analyzer-proposal issue template. Upstream had suggest-a-new-rule.md; this repo has none, so the docs point at /issues/new/choose. Adding one is a team process call.

Verified: ValidateSkill.cs passes, no broken relative links or anchors across the touched markdown, NextDiagnosticId.cs exercised on all six exit paths, and the two properties removed from Package.csproj are provably dead -- I deleted both generated files, rebuilt, and the generator restored them byte-identically.

Note

Drafted with agent assistance; every claim was verified against the source before it went in.

Covers the parts of adding a CA#### rule that are easy to get wrong: allocating a diagnostic ID that does not collide with one already in flight, choosing a RuleLevel, wiring the resx/xlf and release-tracking entries, and splitting work between the analyzer and the fixer. `scripts/NextDiagnosticId.cs` does the ID allocation, checking the working tree and open PRs. ---------- Also de-stales `docs/`. The FxCop port notes, the three RS#### rule pages, and the analyzer documentation templates described either the retired `dotnet/roslyn-analyzers` repo or a docs pipeline that no longer runs, and `netcore-getting-started.md` pointed at paths that moved during the migration into this repo. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

azure-pipelines · 2026-07-28T23:20:33Z

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.

Copilot

Pull request overview

Adds a new GitHub Copilot “skill” to guide contributors through adding/porting .NET analyzers (CA####) in src/Microsoft.CodeAnalysis.NetAnalyzers, along with a helper script for allocating the next available diagnostic ID and a cleanup/refresh of migrated NetAnalyzers documentation.

Changes:

Add .github/skills/add-net-analyzer skill docs plus a NextDiagnosticId.cs helper to avoid diagnostic ID collisions.
Update NetAnalyzers contributor guidance (AGENTS.md) and modernize existing docs (build/validate workflow, configuration docs, and internal cross-links).
Remove stale or upstream-owned documentation that no longer applies post-migration.

Show a summary per file

File	Description
`src/Microsoft.CodeAnalysis.NetAnalyzers/src/Microsoft.CodeAnalysis.NetAnalyzers.Package.csproj`	Removes dead configuration-doc properties and keeps packaging/generation inputs aligned with current generator invocation.
`src/Microsoft.CodeAnalysis.NetAnalyzers/docs/writing-dataflow-analysis-based-analyzers.md`	Retargets links from archived upstream to in-repo sources and current configuration docs.
`src/Microsoft.CodeAnalysis.NetAnalyzers/docs/rules/RS1041.md`	Removes upstream-owned rule documentation from this repo.
`src/Microsoft.CodeAnalysis.NetAnalyzers/docs/rules/RS1038.md`	Removes upstream-owned rule documentation from this repo.
`src/Microsoft.CodeAnalysis.NetAnalyzers/docs/rules/RS1022.md`	Removes upstream-owned rule documentation from this repo.
`src/Microsoft.CodeAnalysis.NetAnalyzers/docs/performance.md`	Deletes stale performance-measurement doc that no longer reflects current infrastructure.
`src/Microsoft.CodeAnalysis.NetAnalyzers/docs/netcore-getting-started.md`	Rewrites getting-started guidance for the `dotnet/sdk` layout, build/test commands, and real-repo validation workflow (including VS redirecting note).
`src/Microsoft.CodeAnalysis.NetAnalyzers/docs/guidelines-for-new-rules.md`	Updates rule proposal guidance for `dotnet/sdk`, fixes paths, and documents using `NextDiagnosticId.cs` to avoid collisions.
`src/Microsoft.CodeAnalysis.NetAnalyzers/docs/FxCopPort/rules-inventory.csv`	Removes obsolete FxCop port inventory content.
`src/Microsoft.CodeAnalysis.NetAnalyzers/docs/FxCopPort/proposed-fxcop-rule-changes-in-roslyn.md`	Removes obsolete FxCop-port-era planning doc.
`src/Microsoft.CodeAnalysis.NetAnalyzers/docs/FxCopPort/porting-fxcop-rules-to-roslyn.md`	Removes obsolete FxCop-port-era planning doc.
`src/Microsoft.CodeAnalysis.NetAnalyzers/docs/documenting-your-analyzers.md`	Removes docs-pipeline guidance that has moved elsewhere (e.g., `dotnet/docs`).
`src/Microsoft.CodeAnalysis.NetAnalyzers/docs/analyzer-reference-page-template.md`	Removes obsolete rule reference template doc.
`src/Microsoft.CodeAnalysis.NetAnalyzers/docs/analyzer-configuration.md`	Updates configuration guidance and refreshes links to current public docs.
`src/Microsoft.CodeAnalysis.NetAnalyzers/AGENTS.md`	Adds/updates agent-facing contributor guidance: layout map, build/test commands, conventions, and test framework notes.
`.github/skills/add-net-analyzer/SKILL.md`	Adds the new skill entry describing the end-to-end workflow for adding/porting CA rules in this repo.
`.github/skills/add-net-analyzer/scripts/NextDiagnosticId.cs`	Adds a helper script to propose the next available CA ID by scanning ranges, working tree, local branches, and open PRs.
`.github/skills/add-net-analyzer/references/porting-from-roslyn-analyzers.md`	Adds porting guidance (path mapping, common breakages like MSTest vs xUnit).
`.github/skills/add-net-analyzer/references/authoring-patterns.md`	Adds analyzer/fixer/test authoring guidance plus repo-specific helper pointers and gotchas (fix-all, options, etc.).
`.github/copilot-instructions.md`	Updates repo-level Copilot instructions to include NetAnalyzers area in the high-level source map.

Copilot's findings

Files reviewed: 19/22 changed files
Comments generated: 0

`/t:UpdateXlf` fails with MSB4057 when passed to `build.cmd`: Arcade routes the target to its own `Build.proj` rather than to the projects being built. Run it against the project that owns the resx instead. Also drop the flat "derive from `OrderedCodeFixProvider`" prescription. Its sealed `RegisterCodeFixesAsync` registers the action for every diagnostic in `context.Diagnostics`, so it does not fit a rule whose ID also covers shapes the fixer cannot handle. And state that a fix must produce compiling code on every shape it offers itself on, rather than narrowing the tests to the safe shapes. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Chaining `.Remove(node)` silently no-ops after the first call, because the returned list re-creates its surviving nodes. It presents as the same fix-all iteration failure as a batch-fixer bug, so it is worth naming as a distinct cause. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

The generator skips the help-link check whenever it runs offline, and the product build forces offline on, so the file stays empty for every rule. Two sessions using the skill lost time chasing a CI failure it cannot produce. Nothing verifies a rule's help page exists, which makes the dotnet/docs PR the only guard. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Two sessions cited the harness compiling fixed output as evidence their fixer was safe, then shipped a fix that emitted CS1503 and one that would emit CS8640. Harness strictness only covers the shapes a test reaches, so the invalid contexts have to be enumerated deliberately. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

It merges at the text-span level through an interval tree, not at the syntax-node level, so two fixes that delete distinct children of one node merge cleanly and the previous "overlap or nest" wording produced false positives. Two insertions at the same position also conflict, which is easy to miss because an empty span cannot overlap anything. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

The merger sees the diff between the original and fixed documents, not the edit, and the cleanup pass can reflow a region wider than the edit. A fixer whose fix is a single RemoveNode merges 33 field initializers cleanly and fails at 18 once properties are involved, so shape does not predict conflict. Also note that a failing fix-all test has to be read: a verbatim literal written with LF into a CRLF file produces a diff that looks like the same bug. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

tannergooding · 2026-07-29T22:55:13Z

#55533 also exists, which moves the fix all provider to be SyntaxEditorFixAllProvider which is almost identical to Roslyn's own SyntaxEditorBasedCodeFixProvider. Depending on which gets merged first, the other PR should ensure the skill is updated to use that name/convention

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

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

333fred

Did not review files outside the markdown of the skill.

@333fred

Answers @333fred's review. The corrections, in the order they appear: - Analyzer fields must hold nothing derived from a compiler API, and every cache belongs in the compilation-start closure. Immutability is not the test -- an ImmutableArray<INamedTypeSymbol> roots its compilation. - Prefer RegisterSymbolStartAction to a compilation-end action, whose diagnostics never surface in the IDE. - The lookup rule is a performance technique for matching invocations against library members, not a non-negotiable, so it moves to Performance and names the situation it applies to. - Severity is a claim about the code; the false-positive rate is the evidence bar that claim has to clear, not the claim itself. - [Shared] is load-bearing wherever a fixer is MEF-composed, but on the analyzer-package path Roslyn reflects for the export attribute and constructs one cached instance per reference, so it is never read there. - An IOperation analyzer does not bring the fixer along for free; the fixer edits syntax, and only about a third of the ones here are shared, so VB is cheap only where the fixer happens to be language-agnostic. - Preserve semantics where that is trivial, rather than declining to fix. Parenthesize unconditionally with Simplifier.Annotation and use the in-repo helper that applies it. - Report the diagnostic; decide separately whether you can fix it. The analyzer is not narrowed to what the fixer handles, and the fixer does not register an action it cannot carry out. - State the compiling-code rule from the fixer's side, not only from the test side, since the skill index no longer carries it. - Split tests by both behavior and language, so a failure shows at a glance whether it is language-specific. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

@333fred

Section 3 already opens with an unconditional 'Read references/authoring-patterns.md first', so the rejection list under it was a third statement of rules the Checklist and the reference doc both carry. Two of @333fred's findings landed on both copies because the wordings had drifted apart. Drop the list. The Checklist gains the one rule it was missing, and the two decisions that remain -- RuleLevel and semantics preservation -- stay because they are choices with no obvious default rather than failure modes to verify. The same drift had left the test bar saying to cover both languages in one test method, which is what the review asked to change. It now matches the reference: split by both behavior and language. RuleLevel drops its copy of the no-false-positives bar, since step 8 is where that gets earned, and keeps only the level boundary that step 8 does not state. Also drop the closed-set framing on what survives: 'The three things reviewers reject on' and 'The decisions that are yours' tell a reader that anything absent from those lists is safe. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

CA1517's row linked ca1516, sending readers to a different rule's page instead of the ca1517 help link its descriptor generates. CA1877 carried a stray capital. These were the only two defects across the 329 linked rows. The skill's guidance said a stray capital ships a link to the wrong page. learn.microsoft.com is case-insensitive, so only the rule ID matters; it now says that, and notes that a new rule's page 404s until the docs PR lands. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

tannergooding · 2026-07-30T15:03:06Z

All the feedback should be addressed and this should be ready for the next round of review.

I also used the general skill here (including feedback) to finish a pass over the existing analyzers and it looks to have done a generally good job, the bulk of which is tests ensuring that the fix-all provider works.

333fred

Couple more pieces of feedback.

- Rephrase 'must produce compiling code on every shape' to be nuanced: make every reasonable effort, but don't withhold a useful fixer over edge cases; decline for the specific shapes you can't handle instead - Use async/await in the test method example instead of returning the Task Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

333fred

Skill is looking good to me. Haven't reviewed the rest of the change.

tannergooding · 2026-07-30T20:54:57Z

Thanks @333fred, really appreciate you taking the time to go over the skill portion and help make sure its correct.

Copilot AI review requested due to automatic review settings July 28, 2026 23:20

tannergooding requested review from a team, lbussell and mthalman as code owners July 28, 2026 23:20

Copilot started reviewing on behalf of tannergooding July 28, 2026 23:20 View session

Copilot AI reviewed Jul 28, 2026

View reviewed changes

333fred self-assigned this Jul 29, 2026

tannergooding and others added 6 commits July 29, 2026 00:13

mthalman reviewed Jul 29, 2026

View reviewed changes

Comment thread .github/skills/add-net-analyzer/references/authoring-patterns.md

Comment thread .github/skills/add-net-analyzer/references/porting-from-roslyn-analyzers.md

Comment thread .github/skills/add-net-analyzer/SKILL.md Outdated

tannergooding and others added 3 commits July 29, 2026 15:55

Point the analyzer skill at the in-repo helpers before the general sk…

11eb216

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

Defer the xUnit to MSTest mapping to the migration skill

9e0b518

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

Leave opening the docs PR to the user

a7fd1af

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

333fred reviewed Jul 29, 2026

View reviewed changes

tannergooding and others added 3 commits July 29, 2026 18:06

333fred reviewed Jul 30, 2026

View reviewed changes

Comment thread .github/skills/add-net-analyzer/references/authoring-patterns.md Outdated

Comment thread .github/skills/add-net-analyzer/references/authoring-patterns.md Outdated

333fred approved these changes Jul 30, 2026

View reviewed changes

jeffhandley approved these changes Jul 30, 2026

View reviewed changes

tannergooding merged commit 6895c4c into dotnet:main Jul 30, 2026
22 checks passed

tannergooding deleted the tannergooding-add-net-analyzer-skill branch July 30, 2026 21:26

dotnet-maestro Bot mentioned this pull request Jul 31, 2026

[main] Source code updates from dotnet/sdk dotnet/dotnet#8053

Merged

dotnet-milestone-bot Bot added this to the 11.0-rc1 milestone Jul 31, 2026

MichaelSimons mentioned this pull request Jul 31, 2026

Microsoft.DotNet.Cli.New.IntegrationTests.CommonTemplatesTests.FeaturesSupport: intermittent hang (test host hang-dump timeout, exit code 7) #55551

Closed

Uh oh!

Conversation

tannergooding commented Jul 28, 2026

Uh oh!

azure-pipelines Bot commented Jul 28, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Copilot's findings

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tannergooding commented Jul 29, 2026

Uh oh!

333fred left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tannergooding commented Jul 30, 2026

Uh oh!

333fred left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

333fred left a comment

Choose a reason for hiding this comment

Uh oh!

tannergooding commented Jul 30, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants