Lambda-Associates/sql-utilities
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

chore(sonar): bulk-fix production INFO analyzer warnings + skill update #12

Merged
bermudalamb merged 7 commits from chore/sonarqube-info-prod-bulk into main 2026-05-26 16:49:02 -05:00
Conversation 0 Commits 7 Files Changed 45
+229 -235
bermudalamb commented 2026-05-26 15:51:15 -05:00
Owner
Copy Link
Copy Source

Summary

Bulk-cleared the production-code INFO analyzer warnings on sql-utilities SonarQube — ~156 issues fixed across 18 CA/IDE rule IDs via dotnet format analyzers --diagnostics …, tiered into seven commits by risk. NUnit test refactors (NUnit2045/2046/2011, ~493 issues) are deferred to a separate PR — they're a much larger test-only diff.

Also rewrites the workflow half of .claude/skills/sonarqube/SKILL.MD around the tiered bulk-fix model (the old per-rule loop was right for surgical fixes but doesn't scale to 100+ issues at once).

Tier 1 — mechanical (one commit)

2c2a8b1 — IDE0028 CA1825 CA1834 CA1845 CA1847 CA1860 CA1866 CA1853 CA1830 CA2249 bulk-fixed via dotnet format analyzers. Pure syntactic rewrites (Contains("x") → Contains('x'), IndexOf(c) == -1 → !Contains(c), etc.). 19 files, 39+/39−. CA1846 ×4, CA1806 ×1, CA1869 ×1 had no batch fixer and stay open.

Tier 2 — judgment (one commit per rule)

Commit Rule What
8b7fc1a CA1510 if (x == null) throw new … → ArgumentNullException.ThrowIfNull(x) — 19 sites, 6 files
84b0655 ! CA1822 make-static, 12 files. BREAKING: two public methods became static — QueryBreakdownGenerator.GenerateMethodChainDiagram, SqlStatementGenerator.GenerateLinqPipelineDiagram. Both are stateless utility methods on Generator classes; all in-repo callers already used the type-name form, but external NuGet consumers using instance.Method(...) will need to switch to Type.Method(...)
f5d539b CA1861 Hoist constant array args to static readonly fields, 8 files. Manually renamed two cryptic fixer-generated field names (stringArray → SnowflakeFunctionNames, arg2 → NotInValues) and deduped three identical separator fields the fixer emitted in the same test class
81254c1 CA1854 ContainsKey(k)+indexer → TryGetValue, 4 files
6af2390 CA1859 Concrete return types for perf — no batch fixer; applied manually after auditing each site for visibility. All 6 sites are private/internal/test (no public-API impact). The public RuleSet.GetAllSingleRules() overload kept its IEnumerable<SingleRule> shape; only the private recursive helper was tightened

Tier 3 — skill update

c38d122 — rewrites section B of .claude/skills/sonarqube/SKILL.MD around tiered batches. Adds:

  • Three-tier model with dotnet format analyzers --diagnostics <ids> examples.
  • Per-rule callouts for the gotchas hit in this PR (CA1822 binary break, CA1861 cryptic field names + duplicate-field collisions, CA1859 no batch fixer, etc).
  • The local-Java-version gotcha: ./scan-sonar.ps1 is blocked here (system JRE 8 vs scanner Java 17); CI handles the upload via actions/setup-java@v4 with temurin 17.

Section A (API access) is unchanged.

Test plan

  • dotnet build … -c Release (full --no-incremental rebuild) — 35 warnings, 0 errors at HEAD vs 39 at branch start (4-warning local drop; most of the 156 fixed issues are info-only locally so don't show in the build summary)
  • dotnet test … -c Release --no-build — all 8 test projects green, 1180 passed / 7 skipped / 0 failed throughout the branch
  • CI SonarQube Analysis job runs the scan on Java 17 — verify on PR open that open INFO count drops by ~150 (from 649) and no new MAJOR/CRITICAL/BLOCKER appear
  • Skill cold-read test — a fresh agent should be able to execute one of the tiers using only the updated SKILL.MD + memory [[sonarqube-wontfix-rules]]

Notes

  • The ! bang on commit 84b0655 (CA1822) is the only deliberate breaking change. The S2342 rename in PR #11 set the precedent for accepting public-API tightening when the change is structurally correct (here: stateless utilities really should be static).
  • After this PR merges, the only INFO issues remaining will be the 493 NUnit test refactors (NUnit2045/2046/2011), 3 production rules with no batch fixer (CA1806, CA1846, CA1869), and a few odd singletons. Those can be a follow-up.

🤖 Generated with Claude Code

## Summary Bulk-cleared the production-code INFO analyzer warnings on `sql-utilities` SonarQube — ~156 issues fixed across 18 CA/IDE rule IDs via `dotnet format analyzers --diagnostics …`, tiered into seven commits by risk. NUnit test refactors (NUnit2045/2046/2011, ~493 issues) are deferred to a separate PR — they're a much larger test-only diff. Also rewrites the workflow half of `.claude/skills/sonarqube/SKILL.MD` around the tiered bulk-fix model (the old per-rule loop was right for surgical fixes but doesn't scale to 100+ issues at once). ### Tier 1 — mechanical (one commit) `2c2a8b1` — `IDE0028 CA1825 CA1834 CA1845 CA1847 CA1860 CA1866 CA1853 CA1830 CA2249` bulk-fixed via `dotnet format analyzers`. Pure syntactic rewrites (`Contains("x")` → `Contains('x')`, `IndexOf(c) == -1` → `!Contains(c)`, etc.). 19 files, 39+/39−. `CA1846 ×4`, `CA1806 ×1`, `CA1869 ×1` had no batch fixer and stay open. ### Tier 2 — judgment (one commit per rule) | Commit | Rule | What | |---|---|---| | `8b7fc1a` | `CA1510` | `if (x == null) throw new …` → `ArgumentNullException.ThrowIfNull(x)` — 19 sites, 6 files | | `84b0655` **!** | `CA1822` | make-static, 12 files. **BREAKING**: two `public` methods became static — `QueryBreakdownGenerator.GenerateMethodChainDiagram`, `SqlStatementGenerator.GenerateLinqPipelineDiagram`. Both are stateless utility methods on Generator classes; all in-repo callers already used the type-name form, but external NuGet consumers using `instance.Method(...)` will need to switch to `Type.Method(...)` | | `f5d539b` | `CA1861` | Hoist constant array args to `static readonly` fields, 8 files. Manually renamed two cryptic fixer-generated field names (`stringArray` → `SnowflakeFunctionNames`, `arg2` → `NotInValues`) and deduped three identical `separator` fields the fixer emitted in the same test class | | `81254c1` | `CA1854` | `ContainsKey(k)`+indexer → `TryGetValue`, 4 files | | `6af2390` | `CA1859` | Concrete return types for perf — no batch fixer; applied manually after auditing each site for visibility. All 6 sites are `private`/`internal`/test (no public-API impact). The public `RuleSet.GetAllSingleRules()` overload kept its `IEnumerable<SingleRule>` shape; only the private recursive helper was tightened | ### Tier 3 — skill update `c38d122` — rewrites section B of `.claude/skills/sonarqube/SKILL.MD` around tiered batches. Adds: - Three-tier model with `dotnet format analyzers --diagnostics <ids>` examples. - Per-rule callouts for the gotchas hit in this PR (CA1822 binary break, CA1861 cryptic field names + duplicate-field collisions, CA1859 no batch fixer, etc). - The local-Java-version gotcha: `./scan-sonar.ps1` is blocked here (system JRE 8 vs scanner Java 17); CI handles the upload via `actions/setup-java@v4` with `temurin` 17. Section A (API access) is unchanged. ## Test plan - [x] `dotnet build … -c Release` (full `--no-incremental` rebuild) — 35 warnings, 0 errors at HEAD vs 39 at branch start (4-warning local drop; most of the 156 fixed issues are info-only locally so don't show in the build summary) - [x] `dotnet test … -c Release --no-build` — all 8 test projects green, 1180 passed / 7 skipped / 0 failed throughout the branch - [ ] CI `SonarQube Analysis` job runs the scan on Java 17 — verify on PR open that open INFO count drops by ~150 (from 649) and no new MAJOR/CRITICAL/BLOCKER appear - [ ] Skill cold-read test — a fresh agent should be able to execute one of the tiers using only the updated SKILL.MD + memory `[[sonarqube-wontfix-rules]]` ## Notes - The `!` bang on commit `84b0655` (CA1822) is the only deliberate breaking change. The S2342 rename in PR #11 set the precedent for accepting public-API tightening when the change is structurally correct (here: stateless utilities really should be static). - After this PR merges, the only INFO issues remaining will be the 493 NUnit test refactors (NUnit2045/2046/2011), 3 production rules with no batch fixer (CA1806, CA1846, CA1869), and a few odd singletons. Those can be a follow-up. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
bermudalamb added 7 commits 2026-05-26 15:51:15 -05:00
chore(sonar): bulk-fix mechanical CA/IDE analyzer warnings 2c2a8b1193 
Applied via `dotnet format analyzers --diagnostics IDE0028 CA1825 CA1834
CA1845 CA1847 CA1860 CA1866 CA1853 CA1830 CA1846 CA1806 CA1869 CA2249
--severity info`. 19 files touched, all mechanical syntactic rewrites:

- CA1847: string.Contains("x") -> string.Contains('x')
- CA2249: s.IndexOf(c) == -1 -> !s.Contains(c)
- CA1830: sb.Append(sb.ToString()) -> sb.Append(sb)
- CA1834: StringBuilder.Append("x") -> Append('x')
- CA1825, CA1860, CA1866, CA1853, IDE0028, CA1845: corresponding fixers

Three rules in the batch had no batch fixer available (CA1846 ×4,
CA1806 ×1, CA1869 ×1) and stay open for separate manual handling.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
chore(sonar): use ArgumentNullException.ThrowIfNull (CA1510) 8b7fc1a327 
Applied via `dotnet format analyzers --diagnostics CA1510 --severity info`.
Replaces `if (x == null) throw new ArgumentNullException(nameof(x));`
blocks with the one-line `ArgumentNullException.ThrowIfNull(x);` —
same behavior, same parameter name, much less noise.

6 files touched across SqlBreakdown, SqlServer, LinqToSql.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
chore(sonar)!: mark instance-data-free members static (CA1822) 84b06557e5 
Applied via `dotnet format analyzers --diagnostics CA1822 --severity info`.
12 files touched. The fixer also updated internal callers in tests to use
the type-name form (e.g. `gen.Method(x)` -> `Generator.Method(x)`); build
and full test suite remain green.

BREAKING CHANGE: two public methods become static and therefore can no
longer be invoked through an instance reference by external consumers:
- Strata.SqlTools.Markdown.LinqToSql.QueryBreakdownGenerator.GenerateMethodChainDiagram
- Strata.SqlTools.Markdown.LinqToSql.SqlStatementGenerator.GenerateLinqPipelineDiagram

Both are stateless utility methods on Generator classes — the static form
is the correct shape; the only callers in this repo already used the
type-name form. External code should change `gen.GenerateMethodChainDiagram(...)`
to `QueryBreakdownGenerator.GenerateMethodChainDiagram(...)`.

All other CA1822 hits in this commit are on private/protected members
(no public-surface impact).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
chore(sonar): hoist constant array literals to static readonly fields (CA1861) f5d539b906 
Applied via `dotnet format analyzers --diagnostics CA1861 --severity info`,
plus manual cleanup:

- Renamed two cryptic fixer-generated field names:
  - QueryBreakdownCollection.stringArray -> SnowflakeFunctionNames (and
    inlined the now-redundant local alias)
  - ExpressionObjectTests.arg2 -> NotInValues
- Deduped three identical `separator = ['\r','\n']` fields the fixer
  emitted in the same test class (kept the first declaration; the other
  two test methods now reuse it).

8 files touched.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
chore(sonar): prefer TryGetValue over ContainsKey+indexer (CA1854) 81254c12f9 
Applied via `dotnet format analyzers --diagnostics CA1854 --severity info`.
Eliminates the duplicate hash lookup in the
`if (d.ContainsKey(k)) d[k]++ else d[k] = 1` pattern. The fixer rewrites
the conditional to `if (d.TryGetValue(k, out var value)) d[k] = ++value;`
which is semantically identical but does the lookup once.

4 files touched.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
chore(sonar): use concrete return types where binary-safe (CA1859) 6af239035b 
CA1859 has no `dotnet format` batch fixer, so applied manually after
verifying each site is private/internal/test (no public-surface impact):

- Markdown.TryParseLogicalOperation: Expression? -> BoolExpr?  (private)
- Markdown.TryParseComparison:       Expression? -> Comparison? (private)
- RuleSet.GetAllSingleRules(IGroup): IEnumerable<SingleRule> -> List<SingleRule> (private overload)
- SqlExpressionClause.SplitOnComma:  IEnumerable<string> -> List<string> (private)
- QueryBreakdownRepositoryTests._repository: IQueryBreakdownRepository -> QueryBreakdownRepository (test private field)
- UnitTest1.StartsWith(...,string):  Expression -> MethodCallExpression (test private helper)

The public `RuleSet.GetAllSingleRules()` overload still returns
IEnumerable<SingleRule> — only the private recursive helper was tightened.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
docs(skill): rewrite sonarqube cleanup section around tiered bulk fixes
SonarQube Analysis / sonarqube (pull_request) Successful in 2m51s
Details
c38d122d76 
Section A (server inspection / API access) is unchanged. Section B is
rewritten around the tiered model:

- Tier 1 — mechanical: bulk fix multiple rule IDs in one dotnet format
  invocation (IDE0028, CA1825, CA1834, etc).
- Tier 2 — judgment: one commit per rule, audit diff before staging.
  Calls out CA1822 public-static = binary break, CA1861 cryptic
  generated field names + duplicate-field collisions, CA1859 has no
  batch fixer.
- Tier 3 — manual / no fixer: csharpsquid:Sxxxx, public-API design
  calls. Reference [[sonarqube-wontfix-rules]] memory for triage.

Adds the local-Java-version gotcha: `./scan-sonar.ps1` is blocked here
(system JRE is 8, scanner needs 17); CI handles the upload via
actions/setup-java@v4 with temurin 17.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bermudalamb merged commit 4bb7898fe8 into main 2026-05-26 16:49:02 -05:00
bermudalamb deleted branch chore/sonarqube-info-prod-bulk 2026-05-26 16:49:03 -05:00
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
Clear labels
bug
Something is not working
 duplicate
This issue or pull request already exists
 enhancement
New feature
 help wanted
Need some help
 invalid
Something is wrong
 question
More information is needed
 wontfix
This won't be fixed
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
bermudalamb (Thom Lamb)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: Lambda-Associates/sql-utilities#12