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; + `); + }); + }); });