From 767e9beb2c0debcdd432b9fbb38db7259b8e54d3 Mon Sep 17 00:00:00 2001 From: MSugiura Date: Thu, 11 Jun 2026 22:54:55 +0900 Subject: [PATCH] fix: preserve formatter comments around SQL expressions --- .changeset/fix-comment-formatting.md | 5 + packages/core/src/parsers/HavingParser.ts | 9 +- .../core/src/parsers/JoinOnClauseParser.ts | 7 + .../core/src/parsers/LimitClauseParser.ts | 9 +- .../core/src/parsers/OffsetClauseParser.ts | 7 + .../core/src/parsers/SqlPrintTokenParser.ts | 36 +- packages/core/src/transformers/SqlPrinter.ts | 174 +++++++- .../CommentStyle.comprehensive.test.ts | 419 ++++++++++++++++++ ...atter.comment-exact-transformation.test.ts | 29 +- .../SqlFormatter.comment-order.test.ts | 8 +- ...ter.comprehensive-comment-fulltext.test.ts | 12 +- .../SqlFormatter.comprehensive.test.ts | 4 +- 12 files changed, 685 insertions(+), 34 deletions(-) create mode 100644 .changeset/fix-comment-formatting.md diff --git a/.changeset/fix-comment-formatting.md b/.changeset/fix-comment-formatting.md new file mode 100644 index 000000000..bd929eee8 --- /dev/null +++ b/.changeset/fix-comment-formatting.md @@ -0,0 +1,5 @@ +--- +"rawsql-ts": patch +--- + +Fix SQL formatter comment handling for comma-prefixed expressions, LIMIT/OFFSET values, function arguments, and parenthesized predicates. Comments are no longer duplicated around function arguments, ORDER BY/GROUP BY items, and parenthesized WHERE predicates, comments after HAVING/JOIN ON/LIMIT/OFFSET keywords are preserved, commented function arguments expand to a readable multiline layout, comments after AND/OR operators indent the following predicate, and comments after list commas now use readable before-comma and after-comma layouts for SELECT, ORDER BY, and GROUP BY items. diff --git a/packages/core/src/parsers/HavingParser.ts b/packages/core/src/parsers/HavingParser.ts index 97b8f14c2..48317672b 100644 --- a/packages/core/src/parsers/HavingParser.ts +++ b/packages/core/src/parsers/HavingParser.ts @@ -27,6 +27,7 @@ export class HavingClauseParser { if (lexemes[idx].value !== 'having') { throw new Error(`Syntax error at position ${idx}: Expected 'HAVING' keyword but found "${lexemes[idx].value}". HAVING clauses must start with the HAVING keyword.`); } + const havingKeywordComments = lexemes[idx].positionedComments; idx++; if (idx >= lexemes.length) { @@ -34,8 +35,14 @@ export class HavingClauseParser { } const item = ValueParser.parseFromLexeme(lexemes, idx); + const afterKeywordComments = havingKeywordComments + ?.filter(comment => comment.position === 'after') + .flatMap(comment => comment.comments) ?? []; + if (afterKeywordComments.length > 0) { + item.value.addPositionedComments('before', afterKeywordComments); + } const clause = new HavingClause(item.value); return { value: clause, newIndex: item.newIndex }; } -} \ No newline at end of file +} diff --git a/packages/core/src/parsers/JoinOnClauseParser.ts b/packages/core/src/parsers/JoinOnClauseParser.ts index 57da9e8a8..d5c32b05f 100644 --- a/packages/core/src/parsers/JoinOnClauseParser.ts +++ b/packages/core/src/parsers/JoinOnClauseParser.ts @@ -6,9 +6,16 @@ export class JoinOnClauseParser { public static tryParse(lexemes: Lexeme[], index: number): { value: JoinOnClause; newIndex: number } | null { let idx = index; if (idx < lexemes.length && lexemes[idx].value === 'on') { + const onKeywordComments = lexemes[idx].positionedComments; idx++; // Skip 'on' keyword // Parse the condition expression const condition = ValueParser.parseFromLexeme(lexemes, idx); + const afterKeywordComments = onKeywordComments + ?.filter(comment => comment.position === 'after') + .flatMap(comment => comment.comments) ?? []; + if (afterKeywordComments.length > 0) { + condition.value.addPositionedComments('before', afterKeywordComments); + } idx = condition.newIndex; const joinOn = new JoinOnClause(condition.value); return { value: joinOn, newIndex: idx }; diff --git a/packages/core/src/parsers/LimitClauseParser.ts b/packages/core/src/parsers/LimitClauseParser.ts index de7cb642a..8e6ccfd5b 100644 --- a/packages/core/src/parsers/LimitClauseParser.ts +++ b/packages/core/src/parsers/LimitClauseParser.ts @@ -27,6 +27,7 @@ export class LimitClauseParser { if (lexemes[idx].value !== 'limit') { throw new Error(`Syntax error at position ${idx}: Expected 'LIMIT' keyword but found "${lexemes[idx].value}". LIMIT clauses must start with the LIMIT keyword.`); } + const limitKeywordComments = lexemes[idx].positionedComments; idx++; if (idx >= lexemes.length) { @@ -35,10 +36,16 @@ export class LimitClauseParser { // Parse LIMIT value const limitItem = ValueParser.parseFromLexeme(lexemes, idx); + const afterKeywordComments = limitKeywordComments + ?.filter(comment => comment.position === 'after') + .flatMap(comment => comment.comments) ?? []; + if (afterKeywordComments.length > 0) { + limitItem.value.addPositionedComments('before', afterKeywordComments); + } idx = limitItem.newIndex; const clause = new LimitClause(limitItem.value); return { value: clause, newIndex: idx }; } -} \ No newline at end of file +} diff --git a/packages/core/src/parsers/OffsetClauseParser.ts b/packages/core/src/parsers/OffsetClauseParser.ts index 8600cd120..97938a065 100644 --- a/packages/core/src/parsers/OffsetClauseParser.ts +++ b/packages/core/src/parsers/OffsetClauseParser.ts @@ -27,6 +27,7 @@ export class OffsetClauseParser { if (lexemes[idx].value !== 'offset') { throw new Error(`Syntax error at position ${idx}: Expected 'OFFSET' keyword but found "${lexemes[idx].value}". OFFSET clauses must start with the OFFSET keyword.`); } + const offsetKeywordComments = lexemes[idx].positionedComments; idx++; if (idx >= lexemes.length) { @@ -35,6 +36,12 @@ export class OffsetClauseParser { // Parse OFFSET value const offsetItem = ValueParser.parseFromLexeme(lexemes, idx); + const afterKeywordComments = offsetKeywordComments + ?.filter(comment => comment.position === 'after') + .flatMap(comment => comment.comments) ?? []; + if (afterKeywordComments.length > 0) { + offsetItem.value.addPositionedComments('before', afterKeywordComments); + } idx = offsetItem.newIndex; // If there is a "row" or "rows" command, skip it diff --git a/packages/core/src/parsers/SqlPrintTokenParser.ts b/packages/core/src/parsers/SqlPrintTokenParser.ts index 6db2383ff..e786e1b65 100644 --- a/packages/core/src/parsers/SqlPrintTokenParser.ts +++ b/packages/core/src/parsers/SqlPrintTokenParser.ts @@ -265,6 +265,7 @@ export class SqlPrintTokenParser implements SqlComponentVisitor { private sourceAliasStyle: SourceAliasStyle; private orderByDefaultDirectionStyle: OrderByDefaultDirectionStyle; private readonly normalizeJoinConditionOrder: boolean; + private readonly listContinuationCommentComponents = new WeakSet(); private joinConditionContexts: Array<{ aliasOrder: Map }> = []; constructor(options?: { @@ -491,6 +492,7 @@ export class SqlPrintTokenParser implements SqlComponentVisitor { private visitQualifiedName(arg: QualifiedName): SqlPrintToken { const token = new SqlPrintToken(SqlPrintTokenType.container, '', SqlPrintTokenContainerType.QualifiedName); + const hasOwnComments = this.hasPositionedComments(arg) || this.hasLegacyComments(arg); if (arg.namespaces) { for (let i = 0; i < arg.namespaces.length; i++) { @@ -517,7 +519,7 @@ export class SqlPrintTokenParser implements SqlComponentVisitor { arg.name.comments = originalNameLegacyComments; // Apply the name's comments to the qualified name token - if (this.hasPositionedComments(arg.name) || this.hasLegacyComments(arg.name)) { + if (!hasOwnComments && (this.hasPositionedComments(arg.name) || this.hasLegacyComments(arg.name))) { this.addComponentComments(token, arg.name); } @@ -1120,8 +1122,22 @@ export class SqlPrintTokenParser implements SqlComponentVisitor { private visitColumnReference(arg: ColumnReference): SqlPrintToken { const token = new SqlPrintToken(SqlPrintTokenType.container, '', SqlPrintTokenContainerType.ColumnReference); + const hasOwnComments = this.hasPositionedComments(arg) || this.hasLegacyComments(arg); + const originalNamePositionedComments = arg.qualifiedName.name.positionedComments; + const originalNameComments = arg.qualifiedName.name.comments; + + if (hasOwnComments) { + arg.qualifiedName.name.positionedComments = null; + arg.qualifiedName.name.comments = null; + } + token.innerTokens.push(arg.qualifiedName.accept(this)); + if (hasOwnComments) { + arg.qualifiedName.name.positionedComments = originalNamePositionedComments; + arg.qualifiedName.name.comments = originalNameComments; + } + this.addComponentComments(token, arg); return token; @@ -1480,6 +1496,14 @@ export class SqlPrintTokenParser implements SqlComponentVisitor { innerAfterComments = arg.expression.getPositionedComments('after'); arg.expression.positionedComments = null; } + if (hasOwnComments && innerBeforeComments.length > 0) { + const innerBeforeSet = new Set(innerBeforeComments); + arg.positionedComments = arg.positionedComments + ?.map(comment => comment.position === 'after' + ? { ...comment, comments: comment.comments.filter(value => !innerBeforeSet.has(value)) } + : comment) + .filter(comment => comment.comments.length > 0) ?? null; + } // Build basic structure first token.innerTokens.push(SqlPrintTokenParser.PAREN_OPEN_TOKEN); @@ -1976,10 +2000,9 @@ export class SqlPrintTokenParser implements SqlComponentVisitor { const afterComments = arg.getPositionedComments('after'); if (beforeComments.length > 0) { - if (arg.value instanceof CaseExpression) { + if (arg.value instanceof CaseExpression || this.listContinuationCommentComponents.has(arg)) { const commentBlocks = this.createCommentBlocks(beforeComments); token.innerTokens.push(...commentBlocks); - token.innerTokens.push(new SqlPrintToken(SqlPrintTokenType.commentNewline, '')); } else { const commentTokens = this.createInlineCommentSequence(beforeComments); token.innerTokens.push(...commentTokens); @@ -2119,6 +2142,13 @@ export class SqlPrintTokenParser implements SqlComponentVisitor { for (let i = 0; i < arg.items.length; i++) { if (i > 0) { token.innerTokens.push(...SqlPrintTokenParser.commaSpaceTokens()); + this.listContinuationCommentComponents.add(arg.items[i]); + try { + token.innerTokens.push(this.visit(arg.items[i])); + } finally { + this.listContinuationCommentComponents.delete(arg.items[i]); + } + continue; } token.innerTokens.push(this.visit(arg.items[i])); } diff --git a/packages/core/src/transformers/SqlPrinter.ts b/packages/core/src/transformers/SqlPrinter.ts index 7fc7c9217..072f112fa 100644 --- a/packages/core/src/transformers/SqlPrinter.ts +++ b/packages/core/src/transformers/SqlPrinter.ts @@ -150,6 +150,8 @@ export class SqlPrinter { private pendingLineCommentBreak: number | null = null; /** Accumulates lines when reconstructing multi-line block comments inside CommentBlocks */ private smartCommentBlockBuilder: { lines: string[]; level: number; mode: 'block' | 'line' } | null = null; + /** Tracks function calls whose arguments must expand because rendered comments are present */ + private commentedFunctionCallDepth = 0; /** * @param options Optional style settings for pretty printing @@ -254,6 +256,7 @@ export class SqlPrinter { this.pendingLineCommentBreak = null; this.smartCommentBlockBuilder = null; this.expandedOneLineFallbackTokens = new WeakSet(); + this.commentedFunctionCallDepth = 0; if (this.linePrinter.lines.length > 0 && level !== this.linePrinter.lines[0].level) { this.linePrinter.lines[0].level = level; } @@ -343,14 +346,21 @@ export class SqlPrinter { } const hasRenderableLeadingComment = leadingCommentContexts.some(item => item.shouldRender); + const leadingCommentFollowsLogicalOperator = hasRenderableLeadingComment && this.currentLineEndsWithLogicalOperator(); const leadingCommentIndentLevel = hasRenderableLeadingComment - ? this.getLeadingCommentIndentLevel(parentContainerType, level) + ? this.getLeadingCommentIndentLevel( + token.containerType === SqlPrintTokenContainerType.SelectItem + ? token.containerType + : parentContainerType, + level + ) : null; if ( hasRenderableLeadingComment && !this.isOnelineMode() && this.shouldAddNewlineBeforeLeadingComments(parentContainerType) + && !this.shouldKeepLeadingCommentOnCommaLine() ) { const currentLine = this.linePrinter.getCurrentLine(); if (currentLine.text.trim().length > 0) { @@ -373,6 +383,21 @@ export class SqlPrinter { leading.context, false ); + if ( + token.containerType === SqlPrintTokenContainerType.SelectItem && + this.smartCommentBlockBuilder?.mode === 'line' + ) { + this.flushSmartCommentBlockBuilder(); + } + } + + if ( + leadingCommentFollowsLogicalOperator && + this.pendingLineCommentBreak === null && + !this.isOnelineMode() && + this.linePrinter.getCurrentLine().text.trim().endsWith('*/') + ) { + this.linePrinter.appendNewline(level + 1); } if (this.smartCommentBlockBuilder && token.containerType !== SqlPrintTokenContainerType.CommentBlock && token.type !== SqlPrintTokenType.commentNewline) { @@ -381,7 +406,10 @@ export class SqlPrinter { if (this.pendingLineCommentBreak !== null) { if (!this.isOnelineMode()) { - this.linePrinter.appendNewline(this.pendingLineCommentBreak); + const pendingBreakLevel = leadingCommentFollowsLogicalOperator + ? this.pendingLineCommentBreak + 1 + : this.pendingLineCommentBreak; + this.linePrinter.appendNewline(pendingBreakLevel); } const shouldSkipToken = token.type === SqlPrintTokenType.commentNewline; this.pendingLineCommentBreak = null; @@ -400,7 +428,17 @@ export class SqlPrinter { if (!this.shouldRenderComment(token, effectiveCommentContext)) { return; } + if (this.shouldKeepLeadingCommentOnCommaLine()) { + this.ensureTrailingSpace(); + } const commentLevel = this.getCommentBaseIndentLevel(level, parentContainerType); + if ( + parentContainerType === SqlPrintTokenContainerType.SelectItem && + effectiveCommentContext.position === 'leading' && + this.linePrinter.getCurrentLine().text.trim() === '' + ) { + this.linePrinter.getCurrentLine().level = commentLevel; + } this.handleCommentBlockContainer(token, commentLevel, effectiveCommentContext); return; } @@ -415,6 +453,8 @@ export class SqlPrinter { this.handleKeywordToken(token, level, parentContainerType, caseContextDepth); } else if (token.type === SqlPrintTokenType.comma) { this.handleCommaToken(token, level, parentContainerType); + } else if (token.type === SqlPrintTokenType.argumentSplitter) { + this.handleArgumentSplitterToken(token, level, parentContainerType); } else if (token.type === SqlPrintTokenType.parenthesis) { this.handleParenthesisToken(token, level, indentParentActive, parentContainerType); } else if (token.type === SqlPrintTokenType.operator && token.text.toLowerCase() === 'and') { @@ -539,6 +579,11 @@ export class SqlPrinter { if (enteredJoinOnClause) { this.joinOnClauseDepth++; } + const enteredCommentedFunctionCall = this.shouldExpandCommentedFunctionCall(token); + if (enteredCommentedFunctionCall) { + this.commentedFunctionCallDepth++; + } + const enteredLogicalOperatorWithComment = this.isLogicalOperatorWithComment(token); for (let i = leadingCommentCount; i < token.innerTokens.length; i++) { const child = token.innerTokens[i]; @@ -593,9 +638,12 @@ export class SqlPrinter { const childCommentContext: CommentRenderContext | undefined = child.containerType === SqlPrintTokenContainerType.CommentBlock ? { position: 'inline', isTopLevelContainer: containerIsTopLevel } : undefined; + const childLevel = enteredCommentedFunctionCall && child.containerType === SqlPrintTokenContainerType.ValueList + ? innerLevel + 1 + : innerLevel; this.appendToken( child, - innerLevel, + childLevel, token.containerType, nextCaseContextDepth, childIndentParentActive, @@ -610,12 +658,24 @@ export class SqlPrinter { } } + if (enteredLogicalOperatorWithComment && !this.isOnelineMode()) { + const currentLine = this.linePrinter.getCurrentLine(); + if (currentLine.text.trim() === '') { + currentLine.level = level + 1; + } else { + this.linePrinter.appendNewline(level + 1); + } + } + if (this.smartCommentBlockBuilder && this.smartCommentBlockBuilder.mode === 'line') { this.flushSmartCommentBlockBuilder(); } if (enteredJoinOnClause) { this.joinOnClauseDepth--; } + if (enteredCommentedFunctionCall) { + this.commentedFunctionCallDepth--; + } // Exit WITH clause context when we finish processing WithClause container if (token.containerType === SqlPrintTokenContainerType.WithClause && this.withClauseStyle === 'full-oneline') { @@ -647,6 +707,33 @@ export class SqlPrinter { } } + private shouldExpandCommentedFunctionCall(token: SqlPrintToken): boolean { + return this.commentExportMode !== 'none' && + token.containerType === SqlPrintTokenContainerType.FunctionCall && + token.innerTokens.some(child => + child.containerType === SqlPrintTokenContainerType.ValueList && + this.containsCommentBlock(child) + ); + } + + private containsCommentBlock(token: SqlPrintToken): boolean { + if (token.containerType === SqlPrintTokenContainerType.CommentBlock) { + return true; + } + return token.innerTokens.some(child => this.containsCommentBlock(child)); + } + + private isLogicalOperatorWithComment(token: SqlPrintToken): boolean { + return this.containsCommentBlock(token) && + token.type === SqlPrintTokenType.operator && + ['and', 'or'].includes(token.text.toLowerCase()); + } + + private currentLineEndsWithLogicalOperator(): boolean { + const trimmed = this.linePrinter.getCurrentLine().text.trim().toLowerCase(); + return trimmed === 'and' || trimmed === 'or'; + } + private isCaseContext(containerType?: SqlPrintTokenContainerType): boolean { switch (containerType) { case SqlPrintTokenContainerType.CaseExpression: @@ -828,6 +915,17 @@ export class SqlPrinter { } } + private handleArgumentSplitterToken(token: SqlPrintToken, level: number, parentContainerType?: SqlPrintTokenContainerType): void { + this.linePrinter.appendText(token.text); + if ( + this.commentedFunctionCallDepth > 0 && + parentContainerType === SqlPrintTokenContainerType.ValueList && + !this.isOnelineMode() + ) { + this.linePrinter.appendNewline(level); + } + } + private handleAndOperatorToken(token: SqlPrintToken, level: number, parentContainerType?: SqlPrintTokenContainerType, caseContextDepth: number = 0): void { const text = this.applyKeywordCase(token.text); @@ -864,6 +962,14 @@ export class SqlPrinter { private handleParenthesisToken(token: SqlPrintToken, level: number, indentParentActive: boolean, parentContainerType?: SqlPrintTokenContainerType): void { if (token.text === '(') { this.linePrinter.appendText(token.text); + if ( + this.commentedFunctionCallDepth > 0 && + parentContainerType === SqlPrintTokenContainerType.FunctionCall && + !this.isOnelineMode() + ) { + this.linePrinter.appendNewline(level + 1); + return; + } if ( (parentContainerType === SqlPrintTokenContainerType.InsertClause || parentContainerType === SqlPrintTokenContainerType.MergeInsertAction) && @@ -886,6 +992,14 @@ export class SqlPrinter { } if (token.text === ')' && !this.isOnelineMode()) { + if ( + this.commentedFunctionCallDepth > 0 && + parentContainerType === SqlPrintTokenContainerType.FunctionCall + ) { + this.linePrinter.appendNewline(level); + this.linePrinter.appendText(token.text); + return; + } if (this.shouldBreakBeforeClosingParen(parentContainerType)) { this.linePrinter.appendNewline(Math.max(level, 0)); this.linePrinter.appendText(token.text); @@ -1019,6 +1133,14 @@ export class SqlPrinter { if (this.smartCommentBlockBuilder && this.smartCommentBlockBuilder.mode === 'line') { this.flushSmartCommentBlockBuilder(); } + if ( + this.commentedFunctionCallDepth > 0 && + parentContainerType === SqlPrintTokenContainerType.ValueList && + previousToken?.type === SqlPrintTokenType.argumentSplitter && + !this.isOnelineMode() + ) { + return; + } const currentLineText = this.linePrinter.getCurrentLine().text; if (this.onelineHelper.shouldSkipInsertClauseSpace(parentContainerType, nextToken, currentLineText)) { return; @@ -1275,6 +1397,12 @@ export class SqlPrinter { return; } + const emptyBlockCommentText = this.getEmptyBlockCommentText(token); + if (emptyBlockCommentText !== null) { + this.linePrinter.appendText(emptyBlockCommentText); + return; + } + const lines = this.collectCommentBlockLines(token); if (lines.length === 0 && !this.smartCommentBlockBuilder) { // No meaningful content; treat as empty line comment to preserve spacing @@ -1297,6 +1425,18 @@ export class SqlPrinter { } } + private getEmptyBlockCommentText(token: SqlPrintToken): string | null { + const commentTokens = (token.innerTokens ?? []).filter(child => child.type === SqlPrintTokenType.comment); + if (commentTokens.length !== 1) { + return null; + } + const trimmed = commentTokens[0].text.trim(); + if (!trimmed.startsWith('/*') || !trimmed.endsWith('*/')) { + return null; + } + return trimmed.slice(2, -2).trim() === '' ? trimmed : null; + } + private normalizeSmartBlockLine(raw: string): string { // Remove trailing whitespace that only carries formatting artifacts let line = raw.replace(/\s+$/g, ''); @@ -1375,7 +1515,11 @@ export class SqlPrinter { } const content = this.extractLineCommentContent(trimmed); if (content !== null) { - lines.push(content); + if (!content && trimmed.startsWith('/*') && trimmed.endsWith('*/')) { + lines.push(this.sanitizeCommentLine(this.escapeCommentDelimiters(trimmed))); + } else { + lines.push(content); + } } } } @@ -1575,6 +1719,12 @@ export class SqlPrinter { if (parentType === SqlPrintTokenContainerType.SelectClause) { return true; } + if ( + parentType === SqlPrintTokenContainerType.OrderByItem || + parentType === SqlPrintTokenContainerType.GroupByClause + ) { + return true; + } if (parentType === SqlPrintTokenContainerType.ExplainStatement) { // Ensure EXPLAIN targets print header comments on a dedicated line. return true; @@ -1586,6 +1736,15 @@ export class SqlPrinter { } private getLeadingCommentIndentLevel(parentType: SqlPrintTokenContainerType | undefined, currentLevel: number): number { + if (parentType === SqlPrintTokenContainerType.SelectItem) { + return currentLevel; + } + if ( + parentType === SqlPrintTokenContainerType.OrderByItem || + parentType === SqlPrintTokenContainerType.GroupByClause + ) { + return currentLevel; + } if (parentType === SqlPrintTokenContainerType.TupleExpression) { return currentLevel + 1; } @@ -1603,6 +1762,13 @@ export class SqlPrinter { return currentLevel; } + private shouldKeepLeadingCommentOnCommaLine(): boolean { + if (this.commaBreak !== 'before') { + return false; + } + return this.linePrinter.getCurrentLine().text.trim() === ','; + } + /** * Determines if the printer is in oneliner mode. * Oneliner mode uses single spaces instead of actual newlines. diff --git a/packages/core/tests/transformers/CommentStyle.comprehensive.test.ts b/packages/core/tests/transformers/CommentStyle.comprehensive.test.ts index dcad30c49..bd08c09b6 100644 --- a/packages/core/tests/transformers/CommentStyle.comprehensive.test.ts +++ b/packages/core/tests/transformers/CommentStyle.comprehensive.test.ts @@ -1,6 +1,8 @@ import { describe, test, expect } from 'vitest'; import { SelectQueryParser } from '../../src/parsers/SelectQueryParser'; +import { SqlPrintToken, SqlPrintTokenContainerType, SqlPrintTokenType } from '../../src/models/SqlPrintToken'; import { SqlFormatter } from '../../src/transformers/SqlFormatter'; +import { SqlPrinter } from '../../src/transformers/SqlPrinter'; /** * CommentStyle - Comprehensive TDD Test @@ -42,6 +44,400 @@ describe('CommentStyle - Comprehensive TDD Test', () => { expect(result.formattedSql).toContain('/* Field comment */'); expect(result.formattedSql).toContain('/* Table comment */'); }); + + test('should keep comma-prefixed CASE select item comments aligned with the select item', () => { + const formatter = new SqlFormatter({ + exportComment: true, + commentStyle: 'block', + commaBreak: 'before', + indentSize: 4, + indentChar: ' ', + keywordCase: 'lower', + newline: '\n' + }); + const sql = ` + select a + , /* explains case */ + case when b is null then 0 else 1 end as b_flag + from t + `; + + const query = SelectQueryParser.parse(sql).toSimpleQuery(); + const result = formatter.format(query); + + expect(result.formattedSql).toContain([ + ' , /* explains case */', + ' case' + ].join('\n')); + expect(result.formattedSql).not.toContain([ + ' ,', + ' /* explains case */', + ' case' + ].join('\n')); + }); + + test('should keep comma-prefixed select item comments on the comma line', () => { + const formatter = new SqlFormatter({ + exportComment: true, + commentStyle: 'block', + commaBreak: 'before', + indentSize: 4, + indentChar: ' ', + keywordCase: 'lower', + newline: '\n' + }); + const sql = ` + select a + , /* explains count */ + count(*) as n + , /* explains b */ + b + from t + `; + + const query = SelectQueryParser.parse(sql).toSimpleQuery(); + const result = formatter.format(query); + + expect(result.formattedSql).toContain([ + ' , /* explains count */', + ' count(*) as "n"', + ].join('\n')); + expect(result.formattedSql).toContain([ + ' , /* explains b */', + ' "b"', + ].join('\n')); + expect(result.formattedSql).not.toContain([ + ' ,', + ' /* explains count */', + ].join('\n')); + expect(result.formattedSql).not.toContain([ + ' ,', + ' /* explains b */', + ].join('\n')); + }); + + test('should keep comma-suffixed select item comments on a dedicated line', () => { + const formatter = new SqlFormatter({ + exportComment: true, + commentStyle: 'block', + commaBreak: 'after', + indentSize: 4, + indentChar: ' ', + keywordCase: 'lower', + newline: '\n' + }); + const sql = ` + select a, + /* explains count */ + count(*) as n, + /* explains b */ + b + from t + `; + + const query = SelectQueryParser.parse(sql).toSimpleQuery(); + const result = formatter.format(query); + + expect(result.formattedSql).toContain([ + ' "a",', + ' /* explains count */', + ' count(*) as "n",', + ' /* explains b */', + ' "b"', + ].join('\n')); + expect(result.formattedSql).not.toContain('/* explains count */ count(*)'); + expect(result.formattedSql).not.toContain('/* explains b */ "b"'); + }); + + test('should not duplicate comma-prefixed comments in function arguments', () => { + const formatter = new SqlFormatter({ + exportComment: true, + commentStyle: 'block', + commaBreak: 'before', + indentSize: 4, + indentChar: ' ', + keywordCase: 'lower', + newline: '\n' + }); + const sql = ` + select concat(a + , /* second arg */ + b + , /* third arg */ + c) as label + from t + `; + + const query = SelectQueryParser.parse(sql).toSimpleQuery(); + const result = formatter.format(query); + + expect(result.formattedSql.match(/second arg/g)).toHaveLength(1); + expect(result.formattedSql.match(/third arg/g)).toHaveLength(1); + }); + + test('should not duplicate comma-prefixed comments in ORDER BY and GROUP BY lists', () => { + const formatter = new SqlFormatter({ + exportComment: true, + commentStyle: 'block', + commaBreak: 'before', + indentSize: 4, + indentChar: ' ', + keywordCase: 'lower', + newline: '\n' + }); + const sql = ` + select a, b, count(*) as n + from t + group by a + , /* second grouping item */ + b + order by a + , /* secondary sort item */ + b desc + `; + + const query = SelectQueryParser.parse(sql).toSimpleQuery(); + const result = formatter.format(query); + + expect(result.formattedSql.match(/second grouping item/g)).toHaveLength(1); + expect(result.formattedSql.match(/secondary sort item/g)).toHaveLength(1); + }); + + test('should keep comma-prefixed ORDER BY and GROUP BY comments on the comma line', () => { + const formatter = new SqlFormatter({ + exportComment: true, + commentStyle: 'block', + commaBreak: 'before', + indentSize: 4, + indentChar: ' ', + keywordCase: 'lower', + newline: '\n' + }); + const sql = ` + select a, b, count(*) as n + from t + group by a + , /* second grouping item */ + b + order by a + , /* secondary sort item */ + b desc + `; + + const query = SelectQueryParser.parse(sql).toSimpleQuery(); + const result = formatter.format(query); + + expect(result.formattedSql).toContain([ + 'group by', + ' "a"', + ' , /* second grouping item */', + ' "b"', + ].join('\n')); + expect(result.formattedSql).toContain([ + 'order by', + ' "a"', + ' , /* secondary sort item */', + ' "b" desc', + ].join('\n')); + expect(result.formattedSql).not.toContain([ + ' ,', + ' /* second grouping item */', + ].join('\n')); + expect(result.formattedSql).not.toContain([ + ' ,', + ' /* secondary sort item */', + ].join('\n')); + }); + + test('should keep comma-suffixed ORDER BY and GROUP BY comments on dedicated lines', () => { + const formatter = new SqlFormatter({ + exportComment: true, + commentStyle: 'block', + commaBreak: 'after', + indentSize: 4, + indentChar: ' ', + keywordCase: 'lower', + newline: '\n' + }); + const sql = ` + select a, b, count(*) as n + from t + group by a, + /* second grouping item */ + b + order by a, + /* secondary sort item */ + b desc + `; + + const query = SelectQueryParser.parse(sql).toSimpleQuery(); + const result = formatter.format(query); + + expect(result.formattedSql).toContain([ + 'group by', + ' "a",', + ' /* second grouping item */', + ' "b"', + ].join('\n')); + expect(result.formattedSql).toContain([ + 'order by', + ' "a",', + ' /* secondary sort item */', + ' "b" desc', + ].join('\n')); + expect(result.formattedSql).not.toContain('/* second grouping item */ "b"'); + expect(result.formattedSql).not.toContain('/* secondary sort item */ "b"'); + }); + + test('should not move or duplicate comments inside parenthesized WHERE predicates', () => { + const formatter = new SqlFormatter({ + exportComment: true, + commentStyle: 'block', + commaBreak: 'before', + indentSize: 4, + indentChar: ' ', + keywordCase: 'lower', + newline: '\n' + }); + const sql = ` + select * + from t + where ( + /* email match */ + email = :keyword + or /* name match */ + name = :keyword + ) + `; + + const query = SelectQueryParser.parse(sql).toSimpleQuery(); + const result = formatter.format(query); + + expect(result.formattedSql.match(/email match/g)).toHaveLength(1); + expect(result.formattedSql.match(/name match/g)).toHaveLength(1); + expect(result.formattedSql).not.toContain(') /* email match */'); + }); + + test('should preserve comments before LIMIT and OFFSET values', () => { + const formatter = new SqlFormatter({ + exportComment: true, + commentStyle: 'block', + commaBreak: 'before', + indentSize: 4, + indentChar: ' ', + keywordCase: 'lower', + newline: '\n' + }); + const sql = ` + select * + from t + order by id + limit /* page size */ + :limit + offset /* page offset */ + :offset + `; + + const query = SelectQueryParser.parse(sql).toSimpleQuery(); + const result = formatter.format(query); + + expect(result.formattedSql.match(/page size/g)).toHaveLength(1); + expect(result.formattedSql.match(/page offset/g)).toHaveLength(1); + expect(result.formattedSql).toMatch(/limit\s+\/\* page size \*\/\s+:limit/); + expect(result.formattedSql).toMatch(/offset\s+\/\* page offset \*\/\s+:offset/); + }); + + test('should preserve comments after HAVING and JOIN ON keywords', () => { + const formatter = new SqlFormatter({ + exportComment: true, + commentStyle: 'block', + commaBreak: 'before', + indentSize: 4, + indentChar: ' ', + keywordCase: 'lower', + newline: '\n', + joinOneLine: false + }); + const sql = ` + select c.customer_id, count(*) as ticket_count + from customers c + join tickets t on /* join key */ t.customer_id = c.customer_id + group by c.customer_id + having /* minimum tickets */ count(*) > 0 + `; + + const query = SelectQueryParser.parse(sql).toSimpleQuery(); + const result = formatter.format(query); + + expect(result.formattedSql).toContain('/* join key */'); + expect(result.formattedSql).toContain('/* minimum tickets */'); + }); + + test('should expand function arguments only when an argument has a comment', () => { + const formatter = new SqlFormatter({ + exportComment: true, + commentStyle: 'block', + commaBreak: 'after', + indentSize: 4, + indentChar: ' ', + keywordCase: 'lower', + newline: '\n' + }); + + const compactQuery = SelectQueryParser.parse(` + select concat(a, b, c) as label + from t + `).toSimpleQuery(); + const commentedQuery = SelectQueryParser.parse(` + select concat(a, /* arg b */ b, c) as label + from t + `).toSimpleQuery(); + + const compactResult = formatter.format(compactQuery); + const commentedResult = formatter.format(commentedQuery); + + expect(compactResult.formattedSql).toContain('concat("a", "b", "c") as "label"'); + expect(commentedResult.formattedSql).toContain([ + ' concat(', + ' "a",', + ' /* arg b */', + ' "b",', + ' "c"', + ' ) as "label"', + ].join('\n')); + }); + + test('should indent predicates after AND and OR comments', () => { + const formatter = new SqlFormatter({ + exportComment: true, + commentStyle: 'block', + andBreak: 'before', + orBreak: 'before', + indentSize: 4, + indentChar: ' ', + keywordCase: 'lower', + newline: '\n' + }); + const sql = ` + select * + from t + where status = 'open' + and /* tenant */ tenant_id = 1 + or /* urgent */ priority = 'high' + `; + + const query = SelectQueryParser.parse(sql).toSimpleQuery(); + const result = formatter.format(query); + + expect(result.formattedSql).toContain([ + 'where', + ' "status" = \'open\'', + ' and /* tenant */', + ' "tenant_id" = 1', + ' or /* urgent */', + ' "priority" = \'high\'', + ].join('\n')); + }); }); describe('Smart style conversion', () => { @@ -129,6 +525,29 @@ describe('CommentStyle - Comprehensive TDD Test', () => { // Assert - Line comment content should be preserved (even if converted to block format) expect(result.formattedSql).toContain('Line comment'); }); + + test('should preserve empty block comments without escaping delimiters', () => { + const commentBlock = new SqlPrintToken( + SqlPrintTokenType.container, + '', + SqlPrintTokenContainerType.CommentBlock + ); + commentBlock.innerTokens.push(new SqlPrintToken(SqlPrintTokenType.comment, '/**/')); + + const printer = new SqlPrinter({ + exportComment: true, + commentStyle: 'smart', + indentSize: 4, + indentChar: ' ', + keywordCase: 'lower', + newline: '\n' + }); + + const result = printer.print(commentBlock); + + expect(result).toBe('/**/'); + expect(result).not.toContain('\\/'); + }); }); describe('Smart style with comma break integration', () => { diff --git a/packages/core/tests/transformers/SqlFormatter.comment-exact-transformation.test.ts b/packages/core/tests/transformers/SqlFormatter.comment-exact-transformation.test.ts index d5e031af4..9f76de4e6 100644 --- a/packages/core/tests/transformers/SqlFormatter.comment-exact-transformation.test.ts +++ b/packages/core/tests/transformers/SqlFormatter.comment-exact-transformation.test.ts @@ -81,11 +81,11 @@ FROM "users" WHERE /* w1 */ - "status" = /* w2 */ - 'active' /* w3 */ - AND /* a1 */ - "created_at" > /* a2 */ - '2023-01-01' /* a3 */`; + "status" = /* w2 */ + 'active' /* w3 */ + AND /* a1 */ + "created_at" > /* a2 */ + '2023-01-01' /* a3 */`; const parsed = SelectQueryParser.parse(originalSql); const formatter = new SqlFormatter(formatterOptions); @@ -196,11 +196,12 @@ FROM /* field2 comment */ name /* after name */ FROM users`; - const expectedTransformed = `SELECT - /* field1 comment */ "id" /* after id */, - /* field2 comment */ "name" /* after name */ -FROM - "users"`; + const expectedTransformed = `SELECT + /* field1 comment */ "id" /* after id */, + /* field2 comment */ + "name" /* after name */ +FROM + "users"`; const parsed = SelectQueryParser.parse(originalSql); const formatter = new SqlFormatter(formatterOptions); @@ -329,10 +330,10 @@ AND /* a1 */ created_at > '2023-01-01' /* a2 */`; FROM "users" AS "u" /* u1 */ WHERE - /* w1 */ - "status" = 'active' /* w2 */ - AND /* a1 */ - "created_at" > '2023-01-01' /* a2 */`; + /* w1 */ + "status" = 'active' /* w2 */ + AND /* a1 */ + "created_at" > '2023-01-01' /* a2 */`; console.log('\n=== COMPLEX SQL TRUE FULL TEXT COMPARISON ==='); console.log('Original:'); diff --git a/packages/core/tests/transformers/SqlFormatter.comment-order.test.ts b/packages/core/tests/transformers/SqlFormatter.comment-order.test.ts index 88f898d28..22ca974ff 100644 --- a/packages/core/tests/transformers/SqlFormatter.comment-order.test.ts +++ b/packages/core/tests/transformers/SqlFormatter.comment-order.test.ts @@ -141,8 +141,10 @@ FROM // Expected formatted SQL - corrected to proper comment positions const expectedFormattedSql = `SELECT /* a1 */ a /* a2 */ - , /* b1 */ b /* b2 */ - , /* c1 */ c /* c2 */ AS /* c3 */ alias_c /* c4 */ + , /* b1 */ + b /* b2 */ + , /* c1 */ + c /* c2 */ AS /* c3 */ alias_c /* c4 */ FROM users`; @@ -182,4 +184,4 @@ FROM // Exact match comparison expect(result.formattedSql).toBe(expectedFormattedSql); }); -}); \ No newline at end of file +}); diff --git a/packages/core/tests/transformers/SqlFormatter.comprehensive-comment-fulltext.test.ts b/packages/core/tests/transformers/SqlFormatter.comprehensive-comment-fulltext.test.ts index 2e4060374..ca0a4b723 100644 --- a/packages/core/tests/transformers/SqlFormatter.comprehensive-comment-fulltext.test.ts +++ b/packages/core/tests/transformers/SqlFormatter.comprehensive-comment-fulltext.test.ts @@ -121,11 +121,11 @@ FROM \"users\" WHERE /* w1 */ - \"status\" = /* w2 */ - 'active' /* w3 */ - AND /* a1 */ - \"created_at\" > /* a2 */ - '2023-01-01' /* a3 */`; + \"status\" = /* w2 */ + 'active' /* w3 */ + AND /* a1 */ + \"created_at\" > /* a2 */ + '2023-01-01' /* a3 */`; console.log('\n=== WHERE ORIGINAL ==='); console.log(originalSql); @@ -236,4 +236,4 @@ WHERE t1.status = 'active' /* status check */ // Should preserve at least some comments expect(result.formattedSql).toMatch(/\/\*.*\*\//); }); -}); \ No newline at end of file +}); diff --git a/packages/core/tests/transformers/SqlFormatter.comprehensive.test.ts b/packages/core/tests/transformers/SqlFormatter.comprehensive.test.ts index 6ff2ac28b..faf4ce549 100644 --- a/packages/core/tests/transformers/SqlFormatter.comprehensive.test.ts +++ b/packages/core/tests/transformers/SqlFormatter.comprehensive.test.ts @@ -220,7 +220,7 @@ describe('SqlFormatter - Comprehensive SQL Output Validation', () => { const formatter = new SqlFormatter({ exportComment: true }); const result = formatter.format(query); - const expectedSql = 'select round("price" /* price value */ /* price value */ * 1.1, 2) as "rounded_price" from "products"'; + const expectedSql = 'select round("price" /* price value */ * 1.1, 2) as "rounded_price" from "products"'; validateCompleteSQL(result.formattedSql, expectedSql); }); @@ -236,7 +236,7 @@ describe('SqlFormatter - Comprehensive SQL Output Validation', () => { const formatter = new SqlFormatter({ exportComment: true }); const result = formatter.format(query); - const expectedSql = 'select ("price" /* base price */ /* base price */ * 1.1 /* with tax */ + 500) /* plus fee */ as "final_price" from "products"'; + const expectedSql = 'select ("price" /* base price */ * 1.1 /* with tax */ + 500) /* plus fee */ as "final_price" from "products"'; validateCompleteSQL(result.formattedSql, expectedSql); });