chore(sonar)!: clear all 17 remaining src/ code smells #16

2026-05-27T15:09:17-05:00

bermudalamb commented

2026-05-27 15:09:17 -05:00

Summary

Drives the SonarQube src/ open-issue count from 17 → 0 in three commits, each with dotnet build -c Release + 1180/1180 tests green.

Commit	Rule(s)	Breaking?	Notes
`1bd6dee`	CA1846, CA1853, S2219	no	Three small mechanical wins: `Substring` → `AsSpan` in an exception path, drop redundant `ContainsKey` guard around `Dictionary.Remove`, collapse the post-S1168 stub `GetQuery<T>()` to an expression-bodied member
`54b9c78`	CA1861 (×3), CA1806, S1135 (×2)	no	Hoist three inline `Split` delimiters to `static readonly char[]` fields with descriptive names; explicit `_ =` discard on `double.TryParse`; rewrite the two `TODO:` comments as plain "Future:" notes that still document the design rationale
`423108a`	S1118 (×8)	yes	Mark all eight `Markdown.{SqlServer,LinqToSql,PostgreSql,Snowflake}.{QueryBreakdownGenerator,SqlStatementGenerator}` as `public static class`. Test fixtures lose the meaningless `_generator = new …()` field + `[SetUp]`

Notes

The S1118 commit is the natural completion of the CA1822 cascade in #15 — once every method on those classes is static, the class itself wants the modifier. Internal call sites in the wrappers (and tests, where the fixer already converted them in #15) all use type-name form, so this only breaks external callers doing new.
The two S1135 sites were both deliberate design notes mis-labeled as TODOs ("inherit base for now", "could be indexed later") — the rewording preserves the developer intent without lying to the analyzer.

Test plan

dotnet build Strata.SqlTools.QueryBreakdown.sln -c Release — 0 errors after each commit
dotnet test ... -c Release --no-build — 1180 passed, 0 failed
Gitea Actions sonarqube.yml runs the SonarScanner upload on this PR
Re-query /api/issues/search?componentKeys=sql-utilities&resolved=false&facets=rules after the scan; src/ count should drop from 17 → 0 (CS8604 in tests/ and the residual NUnit2046/NUnit2045 corner cases remain, all pre-existing)

## Summary Drives the SonarQube `src/` open-issue count from **17 → 0** in three commits, each with `dotnet build -c Release` + 1180/1180 tests green. | Commit | Rule(s) | Breaking? | Notes | |---|---|---|---| | `1bd6dee` | CA1846, CA1853, S2219 | no | Three small mechanical wins: `Substring` → `AsSpan` in an exception path, drop redundant `ContainsKey` guard around `Dictionary.Remove`, collapse the post-S1168 stub `GetQuery<T>()` to an expression-bodied member | | `54b9c78` | CA1861 (×3), CA1806, S1135 (×2) | no | Hoist three inline `Split` delimiters to `static readonly char[]` fields with descriptive names; explicit `_ =` discard on `double.TryParse`; rewrite the two `TODO:` comments as plain "Future:" notes that still document the design rationale | | `423108a` | S1118 (×8) | **yes** | Mark all eight `Markdown.{SqlServer,LinqToSql,PostgreSql,Snowflake}.{QueryBreakdownGenerator,SqlStatementGenerator}` as `public static class`. Test fixtures lose the meaningless `_generator = new …()` field + `[SetUp]` | ## Notes - The S1118 commit is the natural completion of the CA1822 cascade in #15 — once every method on those classes is static, the class itself wants the modifier. Internal call sites in the wrappers (and tests, where the fixer already converted them in #15) all use type-name form, so this only breaks external callers doing `new`. - The two S1135 sites were both deliberate design notes mis-labeled as TODOs ("inherit base for now", "could be indexed later") — the rewording preserves the developer intent without lying to the analyzer. ## Test plan - [x] `dotnet build Strata.SqlTools.QueryBreakdown.sln -c Release` — 0 errors after each commit - [x] `dotnet test ... -c Release --no-build` — 1180 passed, 0 failed - [ ] Gitea Actions `sonarqube.yml` runs the SonarScanner upload on this PR - [ ] Re-query `/api/issues/search?componentKeys=sql-utilities&resolved=false&facets=rules` after the scan; src/ count should drop from 17 → 0 (CS8604 in tests/ and the residual NUnit2046/NUnit2045 corner cases remain, all pre-existing)

bermudalamb added 3 commits 2026-05-27 15:09:17 -05:00

chore(sonar): mechanical src/ cleanups — AsSpan, drop ContainsKey guard, simplify GetQuery (CA1846, CA1853, S2219) 1bd6deec83

- `SqlParseException.cs:90` (CA1846): `sql.Substring(0, 197)` →
  `sql.AsSpan(0, 197)` in the truncated-SQL diagnostic message. Avoids
  an allocation in an already cold exception path.

- `Snowflake/QueryBreakdown.cs:103` (CA1853): drop the redundant
  `Parameters.ContainsKey(...)` guard around `Parameters.Remove(...)`.
  `Dictionary<TKey,TValue>.Remove` is a no-op if the key is absent, so
  the guard only doubled the work and computed the key string twice.

- `LinqQueryBreakdown.cs:194` (S2219): collapse the now-stub
  `GetQuery<T>()` (every branch returned `GetEmptyQueryable<T>()` after
  the S1168 cleanup) to a single expression-bodied member. Updates the
  XML doc to describe the actual current behavior.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

chore(sonar): hoist constant arrays, discard TryParse, defang TODO markers (CA1861, CA1806, S1135) 54b9c7876f

- `SqlUtils.cs:43` and `StatementParser.cs:254,285` (CA1861 ×3): inline
  `new[] { ' ' }` and `new[] { ';' }` Split delimiters hoisted to
  `static readonly char[]` fields next to the existing `separator`
  field. Distinct names (`spaceSeparator`, `semicolonSeparator`) avoid
  collision.

- `SqlUtils.Filters.cs:27` (CA1806): `double.TryParse(value, out var
  dblValue)` had its return value silently discarded — intentional
  (downstream switch branches use `dblValue` only when relevant and
  rely on the default `0.0` on failure). Now uses `_ =` to make the
  discard explicit and extends the comment.

- `FilterCondition.cs:10` and `With.cs:10` (S1135 ×2): rewrite the
  `todo:` / `TODO:` markers as plain "Future:" notes. Both comments
  documented deliberate design choices ("inherit base for now",
  "could be indexed later") rather than tracked work, so the marker
  was misleading anyway.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

chore(sonar)!: mark Markdown generator classes as 'public static class' (S1118)

SonarQube Analysis / sonarqube (pull_request) Successful in 4m22s

Details

423108a7dc

After the CA1822 cascade in #15 left every method on the eight
Markdown generator classes static, the classes themselves were
instantiable shells that consumers couldn't usefully `new`. This
commit flips the `class` modifier to `static class` on all eight:

- `Markdown.SqlServer.QueryBreakdownGenerator`
- `Markdown.SqlServer.SqlStatementGenerator`
- `Markdown.LinqToSql.QueryBreakdownGenerator`
- `Markdown.LinqToSql.SqlStatementGenerator`
- `Markdown.PostgreSql.QueryBreakdownGenerator`
- `Markdown.PostgreSql.SqlStatementGenerator`
- `Markdown.Snowflake.QueryBreakdownGenerator`
- `Markdown.Snowflake.SqlStatementGenerator`

Test fixtures drop the now-meaningless `_generator = new …()` field
and `[SetUp]` (kept the existing Setup body where unrelated state was
also initialized — `QueryMarkdownGenerationTests` and
`LinqToSql.QueryBreakdownGeneratorTests`).

BREAKING CHANGE: External NuGet consumers can no longer write
`new Markdown.Snowflake.SqlStatementGenerator()` (or any of the other
seven classes above) — the type is now a static container and may only
be referenced by name, e.g.
`Markdown.Snowflake.SqlStatementGenerator.GenerateSequenceDiagram(…)`.
The call syntax for the static methods is unchanged.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

bermudalamb merged commit 1345a9219e into main

2026-05-27 15:12:02 -05:00

bermudalamb deleted branch fix/sonarqube-prod-misc-17

2026-05-27 15:12:03 -05:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: Lambda-Associates/sql-utilities#16