Skip to content

feat(detect): recognize C# and its Roslyn analyzers - #616

Merged
theCodeDrift merged 1 commit into
mainfrom
feat/detect-roslyn-analyzers
Nov 7, 2026
Merged

theCodeDrift merged 1 commit into
mainfrom
feat/detect-roslyn-analyzers

Conversation

@theCodeDrift

Copy link
Copy Markdown
Member

taskless detect had no notion of C#, so a .NET repository reported no language and no linter. This adds C#, plus Roslynator, StyleCop and SonarAnalyzer.

Each analyzer is reported under its own name, not as one roslyn entry: people know the tool they installed, not the platform under it.

What gets detected

Packages Own config Keys in .editorconfig / .globalconfig CI and hooks
roslynator Roslynator.Analyzers, .Formatting.Analyzers, .CodeAnalysis.Analyzers, .DotNet.Cli .roslynatorconfig roslynator_*, dotnet_diagnostic.RCS*, category-roslynator roslynator analyze|fix|format, or installing the CLI
stylecop StyleCop.Analyzers stylecop.json, .stylecop.json dotnet_diagnostic.SA* none, runs in the build
sonaranalyzer SonarAnalyzer.CSharp SonarLint.xml dotnet_diagnostic.S<digit>* none, runs in the build
  • C# comes from a .csproj. A .sln does not count, since it can hold any language.
  • Packages are read from PackageReference and GlobalPackageReference in .csproj, .props and .targets, plus packages.config and dotnet-tools.json.
  • A commented-out reference or key is not evidence, and neither is a PackageVersion pin, since central package management can pin a package no project uses.

Worth a reviewer's attention

  • bin and obj are now pruned for every repository, not only .NET ones, as .NET build output.
  • Routing does not change yet. These linters have no legacy recipe, so a rule request still goes to a Taskless rule, as for rubocop and clippy.
  • Telemetry languageStack does not report C#. It probes exact root paths and a project file has no fixed name.
  • Not covered: the SDK's own CA rules, which ship with the toolchain and need signals of their own.

Sources

Package ids, config files and RCS/SA ids come from the Roslynator, StyleCop Analyzers, sonar-dotnet and NuGet docs. Three things do not: Sonar's S<digits> format is inferred from the README's S107 example, and the dotnet-tools.json shape and GlobalPackageReference come from general .NET knowledge. Roslynator's use of .globalconfig is also inferred; its docs name only .editorconfig and the global AnalyzerConfig format of .roslynatorconfig.

Tested with fixtures and one synthetic repository, not a real .NET project.

Verification

pnpm typecheck, pnpm lint and pnpm --filter @taskless/cli test pass. The OpenSpec change is archived in this PR, with cli-detect going from 33 to 44 scenarios and none dropped.

Report C# from a .csproj, and Roslynator, StyleCop and SonarAnalyzer as
separate linters from their NuGet packages, their config files, and their
diagnostic ids in .editorconfig or .globalconfig.
@theCodeDrift

Copy link
Copy Markdown
Member Author

@claude /review

@github-actions

github-actions Bot commented Nov 6, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @theCodeDrift's task in 4m 16s —— View job


Review: feat(detect): recognize C# and its Roslyn analyzers

Review mode: incremental — read 0 prior review thread(s) before reviewing. The only pre-existing content on this PR is the @claude /review trigger comment itself — no prior review threads, review bodies, or substantive comments exist to classify as addressed or still-open. This is effectively the first review of this PR.

Todo

  • Read prior review state (none found)
  • Fetch PR diff, metadata, and changed-file contents
  • Read packages/cli/src/detect/scan.ts in full (not just the diff hunks) to check interaction with existing walk/exclude/manifest logic
  • Cross-check openspec/specs/cli-detect/spec.md requirements against the implementation and tests
  • Verify regex behavior (config-key patterns, NuGet reference parsing, invocation matching) against the new test fixtures
  • Post findings

Assessment

This is a clean, well-scoped addition. I traced the new C#/Roslynator/StyleCop/SonarAnalyzer signals end-to-end against packages/cli/test/detect.test.ts and the spec deltas and didn't find correctness bugs:

  • nugetDepsFromMsbuild/nugetDepsFromPackagesConfig/nugetDepsFromToolManifest correctly strip comments, lowercase ids, and split Include="A;B" lists; PackageVersion is deliberately excluded (central package management isn't evidence), matching the spec's explicit non-evidence list.
  • The configKeys regexes (ROSLYNATOR_CONFIG_KEY, StyleCop's sa\d, SonarAnalyzer's s\d) anchor on ^[ \t]*, so #/;-commented EditorConfig lines correctly fail to match, and the sa\d vs s\d patterns don't collide with each other.
  • bin/obj pruning is wired through the same shouldExclude used by the main glob and the new MSBUILD_PATTERN/tool-manifest globs, so restore output under obj is correctly invisible to the NuGet readers (confirmed against the "restore output under obj" test case).
  • Language attribution for C# comes from two independent paths — a literal .csproj match, and (separately) any matched Roslyn-analyzer signal contributing its own languages: ["C#"] — which explains why fixtures with only .editorconfig/.roslynatorconfig/CI evidence (no .csproj at all) still assert languages contains C#. That's intentional and consistent with how other polyglot-independent linters already work in this file, not a gap.
  • CI/hook invocation regexes for roslynator analyze|fix|format / roslynator.dotnet.cli correctly preserve and report original casing in evidence while matching case-insensitively.

One minor, non-blocking observation:

  • scan.ts:1093-1098 — the new configKeys loop re-reads .editorconfig/.globalconfig from disk once per analyzer signal (roslynator, stylecop, sonaranalyzer each call readAt independently), so a repo with those files present gets them read up to 3× with no caching across signals. This mirrors the pre-existing iniSections pattern in the same function, so it's consistent with the file's existing style rather than a new anti-pattern, but it's worth knowing the cost scales with "signals sharing a config file" now that 3 of them do. Not worth blocking on.

The author's own callout about pruning bin/obj for every repository (not just .NET ones) is reasonable — detect only ever looks for specific named config files/manifests, and bin/obj are unlikely homes for any of those, so the risk of a false negative in, say, a Ruby repo's bin/ is low. No action needed there.

Test coverage is thorough: positive cases per evidence type (project file, shared .props, lowercase GlobalPackageReference, legacy packages.config, local tool manifest, EditorConfig option key, analyzer-id key, severity-category key, committed .roslynatorconfig, CI run/install), and negative cases (commented reference, version-only pin, commented key, another analyzer's id, obj restore output, malformed tool manifest), plus a dedicated "three analyzers from one shared .editorconfig" test and an SDK-CA-id exclusion test. I didn't find a gap in this matrix worth adding to.

No inline comments posted — nothing here rises to the level of a concrete defect.

@theCodeDrift

Copy link
Copy Markdown
Member Author

Re: Claude — "Claude finished @theCodeDrift's task in 4m 16s…"
#616 (comment)

Declining the repeated .editorconfig read: it is a few extra reads of a small file in an on-demand scan, and it matches the existing iniSections pattern, so a cache would add code without a measured need. No other findings to address.

— AI Coding Agent

@theCodeDrift
theCodeDrift merged commit 374a7b6 into main Nov 7, 2026
8 checks passed
@theCodeDrift
theCodeDrift deleted the feat/detect-roslyn-analyzers branch November 7, 2026 00:32
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.

1 participant