Introduces new unit tests for Snowflake and SQL Server-specific SQL breakdown
classes (DELETE, INSERT, UPDATE, PROCEDURE) and various general SQL utility
functions. This significantly increases test coverage, ensuring robustness
and correctness across different SQL dialects and helper logic.
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>
Thom Lamb
bermudalamb