Chore/sonarqube src dedup #20
Assignees
bermudalamb (Thom Lamb)
Clear assignees
bermudalamb (Thom Lamb)
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: Lambda-Associates/sql-utilities#20
Reference in New Issue
Block a user
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>`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>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>