From 07993be681113bdc9981fee94f5a3b32f3a8d288 Mon Sep 17 00:00:00 2001 From: "@royashbrook" Date: Tue, 15 Sep 2026 22:29:56 -0400 Subject: [PATCH 1/2] Add tolerant formatting and correct block layout (refs #2) --- .github/workflows/test.yml | 1 + README.md | 16 ++- SKILL.md | 73 +++++++---- references/formatter-examples.md | 98 ++++++++++++++ references/formatter.md | 45 +++++-- references/style-explained.md | 4 +- references/worked-examples.md | 42 +++--- scripts/RoySql.cs | 211 +++++++++++++++++++++++++++++-- scripts/format-sql.ps1 | 7 +- tests/cli.ps1 | 16 ++- tests/examples.ps1 | 8 +- tests/feedback.ps1 | 94 ++++++++++++++ tests/formatter.ps1 | 91 +++++++++++-- 13 files changed, 611 insertions(+), 95 deletions(-) create mode 100644 tests/feedback.ps1 diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 15d353e..7569516 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -21,4 +21,5 @@ jobs: & ./scripts/install-scriptdom.ps1 $assembly = (Resolve-Path ./scripts/lib/Microsoft.SqlServer.TransactSql.ScriptDom.dll).Path & ./tests/formatter.ps1 -AssemblyPath $assembly + & ./tests/feedback.ps1 -AssemblyPath $assembly & ./tests/cli.ps1 -AssemblyPath $assembly diff --git a/README.md b/README.md index 9c9946f..daafb8a 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,6 @@ # roy-sql -My SQL house style, as an agent skill and a local T-SQL formatter. +My SQL house style, as an agent skill and a local SQL-like formatter with T-SQL awareness. Lowercase keywords, leading commas, alias-first projections, expanded joins and CTEs. Plain SQL, comments that explain why, no query redesign disguised as formatting. @@ -24,6 +24,7 @@ From the repository directory: ```powershell pwsh -NoProfile -File scripts/install-scriptdom.ps1 pwsh -NoProfile -File scripts/format-sql.ps1 -Path query.sql +pwsh -NoProfile -File scripts/format-sql.ps1 -Path query.sql -Strict pwsh -NoProfile -File scripts/format-sql.ps1 -Path query.sql -Check ``` @@ -31,12 +32,15 @@ The first command downloads a hash-pinned Microsoft ScriptDOM dependency. The fo then runs locally, with no model, database, network call or input-file edit. Its implementation is visible PowerShell and C# source; no separate .NET SDK is needed. -SQL goes to stdout. Diagnostics go to stderr. Exit 0 means success, 1 means check-mode -drift, 2 means refusal or setup failure. Save stdout to a different file, never the input path. +SQL goes to stdout. Diagnostics go to stderr. Omit `-Path` to pipe SQL on stdin. +Exit 0 means output produced, 1 means check-mode drift, 2 means strict refusal or an +operational failure. Save stdout to a different file, never the input path. -The mechanical helper covers **T-SQL SELECTs**, not all of the skill's authoring guidance. -Unsupported syntax, including USE, DML, DDL and UNION, refuses with no partial SQL. -Preservation checks are not proof that a query returns the same results on a database. +Recognized **T-SQL SELECTs** get structural formatting. Unfamiliar syntax, DDL and incomplete +SQL get conservative token layout with a warning, or unchanged input if tokenization is unsafe. +`-Strict` retains the original SELECT-focused refusal behavior and emits no partial SQL. +Neither mode executes SQL or proves database validity. There is no dialect translation: +LIMIT stays LIMIT. Missing files/dependencies and write failures still report errors. See [coverage, checks and limitations](references/formatter.md). ## License diff --git a/SKILL.md b/SKILL.md index 5cf6831..8b521c3 100644 --- a/SKILL.md +++ b/SKILL.md @@ -3,7 +3,7 @@ name: roy-sql description: "Write or reformat SQL in the roy-sql house style: lowercase keywords, leading commas, alias-first projections, expanded joins and CTEs, and comments that explain why. Use for roy-sql, match my SQL style, or format this query in this profile. Includes an optional local T-SQL formatter. Preserve existing names and values when reformatting; this is not a database execution or query optimization tool." license: MIT metadata: - version: 2.27.0 + version: 2.28.0 publishable: true --- @@ -15,9 +15,9 @@ When they conflict, the principle wins. Read principles before mechanics. New to the terms? [roy-sql, in plain language](references/style-explained.md) explains each piece with a small example, and separates formatting from query-design choices. -For deterministic expanded T-SQL layout, the [formatter guide](references/formatter.md) -documents the local helper, its admitted syntax and safe refusals. -The compact authoring latitude below is not part of its v1 output. +For expanded T-SQL layout with best-effort handling of unfamiliar SQL-like text, the +[formatter guide](references/formatter.md) documents the local helper and optional strict mode. +The compact authoring latitude below is not part of its automatic output. ## Two modes: authoring vs reformatting @@ -212,7 +212,26 @@ declare @orderid int = try_cast( Default to `left(name, 2)`: no space before `(` or just inside either parenthesis. Hand-formatted SQL may use `left( name, 2 )`, or tier long nested calls across lines with their closing parentheses on separate lines. This is readability latitude, not a fixed-width -rule. The mechanical v1 formatter uses the tight default; automatic nested-call wrapping is deferred. +rule. The mechanical formatter uses the tight default; automatic nested-call wrapping is deferred. + +## CASE blocks and TOP + +Treat `case` / `end` as a block: put each `when` and `else` one indent unit inside, +and close with `end` at the expression's base indentation. Nested CASEs nest another unit. +Very short hand-written CASEs may stay inline. No universal length cutoff is established; +the mechanical formatter expands CASEs rather than guessing that exception. + +```sql + [tier] = case + when amount > 100 then 'high' + when amount > 50 then 'medium' + else 'low' + end +``` + +Use `select top 50`, not `select top (50)`, for an integer row count. Parentheses around +variable/calculated TOP expressions are retained by the formatter. Do not translate another +dialect's LIMIT to TOP: that is not formatting. ## Column lists @@ -254,19 +273,20 @@ Apply these to tables, subqueries/apply blocks and CTEs: ## FROM and joins -`from` stands alone. The first table is one unit in, join targets another unit in, -and each ON condition another unit in. `on` ENDS the join line. Put the new table on +`from` stands alone. The first table and every JOIN/APPLY are one unit in, aligned with +each other. ON conditions are one further unit in. Write `join`, not `inner join`; +LEFT/RIGHT/FULL/CROSS qualifiers retain their meaning. `on` ENDS the join line. Put the new table on the LEFT of `=` and established/parent table on the RIGHT (`od.orderid = o.orderid`). Order joins down the domain hierarchy: orders → details → products → categories. ```sql from orders o - join [order details] od on - od.orderid = o.orderid - join products p on - p.productid = od.productid - and p.discontinued = 0 + join [order details] od on + od.orderid = o.orderid + join products p on + p.productid = od.productid + and p.discontinued = 0 where o.orderid = @orderid and o.shippeddate is not null @@ -309,29 +329,28 @@ collapse expanded SQL just because it fits. When uncertain, expand. where shippeddate is not null -) - --- orders containing at least one beverages line -, withbev as ( +), withbev as ( + -- orders containing at least one beverages line select gd.orderid from gd - join [order details] od on - od.orderid = gd.orderid - join products p on - p.productid = od.productid - join categories c on - c.categoryid = p.categoryid - and c.categoryname = 'beverages' + join [order details] od on + od.orderid = gd.orderid + join products p on + p.productid = od.productid + join categories c on + c.categoryid = p.categoryid + and c.categoryname = 'beverages' ) ``` Use a new-line `;with` to defend against a prior batch without `go`. Keep one blank -line after each opening `(` and before its closing `)`. Subsequent CTEs have a leading -comma and a short `--` comment above naming what they find/exclude. +line after each opening `(` and before its closing `)`. Chain the next CTE on the closing +line: `), next as (`. No blank line between CTEs. A short purpose comment can go inside +the next CTE's body; preserve existing comments when reformatting. CTE names are pure lowercase without underscores by default, snake_case when ambiguity or meaningful prefixes warrant it (`ord_rev`, `ord_pay`), PascalCase last resort. The scoping CTE is conventionally `gd` (good/got data). @@ -390,8 +409,8 @@ Projection brackets and alias rules apply inside subqueries too. ## Indentation One **tab-over unit per construct**, not a single indent per file. Clause keywords sit -at their block level, contents one unit in, joins one further unit from FROM's contents, -ON conditions one further unit, and CTE bodies one unit from their opening line. +at their block level, contents one unit in, joins aligned with FROM's contents, +ON conditions one further unit in, and CTE bodies one unit from their opening line. Use the same width throughout a query. Mechanical output is **4 spaces, no tab bytes**. I type tabs converted by my editor, usually to 4 spaces; where tabs can't be typed, 2 spaces is the minimum visible unit. The rule wins over a conflicting example. diff --git a/references/formatter-examples.md b/references/formatter-examples.md index 6faadd7..39cc3cb 100644 --- a/references/formatter-examples.md +++ b/references/formatter-examples.md @@ -85,3 +85,101 @@ select from [order details] ``` + +## CASE is a block + +Before: + +```sql +SELECT CASE WHEN amount > 100 THEN 'High' WHEN amount > 50 THEN 'Medium' ELSE 'Low' END AS tier FROM totals +``` + +After: + +```sql +select + [tier] = case + when amount > 100 then 'High' + when amount > 50 then 'Medium' + else 'Low' + end +from + totals +``` + +## JOIN alignment and literal TOP + +Before: + +```sql +SELECT TOP (50) o.id FROM orders o INNER JOIN items i ON i.orderid=o.id LEFT JOIN products p ON p.id=i.productid +``` + +After: + +```sql +select top 50 + o.id +from + orders o + join items i on + i.orderid = o.id + left join products p on + p.id = i.productid +``` + +## Adjacent CTEs + +Before: + +```sql +WITH a AS (SELECT id FROM orders), b AS (SELECT id FROM a) SELECT id FROM b +``` + +After: + +```sql +;with a as ( + + select + id + from + orders + +), b as ( + + select + id + from + a + +) + +select + id +from + b +``` + +## LIMIT is retained, not translated + +Before: + +```sql +SELECT id FROM orders ORDER BY id LIMIT 50; +``` + +After: + +```sql +select + id +from + orders +order by + id +limit + 50; +``` + +Best-effort output. A diagnostic is written to stderr, not into the SQL. diff --git a/references/formatter.md b/references/formatter.md index 2988266..8443766 100644 --- a/references/formatter.md +++ b/references/formatter.md @@ -1,8 +1,8 @@ # Mechanical formatter -The helper formats existing **T-SQL SELECTs** into the expanded roy-sql layout. It makes no -model call, connects to no database and never writes the input file. It is not a -whole-language formatter. +The helper gives recognized **T-SQL SELECTs** expanded roy-sql layout, and makes a +best-effort layout pass over other SQL-like text. It makes no model call, connects to no +database and never writes the input file. It is not a whole-language parser or translator. ## Run @@ -12,6 +12,7 @@ Install the dependency once (an explicit download from Microsoft's NuGet package ```powershell pwsh -NoProfile -File scripts/install-scriptdom.ps1 pwsh -NoProfile -File scripts/format-sql.ps1 -Path query.sql +pwsh -NoProfile -File scripts/format-sql.ps1 -Path query.sql -Strict ``` Or supply an existing pinned assembly explicitly: @@ -23,6 +24,10 @@ pwsh -NoProfile -File scripts/format-sql.ps1 -AssemblyPath /path/to/ScriptDom.dl Omit `-Path` to read stdin. SQL goes to stdout, diagnostics to stderr. Exit **0** is successful formatting or an already-formatted check, **1** is check-mode drift, **2** is refusal/setup failure. +Default mode produces output even when the syntax cannot be parsed: a token-layout fallback +reports its reason on stderr. If tokenization cannot retain the text, input is returned +unchanged with a warning. Missing/unreadable files, missing or mismatched dependencies, +compilation and output errors remain failures. `-Strict` opts into the original refusals. Check mode emits no replacement SQL. Save output to a **different** path if wanted; shell redirection to the input path can truncate it before this script runs. @@ -37,20 +42,41 @@ tested by CI; check its actual result before calling a particular build cross-pl ## What it does - Standalone clauses, leading projection commas, local alias/declaration alignment and - four-space construct-relative indentation. CTE interior blank lines, expanded nested queries. + four-space construct-relative indentation. JOIN/APPLY align with the primary table, + with conditions one level deeper. CTE interior blank lines, adjacent `), next as (` chains. +- Expanded CASE blocks. Short-case compaction remains an authoring choice, not an invented + character cutoff. Plain `join` replaces `inner join`; integer `top (50)` becomes `top 50`. + Parenthesized variable/calculated TOP expressions remain unchanged. - Converts a named projection to `[Alias] = expression`, retaining the alias's exact value/case. Adds optional `as` in variable declarations. These are syntax forms, not query-design edits. - Lowercases lexer keywords, native types and recognized unqualified built-ins. Preserves identifiers, variables, literals, date masks, comments and schema-qualified function names. + Common unqualified DATE_TRUNC/ARRAY_AGG are recognized for casing, not execution validity. + In default mode, bare TRUE/FALSE comparison operands are treated as SQL-like boolean words + and lowercased. `-Strict` preserves them as T-SQL identifiers. Quoted/qualified names stay intact. - Preserves LF/CRLF choice. The canonical output ends with a newline. - Reparses output, compares a canonical ScriptDOM-generated representation, and checks exact comment content/order. The generator is used for comparison, not for printing source. -No `isnull` replacement, comparison/BETWEEN rewrite, guard insertion, name change, predicate movement, join reordering or +No `isnull` replacement, comparison/BETWEEN rewrite, guard insertion, predicate movement, join reordering or compact-layout heuristic. Canonical comparison is a regression guard, **not proof of database execution equivalence**. No SQL executes during formatting or the test suite. -## Coverage and refusals +## Best effort versus strict + +Default mode first tries structural formatting. When that is unavailable, its fallback +organizes recognizable clause boundaries, projection/definition lists and parenthesized +query blocks. Unknown words and operators retain their order. Inline spacing is conservative +because a T-SQL token boundary may split another dialect's operator. Comments and quoted +text are preserved; unclosed literals can cause the entire input to remain unchanged. +This is not full support for PostgreSQL, MySQL or arbitrary pseudocode, and layout may be +less polished than the T-SQL path. Warnings are deliberately outside the SQL output. + +`-Strict` requires the admitted structural slice below and retains canonical-representation +and comment checks. It is a grammar/coverage check, not a schema, type or execution validator. +For example, a function name can parse without identifying a real SQL Server function. + +## Strict coverage and refusals The admitted slice is SELECT, normal named/function/derived table sources, joins/APPLY, CTEs, TOP, WHERE/GROUP BY/HAVING/ORDER BY, CASE/window expressions, scalar/EXISTS/IN subqueries, @@ -64,14 +90,15 @@ separator. Comments before a leading comma remain supported. String-valued proje also refuse. Existing table/join hints and ordinary OVER expressions are preserved, not inserted. Other unsupported parser shapes may refuse. SQLCMD commands/repeat counts are outside this slice. -A refusal emits **no partial SQL**. Unsupported valid SQL is a coverage gap, not a claim -that the original query is wrong. Read the diagnostic and format it with the skill's authoring -guidance, or add a reviewed fixture before expanding the helper's scope. +A strict refusal emits **no partial SQL**. Unsupported valid SQL is a coverage gap, not a claim +that the original query is wrong. Parser diagnostics include line, column and the parser's +message. Omit `-Strict` for best-effort output instead. ## Test ```powershell pwsh -NoProfile -File tests/formatter.ps1 -AssemblyPath /path/to/ScriptDom.dll +pwsh -NoProfile -File tests/feedback.ps1 -AssemblyPath /path/to/ScriptDom.dll pwsh -NoProfile -File tests/cli.ps1 -AssemblyPath /path/to/ScriptDom.dll ``` diff --git a/references/style-explained.md b/references/style-explained.md index 1505832..5f0b8f2 100644 --- a/references/style-explained.md +++ b/references/style-explained.md @@ -17,10 +17,10 @@ the [worked examples](worked-examples.md) isolate each point in a short SQL exce | selective brackets | use SQL Server's square-bracket quoting where a name needs it, and for renamed/computed output labels in this style | `[Customer Name]` or `[total]`, but a plain reference stays `o.orderid` | | standalone clause keywords | put a section's opening words on their own line, with its contents underneath | `select` above even one column, `from` above the table, `where` above the conditions; small-query exceptions are in the rules | | block indentation | move inward to show that something belongs inside something else | a CTE's query sits one level inside its parentheses | -| construct-relative indentation | calculate that inward step from the surrounding SQL structure, not the file's left edge | table under `from`, join one level deeper, its conditions another level deeper; normally four spaces per level | +| construct-relative indentation | calculate that inward step from the surrounding SQL structure, not the file's left edge | table and joins aligned under `from`, conditions one level deeper; normally four spaces per level | | vertical alignment | add spaces so related items line up in columns | all output aliases in a SELECT list have their `=` signs in the same column, sized to the longest alias | | leading boolean operators | start each additional condition with its joining word | `and o.shippeddate is not null`; preserve parentheses and the order of `and`/`or` | -| CTE chain | give intermediate queries names so a longer query can be read in stages | `;with gd as (...)` followed by `, totals as (...)`; CTE means common table expression, not a saved table | +| CTE chain | give intermediate queries names so a longer query can be read in stages | `;with gd as (...)` continues on its closing line as `), totals as (`; CTE means common table expression, not a saved table | | comment-friendly layout | give columns and conditions their own lines so they are easier to inspect or temporarily comment out | leading commas and `and` help with later items; the first item still needs care | | why-comments | explain a reason the SQL alone cannot tell the reader | explain why a cutoff exists, not that `where` filters rows | diff --git a/references/worked-examples.md b/references/worked-examples.md index eb290a0..d50e49b 100644 --- a/references/worked-examples.md +++ b/references/worked-examples.md @@ -23,16 +23,16 @@ This illustrates authoring, not permission for a formatter to move predicates. ```sql from [order details] od - join products p on - p.productid = od.productid - and p.discontinued = 0 + join products p on + p.productid = od.productid + and p.discontinued = 0 where od.orderid = @orderid ``` ## 3. CTE spacing and chaining -Blank lines inside each CTE, a leading comma for the next, and a short comment naming its purpose. +Blank lines inside each CTE, with the next name on the closing line: `), ordrollup as (`. The second CTE uses only the orders selected by `gd`. The final SELECT is omitted. ```sql @@ -45,18 +45,16 @@ The second CTE uses only the orders selected by `gd`. The final SELECT is omitte where shippeddate is not null -) - --- line counts for shipped orders -, ordrollup as ( +), ordrollup as ( + -- line counts for shipped orders select gd.orderid , [linecount] = count(*) from gd - join [order details] od on - od.orderid = gd.orderid + join [order details] od on + od.orderid = gd.orderid group by gd.orderid ) @@ -70,16 +68,16 @@ the same customer's greatest order ID below the current one. ```sql from orders o - outer apply ( - select top 1 - [previous_orderid] = oo.orderid - , [previous_orderdate] = oo.orderdate - from - orders oo - where - oo.customerid = o.customerid - and oo.orderid < o.orderid - order by - oo.orderid desc - ) po + outer apply ( + select top 1 + [previous_orderid] = oo.orderid + , [previous_orderdate] = oo.orderdate + from + orders oo + where + oo.customerid = o.customerid + and oo.orderid < o.orderid + order by + oo.orderid desc + ) po ``` diff --git a/scripts/RoySql.cs b/scripts/RoySql.cs index ec4ff23..5de6bf0 100644 --- a/scripts/RoySql.cs +++ b/scripts/RoySql.cs @@ -21,7 +21,7 @@ static TSqlFragment Parse(string sql) { var tree = new TSql160Parser(true).Parse(reader, out var errors); if (errors.Count > 0) - throw new FormatException($"parse refused at {errors[0].Line}:{errors[0].Column} ({errors[0].Number})"); + throw new FormatException($"T-SQL parse at {errors[0].Line}:{errors[0].Column}: {errors[0].Message} ({errors[0].Number})"); if (string.Concat(tree.ScriptTokenStream.Select(t => t.Text)) != sql) throw new FormatException("token retention failed"); return tree; @@ -32,7 +32,7 @@ static TSqlFragment Parse(string sql) static bool Space(TSqlParserToken t) => t.TokenType == TSqlTokenType.WhiteSpace || t.TokenType == TSqlTokenType.EndOfFile; static bool IdentifierToken(TSqlParserToken t) => t.TokenType == TSqlTokenType.Identifier || t.TokenType == TSqlTokenType.QuotedIdentifier; static readonly HashSet Builtins = new HashSet(StringComparer.OrdinalIgnoreCase) - { "count", "sum", "min", "max", "avg", "coalesce", "isnull", "isnumeric", "nullif", "getdate", "getutcdate", "sysdatetime", "sysutcdatetime", "dateadd", "datediff", "datediff_big", "datename", "datepart", "eomonth", "year", "month", "day", "concat", "concat_ws", "left", "right", "substring", "len", "datalength", "ltrim", "rtrim", "trim", "replace", "reverse", "upper", "lower", "quotename", "round", "abs", "ceiling", "floor", "power", "square", "sqrt", "row_number", "rank", "dense_rank", "ntile", "lag", "lead", "first_value", "last_value", "string_agg", "format", "try_cast", "try_convert" }; + { "count", "sum", "min", "max", "avg", "coalesce", "isnull", "isnumeric", "nullif", "getdate", "getutcdate", "sysdatetime", "sysutcdatetime", "dateadd", "datediff", "datediff_big", "datename", "datepart", "eomonth", "year", "month", "day", "concat", "concat_ws", "left", "right", "substring", "len", "datalength", "ltrim", "rtrim", "trim", "replace", "reverse", "upper", "lower", "quotename", "round", "abs", "ceiling", "floor", "power", "square", "sqrt", "row_number", "rank", "dense_rank", "ntile", "lag", "lead", "first_value", "last_value", "string_agg", "format", "try_cast", "try_convert", "date_trunc", "array_agg" }; static HashSet FunctionNames(TSqlFragment tree) { @@ -60,6 +60,10 @@ static string TokenText(TSqlParserToken t, bool function = false) static string SemanticForm(TSqlFragment tree) { + tree.Accept(new Walk(n => { + if (n is TopRowFilter top && top.Expression is ParenthesisExpression p && p.Expression is IntegerLiteral) + top.Expression = p.Expression; + })); var generator = new Sql160ScriptGenerator(new SqlScriptGeneratorOptions { KeywordCasing = KeywordCasing.Lowercase }); generator.GenerateScript(tree, out var sql); var canonical = Parse(sql); @@ -108,6 +112,16 @@ static string NormalizeSyntax(string sql, TSqlFragment tree) { var edits = new List(); tree.Accept(new Walk(n => { + if (n is QualifiedJoin join && join.QualifiedJoinType == QualifiedJoinType.Inner) + for (int i = join.FirstTableReference.LastTokenIndex + 1; i < join.SecondTableReference.FirstTokenIndex; i++) + if (tree.ScriptTokenStream[i].TokenType == TSqlTokenType.Inner) + edits.Add(new Edit(tree.ScriptTokenStream[i].Offset, tree.ScriptTokenStream[i].Text.Length, "")); + if (n is TopRowFilter top && top.Expression is ParenthesisExpression parens && parens.Expression is IntegerLiteral + && !tree.ScriptTokenStream.Skip(parens.FirstTokenIndex).Take(parens.LastTokenIndex - parens.FirstTokenIndex + 1).Any(Comment)) + { + edits.Add(new Edit(parens.StartOffset, 1, " ")); + edits.Add(new Edit(parens.StartOffset + parens.FragmentLength - 1, 1, " ")); + } if (n is SelectStatement select && select.WithCtesAndXmlNamespaces != null) { var with = select.WithCtesAndXmlNamespaces; @@ -165,6 +179,7 @@ sealed class Layout readonly Dictionary indentation = new Dictionary(); readonly HashSet functions; readonly HashSet visitedQueries = new HashSet(); + readonly HashSet visitedCases = new HashSet(); readonly HashSet unary = new HashSet(); readonly HashSet tightOpen = new HashSet(); readonly string newline; @@ -243,6 +258,26 @@ void Expression(TSqlFragment n, int indent) Query(sub.QueryExpression, indent + 4); Line(sub.LastTokenIndex, indent); } + if (part is CaseExpression c && visitedCases.Add(c.FirstTokenIndex)) + { + Span(c, indent); + var clauses = c is SearchedCaseExpression searched + ? searched.WhenClauses.Cast() + : ((SimpleCaseExpression)c).WhenClauses.Cast(); + foreach (var clause in clauses) + { + Span(clause, indent + 4); + Line(clause.FirstTokenIndex, indent + 4); + Expression(clause, indent + 4); + } + if (c.ElseExpression != null) + { + Line(Prev(c.ElseExpression.FirstTokenIndex), indent + 4); + Span(c.ElseExpression, indent + 4); + Expression(c.ElseExpression, indent + 4); + } + Line(c.LastTokenIndex, indent); + } })); } @@ -263,17 +298,17 @@ void Table(TableReference n, int indent) if (n is QualifiedJoin join) { Table(join.FirstTableReference, indent); - Span(join.SecondTableReference, indent + 4); - Line(Next(join.FirstTableReference.LastTokenIndex), indent + 4); - TableBody(join.SecondTableReference, indent + 4); - Predicate(join.SearchCondition, indent + 8, Next(Find(join.SecondTableReference.LastTokenIndex + 1, join.SearchCondition.FirstTokenIndex, "on"))); + Span(join.SecondTableReference, indent); + Line(Next(join.FirstTableReference.LastTokenIndex), indent); + TableBody(join.SecondTableReference, indent); + Predicate(join.SearchCondition, indent + 4, Next(Find(join.SecondTableReference.LastTokenIndex + 1, join.SearchCondition.FirstTokenIndex, "on"))); } else if (n is UnqualifiedJoin unqualified) { Table(unqualified.FirstTableReference, indent); - Span(unqualified.SecondTableReference, indent + 4); - Line(Next(unqualified.FirstTableReference.LastTokenIndex), indent + 4); - TableBody(unqualified.SecondTableReference, indent + 4); + Span(unqualified.SecondTableReference, indent); + Line(Next(unqualified.FirstTableReference.LastTokenIndex), indent); + TableBody(unqualified.SecondTableReference, indent); } else { Span(n, indent); Line(n.FirstTokenIndex, indent); TableBody(n, indent); } } @@ -350,7 +385,11 @@ public string Print(TSqlFragment tree) if (previous >= 0 && tokens[previous].Text == ";") { Line(previous, 0, true); before[with.FirstTokenIndex] = ""; } foreach (var cte in with.CommonTableExpressions) { - if (cte != with.CommonTableExpressions[0]) Line(Prev(cte.FirstTokenIndex), 0, true); + if (cte != with.CommonTableExpressions[0]) + { + before[Prev(cte.FirstTokenIndex)] = ""; + before[cte.FirstTokenIndex] = " "; + } Query(cte.QueryExpression, 4); Line(cte.QueryExpression.FirstTokenIndex, 4, true); Line(Next(cte.QueryExpression.LastTokenIndex), 0, true); @@ -414,11 +453,161 @@ public string Print(TSqlFragment tree) } } + static IList Lex(string sql) + { + using (var reader = new StringReader(sql)) + { + var tokens = new TSql160Parser(true).GetTokenStream(reader, out var errors); + if (errors.Count > 0) + throw new FormatException($"tokenization at {errors[0].Line}:{errors[0].Column}: {errors[0].Message}"); + if (string.Concat(tokens.Select(t => t.Text)) != sql) throw new FormatException("token retention failed"); + return tokens.Where(t => !Space(t)).ToList(); + } + } + + static string LooseText(IList tokens, int i) + { + var t = tokens[i]; + var prior = i > 0 ? tokens[i - 1].Text.ToLowerInvariant() : ""; + var next = i + 1 < tokens.Count ? tokens[i + 1].Text : ""; + if (t.TokenType == TSqlTokenType.Identifier && prior != "." && next != ".") + { + if (Builtins.Contains(t.Text) && next == "(") return t.Text.ToLowerInvariant(); + if ((t.Text.Equals("false", StringComparison.OrdinalIgnoreCase) || t.Text.Equals("true", StringComparison.OrdinalIgnoreCase)) + && (prior == "=" || prior == "<>" || prior == "!=" || prior == "is" || prior == "not")) return t.Text.ToLowerInvariant(); + if (((t.Text.Equals("limit", StringComparison.OrdinalIgnoreCase) || t.Text.Equals("offset", StringComparison.OrdinalIgnoreCase)) + && i + 1 < tokens.Count && tokens[i + 1].TokenType == TSqlTokenType.Integer) + || (t.Text.Equals("filter", StringComparison.OrdinalIgnoreCase) && next == "(")) return t.Text.ToLowerInvariant(); + } + return TokenText(t); + } + + sealed class LooseScope + { + public int Indent; + public bool Block, Columns, Between; + public string Clause = ""; + } + + // ponytail: this fallback lays out tokens, not another SQL grammar. Unknown syntax stays in order. + static string Loose(string sql) + { + var tokens = Lex(sql); + if (tokens.Count == 0) return sql; + var newline = sql.Contains("\r\n") ? "\r\n" : "\n"; + var output = new StringBuilder(); + var scopes = new Stack(); + scopes.Push(new LooseScope { Block = true }); + bool tableDefinition = false; + int prior = -1; + string planned = ""; + Func line = indent => newline + new string(' ', indent); + for (int i = 0; i < tokens.Count; i++) + { + var token = tokens[i]; + var word = token.Text.ToLowerInvariant(); + var p = prior >= 0 ? tokens[prior].Text.ToLowerInvariant() : ""; + var next = i + 1 < tokens.Count ? tokens[i + 1].Text.ToLowerInvariant() : ""; + var scope = scopes.Peek(); + var indent = scope.Indent + (scope.Clause.Length > 0 ? 4 : 0); + // Keep inline spacing: a T-SQL token boundary need not be a boundary in another dialect. + string gap = prior < 0 ? "" : sql.Substring(tokens[prior].Offset + tokens[prior].Text.Length, + token.Offset - tokens[prior].Offset - tokens[prior].Text.Length); + if (gap.Contains("\n") || gap.Contains("\r")) gap = " "; + if (planned.Length > 0) { gap = planned; planned = ""; } + + if (Comment(token)) + { + if (prior >= 0 && token.Line > tokens[prior].Line && !gap.StartsWith(newline)) gap = line(indent); + } + else if (word == "(") + { + bool query = next == "select" || next == "with"; + bool columns = tableDefinition && scope.Block; + bool block = query || columns; + scopes.Push(new LooseScope { Indent = indent, Block = block, Columns = columns }); + if (block) planned = line(indent + 4); + tableDefinition = false; + } + else if (word == ")" && scopes.Count > 1) + { + var closed = scopes.Pop(); + if (closed.Block) gap = line(closed.Indent); + } + else if (scope.Block) + { + if (word == "table" && p == "create") tableDefinition = true; + bool name = (token.TokenType == TSqlTokenType.Identifier || token.IsKeyword()) && p != "." && next != "."; + bool pair = (word == "group" || word == "order") && next == "by"; + bool clause = name && (new[] { "select", "from", "where", "having", "limit", "offset", "returning", "union" }.Contains(word) || pair); + bool join = word == "join" || ((word == "left" || word == "right" || word == "full" || word == "inner" || word == "cross" || word == "outer") && (next == "join" || next == "outer" || next == "apply")); + if (clause && !scope.Columns) + { + gap = line(scope.Indent + (scopes.Count > 1 ? 4 : 0)); + scope.Clause = word; + scope.Between = false; + if (!pair) planned = line(scope.Indent + (scopes.Count > 1 ? 8 : 4)); + } + else if (word == "by" && (p == "order" || p == "group")) planned = line(scope.Indent + (scopes.Count > 1 ? 8 : 4)); + else if (name && join && !new[] { "left", "right", "full", "inner", "cross", "outer" }.Contains(p)) + { + gap = line(scope.Indent + 4); + scope.Clause = "join"; + } + else if (word == "on" && scope.Clause == "join") planned = line(scope.Indent + 8); + else if (word == "between") scope.Between = true; + else if (word == "and" && scope.Between) scope.Between = false; + else if ((word == "and" || word == "or") && (scope.Clause == "where" || scope.Clause == "having" || scope.Clause == "join")) + gap = line(scope.Indent + (scope.Clause == "join" ? 8 : 4)); + else if (word == "," && (scope.Clause == "select" || scope.Columns)) { gap = line(scope.Indent + 4); planned = " "; } + else if (word == ";") { scope.Clause = ""; planned = newline + newline; } + } + if (prior >= 0 && tokens[prior].TokenType == TSqlTokenType.SingleLineComment) gap = line(indent); + if (prior < 0) gap = ""; + output.Append(gap).Append(LooseText(tokens, i)); + prior = i; + } + var result = output.ToString().TrimEnd('\r', '\n') + newline; + var after = Lex(result); + if (tokens.Count != after.Count || !tokens.Select((t, i) => LooseText(tokens, i)).SequenceEqual(after.Select((t, i) => LooseText(after, i)))) + throw new FormatException("token boundaries changed during best-effort layout"); + return result; + } + + public static string Format(string sql, bool strict, out string warning) + { + warning = null; + if (strict) return Format(sql); + try + { + var formatted = Format(sql); + var tokens = Lex(formatted); + // FALSE/TRUE in SQL-like comparisons are identifiers to the T-SQL parser, not literals. + var output = new StringBuilder(formatted); + for (int i = tokens.Count - 1; i >= 0; i--) + { + var text = LooseText(tokens, i); + if (text != tokens[i].Text) output.Remove(tokens[i].Offset, tokens[i].Text.Length).Insert(tokens[i].Offset, text); + } + return output.ToString(); + } + catch (FormatException error) + { + warning = error.Message + "; using best-effort layout (not validated)."; + try { return Loose(sql); } + catch (FormatException fallback) + { + warning += " " + fallback.Message + "; input preserved unchanged."; + return sql; + } + } + } + public static string Format(string sql) { var original = Parse(sql); Gate(original); - var normalized = Parse(NormalizeSyntax(sql, original)); + var normalized = Parse(NormalizeSyntax(sql, original)); string newline = sql.Contains("\r\n") ? "\r\n" : "\n"; var output = new Layout(normalized, newline).Print(normalized); var result = Parse(output); diff --git a/scripts/format-sql.ps1 b/scripts/format-sql.ps1 index 995d118..c3bdefa 100644 --- a/scripts/format-sql.ps1 +++ b/scripts/format-sql.ps1 @@ -2,6 +2,7 @@ param( [string]$Path, [Alias('check')][switch]$CheckOnly, + [switch]$Strict, [string]$AssemblyPath = "$PSScriptRoot/lib/Microsoft.SqlServer.TransactSql.ScriptDom.dll" ) $ErrorActionPreference = 'Stop' @@ -14,10 +15,12 @@ try { Add-Type -Path "$PSScriptRoot/RoySql.cs" -ReferencedAssemblies $references } $sql = if ($Path) { [IO.File]::ReadAllText($ExecutionContext.SessionState.Path.GetUnresolvedProviderPathFromPSPath($Path)) } else { [Console]::In.ReadToEnd() } - $formatted = [RoySql]::Format($sql) + $warning = $null + $formatted = [RoySql]::Format($sql, [bool]$Strict, [ref]$warning) + if ($warning) { [Console]::Error.WriteLine($warning) } if ($CheckOnly) { if ($formatted -cne $sql) { exit 1 }; exit 0 } [Console]::Out.Write($formatted) } catch { - [Console]::Error.WriteLine($_.Exception.Message) + [Console]::Error.WriteLine($_.Exception.GetBaseException().Message) exit 2 } diff --git a/tests/cli.ps1 b/tests/cli.ps1 index 9623546..f34841e 100644 --- a/tests/cli.ps1 +++ b/tests/cli.ps1 @@ -41,14 +41,26 @@ try { } $repeat = Run @('-AssemblyPath', $AssemblyPath, '-Path', $outputFile) if ($repeat.code -ne 0 -or $repeat.stdout -cne $file.stdout) { throw 'CLI not idempotent' } - $refused = Run @('-AssemblyPath', $AssemblyPath) 'select a into newtable from t' + $refused = Run @('-AssemblyPath', $AssemblyPath, '-Strict') 'select a into newtable from t' if ($refused.code -ne 2 -or $refused.stdout -or !$refused.stderr) { throw 'refusal leaked partial output' } + $loose = Run @('-AssemblyPath', $AssemblyPath) 'select a from t limit 50;' + if ($loose.code -ne 0 -or !$loose.stdout.Contains('limit') -or $loose.stderr -notmatch 'best-effort') { throw 'default fallback failed' } + $looseRepeat = Run @('-AssemblyPath', $AssemblyPath) $loose.stdout + if ($looseRepeat.code -ne 0 -or $looseRepeat.stdout -cne $loose.stdout) { throw 'fallback CLI is not stable' } + $looseCheck = Run @('-AssemblyPath', $AssemblyPath, '-Check') $loose.stdout + if ($looseCheck.code -ne 0 -or $looseCheck.stdout -or $looseCheck.stderr -notmatch 'best-effort') { throw 'fallback check contract failed' } + $strict = Run @('-AssemblyPath', $AssemblyPath, '-Strict') 'select a from t limit 50;' + if ($strict.code -ne 2 -or $strict.stdout -or $strict.stderr -notmatch 'T-SQL parse at 1:' -or $strict.stderr -notmatch 'Incorrect syntax' -or $strict.stderr -match 'Exception calling') { throw 'strict diagnostic failed' } + $unclosed = Run @('-AssemblyPath', $AssemblyPath) "select 'unfinished" + if ($unclosed.code -ne 0 -or $unclosed.stdout -cne "select 'unfinished" -or $unclosed.stderr -notmatch 'preserved unchanged') { throw 'damaged input fallback failed' } + $missing = Run @('-AssemblyPath', $AssemblyPath, '-Path', (Join-Path $root 'missing.sql')) + if ($missing.code -ne 2 -or $missing.stdout -or !$missing.stderr) { throw 'missing input was hidden' } foreach ($path in @((Join-Path $root 'missing.dll'), $badFile)) { $failed = Run @('-AssemblyPath', $path) $sample if ($failed.code -ne 2 -or $failed.stdout -or !$failed.stderr) { throw 'dependency refusal failed' } } if ([IO.File]::ReadAllText($inputFile) -cne $sample) { throw 'input changed' } - '9 CLI checks passed' + '15 CLI checks passed' } finally { Remove-Item -LiteralPath $inputFile, $outputFile, $badFile -ErrorAction SilentlyContinue [IO.Directory]::Delete($root) diff --git a/tests/examples.ps1 b/tests/examples.ps1 index 52152ae..fa545c4 100644 --- a/tests/examples.ps1 +++ b/tests/examples.ps1 @@ -7,6 +7,10 @@ $examples = [ordered]@{ 'Expanded nested check' = 'SELECT o.orderid FROM orders o WHERE EXISTS (SELECT 1 FROM [order details] od WHERE od.orderid = o.orderid)' 'Existing predicates stay intact' = "SELECT orderid FROM orders WHERE orderid BETWEEN 10248 AND 10260 AND customerid != 'ALFKI'" 'List-wide alias alignment' = 'SELECT unitprice * quantity AS gross, unitprice * quantity * (1 - discount) AS discounted_total, discount AS d FROM [order details]' + 'CASE is a block' = "SELECT CASE WHEN amount > 100 THEN 'High' WHEN amount > 50 THEN 'Medium' ELSE 'Low' END AS tier FROM totals" + 'JOIN alignment and literal TOP' = 'SELECT TOP (50) o.id FROM orders o INNER JOIN items i ON i.orderid=o.id LEFT JOIN products p ON p.id=i.productid' + 'Adjacent CTEs' = 'WITH a AS (SELECT id FROM orders), b AS (SELECT id FROM a) SELECT id FROM b' + 'LIMIT is retained, not translated' = 'SELECT id FROM orders ORDER BY id LIMIT 50;' } '# Actual formatter output' '' @@ -25,6 +29,8 @@ foreach ($example in $examples.GetEnumerator()) { 'After:' '' '```sql' - [RoySql]::Format($example.Value).TrimEnd("`r", "`n") + $warning = $null + [RoySql]::Format($example.Value, $false, [ref]$warning).TrimEnd("`r", "`n") '```' + if ($warning) { ''; 'Best-effort output. A diagnostic is written to stderr, not into the SQL.' } } diff --git a/tests/feedback.ps1 b/tests/feedback.ps1 new file mode 100644 index 0000000..f95008d --- /dev/null +++ b/tests/feedback.ps1 @@ -0,0 +1,94 @@ +#requires -Version 7.0 +param([Parameter(Mandatory)][string]$AssemblyPath) +$ErrorActionPreference = 'Stop' +Add-Type -LiteralPath $AssemblyPath +Add-Type -Path "$PSScriptRoot/../scripts/RoySql.cs" -ReferencedAssemblies (@([IO.Directory]::GetFiles("$PSHOME/ref", '*.dll')) + (Resolve-Path -LiteralPath $AssemblyPath).Path) +$samples = [ordered]@{ + aggregation = @' +SELECT p.category_id, c.category_name, +COUNT(DISTINCT p.product_id) AS total_products, +SUM(oi.quantity * oi.unit_price) AS total_revenue, +ROUND(AVG(oi.unit_price), 2) AS avg_item_price, +CASE WHEN SUM(oi.quantity * oi.unit_price) > 100000 THEN 'High Performing' +WHEN SUM(oi.quantity * oi.unit_price) BETWEEN 50000 AND 100000 THEN 'Mid Performing' +ELSE 'Low Performing' END AS performance_tier +FROM products p LEFT JOIN categories c ON p.category_id = c.id +INNER JOIN order_items oi ON p.product_id = oi.product_id +INNER JOIN orders o ON oi.order_id = o.id +WHERE o.order_status IN ('SHIPPED', 'DELIVERED') AND o.created_at >= '2025-01-01' +GROUP BY p.category_id, c.category_name HAVING COUNT(DISTINCT p.product_id) >= 5 +ORDER BY total_revenue DESC; +'@ + windows = @' +WITH MonthlySales AS ( +SELECT DATE_TRUNC('month', order_date) AS sales_month, customer_id, +SUM(net_amount) AS monthly_spend, +ROW_NUMBER() OVER (PARTITION BY DATE_TRUNC('month', order_date) ORDER BY SUM(net_amount) DESC) AS spender_rank +FROM customer_orders WHERE is_cancelled = FALSE GROUP BY 1, 2 +), TopCustomers AS ( +SELECT sales_month, customer_id, monthly_spend FROM MonthlySales WHERE spender_rank <= 10 +) +SELECT tc.sales_month, tc.customer_id, tc.monthly_spend, +LAG(tc.monthly_spend, 1) OVER (PARTITION BY tc.customer_id ORDER BY tc.sales_month) AS prev_month_spend, +LEAD(tc.monthly_spend, 1) OVER (PARTITION BY tc.customer_id ORDER BY tc.sales_month) AS next_month_spend +FROM TopCustomers tc ORDER BY tc.sales_month DESC, tc.monthly_spend DESC; +'@ + ddl = @' +CREATE TABLE inventory_audit_log ( +log_id BIGINT GENERATED ALWAYS AS IDENTITY PRIMARY KEY, +product_sku VARCHAR(50) NOT NULL, +warehouse_id INT NOT NULL, +old_quantity INT NOT NULL CHECK (old_quantity >= 0), +new_quantity INT NOT NULL CHECK (new_quantity >= 0), +change_reason VARCHAR(255) DEFAULT 'ROUTINE_ADJUSTMENT', +adjusted_by VARCHAR(100) NOT NULL, +created_at TIMESTAMP WITH TIME ZONE DEFAULT CURRENT_TIMESTAMP, +CONSTRAINT fk_inventory_warehouse FOREIGN KEY (warehouse_id) REFERENCES warehouses (id) +ON DELETE RESTRICT ON UPDATE CASCADE +); +CREATE INDEX idx_inventory_audit_sku_date ON inventory_audit_log (product_sku, created_at DESC); +'@ + operators = @' +-- Test case for inline comments and mixed casing +SeLeCt -- Trailing column comment +tbl.id AS [User ID], /* Block comment test */ +tbl.details->>'email' AS email_address, +ARRAY_AGG(tbl.tags) FILTER (WHERE tbl.tags IS NOT NULL) AS tag_list, +COALESCE(tbl.updated_at, tbl.created_at, '1970-01-01 00:00:00+00') AS effective_date +FROM "Audit"."UserLogs" tbl +WHERE tbl.payload LIKE '%"status": "error"%' +AND tbl.created_at BETWEEN TIMESTAMP '2026-01-01 00:00:00' AND CURRENT_TIMESTAMP +GROUP BY tbl.id, tbl.details->>'email', tbl.updated_at, tbl.created_at; +'@ + limit = "select u.id, u.first_name from users u where u.status='COMPLETED' order by u.id limit 50;" + incomplete = 'select from' + unclosed = "select 'unterminated" + identifiers = 'select Limit, Zone, False, dbo.DATE_TRUNC(d), [FALSE] from t' + comments = "-- keep SELECT FROM`r`nselect a, -- next`r`nb from t limit 2; /* END */" + punctuation = "select details->>'x', payload#>>'{a}', name::text, - -2, 'a`nb' from t limit 4;" + pseudocode = 'select something from wherever MAGIC THING happens limit 1' +} +$passed = 0 +foreach ($sample in $samples.GetEnumerator()) { + $warning = $null + $actual = [RoySql]::Format($sample.Value, $false, [ref]$warning) + $secondWarning = $null + $second = [RoySql]::Format($actual, $false, [ref]$secondWarning) + if ($second -cne $actual) { throw "$($sample.Key) not idempotent:`n$actual`nSECOND:`n$second" } + switch ($sample.Key) { + aggregation { + if ($actual -notmatch 'case\r?\n {8}when' -or $actual -match 'inner join' -or $actual -notmatch '\r?\n {4}left join' -or $warning) { throw 'aggregation layout failed' } + } + windows { + if ($actual -cnotmatch 'date_trunc\(' -or $actual -cnotmatch 'is_cancelled = false' -or $actual -notmatch '\), TopCustomers as \(') { throw 'window/CTE style failed' } + } + ddl { if (!$warning -or !$actual.Contains('ROUTINE_ADJUSTMENT') -or $actual -notmatch '\r?\n {4}, product_sku') { throw 'DDL fallback failed' } } + operators { if (!$warning -or $actual -notmatch "->>\s*'email'" -or !$actual.Contains('"Audit"."UserLogs"') -or !$actual.Contains('/* Block comment test */')) { throw "operator/comment retention failed: $warning`n$actual" } } + limit { if (!$warning -or $actual -notmatch 'limit\r?\n {4}50' -or $actual -match 'top') { throw 'LIMIT was lost or translated' } } + unclosed { if ($actual -cne $sample.Value -or $warning -notmatch 'preserved unchanged') { throw 'unclosed literal was damaged' } } + identifiers { if (!$actual.Contains('Limit') -or !$actual.Contains('Zone') -or !$actual.Contains('False') -or !$actual.Contains('dbo.DATE_TRUNC')) { throw 'identifiers changed' } } + punctuation { if ($actual -notmatch "#>>\s*'\{a\}'" -or $actual -notmatch '::\s*text' -or $actual -notmatch '- -\s*2') { throw "punctuation changed:`n$actual" } } + } + $passed++ +} +"$passed feedback checks passed" diff --git a/tests/formatter.ps1 b/tests/formatter.ps1 index 9cd09b7..276bb7f 100644 --- a/tests/formatter.ps1 +++ b/tests/formatter.ps1 @@ -21,8 +21,8 @@ select , [Total] = sum(od.Price * od.Quantity) from Orders o - join Details od on - od.OrderID = o.OrderID + join Details od on + od.OrderID = o.OrderID where o.OrderID = @OrderID and o.ShippedDate is not null @@ -45,16 +45,16 @@ select , p.b from t o - outer apply ( - select top 1 - x.b - from - t x - where - x.a = o.a - order by - x.b desc - ) p + outer apply ( + select top 1 + x.b + from + t x + where + x.a = o.a + order by + x.b desc + ) p '@ Test-Format 'preamble' "-- keep guard`nset transaction isolation level read uncommitted`nset deadlock_priority -10`nset nocount on`ndeclare @orderid int = try_cast('123' as int)`nselect @orderid" @@ -200,7 +200,7 @@ foreach ($nl in @("`n", "`r`n")) { $sql = @('select a.id', 'from things a', 'outer apply (', ' -- first explanation', ' -- second explanation', ' select top 1 b.id', ' from things b', ' where b.id = a.id', ') found') -join $nl Test-Format 'nested full-line comment block' $sql $comments = @([RoySql]::Format($sql) -split '\r?\n' | Where-Object { $_ -match '^\s*--' }) - if ($comments.Count -ne 2 -or @($comments | Where-Object { $_ -match '^ {12}--' }).Count -ne 2) { throw 'comment block indentation differs' } + if ($comments.Count -ne 2 -or @($comments | Where-Object { $_ -match '^ {8}--' }).Count -ne 2) { throw 'comment block indentation differs' } } Test-Format 'predicate comment block' "select a from t where`n-- first`n/* second */`na=1" @' select @@ -231,6 +231,71 @@ select from Things +'@ +Test-Format 'literal TOP and implicit inner join' 'select top (50) a.id from things a inner join others b on b.id=a.id' @' +select top 50 + a.id +from + things a + join others b on + b.id = a.id + +'@ +Test-Format 'expanded CASE' "select case when x > 10 then 'High' when x between 5 and 10 then 'Middle' else 'Low' end as tier from t" @' +select + [tier] = case + when x > 10 then 'High' + when x between 5 and 10 then 'Middle' + else 'Low' + end +from + t + +'@ +Test-Format 'nested CASE' 'select case when x=1 then case y when 2 then 3 else 4 end else 5 end as result from t' +Test-Format 'TOP expressions remain parenthesized' 'select top (@n + 1) a from t' +Test-Format 'TOP without source whitespace' 'SELECT TOP(50)id FROM things' @' +select top 50 + id +from + things + +'@ +Test-Format 'TOP percent without source whitespace' 'SELECT TOP(10)PERCENT id FROM things' +Test-Format 'TOP comments retained' 'select top (/* count */ 50) a from t' +Test-Format 'join qualifier comment retained' 'select a.id from t a inner /* join reason */ join u b on b.id=a.id' +Test-Format 'adjacent CTEs' 'with a as (select x from t), b as (select x from a) select x from b' @' +;with a as ( + + select + x + from + t + +), b as ( + + select + x + from + a + +) + +select + x +from + b + +'@ +Test-Format 'known common functions and quoted names' "select DATE_TRUNC('month', d), dbo.DATE_TRUNC(d), [FALSE], [Limit] from t" @' +select + date_trunc('month', d) + , dbo.DATE_TRUNC(d) + , [FALSE] + , [Limit] +from + t + '@ Test-Refusal 'USE outside v1' 'use ExampleDatabase; select 1' Test-Refusal 'malformed' 'select from' From 514d5d7411e5fccf1eb1b83d3f250b5231c56581 Mon Sep 17 00:00:00 2001 From: "@royashbrook" Date: Tue, 15 Sep 2026 22:53:42 -0400 Subject: [PATCH 2/2] align first projections in best-effort layout (refs #2) --- references/formatter.md | 6 ++++-- scripts/RoySql.cs | 24 +++++++++++++++++++++--- tests/feedback.ps1 | 12 ++++++++++-- 3 files changed, 35 insertions(+), 7 deletions(-) diff --git a/references/formatter.md b/references/formatter.md index 8443766..9ba199b 100644 --- a/references/formatter.md +++ b/references/formatter.md @@ -66,7 +66,9 @@ execution equivalence**. No SQL executes during formatting or the test suite. Default mode first tries structural formatting. When that is unavailable, its fallback organizes recognizable clause boundaries, projection/definition lists and parenthesized -query blocks. Unknown words and operators retain their order. Inline spacing is conservative +query blocks. A multi-column SELECT pads its first item to align with the names after +leading commas; single-column lists do not. Nested SELECTs count independently. +Unknown words and operators retain their order. Inline spacing is conservative because a T-SQL token boundary may split another dialect's operator. Comments and quoted text are preserved; unclosed literals can cause the entire input to remain unchanged. This is not full support for PostgreSQL, MySQL or arbitrary pseudocode, and layout may be @@ -103,7 +105,7 @@ pwsh -NoProfile -File tests/cli.ps1 -AssemblyPath /path/to/ScriptDom.dll ``` Exact layout fixtures, idempotence and safe-refusal checks use synthetic SQL. CI installs -the pinned dependency and runs both suites cold on Windows, macOS and Linux. +the pinned dependency and runs all three suites cold on Windows, macOS and Linux. `-Check` checks for changes this formatter would make, not every prose rule in the skill. A stable formatting bug can pass `-Check`; expected-output regression tests catch those mistakes. diff --git a/scripts/RoySql.cs b/scripts/RoySql.cs index 5de6bf0..541cd55 100644 --- a/scripts/RoySql.cs +++ b/scripts/RoySql.cs @@ -484,8 +484,8 @@ static string LooseText(IList tokens, int i) sealed class LooseScope { - public int Indent; - public bool Block, Columns, Between; + public int Indent, ProjectionStart = -1; + public bool Block, Columns, Between, FirstProjection; public string Clause = ""; } @@ -509,6 +509,7 @@ static string Loose(string sql) var p = prior >= 0 ? tokens[prior].Text.ToLowerInvariant() : ""; var next = i + 1 < tokens.Count ? tokens[i + 1].Text.ToLowerInvariant() : ""; var scope = scopes.Peek(); + bool firstProjection = scope.FirstProjection && !Comment(token); var indent = scope.Indent + (scope.Clause.Length > 0 ? 4 : 0); // Keep inline spacing: a T-SQL token boundary need not be a boundary in another dialect. string gap = prior < 0 ? "" : sql.Substring(tokens[prior].Offset + tokens[prior].Text.Length, @@ -546,6 +547,8 @@ static string Loose(string sql) gap = line(scope.Indent + (scopes.Count > 1 ? 4 : 0)); scope.Clause = word; scope.Between = false; + scope.FirstProjection = word == "select"; + scope.ProjectionStart = -1; if (!pair) planned = line(scope.Indent + (scopes.Count > 1 ? 8 : 4)); } else if (word == "by" && (p == "order" || p == "group")) planned = line(scope.Indent + (scopes.Count > 1 ? 8 : 4)); @@ -559,11 +562,26 @@ static string Loose(string sql) else if (word == "and" && scope.Between) scope.Between = false; else if ((word == "and" || word == "or") && (scope.Clause == "where" || scope.Clause == "having" || scope.Clause == "join")) gap = line(scope.Indent + (scope.Clause == "join" ? 8 : 4)); - else if (word == "," && (scope.Clause == "select" || scope.Columns)) { gap = line(scope.Indent + 4); planned = " "; } + else if (word == "," && (scope.Clause == "select" || scope.Columns)) + { + // Only a comma in this SELECT's scope proves that its first item needs padding. + if (scope.Clause == "select" && scope.ProjectionStart >= 0) + { + output.Insert(scope.ProjectionStart, " "); + scope.ProjectionStart = -1; + } + gap = line(scope.Indent + (scope.Clause == "select" && scopes.Count > 1 ? 8 : 4)); + planned = " "; + } else if (word == ";") { scope.Clause = ""; planned = newline + newline; } } if (prior >= 0 && tokens[prior].TokenType == TSqlTokenType.SingleLineComment) gap = line(indent); if (prior < 0) gap = ""; + if (firstProjection) + { + scope.ProjectionStart = output.Length + gap.Length; + scope.FirstProjection = false; + } output.Append(gap).Append(LooseText(tokens, i)); prior = i; } diff --git a/tests/feedback.ps1 b/tests/feedback.ps1 index f95008d..ab93b12 100644 --- a/tests/feedback.ps1 +++ b/tests/feedback.ps1 @@ -61,6 +61,10 @@ AND tbl.created_at BETWEEN TIMESTAMP '2026-01-01 00:00:00' AND CURRENT_TIMESTAMP GROUP BY tbl.id, tbl.details->>'email', tbl.updated_at, tbl.created_at; '@ limit = "select u.id, u.first_name from users u where u.status='COMPLETED' order by u.id limit 50;" + single = "select coalesce(a, b) from t limit 1;" + nested = "select (select a, b from t limit 1), c from u limit 2;" + nestedSingle = "select (select a, b from t limit 1) from u limit 2;" + review = "select u.id,u.first_name,u.last_name,o.order_date,o.total_amount from users u inner join orders o on u.id=o.user_id where o.status='COMPLETED' and o.order_date>='2026-01-01' order by o.total_amount desc limit 50;" incomplete = 'select from' unclosed = "select 'unterminated" identifiers = 'select Limit, Zone, False, dbo.DATE_TRUNC(d), [FALSE] from t' @@ -83,8 +87,12 @@ foreach ($sample in $samples.GetEnumerator()) { if ($actual -cnotmatch 'date_trunc\(' -or $actual -cnotmatch 'is_cancelled = false' -or $actual -notmatch '\), TopCustomers as \(') { throw 'window/CTE style failed' } } ddl { if (!$warning -or !$actual.Contains('ROUTINE_ADJUSTMENT') -or $actual -notmatch '\r?\n {4}, product_sku') { throw 'DDL fallback failed' } } - operators { if (!$warning -or $actual -notmatch "->>\s*'email'" -or !$actual.Contains('"Audit"."UserLogs"') -or !$actual.Contains('/* Block comment test */')) { throw "operator/comment retention failed: $warning`n$actual" } } - limit { if (!$warning -or $actual -notmatch 'limit\r?\n {4}50' -or $actual -match 'top') { throw 'LIMIT was lost or translated' } } + operators { if (!$warning -or $actual -notmatch "->>\s*'email'" -or !$actual.Contains('"Audit"."UserLogs"') -or !$actual.Contains('/* Block comment test */') -or $actual -notmatch '\r?\n {6}tbl.id') { throw "operator/comment retention failed: $warning`n$actual" } } + limit { if (!$warning -or $actual -notmatch 'limit\r?\n {4}50' -or $actual -match 'top' -or $actual -notmatch 'select\r?\n {6}u.id\r?\n {4}, u.first_name') { throw 'LIMIT layout failed' } } + single { if (!$warning -or $actual -notmatch 'select\r?\n {4}coalesce\(a, b\)') { throw 'function comma padded a single projection' } } + nested { if (!$warning -or $actual -notmatch 'select\r?\n {6}\(' -or $actual -notmatch 'select\r?\n {14}a\r?\n {12}, b' -or $actual -notmatch '\r?\n {4}, c') { throw "nested projection alignment failed:`n$actual" } } + nestedSingle { if (!$warning -or $actual -notmatch 'select\r?\n {4}\(' -or $actual -notmatch 'select\r?\n {14}a\r?\n {12}, b') { throw 'nested commas padded a single outer projection' } } + review { if (!$warning -or $actual -notmatch 'select\r?\n {6}u.id\r?\n {4}, u.first_name' -or $actual -notmatch '\r?\n {4}, o.total_amount' -or !$actual.Contains("'COMPLETED'")) { throw 'review query alignment or literal retention failed' } } unclosed { if ($actual -cne $sample.Value -or $warning -notmatch 'preserved unchanged') { throw 'unclosed literal was damaged' } } identifiers { if (!$actual.Contains('Limit') -or !$actual.Contains('Zone') -or !$actual.Contains('False') -or !$actual.Contains('dbo.DATE_TRUNC')) { throw 'identifiers changed' } } punctuation { if ($actual -notmatch "#>>\s*'\{a\}'" -or $actual -notmatch '::\s*text' -or $actual -notmatch '- -\s*2') { throw "punctuation changed:`n$actual" } }