From 089d4f600008d986c17c2ca92600e43d90b0bed5 Mon Sep 17 00:00:00 2001 From: Thom Lamb Date: Wed, 27 May 2026 17:23:48 -0500 Subject: [PATCH 1/2] refactor(dedup): extract AppendWithClauseSection for the WITH/CTE block MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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) --- .../Breakdowns/QueryBreakdown.cs | 57 +------- .../Breakdowns/QueryBreakdown.cs | 125 +++++++++--------- 2 files changed, 66 insertions(+), 116 deletions(-) diff --git a/src/Strata.SqlTools.Snowflake/Breakdowns/QueryBreakdown.cs b/src/Strata.SqlTools.Snowflake/Breakdowns/QueryBreakdown.cs index 637a8f1..ae37519 100644 --- a/src/Strata.SqlTools.Snowflake/Breakdowns/QueryBreakdown.cs +++ b/src/Strata.SqlTools.Snowflake/Breakdowns/QueryBreakdown.cs @@ -451,62 +451,7 @@ public class QueryBreakdown : SqlServerQueryBreakdown } } - if (IsUsingWithClause) - { - // Check if any WITH clause is recursive - bool hasRecursive = WithClauses.Any(wc => wc.IsRecursive); - sb.Append("WITH"); - if (hasRecursive) - { - sb.Append(" RECURSIVE"); - } - sb.AppendLine(); - - for (int i = 0; i < WithClauses.Count; i++) - { - var withClause = WithClauses[i]; - - if (i > 0) - { - sb.Append(','); - sb.AppendLine(); - } - - // Include comment if present - if (!string.IsNullOrWhiteSpace(withClause.Comment)) - { - sb.AppendLine($" {withClause.Comment}"); - } - - // Write CTE name with optional column list - var cteName = withClause.TableName; - if (withClause.ColumnList != null && withClause.ColumnList.Count > 0) - { - var columnList = string.Join(", ", withClause.ColumnList); - cteName = $"{withClause.TableName} ({columnList})"; - } - - sb.AppendLine($" {cteName} AS ("); - - if (withClause.IsRecursive && withClause.RecursiveQuery != null) - { - // For recursive CTEs: anchor query UNION ALL recursive query - var anchorSql = withClause.Query?.GetSql(includeSetupFinish: false).Trim() ?? string.Empty; - var recursiveSql = withClause.RecursiveQuery.GetSql(includeSetupFinish: false).Trim() ?? string.Empty; - sb.AppendLine($" {anchorSql}"); - sb.AppendLine(" UNION ALL"); - sb.AppendLine($" {recursiveSql}"); - } - else - { - // For non-recursive CTEs: just the single query - var withSql = withClause.Query?.GetSql(includeSetupFinish: false).Trim() ?? string.Empty; - sb.AppendLine($" {withSql}"); - } - sb.Append(" )"); - } - sb.AppendLine(); - } + AppendWithClauseSection(sb, withClauseIndent: " ", queryBodyIndent: " "); // Snowflake SELECT syntax sb.Append(StatementParser.KeywordSelect); diff --git a/src/Strata.SqlTools.SqlServer/Breakdowns/QueryBreakdown.cs b/src/Strata.SqlTools.SqlServer/Breakdowns/QueryBreakdown.cs index 9d8187c..2382e06 100644 --- a/src/Strata.SqlTools.SqlServer/Breakdowns/QueryBreakdown.cs +++ b/src/Strata.SqlTools.SqlServer/Breakdowns/QueryBreakdown.cs @@ -547,74 +547,79 @@ public class QueryBreakdown : SqlBreakdownBase, IQueryBreakdown return mergedParams; } -#pragma warning disable S3776 // Cognitive Complexity of methods should not be too high + /// + /// Renders the WITH [RECURSIVE] CTE section into if any + /// are present. Shared by dialects (SqlServer uses 5-space + /// outer / 10-space inner indents; Snowflake passes 4-/8-space). + /// + /// The builder accumulating the SQL. + /// Indent used for the CTE name line and the closing ). + /// Indent used for each anchor/recursive query body line and the UNION ALL. + protected virtual void AppendWithClauseSection(StringBuilder sb, string withClauseIndent, string queryBodyIndent) + { + if (!IsUsingWithClause) + { + return; + } + + bool hasRecursive = _withClauses.Any(wc => wc.IsRecursive); + sb.Append("WITH"); + if (hasRecursive) + { + sb.Append(" RECURSIVE"); + } + sb.AppendLine(); + + for (int i = 0; i < _withClauses.Count; i++) + { + var withClause = _withClauses[i]; + var anchorSql = withClause.Query?.GetSql(includeSetupFinish: false).Trim() ?? string.Empty; + + if (i > 0) + { + sb.Append(','); + sb.AppendLine(); + } + + if (!string.IsNullOrWhiteSpace(withClause.Comment)) + { + sb.AppendLine($"{withClauseIndent}{withClause.Comment}"); + } + + var cteName = withClause.TableName; + if (withClause.ColumnList != null && withClause.ColumnList.Count > 0) + { + var columnList = string.Join(", ", withClause.ColumnList); + cteName = $"{withClause.TableName} ({columnList})"; + } + sb.AppendLine($"{withClauseIndent}{cteName} AS ("); + + if (withClause.IsRecursive && withClause.RecursiveQuery != null) + { + var recursiveSql = withClause.RecursiveQuery.GetSql(includeSetupFinish: false).Trim(); + sb.AppendLine($"{queryBodyIndent}{anchorSql}"); + sb.AppendLine($"{queryBodyIndent}UNION ALL"); + sb.AppendLine($"{queryBodyIndent}{recursiveSql}"); + } + else + { + sb.AppendLine($"{queryBodyIndent}{anchorSql}"); + } + + sb.Append($"{withClauseIndent})"); + } + sb.AppendLine(); + } + /// /// Gets the SQL breakdown as a string (query-specific implementation). /// /// The SELECT SQL statement. protected override string GetSqlBreakdown() -#pragma warning restore S3776 { var sb = new StringBuilder(); - if (IsUsingWithClause) - { - // Check if any CTE is recursive - if so, add RECURSIVE keyword - bool hasRecursive = _withClauses.Any(wc => wc.IsRecursive); - sb.Append("WITH "); - if (hasRecursive) - { - sb.AppendLine("RECURSIVE"); - } - else - { - sb.AppendLine(); - } - - for (int i = 0; i < _withClauses.Count; i++) - { - var withClause = _withClauses[i]; - var anchorSql = withClause.Query?.GetSql(includeSetupFinish: false).Trim() ?? string.Empty; - - if (i > 0) - { - sb.Append(','); - sb.AppendLine(); - } - - // Include comment if present - if (!string.IsNullOrWhiteSpace(withClause.Comment)) - { - sb.AppendLine($" {withClause.Comment}"); - } - - // Write CTE name with optional column list - var cteName = withClause.TableName; - if (withClause.ColumnList != null && withClause.ColumnList.Count > 0) - { - var columnList = string.Join(", ", withClause.ColumnList); - cteName = $"{withClause.TableName} ({columnList})"; - } - sb.AppendLine($" {cteName} AS ("); - - if (withClause.IsRecursive && withClause.RecursiveQuery != null) - { - // For recursive CTEs: anchor query UNION ALL recursive query - var recursiveSql = withClause.RecursiveQuery.GetSql(includeSetupFinish: false).Trim(); - sb.AppendLine($" {anchorSql}"); - sb.AppendLine(" UNION ALL"); - sb.AppendLine($" {recursiveSql}"); - } - else - { - // For non-recursive CTEs: just the single query - sb.AppendLine($" {anchorSql}"); - } - - sb.Append(" )"); - } - sb.AppendLine(); - } + AppendWithClauseSection(sb, withClauseIndent: " ", queryBodyIndent: " "); sb.AppendLine("SELECT "); if (!string.IsNullOrEmpty(SelectClause.Comment)) -- 2.54.0 From 4038b3dab532d022f180e3822068ba6fcaaf9a42 Mon Sep 17 00:00:00 2001 From: Thom Lamb Date: Wed, 27 May 2026 17:28:16 -0500 Subject: [PATCH 2/2] refactor(dedup): adapter-based Markdown CollectionGenerator dedup (the big one) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- .../Common/CollectionMarkdownGenerator.cs | 37 ++++++++++++ .../Common/ICollectionMarkdownData.cs | 30 ++++++++++ .../QueryBreakdownCollectionGenerator.cs | 60 +++++++------------ .../QueryBreakdownCollectionGenerator.cs | 60 +++++++------------ 4 files changed, 111 insertions(+), 76 deletions(-) create mode 100644 src/Strata.SqlTools.Markdown/Common/CollectionMarkdownGenerator.cs create mode 100644 src/Strata.SqlTools.Markdown/Common/ICollectionMarkdownData.cs diff --git a/src/Strata.SqlTools.Markdown/Common/CollectionMarkdownGenerator.cs b/src/Strata.SqlTools.Markdown/Common/CollectionMarkdownGenerator.cs new file mode 100644 index 0000000..977042b --- /dev/null +++ b/src/Strata.SqlTools.Markdown/Common/CollectionMarkdownGenerator.cs @@ -0,0 +1,37 @@ +namespace Strata.SqlTools.Markdown.Common; + +/// +/// Shared method bodies for the per-dialect QueryBreakdownCollectionGenerator +/// classes. Each dialect wraps its QueryBreakdownCollection in an +/// adapter and supplies a dialect-specific +/// ; this template handles the rest. +/// +internal static class CollectionMarkdownGenerator +{ + public static string GenerateCollectionReport(ICollectionMarkdownData data, MarkdownDialectFormat format, string? title) + => CollectionReportWriter.CollectionReport(title, new[] + { + GenerateCollectionSummary(data), + GenerateParameterAnalysis(data, format), + GenerateQueryCompositionReport(data, format) + }); + + public static string GenerateCollectionSummary(ICollectionMarkdownData data) + => CollectionReportWriter.CollectionSummary( + data.QueryCount, + data.UniqueParameterCount, + data.TotalSelectedColumns, + data.UniqueTableCount); + + public static string GenerateParameterAnalysis(ICollectionMarkdownData data, MarkdownDialectFormat format) + => CollectionReportWriter.ParameterAnalysis(data.ParameterRows.ToList(), data.QueriesForReport, format); + + public static string GenerateParameterDependencyDiagram(ICollectionMarkdownData data, MarkdownDialectFormat format) + => CollectionReportWriter.ParameterDependencyDiagram(data.QueriesForReport, format); + + public static string GenerateQueryCompositionReport(ICollectionMarkdownData data, MarkdownDialectFormat format) + => CollectionReportWriter.QueryCompositionReport(data.QueriesForReport, format); + + public static string GenerateBatchFlowDiagram(ICollectionMarkdownData data, string? openLabel, string? closeLabel) + => CollectionReportWriter.BatchFlowDiagram(data.QueryCount, openLabel, closeLabel); +} diff --git a/src/Strata.SqlTools.Markdown/Common/ICollectionMarkdownData.cs b/src/Strata.SqlTools.Markdown/Common/ICollectionMarkdownData.cs new file mode 100644 index 0000000..0dd70d1 --- /dev/null +++ b/src/Strata.SqlTools.Markdown/Common/ICollectionMarkdownData.cs @@ -0,0 +1,30 @@ +using SqlServerBreakdowns = Strata.SqlTools.Breakdowns.SqlServer; + +namespace Strata.SqlTools.Markdown.Common; + +/// +/// Dialect-neutral view of a QueryBreakdownCollection as seen by the Markdown +/// generators. Each per-dialect QueryBreakdownCollectionGenerator wraps its +/// dialect-specific collection in an implementation of this interface so the shared +/// template can operate uniformly. +/// +internal interface ICollectionMarkdownData +{ + int QueryCount { get; } + int UniqueParameterCount { get; } + int TotalSelectedColumns { get; } + int UniqueTableCount { get; } + + /// + /// Queries as the base type. Used + /// directly by + /// (which needs ) and via covariance by the other + /// writer methods that accept . + /// + IReadOnlyList QueriesForReport { get; } + + /// + /// Parameter usage already mapped into the writer's dialect-agnostic row type. + /// + IEnumerable ParameterRows { get; } +} diff --git a/src/Strata.SqlTools.Markdown/PostgreSql/QueryBreakdownCollectionGenerator.cs b/src/Strata.SqlTools.Markdown/PostgreSql/QueryBreakdownCollectionGenerator.cs index 4fb82fd..1a28895 100644 --- a/src/Strata.SqlTools.Markdown/PostgreSql/QueryBreakdownCollectionGenerator.cs +++ b/src/Strata.SqlTools.Markdown/PostgreSql/QueryBreakdownCollectionGenerator.cs @@ -23,79 +23,63 @@ public static class QueryBreakdownCollectionGenerator /// /// Generates a comprehensive collection report in Markdown format with PostgreSQL-specific information. /// - /// The QueryBreakdownCollection to document. - /// Optional title for the report. - /// A string containing the Markdown documentation. public static string GenerateCollectionReport(QueryBreakdownCollection collection, string? title = null) - => CollectionReportWriter.CollectionReport(title, new[] - { - GenerateCollectionSummary(collection), - GenerateParameterAnalysis(collection), - GenerateQueryCompositionReport(collection) - }); + => CollectionMarkdownGenerator.GenerateCollectionReport(Adapt(collection), Format, title); /// /// Generates a summary section for the collection. /// - /// The QueryBreakdownCollection to summarize. - /// Markdown summary section. public static string GenerateCollectionSummary(QueryBreakdownCollection collection) - => CollectionReportWriter.CollectionSummary( - collection.QueryBreakdowns.Count, - collection.GetAllUniqueParameters().Count(), - collection.GetTotalSelectedColumns(), - collection.GetUniqueTableReferences().Count()); + => CollectionMarkdownGenerator.GenerateCollectionSummary(Adapt(collection)); /// /// Generates a parameter analysis report with PostgreSQL parameter syntax support. /// - /// The QueryBreakdownCollection to analyze. - /// Markdown parameter analysis section. public static string GenerateParameterAnalysis(QueryBreakdownCollection collection) - => CollectionReportWriter.ParameterAnalysis(MapParameters(collection), collection.QueryBreakdowns, Format); + => CollectionMarkdownGenerator.GenerateParameterAnalysis(Adapt(collection), Format); /// /// Generates a Mermaid diagram showing parameter dependencies across queries. /// - /// The QueryBreakdownCollection to visualize. - /// Mermaid diagram markdown. public static string GenerateParameterDependencyDiagram(QueryBreakdownCollection collection) - => CollectionReportWriter.ParameterDependencyDiagram(collection.QueryBreakdowns, Format); + => CollectionMarkdownGenerator.GenerateParameterDependencyDiagram(Adapt(collection), Format); /// /// Generates a detailed query composition report. /// - /// The QueryBreakdownCollection to report on. - /// Markdown composition report section. public static string GenerateQueryCompositionReport(QueryBreakdownCollection collection) - => CollectionReportWriter.QueryCompositionReport(collection.QueryBreakdowns, Format); + => CollectionMarkdownGenerator.GenerateQueryCompositionReport(Adapt(collection), Format); /// /// Generates a batch execution flow diagram for PostgreSQL. /// - /// The QueryBreakdownCollection to visualize. - /// Whether to show transaction wrapping. - /// Mermaid diagram markdown. public static string GenerateBatchFlowDiagram(QueryBreakdownCollection collection, bool includeTransaction = false) - => CollectionReportWriter.BatchFlowDiagram( - collection.QueryBreakdowns.Count, + => CollectionMarkdownGenerator.GenerateBatchFlowDiagram( + Adapt(collection), includeTransaction ? "BEGIN" : null, includeTransaction ? "COMMIT" : null); - /// - /// Maps the collection's parameter usage report into the writer's dialect-agnostic rows. - /// - private static List MapParameters(QueryBreakdownCollection collection) - => collection.GetParameterUsageReport() - .Select(p => new ParameterUsageRow + private static ICollectionMarkdownData Adapt(QueryBreakdownCollection collection) => new Adapter(collection); + + private sealed class Adapter : ICollectionMarkdownData + { + private readonly QueryBreakdownCollection _c; + public Adapter(QueryBreakdownCollection c) { _c = c; } + public int QueryCount => _c.QueryBreakdowns.Count; + public int UniqueParameterCount => _c.GetAllUniqueParameters().Count(); + public int TotalSelectedColumns => _c.GetTotalSelectedColumns(); + public int UniqueTableCount => _c.GetUniqueTableReferences().Count(); + public IReadOnlyList QueriesForReport => _c.QueryBreakdowns; + public IEnumerable ParameterRows + => _c.GetParameterUsageReport().Select(p => new ParameterUsageRow { ParameterName = p.ParameterName, IsUsedInAllQueries = p.IsUsedInAllQueries, UsedInQueryCount = p.UsedInQueryCount, TotalQueries = p.TotalQueries, Value = p.Value - }) - .ToList(); + }); + } /// /// Formats a parameter using PostgreSQL syntax: $n for positional, :name for named. diff --git a/src/Strata.SqlTools.Markdown/SqlServer/QueryBreakdownCollectionGenerator.cs b/src/Strata.SqlTools.Markdown/SqlServer/QueryBreakdownCollectionGenerator.cs index 723907d..07ba541 100644 --- a/src/Strata.SqlTools.Markdown/SqlServer/QueryBreakdownCollectionGenerator.cs +++ b/src/Strata.SqlTools.Markdown/SqlServer/QueryBreakdownCollectionGenerator.cs @@ -22,79 +22,63 @@ public static class QueryBreakdownCollectionGenerator /// /// Generates a comprehensive collection report in Markdown format. /// - /// The QueryBreakdownCollection to document. - /// Optional title for the report. - /// A string containing the Markdown documentation. public static string GenerateCollectionReport(QueryBreakdownCollection collection, string? title = null) - => CollectionReportWriter.CollectionReport(title, new[] - { - GenerateCollectionSummary(collection), - GenerateParameterAnalysis(collection), - GenerateQueryCompositionReport(collection) - }); + => CollectionMarkdownGenerator.GenerateCollectionReport(Adapt(collection), Format, title); /// /// Generates a summary section for the collection. /// - /// The QueryBreakdownCollection to summarize. - /// Markdown summary section. public static string GenerateCollectionSummary(QueryBreakdownCollection collection) - => CollectionReportWriter.CollectionSummary( - collection.QueryBreakdowns.Count, - collection.GetAllUniqueParameters().Count(), - collection.GetTotalSelectedColumns(), - collection.GetUniqueTableReferences().Count()); + => CollectionMarkdownGenerator.GenerateCollectionSummary(Adapt(collection)); /// /// Generates a parameter analysis report. /// - /// The QueryBreakdownCollection to analyze. - /// Markdown parameter analysis section. public static string GenerateParameterAnalysis(QueryBreakdownCollection collection) - => CollectionReportWriter.ParameterAnalysis(MapParameters(collection), collection.QueryBreakdowns, Format); + => CollectionMarkdownGenerator.GenerateParameterAnalysis(Adapt(collection), Format); /// /// Generates a Mermaid diagram showing parameter dependencies across queries. /// - /// The QueryBreakdownCollection to visualize. - /// Mermaid diagram markdown. public static string GenerateParameterDependencyDiagram(QueryBreakdownCollection collection) - => CollectionReportWriter.ParameterDependencyDiagram(collection.QueryBreakdowns, Format); + => CollectionMarkdownGenerator.GenerateParameterDependencyDiagram(Adapt(collection), Format); /// /// Generates a detailed query composition report. /// - /// The QueryBreakdownCollection to report on. - /// Markdown composition report section. public static string GenerateQueryCompositionReport(QueryBreakdownCollection collection) - => CollectionReportWriter.QueryCompositionReport(collection.QueryBreakdowns, Format); + => CollectionMarkdownGenerator.GenerateQueryCompositionReport(Adapt(collection), Format); /// /// Generates a batch execution flow diagram. /// - /// The QueryBreakdownCollection to visualize. - /// Whether to show transaction wrapping. - /// Mermaid diagram markdown. public static string GenerateBatchFlowDiagram(QueryBreakdownCollection collection, bool includeTransaction = false) - => CollectionReportWriter.BatchFlowDiagram( - collection.QueryBreakdowns.Count, + => CollectionMarkdownGenerator.GenerateBatchFlowDiagram( + Adapt(collection), includeTransaction ? "BEGIN TRANSACTION" : null, includeTransaction ? "COMMIT TRANSACTION" : null); - /// - /// Maps the collection's parameter usage report into the writer's dialect-agnostic rows. - /// - private static List MapParameters(QueryBreakdownCollection collection) - => collection.GetParameterUsageReport() - .Select(p => new ParameterUsageRow + private static ICollectionMarkdownData Adapt(QueryBreakdownCollection collection) => new Adapter(collection); + + private sealed class Adapter : ICollectionMarkdownData + { + private readonly QueryBreakdownCollection _c; + public Adapter(QueryBreakdownCollection c) { _c = c; } + public int QueryCount => _c.QueryBreakdowns.Count; + public int UniqueParameterCount => _c.GetAllUniqueParameters().Count(); + public int TotalSelectedColumns => _c.GetTotalSelectedColumns(); + public int UniqueTableCount => _c.GetUniqueTableReferences().Count(); + public IReadOnlyList QueriesForReport => _c.QueryBreakdowns; + public IEnumerable ParameterRows + => _c.GetParameterUsageReport().Select(p => new ParameterUsageRow { ParameterName = p.ParameterName, IsUsedInAllQueries = p.IsUsedInAllQueries, UsedInQueryCount = p.UsedInQueryCount, TotalQueries = p.TotalQueries, Value = p.Value - }) - .ToList(); + }); + } /// /// Gets the parameter type name from a parameter value. -- 2.54.0