From a39db15fc227a96e13ea13ee2a27487722f713eb Mon Sep 17 00:00:00 2001 From: emrberk Date: Thu, 24 Sep 2026 15:11:03 +0300 Subject: [PATCH] Keep glued lexemes intact when formatting The formatter inserted a space between two word tokens that the lexer split from one QuestDB lexeme: 0x1F became 0 x1F, 16777217.0f became 16777217.0 f, geohash(5c) became geohash(5 c). Some of those formatted queries ran and returned different results. Two word pieces with no whitespace between them in the source now keep no gap, and no clause starts on such a piece. A dot stays apart from any digit-led token, not only NumberLiteral, so `t. 5m` does not become the single token `.5m`. Two new oracles guard this: whitespace-stripped equality catches insertions, and main-lexer token equality catches merges. A new test sweeps every pair of 82 lexemes with three separators. Co-Authored-By: Claude Fable 5.1 --- src/formatter/layout.ts | 20 +++- src/formatter/spacing.ts | 8 +- tests/formatter/lexemes.test.ts | 189 ++++++++++++++++++++++++++++++++ tests/formatter/oracles.ts | 26 +++++ 4 files changed, 238 insertions(+), 5 deletions(-) create mode 100644 tests/formatter/lexemes.test.ts diff --git a/src/formatter/layout.ts b/src/formatter/layout.ts index ea3b43c..50179fc 100644 --- a/src/formatter/layout.ts +++ b/src/formatter/layout.ts @@ -23,7 +23,7 @@ import { pivotSubClauses, windowSubClauses, } from "./phrases" -import { gapBetween, isOperand, isSign, Piece } from "./spacing" +import { gapBetween, isGluedWords, isOperand, isSign, Piece } from "./spacing" import { Statement } from "./statements" type Element = { leading: Doc; doc: Doc } @@ -208,8 +208,20 @@ class StatementBuilder { return { leading, doc: text(this.textOf(piece)) } } + private phraseAt(phrases: Phrase[]): PhraseMatch | null { + const piece = this.peek() + if ( + piece !== undefined && + this.previous !== null && + isGluedWords(this.previous, piece) + ) { + return null + } + return matchPhrase(this.tokens, this.index, phrases) + } + private matchClause(current: Phrase | null): PhraseMatch | null { - const match = matchPhrase(this.tokens, this.index, this.phrases) + const match = this.phraseAt(this.phrases) if (match === null || continuesPhrase(current, match.phrase)) return null return match } @@ -345,7 +357,7 @@ class StatementBuilder { items.length === 0 && current === null && ctx.subClauses.length > 0 && - matchPhrase(this.tokens, this.index, ctx.subClauses) !== null + this.phraseAt(ctx.subClauses) !== null ) { subClauseStarted = true } @@ -369,7 +381,7 @@ class StatementBuilder { const subClause = current !== null && endsOperand(this.previous) - ? matchPhrase(this.tokens, this.index, ctx.subClauses) + ? this.phraseAt(ctx.subClauses) : null if (subClause !== null) { closeItem() diff --git a/src/formatter/spacing.ts b/src/formatter/spacing.ts index 203f193..19710b1 100644 --- a/src/formatter/spacing.ts +++ b/src/formatter/spacing.ts @@ -30,11 +30,16 @@ const noSpaceAfter: ReadonlySet = new Set([ const isComment = (piece: Piece) => piece.token.kind === "lineComment" || piece.token.kind === "blockComment" -const isNumber = (piece: Piece) => piece.token.tokenName === "NumberLiteral" +const isNumber = (piece: Piece) => /^\d/.test(piece.token.image) const isUncertain = (piece: Piece) => piece.token.kind === "opaque" || piece.token.kind === "tolerant" +export const isGluedWords = (previous: Piece, next: Piece) => + previous.token.kind === "word" && + next.token.kind === "word" && + next.gapBefore === "" + export const isSign = (piece: Piece) => piece.token.tokenName === "Minus" || piece.token.tokenName === "Plus" @@ -53,6 +58,7 @@ export const gapBetween = (previous: Piece | null, next: Piece): string => { return preserved(next) } if (previous.unary && !isComment(next)) return "" + if (isGluedWords(previous, next)) return "" const previousName = previous.token.tokenName const nextName = next.token.tokenName if (nextName === "Dot" && isNumber(previous)) return " " diff --git a/tests/formatter/lexemes.test.ts b/tests/formatter/lexemes.test.ts new file mode 100644 index 0000000..7919309 --- /dev/null +++ b/tests/formatter/lexemes.test.ts @@ -0,0 +1,189 @@ +import { describe, expect, it } from "vitest" +import { format } from "../../src/formatter/index" +import { + assertPreserved, + assertSameCharacters, + assertSameLexemes, + assertSameStream, +} from "./oracles" + +const splitLexemes = [ + "0x1F", + "0xDEADbeef", + "16777217.0f", + "1.5f", + "5c", + "7b", + "1500ns", + "1.5L", + "2h30m", + "123abc", + "1e", +] + +const contexts = [ + (lexeme: string) => `SELECT id, ${lexeme} FROM t`, + (lexeme: string) => `SELECT f(${lexeme}) FROM t`, + (lexeme: string) => `SELECT * FROM t WHERE h = ${lexeme}`, + (lexeme: string) => `SELECT -${lexeme}`, + (lexeme: string) => `SELECT a[${lexeme}]`, + (lexeme: string) => `INSERT INTO t VALUES (1, ${lexeme})`, + (lexeme: string) => `CREATE TABLE g (h geohash(${lexeme}), ts timestamp)`, +] + +describe("lexemes the lexer splits into several tokens", () => { + it.each( + splitLexemes.flatMap((lexeme) => + contexts.map((context) => [context(lexeme)] as const), + ), + )("keeps %s glued", (sql) => { + assertPreserved(sql) + }) + + it("formats a hex literal without touching it", () => { + // Given + const sql = "SELECT id, 0x1F FROM t" + + // When + const output = format(sql) + + // Then + expect(output).toBe("SELECT id, 0x1F\nFROM t") + }) + + it("formats a geohash precision without touching it", () => { + // Given + const sql = "CREATE TABLE g (h geohash(5c), ts timestamp) timestamp(ts)" + + // When + const output = format(sql) + + // Then + expect(output).toBe( + "CREATE TABLE g (\n h geohash(5c),\n ts timestamp\n) timestamp(ts)", + ) + }) + + it("does not start a clause inside a glued run", () => { + // Given + const sql = "SELECT 10.5mFROM FROM t" + + // When + const output = format(sql) + + // Then + expect(output).toBe("SELECT 10.5mFROM\nFROM t") + }) + + it("keeps a dot apart from a suffixed number", () => { + // Given + const sql = "SELECT t. 5m" + + // When + const output = format(sql) + + // Then + expect(output).toBe("SELECT t. 5m") + }) +}) + +const lexemes = [ + "SELECT", + "FROM", + "WHERE", + "AND", + "AS", + "NULL", + "key", + "a", + "x1F", + "f", + "café", + "0", + "1.5", + ".5", + "5.", + "1e5", + "1.5E+3", + "1_000", + "0x1F", + "1.5f", + "5c", + "1500ns", + "100L", + "10.5m", + "5m", + "1d", + "#sp052w92p1p8", + "##0101", + "#", + "'a'", + "'a''b'", + "'abc", + '"a"', + '"abc', + "+", + "-", + "*", + "/", + "%", + "=", + "!=", + "<>", + "<", + "<=", + ">", + ">=", + "<<", + "<<=", + ">>", + ">>=", + "||", + "|", + "&", + "^", + "~", + "!~", + "~=", + "::", + ":", + ":=", + "@", + "!", + "?", + "$", + "(", + ")", + "[", + "]", + ",", + ";", + ".", + "--c\n", + "/*c*/", + "/*c", + "@x", + ":name", + "$1", +] + +const separators = ["", " ", "\n"] + +describe("adjacent lexeme pairs", () => { + it("never changes characters or lexemes for any pair", () => { + // Given + const pairs = lexemes.flatMap((left) => + lexemes.flatMap((right) => + separators.map((separator) => `${left}${separator}${right}`), + ), + ) + + // When / Then + for (const sql of pairs) { + const output = format(sql) + assertSameCharacters(sql, output) + assertSameLexemes(sql, output) + assertSameStream(sql, output) + } + }) +}) diff --git a/tests/formatter/oracles.ts b/tests/formatter/oracles.ts index c45a5df..cf22e21 100644 --- a/tests/formatter/oracles.ts +++ b/tests/formatter/oracles.ts @@ -2,6 +2,7 @@ import { expect } from "vitest" import { parseToAst, toSql } from "../../src/index" import { format, FormatOptions } from "../../src/formatter/index" import { scan, StreamToken } from "../../src/formatter/lexer" +import { tokenize } from "../../src/parser/lexer" const significant = (sql: string): StreamToken[] => scan(sql).filter((token) => token.kind !== "whitespace") @@ -36,6 +37,27 @@ export const assertSameOperatorAdjacency = (input: string, output: string) => { expect(adjacencyPairs(output)).toEqual(adjacencyPairs(input)) } +const lexemeSignature = (sql: string): string[] => { + const result = tokenize(sql) + return [ + ...result.tokens.map((token) => `${token.tokenType.name}:${token.image}`), + ...result.errors.map( + (error) => + `error:${sql.slice(error.offset, error.offset + error.length)}`, + ), + ] +} + +export const assertSameLexemes = (input: string, output: string) => { + expect(lexemeSignature(output)).toEqual(lexemeSignature(input)) +} + +const withoutWhitespace = (sql: string) => sql.replace(/\s+/g, "") + +export const assertSameCharacters = (input: string, output: string) => { + expect(withoutWhitespace(output)).toBe(withoutWhitespace(input)) +} + export const assertIdempotent = (sql: string, options?: FormatOptions) => { const once = format(sql, options) expect(format(once, options)).toBe(once) @@ -52,6 +74,10 @@ export const assertPreserved = (input: string, options?: FormatOptions) => { const output = format(input, options) assertSameStream(input, output) assertSameOperatorAdjacency(input, output) + if (!options?.capitalize) { + assertSameCharacters(input, output) + assertSameLexemes(input, output) + } assertIdempotent(input, options) if (parsesCleanly(input)) assertSameAst(input, output) }