Add Roslyn analyzers and source generators skill 🤖🤖🤖 - #2739
Conversation
|
🟡 Contributor Reputation Check: MEDIUM risk
Maintainers: please review this contributor before merging. |
🔒 PR Risk Scan ResultsScanned 3 changed file(s).
✅ No matching risk patterns were detected in changed files.
|
🔍 Vally Lint Results✅ All checks passed
Summary
Full linter output |
There was a problem hiding this comment.
Pull request overview
Adds a reusable skill for developing Roslyn analyzers, code fixes, and incremental source generators.
Changes:
- Adds comprehensive Roslyn development guidance.
- Bundles source-writing and test-verifier templates.
- Registers the skill in documentation.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
skills/roslyn-analyzers/SKILL.md |
Defines the skill workflow and guidance. |
skills/roslyn-analyzers/SourceWriter.cs |
Provides deterministic source generation utilities. |
skills/roslyn-analyzers/CSharpCodeFixVerifier.cs |
Provides analyzer/code-fix test helpers. |
docs/README.skills.md |
Adds the skill to the catalog. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
Suppressed comments (4)
skills/roslyn-analyzers/SKILL.md:84
- This preferred sample identifies
Taskby display text, even though the workflow requires symbol identity. A different symbol with the same fully qualified display name can be treated as the BCLTask; it also repeats formatting work for every invocation. Resolve the type once per compilation and compare symbols.
if (method.Name == KnownApis.Task.Wait &&
method.ContainingType.ToDisplayString() == KnownApis.Task.FullName)
skills/roslyn-analyzers/SKILL.md:256
- The sample uses
PackageVersion Updatefor several dependencies, butUpdateonly changes an item already declared by the repository's central package file; it does not create a pin for a transitive-only package. Since this workflow explicitly supports packages absent from the root declarations, the tailoring instructions must distinguishIncludefor new transitive pins fromUpdatefor existing entries.
Third, copy this `Directory.Packages.Analyzers.props` structure and tailor the baseline comment, Roslyn version, and transitive versions to the oldest supported compiler or SDK host:
skills/roslyn-analyzers/SKILL.md:33
- Correct the spelling of “formatable” to “formattable.”
7. Add or update the code fix in the separate code-fix assembly. Preserve trivia, use syntax generators or typed syntax APIs, annotate simplifiable/formatable nodes where appropriate, provide stable equivalence keys for distinct actions, and offer `FixAllProvider` only when batch application is correct.
skills/roslyn-analyzers/SKILL.md:234
- This scope excludes code-fix projects, but the sample later adds Workspaces versions and line 286 says those entries pin code-fix dependencies. With the shown conditional import, a code-fix project never sees those versions. Include shipping code-fix projects in the compatibility marker/import scope, or define a separate code-fix import.
With central package management, create a dedicated `Directory.Packages.Analyzers.props` at the repository root and import it only for analyzer and source-generator projects. Do not fold these overrides into the general `Directory.Packages.props`: the separate file makes the compatibility boundary visible and lets dependency automation treat it specially.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Suppressed comments (4)
skills/roslyn-analyzers/SKILL.md:260
- In a normal SDK-style project,
Directory.Build.propsis evaluated before imports in the project body. If step one importsAnalyzerCompatibility.propsfrom the project,IsAnalyzerCompatibilityProjectis therefore still unset when a repository-wide props file evaluates this condition, and the compatibility pins are silently skipped. Place this conditional import inDirectory.Build.targets(or arrange for the marker itself to be set earlier).
Second, conditionally import the dedicated package file from a repository-wide props or targets file after central package versions are available:
skills/roslyn-analyzers/SourceWriter.cs:169
GetNextLineonly recognizes\n, so a valid lone\rline break is copied into the output instead of being normalized toNewLine. This leaves mixed newline styles in the supposedly deterministic generated source. Split on both CR and LF, consuming CRLF as one separator.
int lineLength = remainingText.IndexOf('\n');
skills/roslyn-analyzers/SKILL.md:349
assemblyVersion.precision: revisionputs Git version height in the assembly revision, but version height is not unique across sibling commits at the same depth. This configuration therefore does not guarantee the per-commit assembly identity claimed throughout this section. Either narrow the guarantee to a linear history or document/configure the repository's strategy for incorporating a commit-unique build value.
"assemblyVersion": {
"precision": "revision"
}
skills/roslyn-analyzers/SKILL.md:22
- This unconditional rule conflicts with the later requirement to target the oldest supported compiler host:
IIncrementalGeneratoris unavailable before Roslyn 4.x, whileISourceGeneratorremains the compatible extension point there. For repositories supporting older hosts, following this rule makes the generator fail to build/load. Prefer incremental generators when the minimum host supports them, and require an explicit baseline increase otherwise; update the repeated “always” rule and checklist entry too.
This issue also appears on line 260 of the same file.
- Source generators must implement `IIncrementalGenerator`, not `ISourceGenerator`. Design the provider graph so unchanged inputs remain cached and do not regenerate output.
aaronpowell
left a comment
There was a problem hiding this comment.
The C# files shouldn't be in the skills root, they should be in a references folder per the spec
|
Addressed in 7ce98a7. Both C# templates now live under Validation: |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Suppressed comments (2)
skills/roslyn-analyzers/references/SourceWriter.cs:41
char.IsWhiteSpacealso accepts\r,\n, form feed, and other line-separator characters. Using one of them as indentation inserts uncontrolled line breaks before every indented line, contradicting this helper's deterministic-newline guarantee. Restrict indentation to horizontal space or tab.
if (!char.IsWhiteSpace(indentationChar))
skills/roslyn-analyzers/SKILL.md:349
- Revision precision does not guarantee a unique assembly identity per commit: Nerdbank.GitVersioning derives this numeric component from version height, so sibling commits on diverging branches can have the same
AssemblyVersion. This leaves the caching collision that lines 339 and 357 claim to prevent. Qualify this as a linear-history guarantee and document a branch/CI-specific numeric revision scheme when identities must be globally commit-unique.
"assemblyVersion": {
"precision": "revision"
}
Pull Request Checklist
npm startand verified thatREADME.mdis up to date.mainbranch for this pull request.Description
Adds a comprehensive skill for building, reviewing, testing, packaging, and maintaining Roslyn diagnostic analyzers, code fixes, and incremental source generators. It includes concrete guidance for operation-based analysis, incremental pipelines, compatibility-pinned dependencies, dependency automation, diagnostic documentation, assembly versioning, test structure, and NuGet packaging.
The skill bundles two referenced starter assets:
SourceWriter.csfor deterministic generated sourceCSharpCodeFixVerifier.csfor analyzer/code-fix testingValidation performed:
npm run skill:validatenpm startbash eng/fix-line-endings.shgit diff --checkType of Contribution
Additional Notes
The dependency-automation guidance deliberately prevents Renovate and Dependabot from updating the analyzer compatibility baseline, including patch updates. The skill does not rely on paid services.
By submitting this pull request, I confirm that my contribution abides by the Code of Conduct and will be licensed under the MIT License.