Skip to content

Apply various formatting fixes #80

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
joelmukuthu wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Apply various formatting fixes #80

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
41a21c5
Break long WHEN/THEN clauses into separate lines
joelmukuthu Aug 10, 2026
3301083
Break long COMMENT ON clauses into separate lines
joelmukuthu Aug 10, 2026
8af6980
Break long CREATE INDEX clauses into separate lines
joelmukuthu Aug 10, 2026
9714459
Break long binary expressions into separate lines
joelmukuthu Aug 10, 2026
120b664
Avoid line-break between empty parenthesis
joelmukuthu Aug 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions src/syntax/comment.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
import { AllCommentNodes } from "sql-parser-cst";
import { group, indent, line } from "../print_utils";
import { group, line } from "../print_utils";
import { CstToDocMap } from "../CstToDocMap";

export const commentMap: CstToDocMap<AllCommentNodes> = {
comment_stmt: (print) =>
group([
print.spaced(["commentKw", "onKw", "target", "isKw"]),
indent([line, print("message")]),
print.spaced(["commentKw", "onKw"]),
line,
print("target"),
line,
print.spaced(["isKw", "message"]),
]),

comment_target_aggregate: (print) =>
Expand Down
44 changes: 41 additions & 3 deletions src/syntax/expr.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,13 @@ export const exprMap: CstToDocMap<AllExprNodes> = {
) {
return print("expr");
}
if (
isEmptyParenContent(node.expr) &&
!hasComments(node) &&
!hasComments(node.expr)
) {
return ["(", print("expr"), ")"];
}
const lineStyle =
isCreateTableStmt(parent) && print.dynamicLine() === hardline
? hardline
Expand All @@ -101,7 +108,10 @@ export const exprMap: CstToDocMap<AllExprNodes> = {
// Some operators are better formatted without spaces around them
return print(["left", "operator", "right"]);
}
return print.spaced(["left", "operator", "right"]);
return group([
print("left"),
group([" ", print.spaced("operator"), indent([line, print("right")])]),
]);
},
prefix_op_expr: (print, node) =>
(isString(node.operator) ? print : print.spaced)(["operator", "expr"]),
Expand All @@ -117,11 +127,19 @@ export const exprMap: CstToDocMap<AllExprNodes> = {
case_when: (print, node) => {
if (isProgram(node.result)) {
return [
print.spaced(["whenKw", "condition", "thenKw"]),
group([
group([print("whenKw"), indent([line, print("condition")])]),
line,
print("thenKw"),
]),
indent([hardline, stripTrailingHardline(print("result"))]),
];
}
return print.spaced(["whenKw", "condition", "thenKw", "result"]);
return group([
group([print("whenKw"), indent([line, print("condition")])]),
line,
group([print("thenKw"), indent([line, print("result")])]),
]);
},
case_else: (print, node) => {
if (isProgram(node.result)) {
Expand Down Expand Up @@ -317,3 +335,23 @@ const isFunctionContext = (
const isBooleanOp = ({ name }: Keyword) => name === "AND" || name === "OR";

const isCompactOp = (op: string) => op === "->" || op === "->>";

const isEmptyParenContent = (expr: Node): boolean => {

@nene nene Aug 20, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would turn this function into:

const isEmptyParenExpr = (expr: ParenExpr): boolean => {

Then all that logic for determining whether it's an empty parenthesis would live in one place, and also the name of the function would IMHO be easier to understand.

if (isFuncArgs(expr)) {
return (
expr.args.items.length === 0 &&
!expr.distinctKw &&
!expr.nullHandlingKw &&
!expr.orderBy &&
!expr.limit &&
!expr.having
Comment on lines +343 to +347

@joelmukuthu joelmukuthu Aug 10, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a way to avoid handling each case individually here?

@nene nene Aug 20, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At the moment I think not much can be done here. This needs some changes on the parser side to make it easier to handle within the prettier plugin.

);
}
if (isListExpr(expr)) {
return expr.items.length === 0;
}
return false;
};

const hasComments = (node: Node): boolean =>
Boolean((node as Node & { comments?: unknown[] }).comments?.length);
Comment on lines +356 to +357

@nene nene Aug 20, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A better way to implement this, would be:

const hasComments = (node: Node): boolean =>
  (node.leading?.length ?? 0) > 0 || (node.trailing?.length ?? 0) > 0;

The comments field is added to nodes by the Prettier engine. But the leading & trailing fields come directly from our parser.

Given that in here we really are interested from where the comments were in the original source code, we're better off using the leading & trailing. Plus we don't need the type-casts. Additionally it should be more resilient against changes in Prettier.

36 changes: 21 additions & 15 deletions src/syntax/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,24 +1,30 @@
import { AllIndexNodes } from "sql-parser-cst";
import { group, join } from "../print_utils";
import { group, join, line } from "../print_utils";
import { CstToDocMap } from "../CstToDocMap";

export const indexMap: CstToDocMap<AllIndexNodes> = {
create_index_stmt: (print) =>
create_index_stmt: (print, node) =>
group(
join(print.dynamicLine(), [
print.spaced([
"createKw",
"orReplaceKw",
"indexTypeKw",
"indexKw",
"concurrentlyKw",
"ifNotExistsKw",
"name",
"onKw",
"table",
"using",
"columns",
]),
group(
join(line, [
print.spaced([
"createKw",
"orReplaceKw",
"indexTypeKw",
"indexKw",
"concurrentlyKw",
"ifNotExistsKw",
"name",
]),
print.spaced([
"onKw",
"table",
...(node.using ? [] : (["columns"] as const)),
]),
...(node.using ? [print.spaced(["using", "columns"])] : []),
]),
),
...print("clauses"),
]),
),
Expand Down
3 changes: 2 additions & 1 deletion test/ddl/create_table.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -395,7 +395,8 @@ describe("create table", () => {
OPTIONS (
expiration_timestamp = TIMESTAMP "2025-01-01 00:00:00 UTC",
partition_expiration_days = 1,
description = "a table that expires in 2025, with each partition living for 24 hours",
description =
"a table that expires in 2025, with each partition living for 24 hours",
labels = [("org_unit", "development")]
)
`);
Expand Down
46 changes: 38 additions & 8 deletions test/ddl/index.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,19 +42,49 @@ describe("index", () => {
`);
});

it(`breaks long CREATE INDEX across CREATE, ON, and USING`, async () => {
await testPostgresql(
dedent`
CREATE INDEX my_index
ON my_table
USING btree (col)
`,
{ printWidth: 50 },
);
});

it(`breaks long CREATE UNIQUE INDEX across CREATE, ON, and USING`, async () => {
await testPostgresql(
dedent`
CREATE UNIQUE INDEX my_index
ON my_table
USING btree (
col_one,
col_two
)
`,
{ printWidth: 30 },
);
});

it(`formats long columns list on multiple lines`, async () => {
await test(dedent`
CREATE UNIQUE INDEX IF NOT EXISTS my_index ON my_table (
column_name_one,
column_name_two,
column_name_three
)
`);
await test(
dedent`
CREATE UNIQUE INDEX IF NOT EXISTS my_index
ON my_table (
col_one,
col_two,
col_three
)
`,
{ printWidth: 40 },
);
});

it(`formats column list with various index parameters`, async () => {
await testPostgresql(dedent`
CREATE INDEX my_index ON my_table (
CREATE INDEX my_index
ON my_table (
column_name_one COLLATE "C" ASC NULLS FIRST,
column_name_two DESC NULLS LAST,
(col3 + col4) my_opclass (foo = 'bar', baz = 'qux') ASC
Expand Down
80 changes: 80 additions & 0 deletions test/expr/expr.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,27 @@ describe("expr", () => {
`);
});

it(`keeps short binary expressions on one line`, async () => {
await test(dedent`
SELECT *
FROM foo
WHERE bar = short_func()
`);
});

it(`breaks long binary expressions into multiple lines with indentation`, async () => {
await test(
dedent`
SELECT *
FROM foo
WHERE
bar =
my_func()
`,
{ printWidth: 15 },
);
});

it(`formats IN expressions`, async () => {
await test(`SELECT col1 IN (1, 2, 3), col2 NOT IN (4, 5, 6)`);
});
Expand Down Expand Up @@ -171,6 +192,65 @@ describe("expr", () => {
END
`);
});

it(`breaks long WHEN/THEN into separate lines`, async () => {
await test(
dedent`
SELECT
CASE
WHEN column_name = 1
THEN result_name
END
`,
{ printWidth: 40 },
);
});

it(`breaks multiple long WHEN/THEN clauses without blank lines between them`, async () => {
await test(
dedent`
SELECT
CASE
WHEN column_name = 1
THEN result_name
WHEN column_name = 2
THEN other_result
ELSE foo
Comment on lines +212 to +218

@nene nene Aug 20, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find this not so easy to read, because WHEN and THEN blocks are indented the same amount. IMHO it's sort of like formatting if-else in some other language like so:

if x > 10
return 15
else if x < 10
return 20

It also doesn't match with how the procedural version of CASE expression gets formatted:

CASE
  WHEN column_name = 1 THEN
    SELECT \good'
  WHEN columne_name = 2 THEN
    SELECT 'bad'
  ELSE
    SELECT 'other'
END CASE

See case.test.ts

I would go with similar indentation for the long WHEN..THEN blocks in general:

CASE
  WHEN column_name = 1 THEN
    result_name
  WHEN columne_name = 2 THEN
    other_result
  ELSE
    foo
END CASE

END
`,
{ printWidth: 40 },
);
});

it(`indents multi-condition WHEN clauses and keeps ORs parenthesized`, async () => {
await test(
dedent`
SELECT
CASE
WHEN
column_name = 1
AND (other_name = 2 OR other_name = 3)
THEN result_name
END
`,
{ printWidth: 50 },
);
});

it(`indents multi-expression THEN clauses and keeps ORs parenthesized`, async () => {
await test(
dedent`
SELECT
CASE
WHEN column_name = 1
THEN
result_name = 1
AND (other_name = 2 OR other_name = 3)
END
`,
{ printWidth: 45 },
);
});
});

it(`formats quantifier expressions`, async () => {
Expand Down
60 changes: 60 additions & 0 deletions test/expr/func.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,66 @@ describe("functions", () => {
`);
});

it(`keeps empty function args on one line`, async () => {
expect(await pretty(`SELECT my_func()`, { printWidth: 10 })).toBe(dedent`
SELECT
my_func()
`);
});

it(`does not treat count(DISTINCT) as empty function args`, async () => {
expect(await pretty(`SELECT count(DISTINCT id)`, { printWidth: 25 }))
.toBe(dedent`
SELECT
count(DISTINCT id)
`);
});

it(`keeps empty CREATE FUNCTION params on one line`, async () => {
expect(
await pretty(
`CREATE FUNCTION my_func() AS (SELECT 1)`,
{ printWidth: 10, dialect: "bigquery" },
),
).toBe(dedent`
CREATE FUNCTION my_func() AS
(
SELECT
1
)
`);
});

it(`preserves block comments inside empty function args`, async () => {
expect(
await pretty(`SELECT my_func(/* comment */)`, { printWidth: 25 }),
).toBe(dedent`
SELECT
my_func(
/* comment */
)
`);
});

it(`preserves line comments inside empty function args`, async () => {
expect(
await pretty(
dedent`
SELECT my_func(
-- comment
)
`,
{ printWidth: 25 },
),
).toBe(dedent`
SELECT
my_func(
-- comment

)
`);
});

it(`formats named function arguments`, async () => {
await testBigquery(
`SELECT concat_lower_or_upper(a => 'Hello', b => 'World', uppercase => TRUE)`,
Expand Down
Loading
Loading