diff --git a/server/scripts/test-sql-ratchet.mjs b/server/scripts/test-sql-ratchet.mjs index 73980c018..9fd995daa 100644 --- a/server/scripts/test-sql-ratchet.mjs +++ b/server/scripts/test-sql-ratchet.mjs @@ -21,8 +21,9 @@ * `exempt` with the reason; it is not counted, and an exemption for a * file that is gone fails; * - a single statement that has to stay raw carries a comment with - * `test-sql-allow:` and a reason, on its own line or the line above. - * A marker with no reason does not count. + * `test-sql-allow:` and a reason, on its own line or the line above + * (a statement broken into a method chain stays covered on the chain + * lines that continue it). A marker with no reason does not count. * * npm run lint:test-sql check against the baseline (CI) * npm run lint:test-sql -- --update lower the counts to what the files hold now; @@ -45,17 +46,39 @@ const SOURCE = /\.(?:[cm]?[jt]s)$/; const PREPARE = /\.prepare\(/g; const ALLOW = /test-sql-allow:\s*\S/; +/** A line that continues the method chain of the line above it (`.prepare(...)` below `db`). */ +const CHAIN = /^\s*\./; + +/** + * The lines an allow marker covers: its own, the line right below it, and the + * method-chain lines that continue that statement. Prettier breaks a long + * `db.prepare('...').all()` into `db` on one line and `.prepare(` on the next, + * which puts the call two lines below the marker without changing what the + * marker is about. + */ +function allowedLines(lines) { + const allowed = new Set(); + lines.forEach((line, i) => { + if (!ALLOW.test(line)) return; + allowed.add(i); + allowed.add(i + 1); + for (let j = i + 2; j < lines.length && CHAIN.test(lines[j]); j++) allowed.add(j); + }); + return allowed; +} + /** * The raw statements in one file's text: every `.prepare(` except those on a - * line that carries an allow marker with a reason, or right below one. + * line that carries an allow marker with a reason, or in the statement right + * below one (its first line and the chain lines that continue it). */ export function countPrepares(text) { const lines = text.replace(/\r\n?/g, '\n').split('\n'); + const allowed = allowedLines(lines); let total = 0; lines.forEach((line, i) => { const hits = line.match(PREPARE)?.length ?? 0; - if (hits === 0) return; - if (ALLOW.test(line) || (i > 0 && ALLOW.test(lines[i - 1]))) return; + if (hits === 0 || allowed.has(i)) return; total += hits; }); return total; diff --git a/server/tests/unit/scripts/test-sql-ratchet.test.ts b/server/tests/unit/scripts/test-sql-ratchet.test.ts index 9f82c629a..786f43b10 100644 --- a/server/tests/unit/scripts/test-sql-ratchet.test.ts +++ b/server/tests/unit/scripts/test-sql-ratchet.test.ts @@ -118,6 +118,19 @@ describe('test-sql-ratchet.mjs', () => { expect(run(serverRoot({ 'tests/a.test.ts': text })).status).toBe(0); }); + it('TSQL-005b: a marker covers a statement broken into a method chain, and no further', () => { + const chained = + `// test-sql-allow: the raw statement is the legacy oracle\n` + + `const legacy = testDb\n ${CALL}'SELECT 1')\n .all();\n`; + expect(run(serverRoot({ 'tests/a.test.ts': chained })).status).toBe(0); + + // The chain ends at the first line that does not start with a dot. + const past = `${chained}const other = testDb\n ${CALL}'SELECT 2')\n .all();\n`; + const { status, out } = run(serverRoot({ 'tests/b.test.ts': past })); + expect(status).toBe(1); + expect(out).toContain('tests/b.test.ts: 1 raw statement(s)'); + }); + it('TSQL-006: a marker without a reason, or two lines up, does not count', () => { const bare = run(serverRoot({ 'tests/a.test.ts': `// test-sql-allow:\n${stmt(1)}` })); expect(bare.status).toBe(1);