Commit Graph
9 Commits
Author SHA1 Message Date
Thom LambandClaude Opus 4.7 089d4f6000 refactor(dedup): extract AppendWithClauseSection for the WITH/CTE block
`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>
2026-05-27 17:23:48 -05:00
Thom LambandClaude Opus 4.7 85cc79d5a1 refactor(dedup): share clause-with-comments ingestion (PG/Snowflake QueryBreakdown ctors)
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>
2026-05-27 17:06:51 -05:00
Thom LambandClaude Opus 4.7 f577c06558 refactor(dedup): cross-dialect QueryBreakdown + StatementParser helpers (Cluster E)
SonarQube Analysis / sonarqube (pull_request) Successful in 4m5s
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>
2026-05-27 16:39:36 -05:00
Thom LambandClaude Opus 4.7 1bd6deec83 chore(sonar): mechanical src/ cleanups — AsSpan, drop ContainsKey guard, simplify GetQuery (CA1846, CA1853, S2219)
- `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>
2026-05-27 15:05:19 -05:00
Thom LambandClaude Opus 4.7 2c23ba4a88 chore(sonar)!: return empty IQueryable instead of null from GetQuery<T> (S1168)
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>
2026-05-27 14:18:31 -05:00
Thom LambandClaude Opus 4.7 2c2a8b1193 chore(sonar): bulk-fix mechanical CA/IDE analyzer warnings
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>
2026-05-26 15:38:54 -05:00
Thom Lamb 2d9148547f fix(serialization): Remove legacy BinaryFormatter support
SonarQube Analysis / sonarqube (pull_request) Successful in 4m42s
The `[Serializable]` attribute and corresponding `[OnDeserialized]` methods have been removed from various breakdown classes. This eliminates reliance on `BinaryFormatter`, which is a deprecated and insecure serialization mechanism in modern .NET.

This change also resolves SonarQube rule S5766 warnings by removing the context in which they apply, leading to cleaner and more secure code.
2026-05-20 17:44:21 -05:00
Thom Lamb e3153e58c4 fix(security): Resolve SonarQube security hotspots
SonarQube Analysis / sonarqube (pull_request) Successful in 3m9s
Introduce a default regex match timeout across the library to prevent potential ReDoS attacks (SonarQube rule S6444).
Implement `[OnDeserialized]` methods to re-establish object invariants and validate state after deserialization, addressing SonarQube rule S5766.
2026-05-20 17:19:17 -05:00
Thom Lamb 5e467bcc9c chore: initial git load of code space 2026-05-12 08:52:33 -05:00