From 0a9f729c4ceea9359e5a351f498ec53d5d04c321 Mon Sep 17 00:00:00 2001 From: Yarchik Date: Thu, 25 Jun 2026 20:21:48 +0100 Subject: [PATCH 1/2] fix: substitute placeholders near non-comment `--` and executable comments `skipSqlContext` treated every `--` as a line comment and every `/* */` as a regular comment, so placeholders were dropped or shifted in two cases MySQL parses differently: - `--` only starts a comment when followed by whitespace/control/EOF, so `format("SELECT 1--2, ?", ["VAL"])` returned `"SELECT 1--2, ?"` (placeholder dropped) instead of `"SELECT 1--2, 'VAL'"`. - `/*! ... */` and `/*+ ... */` are executable comments / optimizer hints that MySQL evaluates, so a `?` inside them is a live placeholder. `format("SELECT /*!40101 ? */ , ?", ["A", "B"])` returned `"SELECT /*!40101 ? */ , 'A'"` (value shifted to the wrong slot) instead of `"SELECT /*!40101 'A' */ , 'B'"`. Require a whitespace/control/EOF suffix before skipping `--`, and exclude `/*!`/`/*+` from the block-comment skip. Regular `--` and `/* */` comments are still skipped, matching the comment-aware behavior added in #22. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/index.ts | 23 +++++++++++++++++++++-- test/delimiters.test.ts | 19 +++++++++++++++++++ 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/src/index.ts b/src/index.ts index 3cee1fa..fd7404c 100644 --- a/src/index.ts +++ b/src/index.ts @@ -34,6 +34,8 @@ const charCode = { dash: 45, slash: 47, asterisk: 42, + exclamation: 33, + plus: 43, questionMark: 63, newline: 10, space: 32, @@ -129,11 +131,28 @@ const skipSqlContext = (sql: string, position: number): number => { } if (currentChar === charCode.dash && nextChar === charCode.dash) { - const lineBreak = sql.indexOf('\n', position + 2); - return lineBreak === -1 ? sql.length : lineBreak + 1; + // MySQL starts a `--` comment only when the dashes are followed by a + // whitespace/control character (or end of input); `1--2` is an operator + // sequence, not a comment, so it must not swallow following placeholders. + const afterDash = sql.charCodeAt(position + 2); + + if (Number.isNaN(afterDash) || afterDash <= charCode.space) { + const lineBreak = sql.indexOf('\n', position + 2); + return lineBreak === -1 ? sql.length : lineBreak + 1; + } + + return -1; } if (currentChar === charCode.slash && nextChar === charCode.asterisk) { + // `/*! ... */` (executable comments) and `/*+ ... */` (optimizer hints) + // are evaluated by MySQL, so a `?` inside them is a live placeholder and + // must not be skipped like a regular `/* ... */` comment. + const markerChar = sql.charCodeAt(position + 2); + + if (markerChar === charCode.exclamation || markerChar === charCode.plus) + return -1; + const commentEnd = sql.indexOf('*/', position + 2); return commentEnd === -1 ? sql.length : commentEnd + 2; } diff --git a/test/delimiters.test.ts b/test/delimiters.test.ts index 3488b97..bdeb056 100644 --- a/test/delimiters.test.ts +++ b/test/delimiters.test.ts @@ -338,6 +338,25 @@ describe('Query comments for debugging', () => { "SELECT * FROM users -- get all users\nWHERE status = 'active'" ); }); + + test('double dash without trailing whitespace is not a comment', () => { + // MySQL only treats `--` as a comment when followed by whitespace/control; + // `1--2` is an operator sequence, so the placeholder must be substituted. + const sql = format('SELECT 1--2, ?', ['VAL']); + assert.equal(sql, "SELECT 1--2, 'VAL'"); + }); + + test('executable comment placeholder is substituted, not skipped', () => { + // `/*! ... */` is run by MySQL, so a `?` inside it is a live placeholder + // and the following placeholder must not shift into its slot. + const sql = format('SELECT /*!40101 ? */ , ?', ['A', 'B']); + assert.equal(sql, "SELECT /*!40101 'A' */ , 'B'"); + }); + + test('optimizer hint placeholder is substituted, not skipped', () => { + const sql = format('SELECT /*+ MAX_EXECUTION_TIME(?) */ id FROM t', [1000]); + assert.equal(sql, 'SELECT /*+ MAX_EXECUTION_TIME(1000) */ id FROM t'); + }); }); describe('Real edge cases', () => { From 8ae49df04c4f998b6dc0f8cc7fbf6c6a6904b657 Mon Sep 17 00:00:00 2001 From: Yarchik Date: Thu, 25 Jun 2026 21:11:51 +0100 Subject: [PATCH 2/2] refactor: drop explanatory comments per review The variable names and test descriptions already convey intent, so the inline comments in the implementation and tests are redundant. Logic and assertions are unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/index.ts | 6 ------ test/delimiters.test.ts | 4 ---- 2 files changed, 10 deletions(-) diff --git a/src/index.ts b/src/index.ts index fd7404c..b495664 100644 --- a/src/index.ts +++ b/src/index.ts @@ -131,9 +131,6 @@ const skipSqlContext = (sql: string, position: number): number => { } if (currentChar === charCode.dash && nextChar === charCode.dash) { - // MySQL starts a `--` comment only when the dashes are followed by a - // whitespace/control character (or end of input); `1--2` is an operator - // sequence, not a comment, so it must not swallow following placeholders. const afterDash = sql.charCodeAt(position + 2); if (Number.isNaN(afterDash) || afterDash <= charCode.space) { @@ -145,9 +142,6 @@ const skipSqlContext = (sql: string, position: number): number => { } if (currentChar === charCode.slash && nextChar === charCode.asterisk) { - // `/*! ... */` (executable comments) and `/*+ ... */` (optimizer hints) - // are evaluated by MySQL, so a `?` inside them is a live placeholder and - // must not be skipped like a regular `/* ... */` comment. const markerChar = sql.charCodeAt(position + 2); if (markerChar === charCode.exclamation || markerChar === charCode.plus) diff --git a/test/delimiters.test.ts b/test/delimiters.test.ts index bdeb056..c337018 100644 --- a/test/delimiters.test.ts +++ b/test/delimiters.test.ts @@ -340,15 +340,11 @@ describe('Query comments for debugging', () => { }); test('double dash without trailing whitespace is not a comment', () => { - // MySQL only treats `--` as a comment when followed by whitespace/control; - // `1--2` is an operator sequence, so the placeholder must be substituted. const sql = format('SELECT 1--2, ?', ['VAL']); assert.equal(sql, "SELECT 1--2, 'VAL'"); }); test('executable comment placeholder is substituted, not skipped', () => { - // `/*! ... */` is run by MySQL, so a `?` inside it is a live placeholder - // and the following placeholder must not shift into its slot. const sql = format('SELECT /*!40101 ? */ , ?', ['A', 'B']); assert.equal(sql, "SELECT /*!40101 'A' */ , 'B'"); });