fix(sql): keep select column trailing comments on the same line

This commit is contained in:
zipg
2026-10-19 02:51:38 +08:00
committed by GitHub
parent 0bad902bbd
commit d470f4f660
4 changed files with 69 additions and 17 deletions
+20 -4
View File
@@ -159,13 +159,13 @@ describe("sqlFormatter", () => {
});
it("keeps a view's trailing line comment from swallowing the next column (issue #10278)", async () => {
// A `--` comment consumes the rest of its line, so the column that follows
// it in the view text has to start a new line; appending it to the comment
// would silently drop it from the DDL shown to the user.
// A `--` comment consumes the rest of its line. Keeping it on the previous
// field's line is safe only when the following column starts a new line,
// otherwise the column would silently disappear from the DDL shown to the user.
const viewDdl = `CREATE VIEW "TEMP_TEST_VIEW" AS SELECT trunc(sysdate) AS dates, -- 测试\n(SELECT sysdate FROM dual t) AS nows\nFROM dual;`;
const formatted = await formatSqlForDisplay(viewDdl, sqlFormatDialectForDbType("oracle"));
expect(formatted).toBe('CREATE VIEW "TEMP_TEST_VIEW" AS\nSELECT trunc(sysdate) AS dates,\n -- 测试\n (SELECT sysdate FROM dual t) AS nows\nFROM dual;');
expect(formatted).toBe('CREATE VIEW "TEMP_TEST_VIEW" AS\nSELECT trunc(sysdate) AS dates, -- 测试\n (SELECT sysdate FROM dual t) AS nows\nFROM dual;');
});
it("keeps a short OceanBase Oracle view projection readable (issue #7540)", async () => {
@@ -505,6 +505,22 @@ LIMIT 100;`);
await expect(formatSqlForEditing("SELECT 1 .\n", "dameng")).resolves.toBe("SELECT 1 .\n");
});
it("keeps trailing SELECT comments on the field line (#12615)", async () => {
const source = `select a, -- first field\n b, -- second field\n c -- third field\nfrom t;`;
const formatted = await formatSqlForEditing(source, "postgres");
expect(formatted).toContain("SELECT a, -- first field\n");
expect(formatted).toContain(" b, -- second field\n");
expect(formatted).toContain(" c -- third field\nFROM t;");
});
it("keeps source-standalone SELECT comments on their own line", async () => {
const source = `select a,\n-- heading for b\n b\nfrom t;`;
const formatted = await formatSqlForEditing(source, "postgres");
expect(formatted).toContain("SELECT a,\n -- heading for b\n b\nFROM t;");
});
it("preserves the newline before a trailing dot after a line comment", async () => {
await expect(formatSqlForEditing("DELETE FROM accounts -- comment\n .", "dameng")).resolves.toBe("DELETE FROM accounts -- comment\n .");
});
+2 -2
View File
@@ -285,10 +285,10 @@ describe("sql layout", () => {
expect(await format("SELECT a FROM t UNION ALL SELECT b FROM u;")).toBe(lines("SELECT a", "FROM t", "UNION ALL", "SELECT b", "FROM u;"));
});
it("keeps comments and does not fold them onto a line", async () => {
it("keeps a trailing comment on its line without swallowing the next item", async () => {
const sql = lines("-- heading", "SELECT a, -- why", " b", "FROM t; /* trailing */");
expect(await format(sql)).toBe(lines("-- heading", "SELECT a,", " -- why", " b", "FROM t;", "", "/* trailing */"));
expect(await format(sql)).toBe(lines("-- heading", "SELECT a, -- why", " b", "FROM t;", "", "/* trailing */"));
});
it("keeps PostgreSQL casts tight", async () => {
+40 -3
View File
@@ -78,10 +78,35 @@ function printClause(writer: Writer, clause: ClauseNode, baseColumn: number, ctx
}
items.forEach((item, index) => {
// A line comment written after the previous field's comma is a trailing
// annotation for that field, not a separate heading. sql-formatter keeps
// its original preceding whitespace; only comments that did not start a
// new source line are moved back onto the preceding output line. Comments
// that were on their own line keep their own line.
const leadingComments = index > 0 && ctx.options.commaPosition !== "before" ? leadingTrailingComments(item) : 0;
let itemNodes = item;
let leadingText: string | null = null;
if (leadingComments > 0) {
// `renderInline` intentionally drops comments; a trailing comment is
// copied verbatim, and only when it fits the remaining line width.
const commentText = item
.slice(0, leadingComments)
.map((node) => ("text" in node ? node.text : ""))
.join(" ");
if (commentText && writer.column + 1 + commentText.length <= ctx.options.lineWidth) {
leadingText = commentText;
itemNodes = item.slice(leadingComments);
}
}
if (index === 0) {
writer.write(keyword);
if (sourceOnOwnLine) writer.newline(baseColumn + ctx.options.indentWidth);
else writer.space();
} else if (leadingText) {
writer.space();
writer.write(leadingText);
if (itemNodes.length === 0) return;
writer.newline(itemColumn);
} else {
if (ctx.options.commaPosition === "before") {
const leadingIndent = sourceOnOwnLine ? baseColumn + ctx.options.indentWidth : Math.max(baseColumn, itemColumn - 2);
@@ -91,22 +116,34 @@ function printClause(writer: Writer, clause: ClauseNode, baseColumn: number, ctx
writer.newline(itemColumn);
}
}
if (alignedAliases) {
if (alignedAliases && !leadingText) {
const aligned = alignedAliases[index];
if (aligned) {
writer.write(aligned.expression);
writer.write(" ".repeat(aligned.aliasColumn - aligned.expression.length));
writer.write(`${aligned.asKeyword} ${aligned.alias}`);
} else {
printElement(writer, item, baseColumn, childCtx);
printElement(writer, itemNodes, baseColumn, childCtx);
}
} else {
printElement(writer, item, baseColumn, childCtx);
printElement(writer, itemNodes, baseColumn, childCtx);
}
if (ctx.options.commaPosition !== "before" && index < items.length - 1) writer.write(",");
});
}
/** Count leading comments that were written on the previous source line. */
function leadingTrailingComments(item: AstNode[]): number {
let count = 0;
while (count < item.length) {
const node = item[count];
if (!node || (node.type !== "line_comment" && node.type !== "block_comment")) break;
if (/[\r\n]/.test(node.precedingWhitespace ?? "")) break;
count += 1;
}
return count;
}
interface AlignedProjectionItem {
expression: string;
alias: string;
+7 -8
View File
@@ -76,14 +76,13 @@ test("rejects very large SQL before loading formatter work", async () => {
await assert.rejects(() => formatSqlText("x".repeat(MAX_SQL_FORMAT_CHARS + 1), "generic"), /too large/i);
});
test("keeps a trailing line comment from swallowing the column after it", async () => {
// Oracle stores a view's text as written, so a comment at the end of a column
// line is followed by the next column on its own line. A `--` comment consumes
// the rest of its line, so the next column has to start a new line instead of
// being appended to that comment.
test("keeps a trailing line comment on its field line without swallowing the next column", async () => {
// A `--` comment consumes the rest of its source line. When the comment was
// written after the field's comma, keep it there and put the next column on
// a new line so it is still valid SQL (#12615).
const formatted = await formatSqlText('CREATE VIEW "TEMP_TEST_VIEW" AS SELECT trunc(sysdate) AS dates, -- 测试\n(SELECT sysdate FROM dual t) AS nows\nFROM dual;', "oracle", { keywordCase: "upper" });
assert.equal(formatted, 'CREATE VIEW "TEMP_TEST_VIEW" AS\nSELECT trunc(sysdate) AS dates,\n -- 测试\n (SELECT sysdate FROM dual t) AS nows\nFROM dual;');
assert.equal(formatted, 'CREATE VIEW "TEMP_TEST_VIEW" AS\nSELECT trunc(sysdate) AS dates, -- 测试\n (SELECT sysdate FROM dual t) AS nows\nFROM dual;');
});
test("keeps a line comment from swallowing the separator after it", async () => {
@@ -98,10 +97,10 @@ test("keeps a line comment from swallowing the statement terminator", async () =
assert.equal(formatted, "SELECT a -- c\n;");
});
test("formats a comment on its own line without extra breaks", async () => {
test("keeps a source trailing comment on the field line", async () => {
const formatted = await formatSqlText("select a, -- c\nb from t", "postgres", { keywordCase: "upper" });
assert.equal(formatted, "SELECT a,\n -- c\n b\nFROM t");
assert.equal(formatted, "SELECT a, -- c\n b\nFROM t");
});
test("compressSqlText collapses whitespace into single spaces", () => {