diff --git a/.claude/skills/sonarqube/SKILL.MD b/.claude/skills/sonarqube/SKILL.MD index 97ad648..8a66a17 100644 --- a/.claude/skills/sonarqube/SKILL.MD +++ b/.claude/skills/sonarqube/SKILL.MD @@ -38,29 +38,74 @@ Invoke-RestMethod -Uri "$url/api/issues/search?componentKeys=sql-utilities&resol `SONARQUBE_URL` / `SONARQUBE_TOKEN` are read here; `scan-sonar.ps1` reads the separate `SONAR_TOKEN` / `SONAR_HOST_URL` for the *upload* path — do not conflate them. -## B — Cleanup loop +## B — Cleanup loop (tiered) ```dot digraph cleanup { - query [label="Query open issues + facets"]; - pick [label="Pick one rule group"]; - edit [label="Edit code"]; - build [label="dotnet build -c Release"]; - test [label="dotnet test -c Release --no-build"]; - commit [label="git commit\nchore(sonar): … (Sxxxx)"]; - more [label="More groups?" shape=diamond]; - scan [label="scan-sonar.ps1"]; - verify [label="Query API: confirm net-down"]; + query [label="Query open issues + facets=rules"]; + triage [label="Tier each rule:\nmechanical / judgment / API-impact"]; + t1 [label="Tier 1 — mechanical:\nbulk-fix in one commit"]; + t2 [label="Tier 2 — judgment:\none commit per rule, audit diff"]; + t3 [label="Tier 3 — API-impact:\nmanual, accept !breaking or skip"]; + build [label="dotnet build + dotnet test\nafter EACH commit"]; + push [label="git push + open PR"]; + ci [label="CI runs SonarScanner\non Java 17"]; + verify [label="Re-query API:\nconfirm rule counts dropped"]; - query -> pick -> edit -> build -> test -> commit -> more; - more -> pick [label="yes"]; - more -> scan [label="no"]; - scan -> verify; + query -> triage; + triage -> t1 -> build; + triage -> t2 -> build; + triage -> t3 -> build; + build -> push -> ci -> verify; } ``` -1. **Query.** Start with `severities=MAJOR,MINOR` + `facets=rules` to see what's worth fixing. -2. **Pick a rule group.** One Sxxxx (or one tightly-related cluster) per commit, easiest first so a late blocker doesn't strand the others. -3. **Edit → build → test.** Build must succeed; the warning count for the touched rule must drop. Tests must stay green. If a fix would degrade clarity or break a tested contract, prefer **Won't Fix on the server with a justification** over a forced code change — see [[sonarqube-wontfix-rules]] for the catalog of rules already triaged that way (S1168, S3925, CS8601, S107). -4. **Commit.** `chore(sonar): (Sxxxx)` — bang (`!`) if it's a breaking rename. -5. **After all groups:** `./scan-sonar.ps1` (needs `$env:SONAR_TOKEN`). Wait ~30-60s for the CE task to finish, then re-query the issues endpoint and confirm the open MAJOR/MINOR count fell by the expected number. +**The loop has shifted from per-rule commits to *tiered batches* once a project has more than a handful of issues left.** Use `dotnet format analyzers --diagnostics --severity info` — it runs all the Roslyn-shipped code fixers for the listed diagnostics, including `CA*`, `IDE*`, and `NUnit*`. (The `csharpsquid:Sxxxx` family from SonarAnalyzer.CSharp usually has no `dotnet format` fixer; those still need manual edits.) + +### Tier 1 — mechanical (one bulk commit) + +Rules where the fixer's rewrite is purely syntactic and can't change behavior — pile them into one `dotnet format` invocation, audit the diff for sanity, build, test, single commit: + +```powershell +dotnet format analyzers Strata.SqlTools.QueryBreakdown.sln ` + --diagnostics IDE0028 CA1825 CA1834 CA1845 CA1847 CA1860 CA1866 CA1853 CA1830 CA2249 ` + --severity info --verbosity normal +``` + +Typical safe rules: `IDE0028`, `CA1825`, `CA1834`, `CA1845`, `CA1847`, `CA1853`, `CA1860`, `CA1866`, `CA2249`, `CA1830`. Some rules in this family report "no associated code fix" — they'll need manual handling separately. + +### Tier 2 — judgment (one commit per rule) + +Rules whose fixer can produce mediocre output or surprise the reader — bulk-fix but **audit before staging**: + +- `CA1510` — `ArgumentNullException.ThrowIfNull` rollup; always safe but voluminous, deserves its own commit. +- `CA1822` — make-method-static; the fixer also rewrites internal callers to use type-name form. **Public methods becoming static are a binary-break for external NuGet consumers** — commit with `chore(sonar)!:` and a `BREAKING CHANGE:` footer naming each affected member, or revert those file diffs and apply only the private-helper changes. +- `CA1861` — hoists constant array args to `static readonly` fields. The fixer's field names are sometimes cryptic (`stringArray`, `arg2`) or it emits duplicate `separator` fields in the same class. After running, rename ugly fields and dedupe collisions before committing. +- `CA1854` — `TryGetValue`-style double-lookup elimination; trivial. +- `CA1859` — concrete return type for perf. **No batch fixer**: edit each site manually after checking visibility. Apply only to private/internal/test; on public/protected, either revert or treat as a `!breaking` change (most cases are private helpers, so this is usually fine). + +### Tier 3 — manual (no fixer) + +`CA1806`, `CA1846`, `CA1869`, the `csharpsquid:Sxxxx` family that has no `dotnet format` fixer, and individual public-API rules that need design judgment. Edit by hand, one rule at a time. If a fix would degrade clarity or break a tested contract, prefer **server-side Won't Fix with justification** — see [[sonarqube-wontfix-rules]] for the catalog already triaged that way (S1168, S3925, CS8601, S107). + +### Guardrail + +After every commit (Tier 1, 2, or 3): + +```powershell +dotnet build Strata.SqlTools.QueryBreakdown.sln -c Release --nologo +dotnet test Strata.SqlTools.QueryBreakdown.sln -c Release --no-build --nologo +``` + +Build succeeds with no new warnings beyond the baseline. Tests stay green. If either fails, fix or revert before continuing to the next tier — incremental builds can mask warning regressions, so re-run with `--no-incremental` if the warning count looks suspicious. + +### Scan & verify + +**Local `./scan-sonar.ps1` is blocked here** — the system JRE is 8 and the SonarScanner CLI requires Java 17 (`UnsupportedClassVersionError`). CI handles the scan: `.gitea/workflows/sonarqube.yml` runs `actions/setup-java@v4` with `temurin` 17 and uploads on every push and PR. So the path is **push → open PR → CI scans → re-query the API to confirm net-down**: + +```powershell +$h = @{ Authorization = "Basic $b64" } +Invoke-RestMethod -Uri "$url/api/issues/search?componentKeys=sql-utilities&resolved=false&facets=rules&ps=500" -Headers $h +``` + +The targeted rule IDs should each drop to 0 (or near-0 if the fixer left a few cases it couldn't batch-resolve). Commit subject style stays `chore(sonar): … (RuleId)` — bang (`!`) if the change is a deliberate breaking rename or visibility shift.