From 89459775d6b2479cb37c01edc1759b6c05c594a4 Mon Sep 17 00:00:00 2001 From: Jeremy-xuan <2938717844@qq.com> Date: Sat, 19 Sep 2026 16:48:01 +0800 Subject: [PATCH] fix: avoid gluing a slash onto a comment in dialects where // starts a line comment Layout.add() had a guard that kept a layout item starting with "-" from being glued onto one ending with "-", because that forms "--", which re-parses as a line comment and swallows the rest of the line. The same hazard exists for any other line comment marker a dialect uses, and Snowflake is the only dialect where "//" starts a line comment: SELECT a / /* c */ b FROM t; with denseOperators came out as SELECT a//* c */ b FROM t; so "b" became comment text and the statement quietly changed meaning. It also swallowed a following /* sql-formatter-disable */ marker. The guard now works off the dialect's own lineCommentTypes instead of a hard-coded "--", following the same path already used for identifierDashes. Behavior for "--" is unchanged, and dialects that do not use "//" are unaffected. --- src/dialect.ts | 1 + src/formatter/ExpressionFormatter.ts | 4 ++++ src/formatter/Formatter.ts | 5 +++- src/formatter/Layout.ts | 32 ++++++++++++++++++++------ test/snowflake.test.ts | 34 ++++++++++++++++++++++++++++ 5 files changed, 68 insertions(+), 8 deletions(-) diff --git a/src/dialect.ts b/src/dialect.ts index 5a4b40ec77..82228841dc 100644 --- a/src/dialect.ts +++ b/src/dialect.ts @@ -47,4 +47,5 @@ const processDialectFormatOptions = ({ (options.tabularOnelineClauses ?? options.onelineClauses).map(name => [name, true]) ), identifierDashes: Boolean(tokenizerOptions.identChars?.dashes), + lineCommentTypes: tokenizerOptions.lineCommentTypes ?? ['--'], }); diff --git a/src/formatter/ExpressionFormatter.ts b/src/formatter/ExpressionFormatter.ts index 2c1fdc634c..9c71f99db9 100644 --- a/src/formatter/ExpressionFormatter.ts +++ b/src/formatter/ExpressionFormatter.ts @@ -66,6 +66,10 @@ export interface ProcessedDialectFormatOptions { // In such dialects the "-" operator must keep its surrounding spaces, // otherwise "a - b" densed to "a-b" would re-parse as a single identifier. identifierDashes: boolean; + // Line comment markers of the dialect (e.g. "--", "//", "#"). Used by Layout + // to avoid gluing a layout item onto a preceding one in a way that would + // form a line comment and swallow the rest of the line. + lineCommentTypes: string[]; } /** Formats a generic SQL expression */ diff --git a/src/formatter/Formatter.ts b/src/formatter/Formatter.ts index 8f10f87791..ce731670d9 100644 --- a/src/formatter/Formatter.ts +++ b/src/formatter/Formatter.ts @@ -48,7 +48,10 @@ export default class Formatter { cfg: this.cfg, dialectCfg: this.dialect.formatOptions, params: this.params, - layout: new Layout(new Indentation(indentString(this.cfg))), + layout: new Layout( + new Indentation(indentString(this.cfg)), + this.dialect.formatOptions.lineCommentTypes + ), }).format(statement.children); if (!statement.hasSemicolon) { diff --git a/src/formatter/Layout.ts b/src/formatter/Layout.ts index 39fd4071b7..877fc7f943 100644 --- a/src/formatter/Layout.ts +++ b/src/formatter/Layout.ts @@ -25,7 +25,12 @@ export type LayoutItem = WS.SPACE | WS.SINGLE_INDENT | WS.NEWLINE | WS.MANDATORY export default class Layout { private items: LayoutItem[] = []; - constructor(public indentation: Indentation) {} + constructor( + public indentation: Indentation, + // Line comment markers of the dialect. Any two adjacent layout items whose + // boundary would form one of these must stay separated. + private lineCommentTypes: string[] = ['--'] + ) {} /** * Appends token strings and whitespace modifications to SQL string. @@ -57,10 +62,12 @@ export default class Layout { this.items.push(WS.SINGLE_INDENT); break; default: - // Don't glue a layout item starting with "-" directly onto one ending with - // "-": that forms "--", which re-parses as a line comment and - // swallows the rest of the line (e.g. densing "a - -b" into "a--b"). - if (item.startsWith('-') && this.lastItemEndsWith('-')) { + // Don't glue two layout items together when the boundary forms a line + // comment marker of this dialect: the result would re-parse as a + // comment and swallow the rest of the line (e.g. densing "a - -b" + // into "a--b", or a "/" operator directly before a block comment in + // Snowflake, where "//" starts a line comment). + if (this.wouldFormLineComment(item)) { this.items.push(WS.SPACE); } this.items.push(item); @@ -68,9 +75,20 @@ export default class Layout { } } - private lastItemEndsWith(suffix: string): boolean { + /** + * True when appending the given item directly onto the last one would form a + * line comment marker of this dialect, e.g. "-" after "-" in every dialect, + * or "/" after "/" in dialects where "//" starts a comment. + */ + private wouldFormLineComment(item: string): boolean { const lastItem = last(this.items); - return typeof lastItem === 'string' && lastItem.endsWith(suffix); + if (typeof lastItem !== 'string') { + return false; + } + return this.lineCommentTypes.some( + marker => + marker.length > 1 && lastItem.endsWith(marker[0]) && item.startsWith(marker.slice(1)) + ); } private trimHorizontalWhitespace() { diff --git a/test/snowflake.test.ts b/test/snowflake.test.ts index a4c23913fd..5bc89364c2 100644 --- a/test/snowflake.test.ts +++ b/test/snowflake.test.ts @@ -221,4 +221,38 @@ describe('SnowflakeFormatter', () => { CREATE TABLE identifier($foo); `); }); + describe('Snowflake line comments', () => { + // Snowflake is the only dialect where "//" starts a line comment, so a "/" + // operator directly in front of a comment used to glue into "//" and turn + // the rest of the line into comment text. + it('does not glue a slash operator onto a following block comment', () => { + const result = format('SELECT a / /* c */ b FROM t;', { denseOperators: true }); + expect(result).toBe(dedent` + SELECT + a/ /* c */ b + FROM + t; + `); + // The regression: "b" used to become part of the "//" comment. + expect(result).toContain('b'); + expect(result).not.toContain('a//'); + }); + + it('does not glue a slash operator onto a formatter-disable comment', () => { + const result = format('SELECT a / /* sql-formatter-disable */ b FROM t;', { + denseOperators: true, + }); + expect(result).toContain('a/ /* sql-formatter-disable */ b'); + expect(result).not.toContain('a//'); + }); + + it('keeps densing a plain division', () => { + expect(format('SELECT a / b FROM t;', { denseOperators: true })).toBe(dedent` + SELECT + a/b + FROM + t; + `); + }); + }); });