fix(format): indent the body of a stored procedure (#13015)
* fix(format): indent the body of a stored procedure Formatting a routine definition left every statement inside its `BEGIN ... END` body at column 0. sql-formatter's grammar has no block nodes, so a routine parses as a flat clause list — `CREATE PROCEDURE`, `SELECT`, `FROM`, then a separate statement holding the leftover `END` keywords — and the clause layout printer, which only knows how to start a line at a clause boundary, had nothing to hang the body's structure on. `CREATE PROCEDURE AAA AS BEGIN SELECT * FROM SysUser su ... END END` came back with the body flush left, and on Oracle the signature itself was split one token per line. `indentRoutineBlocks` restores the block structure from the token stream after the clause layout has run: it keeps the printer's layout inside each statement and adds the breaks and indentation that `BEGIN` / `END`, `IF ... THEN` / `ELSE` / `END IF`, `LOOP` / `END LOOP` and `TRY` / `CATCH` imply. It only inserts whitespace, so no token is rewritten or dropped, and it declines any input that is not a routine definition with a block body, which keeps ordinary queries out of it. `BEGIN TRANSACTION` is not a block and never opens an indentation level; a `CASE ... END` expression closes with the same keyword as a block and is tracked separately. Closes #12731 * fix(sql): preserve MySQL hash comment boundaries in routines --------- Co-authored-by: zipg <4047349+zipg@users.noreply.github.com>
This commit is contained in:
1 parent
163a4e5030
commit
a785a0e6c8
3 files changed
+317
-2
No files matched your search
@@ -0,0 +1,99 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { formatSqlText } from "@/lib/sql/sqlFormatter";
|
||||
import { indentRoutineBlocks } from "@/lib/sql/routineBlockLayout";
|
||||
import { tokenizeSqlSemantic } from "@/lib/sql/semantic/tokens";
|
||||
|
||||
function tokenStream(sql: string, dialect: string): string[] {
|
||||
return tokenizeSqlSemantic(sql, dialect).map((tokenValue) => tokenValue.normalized);
|
||||
}
|
||||
|
||||
const issueProcedure = `CREATE PROCEDURE AAA AS BEGIN
|
||||
SELECT *
|
||||
FROM SysUser su IF EXISTS (SELECT TOP 1 1 FROM SysUser su2) BEGIN PRINT '123';
|
||||
|
||||
END END`;
|
||||
|
||||
describe("routine block layout", () => {
|
||||
it("indents the body of a T-SQL procedure", async () => {
|
||||
expect(await formatSqlText(issueProcedure, "sqlserver")).toBe(["CREATE PROCEDURE AAA", "AS", "BEGIN", " SELECT *", " FROM SysUser su", " IF EXISTS (SELECT TOP 1 1 FROM SysUser su2)", " BEGIN", " PRINT '123';", " END", "END"].join("\n"));
|
||||
});
|
||||
|
||||
it("follows the configured indent width", async () => {
|
||||
const formatted = await formatSqlText(issueProcedure, "sqlserver", { tabWidth: 4 });
|
||||
|
||||
expect(formatted).toContain("\n SELECT *");
|
||||
expect(formatted).toContain("\n PRINT '123';");
|
||||
expect(formatted).toContain("\n END\nEND");
|
||||
});
|
||||
|
||||
it("indents with tabs when the settings ask for tabs", async () => {
|
||||
const formatted = await formatSqlText(issueProcedure, "sqlserver", { useTabs: true, tabWidth: 4 });
|
||||
|
||||
expect(formatted).toContain("\n\tSELECT *");
|
||||
expect(formatted).toContain("\n\t\tPRINT '123';");
|
||||
});
|
||||
|
||||
it.each([
|
||||
["sqlserver", issueProcedure],
|
||||
["oracle", "CREATE OR REPLACE PROCEDURE p(x IN NUMBER) AS BEGIN IF x > 0 THEN DBMS_OUTPUT.PUT_LINE('pos'); ELSE DBMS_OUTPUT.PUT_LINE('neg'); END IF; END;"],
|
||||
["mysql", "CREATE PROCEDURE p() BEGIN SELECT 1; END"],
|
||||
])("keeps every token of a %s routine", async (dialect, sql) => {
|
||||
const formatted = await formatSqlText(sql, dialect);
|
||||
|
||||
expect(tokenStream(formatted, dialect)).toEqual(tokenStream(sql, dialect));
|
||||
});
|
||||
|
||||
it("is idempotent", async () => {
|
||||
const once = await formatSqlText(issueProcedure, "sqlserver");
|
||||
|
||||
expect(await formatSqlText(once, "sqlserver")).toBe(once);
|
||||
});
|
||||
|
||||
it("preserves the end of MySQL hash comments before a routine header", () => {
|
||||
const sql = "# keep this comment\nCREATE PROCEDURE p() BEGIN SELECT 1; END;";
|
||||
const formatted = indentRoutineBlocks(sql, "mysql", 2, false);
|
||||
|
||||
expect(formatted).toMatch(/^# keep this comment\nCREATE PROCEDURE/);
|
||||
expect(tokenStream(formatted, "mysql")).toEqual(tokenStream(sql, "mysql"));
|
||||
});
|
||||
|
||||
it("keeps a CASE expression on the line its clause produced", async () => {
|
||||
const formatted = await formatSqlText("CREATE PROCEDURE p AS BEGIN SELECT CASE WHEN a = 1 THEN 'x' ELSE 'y' END AS label FROM t; END", "sqlserver");
|
||||
|
||||
expect(formatted).toContain("SELECT CASE WHEN a = 1 THEN 'x' ELSE 'y' END AS label");
|
||||
expect(formatted.endsWith("\nEND")).toBe(true);
|
||||
});
|
||||
|
||||
it("does not nest the statements after BEGIN TRANSACTION", async () => {
|
||||
const formatted = await formatSqlText("CREATE PROCEDURE p AS BEGIN BEGIN TRANSACTION; UPDATE t SET a = 1; COMMIT; END", "sqlserver");
|
||||
|
||||
expect(formatted).toBe(["CREATE PROCEDURE p", "AS", "BEGIN", " BEGIN TRANSACTION;", " UPDATE t SET a = 1;", " COMMIT;", "END"].join("\n"));
|
||||
});
|
||||
|
||||
it("keeps BEGIN TRY / END CATCH together and nests their bodies", async () => {
|
||||
const formatted = await formatSqlText("CREATE PROCEDURE p AS BEGIN BEGIN TRY SELECT 1; END TRY BEGIN CATCH SELECT 2; END CATCH END", "sqlserver");
|
||||
|
||||
expect(formatted).toContain("\n BEGIN TRY\n");
|
||||
expect(formatted).toContain("\n END TRY\n");
|
||||
expect(formatted).toContain("\n BEGIN CATCH\n");
|
||||
expect(formatted).toContain("\n END CATCH\n");
|
||||
});
|
||||
|
||||
it("indents a PL/SQL IF ... THEN body and its ELSE branch", async () => {
|
||||
const formatted = await formatSqlText("CREATE OR REPLACE PROCEDURE p AS BEGIN IF x > 0 THEN DBMS_OUTPUT.PUT_LINE('pos'); ELSE DBMS_OUTPUT.PUT_LINE('neg'); END IF; END;", "oracle");
|
||||
|
||||
expect(formatted).toBe(["CREATE OR REPLACE PROCEDURE p", "AS", "BEGIN", " IF x > 0 THEN", " DBMS_OUTPUT.PUT_LINE('pos');", " ELSE", " DBMS_OUTPUT.PUT_LINE('neg');", " END IF;", "END;"].join("\n"));
|
||||
});
|
||||
|
||||
it("collapses a signature the clause layout split one token per line", () => {
|
||||
const split = ["CREATE", "OR", "REPLACE", "PROCEDURE", "p", "(x IN NUMBER)", "AS", "BEGIN", "SELECT 1;", "END;"].join("\n");
|
||||
|
||||
expect(indentRoutineBlocks(split, "oracle", 2, false)).toBe(["CREATE OR REPLACE PROCEDURE p (x IN NUMBER)", "AS", "BEGIN", " SELECT 1;", "END;"].join("\n"));
|
||||
});
|
||||
|
||||
it("leaves input that is not a routine definition alone", () => {
|
||||
for (const sql of ["SELECT 1", "BEGIN TRANSACTION;\nSELECT 1;\nCOMMIT;", "CREATE TABLE t (a int)", "CREATE PROCEDURE p AS SELECT 1"]) {
|
||||
expect(indentRoutineBlocks(sql, "sqlserver", 2, false)).toBe(sql);
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,210 @@
|
||||
import { tokenizeSqlSemantic } from "@/lib/sql/semantic/tokens";
|
||||
import type { SqlSemanticToken } from "@/lib/sql/semantic/types";
|
||||
|
||||
/**
|
||||
* Routine bodies are the one shape the clause layout printer cannot structure.
|
||||
*
|
||||
* sql-formatter's grammar has no block nodes, so `CREATE PROCEDURE ... AS BEGIN
|
||||
* ... END` parses as a flat clause list — `CREATE PROCEDURE`, `SELECT`, `FROM`,
|
||||
* and then a separate statement holding the leftover `END` keywords. The printer
|
||||
* therefore emits one clause per line at column 0, and the body keeps no
|
||||
* indentation at all, which is what users see when they format a stored
|
||||
* procedure.
|
||||
*
|
||||
* This pass restores the block structure from the token stream *after*
|
||||
* formatting: it keeps the printer's clause layout inside each statement and
|
||||
* adds the line breaks and indentation the block keywords imply. It only
|
||||
* inserts whitespace — every token is emitted verbatim, so the statement the
|
||||
* user ends up with is the one they started with, plus the missing structure.
|
||||
*/
|
||||
|
||||
const ROUTINE_KEYWORDS = new Set(["procedure", "proc", "function", "trigger", "package"]);
|
||||
|
||||
/**
|
||||
* `BEGIN` also starts a transaction, which is a statement rather than a block.
|
||||
* Those forms must not open an indented body, or every following statement
|
||||
* would be nested under it.
|
||||
*/
|
||||
const NON_BLOCK_BEGIN = new Set(["transaction", "tran", "distributed", "work"]);
|
||||
|
||||
/** `END` closes a block together with the word that names it: `END IF`, `END LOOP`, ... */
|
||||
const END_SUFFIXES = new Set(["if", "loop", "while", "case", "try", "catch", "repeat"]);
|
||||
|
||||
/**
|
||||
* Statements inside a routine body that the clause layout cannot start a line
|
||||
* for on its own — it only breaks at clause boundaries, so a `PRINT` or an `IF`
|
||||
* that follows a `FROM` would otherwise stay on that line.
|
||||
*/
|
||||
const STATEMENT_STARTERS = new Set(["if", "while", "else", "return", "print", "exec", "execute", "throw", "raiserror", "waitfor", "goto", "break", "continue"]);
|
||||
|
||||
function previousSignificantToken(tokens: SqlSemanticToken[], index: number): SqlSemanticToken | undefined {
|
||||
for (let cursor = index - 1; cursor >= 0; cursor -= 1) {
|
||||
if (tokens[cursor].kind !== "comment") return tokens[cursor];
|
||||
}
|
||||
return undefined;
|
||||
}
|
||||
|
||||
function nextSignificantToken(tokens: SqlSemanticToken[], index: number): SqlSemanticToken | undefined {
|
||||
for (let cursor = index + 1; cursor < tokens.length; cursor += 1) {
|
||||
if (tokens[cursor].kind !== "comment") return tokens[cursor];
|
||||
}
|
||||
return undefined;
|
||||
}
|
||||
|
||||
function wordAt(tokens: SqlSemanticToken[], index: number): string {
|
||||
const tokenValue = tokens[index];
|
||||
return tokenValue?.kind === "word" ? tokenValue.normalized : "";
|
||||
}
|
||||
|
||||
/** `CREATE [OR ALTER|OR REPLACE] PROCEDURE|PROC|FUNCTION|TRIGGER|PACKAGE ...` */
|
||||
function isRoutineDefinition(tokens: SqlSemanticToken[]): boolean {
|
||||
let cursor = 0;
|
||||
while (cursor < tokens.length && tokens[cursor].kind === "comment") cursor += 1;
|
||||
if (wordAt(tokens, cursor) !== "create") return false;
|
||||
cursor += 1;
|
||||
|
||||
const next = wordAt(tokens, cursor);
|
||||
if (next === "or") {
|
||||
const modifier = wordAt(tokens, cursor + 1);
|
||||
if (modifier !== "alter" && modifier !== "replace") return false;
|
||||
cursor += 2;
|
||||
}
|
||||
return ROUTINE_KEYWORDS.has(wordAt(tokens, cursor));
|
||||
}
|
||||
|
||||
/** A routine body only needs re-indenting when it actually contains a block. */
|
||||
function hasBlockBody(tokens: SqlSemanticToken[]): boolean {
|
||||
return tokens.some((tokenValue, index) => {
|
||||
if (tokenValue.kind !== "word" || tokenValue.normalized !== "begin" || tokenValue.depth !== 0) return false;
|
||||
return !NON_BLOCK_BEGIN.has(nextSignificantToken(tokens, index)?.normalized ?? "");
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Re-indents the block structure of a routine definition, leaving every other
|
||||
* input untouched.
|
||||
*
|
||||
* Returns `sql` unchanged unless it is a routine definition with a `BEGIN`
|
||||
* block, so callers can apply this unconditionally to formatted output.
|
||||
*/
|
||||
export function indentRoutineBlocks(sql: string, dialectId: string, indentWidth: number, useTabs: boolean): string {
|
||||
if (!sql.trim()) return sql;
|
||||
|
||||
const tokens = tokenizeSqlSemantic(sql, dialectId);
|
||||
if (!isRoutineDefinition(tokens) || !hasBlockBody(tokens)) return sql;
|
||||
|
||||
const indentUnit = useTabs ? "\t" : " ".repeat(Math.max(1, indentWidth));
|
||||
const lines: string[] = [];
|
||||
let current = "";
|
||||
let level = 0;
|
||||
// `CASE ... END` closes with the same keyword as a block, so its depth is
|
||||
// tracked separately and its `END` is never treated as a block terminator.
|
||||
let caseDepth = 0;
|
||||
// `IF ... THEN` (PL/SQL) opens a body without a `BEGIN`, so its depth is
|
||||
// counted apart from `BEGIN` blocks to let `ELSE` close and reopen it.
|
||||
let thenBlocks = 0;
|
||||
let headerDone = false;
|
||||
let forceBreak = false;
|
||||
let previousWord = "";
|
||||
let previousToken: SqlSemanticToken | undefined;
|
||||
|
||||
const flush = () => {
|
||||
const trimmed = current.replace(/\s+$/, "");
|
||||
if (trimmed) lines.push(trimmed);
|
||||
current = "";
|
||||
};
|
||||
const startLine = () => {
|
||||
if (current.trim()) flush();
|
||||
};
|
||||
const write = (text: string) => {
|
||||
current = current.trim() ? current + text : indentUnit.repeat(level) + text;
|
||||
};
|
||||
|
||||
for (let index = 0; index < tokens.length; index += 1) {
|
||||
const tokenValue = tokens[index];
|
||||
const gap = previousToken ? sql.slice(previousToken.span.end, tokenValue.span.start) : "";
|
||||
|
||||
const word = tokenValue.kind === "word" ? tokenValue.normalized : "";
|
||||
const nextWord = nextSignificantToken(tokens, index)?.normalized ?? "";
|
||||
const previous = previousSignificantToken(tokens, index);
|
||||
const isBlockBegin = word === "begin" && tokenValue.depth === 0 && !NON_BLOCK_BEGIN.has(nextWord);
|
||||
// A `CASE` expression owns its `THEN`/`ELSE`/`END`; none of them is block syntax.
|
||||
const closesCase = word === "end" && tokenValue.depth === 0 && caseDepth > 0;
|
||||
const isBlockEnd = word === "end" && tokenValue.depth === 0 && caseDepth === 0;
|
||||
// `IF(...)` as a function call and `a = IF(...)` keep their line: only a
|
||||
// statement position starts a new one.
|
||||
const atStatementPosition = previous === undefined || (previous.kind === "word" && previous.normalized !== "select") || (previous.kind === "punctuation" && previous.text !== ",");
|
||||
// A `CASE` expression owns its `THEN`/`ELSE`/`END`; none of them is block syntax.
|
||||
const isStatementStart = STATEMENT_STARTERS.has(word) && headerDone && level > 0 && tokenValue.depth === 0 && caseDepth === 0 && previousWord !== "end" && atStatementPosition;
|
||||
const closesThenBlock = word === "else" && thenBlocks > 0;
|
||||
const breaksBeforeElse = word === "else" && caseDepth === 0;
|
||||
// The header's `AS`/`IS` separates the signature from the body and always
|
||||
// sits on a line of its own.
|
||||
const isHeaderSeparator = !headerDone && (word === "as" || word === "is") && tokenValue.depth === 0;
|
||||
// A signature stays on one line even when the printer split it: the clause
|
||||
// rules have no clause to hang a routine header on, so some dialects emit it
|
||||
// one token per line.
|
||||
const joinsHeader = !headerDone && !isHeaderSeparator && !isBlockBegin;
|
||||
// `BEGIN TRY` / `END CATCH` name their block with a second keyword, which
|
||||
// the clause layout treats as its own token and breaks away.
|
||||
const joinsBlockSuffix = (previousWord === "begin" && (word === "try" || word === "catch")) || (previousWord === "end" && END_SUFFIXES.has(word));
|
||||
|
||||
if (forceBreak || (gap.includes("\n") && !joinsHeader && !joinsBlockSuffix)) {
|
||||
startLine();
|
||||
forceBreak = false;
|
||||
}
|
||||
if (isStatementStart || isBlockBegin || isBlockEnd || closesThenBlock || isHeaderSeparator || breaksBeforeElse) startLine();
|
||||
if (isBlockEnd || closesThenBlock) level = Math.max(0, level - 1);
|
||||
|
||||
// The gap is empty exactly where the input had no whitespace (`a.b`, `(x)`),
|
||||
// so reusing it keeps the printer's spacing inside a line. A statement
|
||||
// terminator never takes a space, even where the printer emitted one.
|
||||
const isTerminator = tokenValue.kind === "punctuation" && tokenValue.text === ";";
|
||||
const spacing = isTerminator ? "" : gap.includes("\n") ? " " : gap;
|
||||
write((current.trim() ? spacing : "") + tokenValue.text);
|
||||
|
||||
if (closesCase) {
|
||||
caseDepth -= 1;
|
||||
} else if (isBlockBegin) {
|
||||
headerDone = true;
|
||||
level += 1;
|
||||
// `BEGIN TRY` / `BEGIN CATCH` name their block, so the suffix stays on the
|
||||
// keyword's line and the break happens after it instead.
|
||||
forceBreak = !END_SUFFIXES.has(nextWord);
|
||||
} else if (word === "try" || word === "catch") {
|
||||
forceBreak = true;
|
||||
} else if (isBlockEnd) {
|
||||
forceBreak = !END_SUFFIXES.has(nextWord) && nextSignificantToken(tokens, index)?.text !== ";";
|
||||
} else if (END_SUFFIXES.has(word) && previousWord === "end") {
|
||||
if (word === "if") thenBlocks = Math.max(0, thenBlocks - 1);
|
||||
forceBreak = nextSignificantToken(tokens, index)?.text !== ";";
|
||||
} else if (word === "case") {
|
||||
caseDepth += 1;
|
||||
} else if (word === "then" && tokenValue.depth === 0 && caseDepth === 0) {
|
||||
thenBlocks += 1;
|
||||
level += 1;
|
||||
forceBreak = true;
|
||||
} else if (word === "loop" && tokenValue.depth === 0 && previousWord !== "end") {
|
||||
level += 1;
|
||||
forceBreak = true;
|
||||
} else if (closesThenBlock) {
|
||||
level += 1;
|
||||
forceBreak = true;
|
||||
} else if (breaksBeforeElse) {
|
||||
forceBreak = true;
|
||||
} else if (tokenValue.kind === "punctuation" && tokenValue.text === ";" && tokenValue.depth === 0) {
|
||||
forceBreak = true;
|
||||
} else if (tokenValue.kind === "comment" && (tokenValue.text.startsWith("--") || tokenValue.text.startsWith("#"))) {
|
||||
forceBreak = true;
|
||||
} else if (isHeaderSeparator) {
|
||||
headerDone = true;
|
||||
forceBreak = true;
|
||||
}
|
||||
|
||||
if (word) previousWord = word;
|
||||
previousToken = tokenValue;
|
||||
}
|
||||
|
||||
flush();
|
||||
return lines.join("\n");
|
||||
}
|
||||
@@ -2,6 +2,7 @@ import { DEFAULT_SQL_FORMATTER_SETTINGS, normalizeSqlFormatterSettings, sqlForma
|
||||
import { formatSqlLayout, type SqlLayoutOptions } from "@/lib/sql/layout";
|
||||
import { looksLikeXml } from "@/lib/sql/autoFormat";
|
||||
import { compressCypherText, formatCypherText } from "@/lib/sql/cypherFormatter";
|
||||
import { indentRoutineBlocks } from "@/lib/sql/routineBlockLayout";
|
||||
import { tokenizeSqlSemantic } from "@/lib/sql/semantic/tokens";
|
||||
|
||||
export type SqlFormatDialect = "mysql" | "postgres" | "sqlite" | "sqlserver" | "oracle" | "clickhouse" | "dameng" | "duckdb" | "cypher" | "generic";
|
||||
@@ -571,14 +572,19 @@ export async function formatSqlText(sql: string, dialect: SqlFormatDialect = "ge
|
||||
return laidOut ?? formatWithFallback(input);
|
||||
};
|
||||
|
||||
// Routine bodies have no block nodes in sql-formatter's grammar, so the
|
||||
// clause layout prints their statements at column 0. Re-indent them once all
|
||||
// other passes have run, i.e. on text that is already the final SQL.
|
||||
const applyRoutineBlockLayout = (formatted: string): string => indentRoutineBlocks(formatted, dialect, normalizedSettings.tabWidth, normalizedSettings.useTabs);
|
||||
|
||||
try {
|
||||
return finalizeFormattedSql(await formatOnce(protectedInput.sql));
|
||||
return applyRoutineBlockLayout(finalizeFormattedSql(await formatOnce(protectedInput.sql)));
|
||||
} catch (err) {
|
||||
const trailingDot = dialect === "dameng" ? splitTrailingStandaloneDot(protectedInput.sql) : null;
|
||||
if (!trailingDot) throw err;
|
||||
|
||||
try {
|
||||
return finalizeFormattedSql(`${await formatOnce(trailingDot.body)}${trailingDot.suffix}`);
|
||||
return applyRoutineBlockLayout(finalizeFormattedSql(`${await formatOnce(trailingDot.body)}${trailingDot.suffix}`));
|
||||
} catch {
|
||||
throw err;
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user