chore(sonar)!: production-code cleanup — S1168 GetQuery + CA1822 static methods #15

2026-05-27T14:23:54-05:00

bermudalamb commented

2026-05-27 14:23:54 -05:00

Summary

Clears the two production-code SonarQube rules called out next on the cleanup list:

S1168 (1 instance) — GetQuery<T>() no longer returns IQueryable<T>? / null; now returns a non-nullable IQueryable<T> that is Enumerable.Empty<T>().AsQueryable() when reconstruction isn't possible. Three tests in LinqQueryBreakdownTests.cs that asserted Is.Null are renamed and now assert Is.Empty.
CA1822 (5 instances) — five member methods that don't access instance state are now static. Three are public methods on the Markdown.SqlServer generators (binary-breaking for NuGet consumers); two are private helpers (non-breaking). The auto-fixer also caught one additional private helper not in the original list (LinqExpressionVisitor.ExtractSelectExpression).

Three commits, each verified with dotnet build -c Release and the full test suite (1180/1180 passing throughout):

Commit	Rule	Breaking?	What changed
`2c23ba4`	S1168	yes	`GetQuery<T>` signature + empty returns + test contract updates
`3c602a4`	CA1822 (private)	no	`LinqExpressionVisitor.ExtractMemberName`, `Markdown.Expressions.ExpressionGenerator.GenerateMermaidDiagram(Expression)`
`5202d93`	CA1822 (public)	yes	`Markdown.SqlServer.QueryBreakdownGenerator.GenerateMermaidDiagram(QueryBreakdown, string?)`, `Markdown.SqlServer.SqlStatementGenerator.GenerateSequenceDiagram(ISqlBreakdown, string?)` + `GenerateEntityRelationshipDiagram(IEnumerable<string>, string?)`. Plus a bonus `ExtractSelectExpression` the fixer caught.

Notes

The S1168 site was previously tracked as a "won't-fix / deliberate design" decision (memory note from 2026-05-22). The user explicitly opted to code-fix it this round; the memory has been updated to record the reversal.
The CA1822 public-method change leaves the Snowflake / LinqToSql / PostgreSql wrapper generators with an unused _baseGenerator field. That S4487 / IDE0052 cleanup is intentionally a follow-up — keeps the breaking-change blast radius limited to the three methods explicitly named in the BREAKING CHANGE: footer of 5202d93.

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 completes; S1168 and CA1822 should both drop to 0

## Summary Clears the two production-code SonarQube rules called out next on the cleanup list: - **S1168** (1 instance) — `GetQuery<T>()` no longer returns `IQueryable<T>?` / `null`; now returns a non-nullable `IQueryable<T>` that is `Enumerable.Empty<T>().AsQueryable()` when reconstruction isn't possible. Three tests in `LinqQueryBreakdownTests.cs` that asserted `Is.Null` are renamed and now assert `Is.Empty`. - **CA1822** (5 instances) — five member methods that don't access instance state are now `static`. Three are public methods on the `Markdown.SqlServer` generators (binary-breaking for NuGet consumers); two are private helpers (non-breaking). The auto-fixer also caught one additional private helper not in the original list (`LinqExpressionVisitor.ExtractSelectExpression`). Three commits, each verified with `dotnet build -c Release` and the full test suite (1180/1180 passing throughout): | Commit | Rule | Breaking? | What changed | |---|---|---|---| | `2c23ba4` | S1168 | yes | `GetQuery<T>` signature + empty returns + test contract updates | | `3c602a4` | CA1822 (private) | no | `LinqExpressionVisitor.ExtractMemberName`, `Markdown.Expressions.ExpressionGenerator.GenerateMermaidDiagram(Expression)` | | `5202d93` | CA1822 (public) | yes | `Markdown.SqlServer.QueryBreakdownGenerator.GenerateMermaidDiagram(QueryBreakdown, string?)`, `Markdown.SqlServer.SqlStatementGenerator.GenerateSequenceDiagram(ISqlBreakdown, string?)` + `GenerateEntityRelationshipDiagram(IEnumerable<string>, string?)`. Plus a bonus `ExtractSelectExpression` the fixer caught. | ## Notes - The S1168 site was previously tracked as a "won't-fix / deliberate design" decision (memory note from 2026-05-22). The user explicitly opted to code-fix it this round; the memory has been updated to record the reversal. - The CA1822 public-method change leaves the Snowflake / LinqToSql / PostgreSql wrapper generators with an unused `_baseGenerator` field. That `S4487 / IDE0052` cleanup is intentionally a follow-up — keeps the breaking-change blast radius limited to the three methods explicitly named in the `BREAKING CHANGE:` footer of `5202d93`. ## 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 completes; S1168 and CA1822 should both drop to 0

bermudalamb added 3 commits 2026-05-27 14:23:54 -05:00

chore(sonar)!: return empty IQueryable instead of null from GetQuery<T> (S1168) 2c23ba4a88

Replaces `null` returns from `SqlBreakdownBase.GetQuery<T>()` and its four
overrides (LinqQueryBreakdown, SqlServer/PostgreSql/Snowflake QueryBreakdown)
with `Enumerable.Empty<T>().AsQueryable()`, and tightens the signature from
`IQueryable<T>?` to `IQueryable<T>`. The "we can't reconstruct" semantic
now lives in "the query yields zero rows" rather than a nullable return,
which is what callers in LINQ pipelines actually want.

Three LinqQueryBreakdownTests tests asserting `Is.Null` are renamed and
updated to assert `Is.Empty`.

BREAKING CHANGE: SqlBreakdownBase.GetQuery<T> and the SqlServer / PostgreSql /
Snowflake / LinqToSql QueryBreakdown.GetQuery<T> overrides no longer return
`IQueryable<T>?`; they now return a non-nullable `IQueryable<T>` that is
empty when reconstruction isn't possible. External NuGet consumers null-
checking the result must switch to `.Any()` / `Is.Empty` checks instead.

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

chore(sonar): mark private helpers static (CA1822) 3c602a4625

Two private methods that don't touch instance state get the `static`
keyword:

- `LinqExpressionVisitor.ExtractMemberName`
- `Markdown.Expressions.ExpressionGenerator.GenerateMermaidDiagram(Expression)`

Both have only intra-class callers, so this is a non-breaking change —
the call sites continue to work unchanged under C# method-resolution.

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

chore(sonar)!: mark public Markdown generator methods static (CA1822)

SonarQube Analysis / sonarqube (pull_request) Successful in 3m30s

Details

5202d93e8e

Three public methods on the SqlServer-namespaced Markdown generators no
longer touch instance state and now carry the `static` keyword:

- `Markdown.SqlServer.QueryBreakdownGenerator.GenerateMermaidDiagram(QueryBreakdown, string?)`
- `Markdown.SqlServer.SqlStatementGenerator.GenerateSequenceDiagram(ISqlBreakdown, string?)`
- `Markdown.SqlServer.SqlStatementGenerator.GenerateEntityRelationshipDiagram(IEnumerable<string>, string?)`

Plus one private bonus the analyzer caught on the same pass:
- `LinqExpressionVisitor.ExtractSelectExpression` → static (non-breaking).

Internal callers in the Snowflake/LinqToSql/PostgreSql wrapper classes
and in the test fixtures are updated to the type-name form
(`SqlServer.SqlStatementGenerator.GenerateSequenceDiagram(...)`).
The wrappers retain their `_baseGenerator` field for now even though it
is no longer used — that S4487 / unused-field cleanup is its own commit.

BREAKING CHANGE: External NuGet consumers calling
`generatorInstance.GenerateMermaidDiagram(...)`,
`generatorInstance.GenerateSequenceDiagram(...)`, or
`generatorInstance.GenerateEntityRelationshipDiagram(...)` on the
SqlServer-namespaced generators must switch to type-name form, e.g.
`Markdown.SqlServer.SqlStatementGenerator.GenerateSequenceDiagram(...)`.
Calls through the Snowflake / LinqToSql / PostgreSql wrapper classes are
unaffected at the call site.

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

bermudalamb added 1 commit 2026-05-27 14:33:52 -05:00

refactor(linq): extract GetEmptyQueryable<T> helper for empty-return paths

SonarQube Analysis / sonarqube (pull_request) Successful in 3m28s

Details

3ba7a7e9f3

DRYs the four `Enumerable.Empty<T>().AsQueryable()` returns added in
`2c23ba4` (S1168 fix) into a single `public static IQueryable<T>
GetEmptyQueryable<T>()` helper on `LinqQueryBreakdown`. No behavior
change — all 1180 tests stay green.

Co-Authored-By: Thom Lamb <thomlamb@gmail.com>

bermudalamb added 1 commit 2026-05-27 14:39:03 -05:00

chore(sonar)!: cascade CA1822 through Markdown dialect wrappers

SonarQube Analysis / sonarqube (pull_request) Successful in 2m50s

Details

6175dcde96

Commit 5202d93 made the SqlServer-namespaced Markdown generator methods
static, which left the LinqToSql / PostgreSql / Snowflake wrapper
classes' instance methods delegating to nothing but a static call.
SonarQube re-flagged those 10 wrapper methods as CA1822 on the next
scan.

This sweep:

- Makes all 10 wrapper instance methods `static` (`dotnet format` driven).
- Makes `Markdown.LinqToSql.QueryBreakdownGenerator.GenerateCombinedDiagram`
  static preemptively — it composes two static helpers and would otherwise
  be the next-iteration cascade flag.
- Removes the now-dead `_baseGenerator` field and its initializing
  constructor from all six dialect wrappers (LinqToSql / PostgreSql /
  Snowflake × QueryBreakdownGenerator + SqlStatementGenerator). The
  classes keep their implicit parameterless constructor so `new
  Snowflake.QueryBreakdownGenerator()` still compiles.
- Updates the one test call site (`GenerateCombinedDiagram`) the fixer
  didn't catch to use type-name form.

BREAKING CHANGE: External NuGet consumers calling
`instance.Generate*Diagram(...)` on `Markdown.LinqToSql.*`,
`Markdown.PostgreSql.*`, or `Markdown.Snowflake.*` generators must
switch to type-name form, e.g. `Markdown.Snowflake.SqlStatementGenerator.GenerateSequenceDiagram(...)`.
The class types and parameterless constructors remain — only the call
syntax for these methods changes.

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

bermudalamb merged commit cbe891dc0b into main

2026-05-27 14:54:06 -05:00

bermudalamb deleted branch fix/sonarqube-prod-s1168-ca1822

2026-05-27 14:54:06 -05:00

bermudalamb referenced this issue from a commit

2026-05-27 15:08:58 -05:00

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

bermudalamb referenced this pull request

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

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

Sign in to join this conversation.