Replaces the per-dialect <c>Adapter</c> nested classes (which were
themselves the leftover duplication after PR #22's first cut) with a
shared <c>IQueryBreakdownCollectionView</c> interface implemented
directly on each dialect's <c>QueryBreakdownCollection</c>.
New foundation types in <c>Strata.SqlTools.Breakdowns.SqlServer</c>:
- **<c>IQueryBreakdownCollectionView</c>** — dialect-neutral view
exposing QueryCount, UniqueParameterCount, TotalSelectedColumns,
UniqueTableCount, QueriesForReport (typed against the SqlServer
<c>QueryBreakdown</c> base — PG/Snowflake satisfy via <c>IReadOnlyList</c>
covariance), and ParameterUsageRecords.
- **<c>ParameterUsageRecord</c>** — record type for per-parameter usage
stats, projected from each dialect's <c>ParameterUsageReport</c>.
The three dialect <c>QueryBreakdownCollection</c> classes now implement
the interface explicitly — a handful of one-line forwarders per class.
The Markdown layer's old <c>ICollectionMarkdownData</c> interface and
the writer-internal <c>ParameterUsageRow</c> type are deleted; the
template and writer take <c>IQueryBreakdownCollectionView</c> and
<c>ParameterUsageRecord</c> directly.
Net effect on the Markdown wrappers:
- <c>Markdown.SqlServer.QueryBreakdownCollectionGenerator</c> loses
its <c>Adapter</c> nested class and its 6 forwarders pass <c>collection</c>
straight through.
- <c>Markdown.PostgreSql.QueryBreakdownCollectionGenerator</c> ditto.
- <c>Markdown.Snowflake.QueryBreakdownCollectionGenerator</c> migrated
to the same pattern; its dialect-specific
<c>GenerateSnowflakeFeaturesAnalysis</c> and feature-aware
<c>QueryCompositionReport</c> callback stay intact (they still call
<c>CollectionReportWriter</c> directly).
Public API additions: <c>IQueryBreakdownCollectionView</c> and
<c>ParameterUsageRecord</c> (both new, both opt-in). Public API
removals: none — the dialect <c>ParameterUsageReport</c> classes are
untouched and the dialect <c>QueryBreakdownCollectionGenerator</c>
public surface is identical.
All 1180 tests stay green.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Final cluster Sonar was reporting: the 82-line copy-paste between
`Markdown.SqlServer.QueryBreakdownCollectionGenerator` and
`Markdown.PostgreSql.QueryBreakdownCollectionGenerator`. Both classes
existed because each dialect has a different concrete
`QueryBreakdownCollection` type with its own `ParameterUsageReport`
class — no shared base for the methods to operate on.
Resolves it with an adapter pattern in `Markdown.Common`:
- **`ICollectionMarkdownData`** (new, internal): dialect-neutral view
exposing query count, parameter / column / table totals, queries-
for-report list, and parameter-rows (already-mapped to the writer's
`ParameterUsageRow` type).
- **`CollectionMarkdownGenerator`** (new, internal static): single
template that takes the data + `MarkdownDialectFormat` and routes
through `CollectionReportWriter`. The six `GenerateX` methods that
were duplicated three times now live here once.
- **SqlServer / PostgreSql wrappers**: shrunk to a `Format` static, a
thin one-line forwarder per public method, and a private sealed
`Adapter : ICollectionMarkdownData` nested class that does the
dialect-specific extraction (including the `ParameterUsageReport →
ParameterUsageRow` mapping that was previously duplicated three
times as `MapParameters`).
Public API unchanged — the existing `Markdown.SqlServer.QueryBreakdownCollectionGenerator.GenerateCollectionReport(collection, title)`
etc. continue to work as before; their bodies just delegate. The
three dialect-specific `ParameterUsageReport` classes are
deliberately *not* unified yet — their `ToString()` overrides differ
meaningfully per dialect and unifying would be a separate API
discussion.
Snowflake wrapper not touched in this commit — Sonar didn't flag it
(its `GenerateSnowflakeFeaturesAnalysis` and feature-aware
`QueryCompositionReport` callback make it structurally distinct).
Consistency follow-up could move it onto the same adapter pattern
without behavior change.
All 1180 tests stay green.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`SqlServer.QueryBreakdown.GetSqlBreakdown` and
`Snowflake.QueryBreakdown.GetSql` each carried a 24-line copy of the
same CTE-rendering loop ("WITH" keyword, optional RECURSIVE, per-clause
header, anchor/UNION ALL/recursive query, closing parens). The two
copies differed only by indent (5/10 spaces vs 4/8) and a trailing
space after the keyword.
Hoist the loop into `protected virtual void AppendWithClauseSection(
StringBuilder, string withClauseIndent, string queryBodyIndent)` on
`SqlServer.QueryBreakdown`. Each caller invokes it with its dialect's
preferred indents; Snowflake's mid-method copy is deleted entirely.
Standardizes on the no-trailing-space "WITH" form (Snowflake's) — was
"WITH " (trailing space) in the SqlServer original. Visible only as a
trailing space before the newline in non-recursive output, which no
tests assert on.
All 1180 tests stay green.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`QueryBreakdownGenerator`, `SqlStatementGenerator`, and
`ExpressionGenerator` each had a 1-line `private static string
TruncateText(...)` forwarder to `Internal.MarkdownTextHelpers.TruncateText`.
The forwarders existed only to keep existing call sites short
(`TruncateText(x, 50)` instead of the fully-qualified form).
Each file now imports `using static MarkdownTextHelpers;` once at the
top, so call sites continue to read identically and the local
forwarders are deleted. Removes the structural duplication Sonar was
flagging (two private static helpers — `EscapeMermaidText` + the
TruncateText forwarder — appearing in both `QueryBreakdownGenerator`
and `SqlStatementGenerator` with the same shape).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The PostgreSql-specific operator dispatch in
`StatementReader.TryHandleAdditionalCharacter` had four similar 4-7
line blocks (each handling a single-char operator with one or more
two-char variants — \<, \>, \|, \=). Sonar flagged it as a
self-duplication.
Extract a small `TryMatchTwoCharOperator(char, string)` helper that
encapsulates the "if next char matches, advance and emit two-char
operator" pattern. Each operator handler now reads as a small list:
if (CurrentCharacter == '<')
{
MovePosition();
if (TryMatchTwoCharOperator('=', "<=")) return true;
if (TryMatchTwoCharOperator('>', "<>")) return true;
if (TryMatchTwoCharOperator('<', "<<")) return true;
_currentToken = new Token(TokenType.Operator, "<");
return true;
}
Reverses my earlier "extracting would obscure intent" call after
re-reading — the helper-based form actually surfaces the intent
("two-char operator dispatch") more clearly than the original.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The `QueryBreakdown(string select, string from, ...)` constructors on
PostgreSql.QueryBreakdown and Snowflake.QueryBreakdown each ran the
same six-line pattern twice (once per clause): call
`parser.ExtractSqlComments`, set the clause to the trimmed result,
join the comment list into the Comment property.
New `StatementParser.PopulateClauseWithComments(rawText, target)`
instance method does both halves. Each ctor now reads:
parser.PopulateClauseWithComments(selectClause, SelectClause);
parser.PopulateClauseWithComments(fromClause, FromClause);
Same behavior; the helper is a pure refactor of existing semantics.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two follow-ups to the prior dedup pass:
- **Delete `Snowflake.UpdateBreakdown.GetSqlBreakdown`**: it was a
byte-for-byte copy of the SqlServer base's `GetSqlBreakdown` (modulo
one explanatory comment). Snowflake's UPDATE syntax — including the
FROM clause — is identical at the formatter level, so the override
was pure inheritance noise. Now inherits.
- **Extract `ParsePreparation.TryMatchInsertSql`**: the regex match +
group extraction + failure message at the end of
`SqlServer.InsertBreakdown.TryParse` and
`Snowflake.InsertBreakdown.TryParse` was duplicated. Hoist the
shared piece next to `TryRunPrelude` on `ParsePreparation`. Both
callers continue to construct their own `InsertBreakdown` instance
(the constructor signatures differ slightly between dialects).
All 1180 tests stay green.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two shared scaffolds for blocks Sonar flagged across the SqlServer,
Snowflake, and PostgreSQL dialects:
1. **`AppendToClause` on `SqlBreakdownBase`** — collapses the "if
clause is empty set it, else append `{operation} {sql}`; then merge
comment with same rule" pattern that was repeated three times in
each of SqlServer/Snowflake `QueryBreakdown`. The matching
`AddWhereExpression` / `AddHavingExpression` / `AddWhereClause(string)`
sites in both files now delegate to a single `protected static`
helper. Operates against `ISqlClause`, so it works for both the
`WhereClause` and `HavingClause` properties.
2. **`HandleDoubleQuoteAsIdentifier` on `SqlServer.StatementParser`** —
PostgreSQL and Snowflake both override SqlServer's
`HandleDoubleQuote` (which produces a string-literal token) to
instead produce a `ColumnIdentifier` token. The two overrides had
identical 14-line bodies. The shared logic now lives once, and
each dialect's override is a one-liner that calls the helper.
Deliberately *not* refactored in this commit:
- The CTE WITH-clause SQL generation in SqlServer/Snowflake QueryBreakdown
(lines ~537-560 / ~579-601 Sonar flagged) — the surrounding logic
differs enough between the two that an extraction would obscure
rather than clarify.
- The PG/Snowflake QueryBreakdown constructor pair (lines 40-58 /
43-61) — only ~10 lines × 2; extracting requires either a new
shared helper for ~20 lines of savings or moving up the inheritance
chain, neither pays for itself.
All 1180 tests stay green.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The standard `TryParse(...)` prelude — null/empty check, parser
construction, comment-preserving normalize, statement-prefix regex
validation, setup/finish clause extraction — was copy-pasted in
**eight** breakdown classes across the SqlServer and Snowflake
dialects. Sonar flagged it as a six-way duplicate cluster on the
shorter (~17-line) common block, and as additional pairwise
duplicates on the longer (~30-line) version.
Introduces `Strata.SqlTools.Statements.SqlServer.ParsePreparation`
with a single `TryRunPrelude(sql, parser, prefixRegex,
prefixDescription, out ...)` method. Each `TryParse` now calls it
once and proceeds straight to dialect-specific match logic.
Touched callers:
- `SqlServer.InsertBreakdown`, `SqlServer.DeleteBreakdown`,
`SqlServer.UpdateBreakdown`, `SqlServer.ProcedureBreakdown`
- `Snowflake.InsertBreakdown`, `Snowflake.DeleteBreakdown`,
`Snowflake.UpdateBreakdown`, `Snowflake.ProcedureBreakdown`
The Microsoft-SQL fallback path in the Snowflake breakdowns (which
delegates to the SqlServer breakdown's TryParse before the prelude
even runs) is preserved unchanged.
`ParsePreparation` is `public` because it sits in the SqlServer
assembly and is consumed cross-assembly by Snowflake/PostgreSql.
This is a new public type but it's deliberately a thin scaffold —
external consumers should still be calling the breakdown
classes' own `TryParse` methods.
All 1180 tests stay green.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`TruncateText` was copy-pasted verbatim in three Markdown generators
(`SqlServer.QueryBreakdownGenerator`, `SqlServer.SqlStatementGenerator`,
`Expressions.ExpressionGenerator`). Pulled out to a new
`Strata.SqlTools.Markdown.Internal.MarkdownTextHelpers` static class
(internal — no public-API change).
Each call site keeps its own one-line private wrapper for source
readability so existing `TruncateText(...)` calls in the generators
need no edits.
Deliberately *not* unified across the same three files:
- `EscapeMermaidText` (QueryBreakdownGenerator) vs `EscapeMermaidText`
(SqlStatementGenerator) — the QBG version intentionally escapes
`[]{}()` for Mermaid node syntax; the SSG version only escapes
quotes/newlines because it writes into `Note right of DB: ...`
contexts where brackets render fine.
- `EscapeMarkdown` (ExpressionGenerator) — a different escape set
again, targeting Markdown rather than Mermaid.
Also deliberately *not* refactored: `SqlServer.UpdateBreakdown.TryParse`
≡ `SqlServer.ProcedureBreakdown.TryParse` prelude (empty-check +
parser + prefix regex + extract setup/finish clauses). The 30-line
duplication is real, but every extraction shape (tuple return,
`out`-flavored helper, context type) is measurably worse than the
duplicated original. Leaving it.
All 1180 tests stay green.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Tackles the in-file copy-paste duplications SonarQube flagged on
`sql-utilities`, narrowing the dedup target to the cases where the
extraction is a clear readability win.
- `LinqToSql.Converters.ReverseConverterExtensions`: the three
`ToLinqQueryBreakdown` overloads (SqlServer / PostgreSql / Snowflake)
had identical 26-line bodies. Routes all three through a single
`BuildLinqBreakdownFrom(QueryBreakdown)` private helper — works
because Snowflake/PostgreSql `QueryBreakdown` derive from the
SqlServer one, so the parameter type accepts all three. Public API
preserved.
- `Markdown.Expressions.ExpressionGenerator`: `VisitInExpression` and
`VisitNotInExpression` had identical 18-line bodies differing only in
the "IN"/"NOT IN" label. Both now delegate to a new private
`RenderInList(label, searchExpression, values)`.
- `PostgreSql.Statements.StatementExpressionParser`: the qualified-
column-name building loop and the column-id switch were duplicated
across `HandleStringToken` (qualified-column branch) and
`GrabColumnExpression`. Extracted to a shared
`BuildQualifiedColumnExpression(seededBuilder, reader)` private helper.
Deliberately *not* refactored: `PostgreSql.Statements.StatementReader`'s
`<` / `>` operator handlers, which Sonar also flags as duplicate. The
shared pattern there is a structural sequence of "MovePosition;
character check; emit Token; return" repeated across single-/two-char
operator variants; folding it into a helper would replace four short,
self-explanatory inline checks with `TryMatchTwoCharOperator('=', ...)`
indirection that obscures what each branch actually emits. The dedup
isn't worth the readability tax.
All 1180 tests stay green.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The Gitea Actions sonarqube workflow was running `dotnet build` and
sonarscanner begin/end without ever invoking `dotnet test`, so Sonar
was reporting coverage = 0% on the server despite a 1180-test suite
already exercising the code locally. `scan-sonar.ps1` (the local
equivalent) has had the coverage flags + test step the whole time —
this aligns CI with it.
Changes:
- `sonarscanner begin` now passes
`/d:sonar.cs.opencover.reportsPaths="tests/**/TestResults/**/coverage.opencover.xml"`
so the scanner picks up the per-project OpenCover reports coverlet
emits, and
`/d:sonar.coverage.exclusions="tests/**,**/*.Tests/**"` so the tests
themselves don't count toward the coverage denominator.
- The dormant step 8 (a disabled `if: false` curl-debug block from an
earlier auth-troubleshooting session) is replaced with the actual
test run: `dotnet test ... --no-build --settings
coverlet.runsettings --collect "XPlat Code Coverage"`.
- Step 9 (`sonarscanner end`) unchanged — it now has coverage data
available to upload.
Once this lands, the next scan will surface the *real* coverage number
for sql-utilities. Until we see that number we don't know which files
actually need test investment vs. were already covered by the existing
suite.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- **NUnit2046 (15)**: extend the prior regex sweep to also catch the
`Is.GreaterThan(n)` / `Is.GreaterThanOrEqualTo(n)` variants on
`.Count` and `.Length` — the previous pass only handled `Is.EqualTo`.
Affects PG/Snowflake `QueryBreakdownTests`, PG `StatementReaderTests`,
`LinqToSql` `QueryComparatorTests` / `QueryValidatorTests`,
`Markdown.Tests` `ExpressionGeneratorTests` /
`QueryBreakdownGeneratorTests`, and `JsonTokenReaderTests`.
- **CS8604 (8)**: `merged[someKey]` access inside `Assert.Multiple(() =>
{ ... })` lambdas where `someKey` is `string?` from `FirstOrDefault`.
The preceding `Assert.That(someKey, Is.Not.Null)` does not propagate
null-narrowing into the lambda scope, so add `!` to the dictionary
index. (Tests fail loud with a meaningful message if the key really
is null, so this is safe.)
- **NUnit2045 (1)**: wrap a four-assert block in
`JsonTokenReaderTests.cs:65-68` in `Assert.Multiple`. Mirrors the
surrounding two `Assert.Multiple` groups in that method.
All 1180 tests stay green.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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>
- `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>
- `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>
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>
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>
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>
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>
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>
Re-runs `dotnet format analyzers --diagnostics NUnit2046 NUnit2045` after
the Tier 2 Assert.Multiple wrap, which exposed:
- `Has.Length.EqualTo(n)` rewrites for `string[]`/array `.Length` checks
(the first pass only knew about `.Count`).
- `Is.Not.Empty` rewrites for `Count, Is.GreaterThan(0)`.
- A handful of new NUnit2045 groups that became wrappable once the
initial Multiple blocks settled the surrounding indentation.
Tests still 1180/1180 passing.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Promote the `new JsonSerializerOptions { Converters = { ... } }` instance
that `OneTimeSetup` was constructing on each fixture run to a
`private static readonly JsonSerializerOptions _jsonOptions` field, so
the converter list isn't rebuilt per fixture. Strictly cosmetic here
(OneTimeSetup runs once) but it's the change the analyzer wants and the
field is the more idiomatic JsonSerializer pattern.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two inline `new[] { ... }` literals inside TestCaseSource yield-returns
get hoisted to `static readonly string[]` fields with descriptive names
(`monthListValues`, `calendarRange`) matching the surrounding test-case
identifiers. Avoids reconstructing the same array on each call.
Only the test-file CA1861 sites are touched; the three production-code
sites flagged for the same rule are out of scope for this branch and
will land separately.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Driven by `dotnet format analyzers --diagnostics NUnit2045`. The fixer
groups consecutive independent `Assert.That(...)` calls into
`Assert.Multiple(() => { ... })`, so a failing assertion no longer
short-circuits the block — every failure inside the group is reported,
which gives much better diagnostics on multi-property tests.
Audit confirmed no Assert.Throws / Assert.Fail / Assert.Catch / Assert.Pass
/ Assert.DoesNotThrow got pulled inside a Multiple block (those need to
short-circuit). All 1180 tests pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- NUnit2046: `Assert.That(x.Count, Is.EqualTo(n))` → `Assert.That(x, Has.Count.EqualTo(n))` (or `Is.Empty` when n==0)
- NUnit2011: `Assert.That(s.Contains(x))` → `Assert.That(s, Does.Contain(x))` for richer failure messages
- CA1866: `.StartsWith("$"|"@"|":")` → `.StartsWith('$'|'@'|':')` char overload
Driven by `dotnet format analyzers --diagnostics NUnit2046 NUnit2011 CA1866`
for the cases the Roslyn fixer handles, plus a regex sweep for the remaining
`Count == n` (n>0) cases which the fixer doesn't address. CA1866 had no
associated code fix and was edited by hand (3 sites in 2 files). All tests
green (1180 passing).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Manual sweep of all 42 IDE0028 sites flagged by SonarQube — `dotnet format
analyzers --diagnostics IDE0028` declined to fix these (no .editorconfig
opt-in for `dotnet_style_prefer_collection_expression`), so applied by
hand. The repo already targets `<LangVersion>latest</LangVersion>` on
net8.0, so C# 12 collection expressions are available.
Pattern: `new List<T>()` / `new Dictionary<K,V>()` / `new ArrayList()` /
`new()` -> `[]` for empty; `new List<T> { ... }` -> `[...]` for literal.
24 files touched in src/{EFCore, LinqToSql, Query, Snowflake, SqlBreakdown,
SqlServer}; tests untouched (no IDE0028 sites in test code).
Build clean (35 warnings unchanged from baseline, 0 errors). All tests
remain green.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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>
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>
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>
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>
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>
bermudalamb
Thom Lamb