From 0a022f57cb8e642978dbdda5ffb135fb765981a9 Mon Sep 17 00:00:00 2001 From: Jeremy-xuan <2938717844@qq.com> Date: Sat, 19 Sep 2026 15:30:14 +0800 Subject: [PATCH] fix: ignore comments when post-processing tokens The dialect postProcess() steps look at the tokens adjacent to the one they are inspecting. Comments are tokens too, so a comment between two syntax elements used to defeat these checks: - BigQuery threw a Parse error on arr[/* c */ OFFSET(0)], because detectArraySubscripts() only recognized OFFSET when the previous token was literally "[", leaving OFFSET a clause token. - ClickHouse formatted EXPLAIN AST /* c */ SELECT and CHECK GRANT /* c */ SELECT differently than without the comment, because the placement of SELECT looks at a neighbouring clause or comma. - Spark formatted window /* c */ (time) as a WINDOW clause instead of a function call, because the function check compares the next token with "(". - MySQL/MariaDB/TiDB/SingleStoreDB stopped treating SET( and VALUES() as functions, because the check compares the next token with "(". These checks are about the surrounding syntax, so they now skip comment tokens, using the same idea as the existing plsql postProcess which already tracks the previous reserved token instead of the previous token. --- src/languages/bigquery/bigquery.formatter.ts | 14 +++--- .../clickhouse/clickhouse.formatter.ts | 12 ++++-- src/languages/mariadb/likeMariaDb.ts | 12 ++++-- src/languages/spark/spark.formatter.ts | 12 ++++-- src/lexer/token.ts | 26 +++++++++++ test/behavesLikeMariaDbFormatter.ts | 18 ++++++++ test/bigquery.test.ts | 23 ++++++++++ test/clickhouse.test.ts | 43 +++++++++++++++++++ test/mysql.test.ts | 8 ++++ test/spark.test.ts | 22 ++++++++++ 10 files changed, 173 insertions(+), 17 deletions(-) diff --git a/src/languages/bigquery/bigquery.formatter.ts b/src/languages/bigquery/bigquery.formatter.ts index 398951015a..fb92dca999 100644 --- a/src/languages/bigquery/bigquery.formatter.ts +++ b/src/languages/bigquery/bigquery.formatter.ts @@ -1,6 +1,6 @@ import { DialectOptions } from '../../dialect.js'; import { expandPhrases } from '../../expandPhrases.js'; -import { EOF_TOKEN, isToken, Token, TokenType } from '../../lexer/token.js'; +import { isToken, prevNonCommentToken, Token, TokenType } from '../../lexer/token.js'; import { functions } from './bigquery.functions.js'; import { dataTypes, keywords } from './bigquery.keywords.js'; @@ -204,16 +204,14 @@ function postProcess(tokens: Token[]): Token[] { // Converts OFFSET token inside array from RESERVED_CLAUSE to RESERVED_FUNCTION_NAME // See: https://cloud.google.com/bigquery/docs/reference/standard-sql/functions-and-operators#array_subscript_operator +// Comments between the "[" and OFFSET are skipped, so "arr[/* c */ OFFSET(0)]" +// is recognized the same way as "arr[OFFSET(0)]". function detectArraySubscripts(tokens: Token[]) { - let prevToken = EOF_TOKEN; - return tokens.map(token => { - if (token.text === 'OFFSET' && prevToken.text === '[') { - prevToken = token; + return tokens.map((token, i) => { + if (token.text === 'OFFSET' && prevNonCommentToken(tokens, i).text === '[') { return { ...token, type: TokenType.RESERVED_FUNCTION_NAME }; - } else { - prevToken = token; - return token; } + return token; }); } diff --git a/src/languages/clickhouse/clickhouse.formatter.ts b/src/languages/clickhouse/clickhouse.formatter.ts index 71aa28b32d..106ecd96f0 100644 --- a/src/languages/clickhouse/clickhouse.formatter.ts +++ b/src/languages/clickhouse/clickhouse.formatter.ts @@ -1,6 +1,12 @@ import { DialectOptions } from '../../dialect.js'; import { expandPhrases } from '../../expandPhrases.js'; -import { EOF_TOKEN, isToken, Token, TokenType } from '../../lexer/token.js'; +import { + isToken, + nextNonCommentToken, + prevNonCommentToken, + Token, + TokenType, +} from '../../lexer/token.js'; import { functions } from './clickhouse.functions.js'; import { dataTypes, keywords } from './clickhouse.keywords.js'; @@ -303,8 +309,8 @@ export const clickhouse: DialectOptions = { */ function postProcess(tokens: Token[]): Token[] { return tokens.map((token, i) => { - const nextToken = tokens[i + 1] || EOF_TOKEN; - const prevToken = tokens[i - 1] || EOF_TOKEN; + const nextToken = nextNonCommentToken(tokens, i); + const prevToken = prevNonCommentToken(tokens, i); // If we have queries like // > GRANT SELECT, INSERT ON db.table TO john diff --git a/src/languages/mariadb/likeMariaDb.ts b/src/languages/mariadb/likeMariaDb.ts index 63c7cba1f7..67df4f2193 100644 --- a/src/languages/mariadb/likeMariaDb.ts +++ b/src/languages/mariadb/likeMariaDb.ts @@ -1,15 +1,21 @@ -import { EOF_TOKEN, isToken, Token, TokenType } from '../../lexer/token.js'; +import { + isToken, + nextNonCommentToken, + prevNonCommentToken, + Token, + TokenType, +} from '../../lexer/token.js'; // Shared functionality used by all MariaDB-like SQL dialects. export function postProcess(tokens: Token[]) { return tokens.map((token, i) => { - const nextToken = tokens[i + 1] || EOF_TOKEN; + const nextToken = nextNonCommentToken(tokens, i); if (isToken.SET(token) && nextToken.text === '(') { // This is SET datatype, not SET statement return { ...token, type: TokenType.RESERVED_FUNCTION_NAME }; } - const prevToken = tokens[i - 1] || EOF_TOKEN; + const prevToken = prevNonCommentToken(tokens, i); if (isToken.VALUES(token) && prevToken.text === '=') { // This is VALUES() function, not VALUES clause return { ...token, type: TokenType.RESERVED_FUNCTION_NAME }; diff --git a/src/languages/spark/spark.formatter.ts b/src/languages/spark/spark.formatter.ts index 192675eadb..c288917756 100644 --- a/src/languages/spark/spark.formatter.ts +++ b/src/languages/spark/spark.formatter.ts @@ -1,6 +1,12 @@ import { DialectOptions } from '../../dialect.js'; import { expandPhrases } from '../../expandPhrases.js'; -import { EOF_TOKEN, isToken, Token, TokenType } from '../../lexer/token.js'; +import { + isToken, + nextNonCommentToken, + prevNonCommentToken, + Token, + TokenType, +} from '../../lexer/token.js'; import { dataTypes, keywords } from './spark.keywords.js'; import { functions } from './spark.functions.js'; @@ -155,8 +161,8 @@ export const spark: DialectOptions = { function postProcess(tokens: Token[]) { return tokens.map((token, i) => { - const prevToken = tokens[i - 1] || EOF_TOKEN; - const nextToken = tokens[i + 1] || EOF_TOKEN; + const prevToken = prevNonCommentToken(tokens, i); + const nextToken = nextNonCommentToken(tokens, i); // [WINDOW](...) if (isToken.WINDOW(token) && nextToken.type === TokenType.OPEN_PAREN) { diff --git a/src/lexer/token.ts b/src/lexer/token.ts index 345a46e220..fff70d0648 100644 --- a/src/lexer/token.ts +++ b/src/lexer/token.ts @@ -111,3 +111,29 @@ export const isReserved = (type: TokenType): boolean => export const isLogicalOperator = (type: TokenType): boolean => type === TokenType.AND || type === TokenType.OR || type === TokenType.XOR; + +/** Checks if token is a comment */ +export const isComment = (type: TokenType): boolean => + type === TokenType.LINE_COMMENT || + type === TokenType.BLOCK_COMMENT || + type === TokenType.DISABLE_COMMENT; + +/** Returns the closest non-comment token preceding the given index, or EOF_TOKEN. */ +export const prevNonCommentToken = (tokens: Token[], index: number): Token => { + for (let i = index - 1; i >= 0; i--) { + if (!isComment(tokens[i].type)) { + return tokens[i]; + } + } + return EOF_TOKEN; +}; + +/** Returns the closest non-comment token following the given index, or EOF_TOKEN. */ +export const nextNonCommentToken = (tokens: Token[], index: number): Token => { + for (let i = index + 1; i < tokens.length; i++) { + if (!isComment(tokens[i].type)) { + return tokens[i]; + } + } + return EOF_TOKEN; +}; diff --git a/test/behavesLikeMariaDbFormatter.ts b/test/behavesLikeMariaDbFormatter.ts index 1a496df175..1218ee95fe 100644 --- a/test/behavesLikeMariaDbFormatter.ts +++ b/test/behavesLikeMariaDbFormatter.ts @@ -191,4 +191,22 @@ export default function behavesLikeMariaDbFormatter(format: FormatFn) { GRANT ALL ON *.* TO user2; `); }); + // Comments between a function name and its parenthesis must not stop the + // dialect post-processing from recognizing the function. + it('formats VALUES() as a function with a comment before it', () => { + expect(format('UPDATE t SET a = /*x*/ VALUES(b);')).toBe(dedent` + UPDATE t + SET + a = /*x*/ VALUES(b); + `); + }); + + it('formats VALUES() as a function with a line comment before it', () => { + expect(format('UPDATE t SET a = --x\nVALUES(b);')).toBe(dedent` + UPDATE t + SET + a = --x + VALUES(b); + `); + }); } diff --git a/test/bigquery.test.ts b/test/bigquery.test.ts index d92e089b67..4edcf6e729 100644 --- a/test/bigquery.test.ts +++ b/test/bigquery.test.ts @@ -636,4 +636,27 @@ describe('BigQueryFormatter', () => { expect(format(input, { linesBetweenQueries: 0 })).toBe(input); }); }); + describe('BigQuery array subscripts with comments', () => { + // A comment between "[" and OFFSET used to leave OFFSET as a clause token, + // which made the parser throw a Parse error. + it('formats a block comment before OFFSET', () => { + expect(format('SELECT arr[/* c */ OFFSET(0)] FROM t;')).toBe(dedent` + SELECT + arr[/* c */ OFFSET(0)] + FROM + t; + `); + }); + + it('formats a line comment before OFFSET', () => { + expect(format('SELECT arr[-- c\nOFFSET(0)] FROM t;')).toBe(dedent` + SELECT + arr[ -- c + OFFSET(0) + ] + FROM + t; + `); + }); + }); }); diff --git a/test/clickhouse.test.ts b/test/clickhouse.test.ts index 225b0480f0..9da08d4e19 100644 --- a/test/clickhouse.test.ts +++ b/test/clickhouse.test.ts @@ -1737,4 +1737,47 @@ describe('ClickhouseFormatter', () => { tuple(); `); }); + describe('comments in post-processed statements', () => { + // Comments between tokens must not affect the post-processing that decides + // whether SELECT is a clause or a privilege keyword. + it('formats CHECK GRANT with a comment before SELECT', () => { + expect(format('CHECK GRANT /* c */ SELECT ON db.table')).toBe(dedent` + CHECK GRANT + /* c */ + SELECT ON db.table + `); + }); + + // Comments must not stop SET( from being recognized as a function. + it('formats SET( as a function with a comment before the parens', () => { + expect(format('SELECT SET /* c */ (100) FROM t;')).toBe(dedent` + SELECT + SET/* c */ (100) + FROM + t; + `); + }); + + it('formats EXPLAIN AST with a comment before SELECT', () => { + expect(format('EXPLAIN AST /*x*/ SELECT 1;')).toBe(dedent` + EXPLAIN AST /*x*/ SELECT 1; + `); + }); + + it('formats EXPLAIN AST with a line comment before SELECT', () => { + expect(format('EXPLAIN AST --x\nSELECT 1;')).toBe(dedent` + EXPLAIN AST --x + SELECT 1; + `); + }); + + it('formats GRANT with a comment before SELECT', () => { + expect(format('GRANT /*x*/ SELECT ON db.table TO john;')).toBe(dedent` + GRANT + /*x*/ + SELECT ON db.table + TO john; + `); + }); + }); }); diff --git a/test/mysql.test.ts b/test/mysql.test.ts index e6bcd37af9..9a7b3ba864 100644 --- a/test/mysql.test.ts +++ b/test/mysql.test.ts @@ -114,4 +114,12 @@ describe('MySqlFormatter', () => { DROP DEFAULT; `); }); + describe('comments in post-processed statements', () => { + // Comments must not stop SET( from being recognized as a function. + it('formats SET( as a function with a comment before the parens', () => { + expect(format('CREATE TABLE t (a SET /* c */ (1,2));')).toBe(dedent` + CREATE TABLE t (a SET/* c */ (1, 2)); + `); + }); + }); }); diff --git a/test/spark.test.ts b/test/spark.test.ts index 62a9786eea..1162818fa6 100644 --- a/test/spark.test.ts +++ b/test/spark.test.ts @@ -152,4 +152,26 @@ describe('SparkFormatter', () => { ALTER COLUMN FirstName COMMENT "new comment"; `); }); + describe('Spark WINDOW() with comments', () => { + // A comment between WINDOW and its parenthesis used to leave WINDOW a clause + // token, so it was formatted as a clause instead of a function call. + it('formats WINDOW() as a function with a block comment before the parens', () => { + expect(format('SELECT window /*x*/ (time) FROM tbl;')).toBe(dedent` + SELECT + window/*x*/ (time) + FROM + tbl; + `); + }); + + it('formats WINDOW() as a function with a line comment before the parens', () => { + expect(format('SELECT window --x\n(time) FROM tbl;')).toBe(dedent` + SELECT + window --x + (time) + FROM + tbl; + `); + }); + }); });