diff --git a/changelog.d/gsta-145-literals-history.md b/changelog.d/gsta-145-literals-history.md index 372f06332..cf835926b 100644 --- a/changelog.d/gsta-145-literals-history.md +++ b/changelog.d/gsta-145-literals-history.md @@ -1,8 +1,8 @@ **pregate `literals` no longer flags a version-history table that the branch extended.** A test that keeps one row per past version of a constant (`[45, ""]`, then `[46, ""]`) used to fail `literals` on every bump, because the old row still holds the old value. Truther turned the check down to a warning over it, which also hid real stale tests. ### Fixed -- `lib/pregate/checks/literals.ts`: a row that starts with the old value is skipped when the same table holds a new-value row the branch added (the file at the merge base lacks it). The scan continues to the next line holding the old value, so `expect(PROOF_CHECKS).toBe(45)` below an extended table still fails. A table the branch did not extend, a file that already held a new-value row at the merge base (read through a rename), and a merge base that cannot be read still fail. +- `lib/pregate/checks/literals.ts`: a row that starts with the old value is skipped when the same table holds a new-value row the branch added (the file at the merge base lacks it). Only the row's first field is exempt, and the scan continues to the next line holding the old value, so `expect(PROOF_CHECKS).toBe(45)` below an extended table still fails. A table the branch did not extend, a file that already held a new-value row at the merge base (read through a rename git detects), and a merge base that cannot be read still fail. - docs/pregate.md describes the history-row rule and the table shapes it recognizes. ### Tests -- `test/pregate-selection.test.ts`: an extended table, a newest-first table and a new file pass; an unextended table, a stale assertion below an extended table, an unextended table beside an extended one, and a table that already held the new row fail at the right line; string-valued and quoted semver histories pass; and an end-to-end `gstack-pregate --only literals` run fails before the table is extended, passes after, and still fails for a renamed file whose table already held the new row. +- `test/pregate-selection.test.ts`: an extended table, a newest-first table and a new file pass; an unextended table, a stale assertion below an extended table, an unextended table beside an extended one, and a table that already held the new row fail at the right line; string-valued and quoted semver histories pass; a stale assertion on the same line as a history row fails; a 20,000-row table is checked in linear time; and an end-to-end `gstack-pregate --only literals` run fails before the table is extended, passes after, and still fails for a renamed file whose table already held the new row. diff --git a/docs/pregate.md b/docs/pregate.md index f37455562..5db84b139 100644 --- a/docs/pregate.md +++ b/docs/pregate.md @@ -172,8 +172,8 @@ Two stages with separate clocks, both printed: `file:line`. A version-history row, a line that starts a bracketed row with the old value (`[45, ""]`), is skipped when the same table (a run of consecutive lines starting with `[`) also holds a row for the new value that - the branch added (the file at the merge base, read through a rename, has no - new-value row). A table the branch did not extend, a file that already held a + the branch added (the file at the merge base, read through a rename git + detects, has no new-value row). A table the branch did not extend, a file that already held a new-value row at the merge base, a merge base that cannot be read, and every other line holding the old value still fail. Rows in object form (`{ v: 45 }`) or spread over several lines are not recognized as history rows. diff --git a/lib/pregate/checks/literals.ts b/lib/pregate/checks/literals.ts index 5c428c6ed..e6ce81021 100644 --- a/lib/pregate/checks/literals.ts +++ b/lib/pregate/checks/literals.ts @@ -78,7 +78,7 @@ function unquote(v: string): string { return /^['"`]/.test(v) ? v.slice(1, -1) : export function consumersOf(repoRoot: string, testFiles: readonly string[], change: ChangedLiteral, baseText?: (rel: string) => string | null | undefined): string[] { const value = unquote(change.old); const valueRe = /^-?\d/.test(value) ? new RegExp(`(? new RegExp(`^\\s*\\[\\s*${/^['"`]/.test(raw) ? `['"\`]${escape(unquote(raw))}['"\`]` : escape(raw)}\\s*,`, 'm'); + const row = (raw: string) => new RegExp(`^[ \\t]*\\[[ \\t]*${/^['"`]/.test(raw) ? `['"\`]${escape(unquote(raw))}['"\`]` : escape(raw)}[ \\t]*,`, 'm'); const oldRowRe = row(change.old); const newRowRe = row(change.new); const base = path.basename(change.file); @@ -92,13 +92,19 @@ export function consumersOf(repoRoot: string, testFiles: readonly string[], chan try { text = fs.readFileSync(path.join(repoRoot, rel), 'utf8'); } catch { continue; } if (!(nameRe.test(text) || (!nameRequired && moduleRe.test(text)))) continue; const lines = text.split('\n'); + const table: number[] = []; + const tableHasNew: boolean[] = []; + for (let i = 0; i < lines.length; i++) { + if (!/^[ \t]*\[/.test(lines[i]!)) { table.push(-1); continue; } + if (table[i - 1] === undefined || table[i - 1] === -1) tableHasNew.push(false); + table.push(tableHasNew.length - 1); + if (newRowRe.test(lines[i]!)) tableHasNew[tableHasNew.length - 1] = true; + } let extended: boolean | undefined; const skip = (i: number) => { - if (!oldRowRe.test(lines[i]!)) return false; - let a = i; while (a > 0 && /^\s*\[/.test(lines[a - 1]!)) a--; - let b = i; while (b < lines.length - 1 && /^\s*\[/.test(lines[b + 1]!)) b++; - if (!lines.slice(a, b + 1).some(l => newRowRe.test(l))) return false; - if (extended === undefined) { const b = baseText?.(rel); extended = b === null || (b !== undefined && !newRowRe.test(b)); } + const line = lines[i]!; + if (!oldRowRe.test(line) || !tableHasNew[table[i]!] || valueRe.test(line.replace(oldRowRe, '['))) return false; + if (extended === undefined) { const base = baseText?.(rel); extended = base === null || (base !== undefined && !newRowRe.test(base)); } return extended; }; for (let i = 0; i < lines.length; i++) if (valueRe.test(lines[i]!) && !skip(i)) { hits.push(`${rel}:${i + 1}`); break; } diff --git a/test/pregate-selection.test.ts b/test/pregate-selection.test.ts index 027f587c4..2a4eda364 100644 --- a/test/pregate-selection.test.ts +++ b/test/pregate-selection.test.ts @@ -227,7 +227,7 @@ describe('literals (pure)', () => { const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-literals-history-')); const nl = String.fromCharCode(10); const head = "import { PROOF_CHECKS } from '../src/proof-checks.ts';"; - const rows = (...ns: number[]) => ns.map(n => ` [${n}, "digest-${n}"],`); + const rows = (...ns: number[]) => ns.map(n => ` [${n}, "sha${n}"],`); const files: Record = { 'test/history.test.ts': [[head, 'const versions = [', ...rows(44, 45, 46), '];', 'expect(PROOF_CHECKS).toBe(46);'], [head, 'const versions = [', ...rows(44, 45), '];', 'expect(PROOF_CHECKS).toBe(45);']], 'test/newest-first.test.ts': [[head, 'const versions = [', ...rows(46, 45, 44), '];'], [head, 'const versions = [', ...rows(45, 44), '];']], @@ -236,15 +236,17 @@ describe('literals (pure)', () => { 'test/two-tables.test.ts': [[head, 'const a = [', ...rows(44, 45), '];', 'const b = [', ...rows(45, 46), '];'], [head, 'const a = [', ...rows(44, 45), '];', 'const b = [', ...rows(45), '];']], 'test/boundary.test.ts': [[head, 'test.each([', ...rows(45, 46), '])("%d", () => {});'], [head, 'test.each([', ...rows(45, 46), '])("%d", () => {});']], 'test/new-file.test.ts': [[head, 'const versions = [', ...rows(45, 46), '];'], null], + 'test/same-line.test.ts': [[head, 'const versions = [', ' [45, "sha45"], expect(PROOF_CHECKS).toBe(45),', ' [46, "sha46"],', '];'], [head, 'const versions = [', ' [45, "sha45"], expect(PROOF_CHECKS).toBe(45),', '];']], }; for (const [rel, [now]] of Object.entries(files)) { fs.mkdirSync(path.dirname(path.join(root, rel)), { recursive: true }); fs.writeFileSync(path.join(root, rel), now.join(nl) + nl); } const baseText = (rel: string) => files[rel]?.[1]?.join(nl) ?? null; const lineOf = (rel: string, text: string) => `${rel}:${files[rel]![0].indexOf(text) + 1}`; expect(consumersOf(root, Object.keys(files), { name: 'PROOF_CHECKS', old: '45', new: '46', file: 'src/proof-checks.ts' }, baseText).sort()).toEqual([ - lineOf('test/boundary.test.ts', ' [45, "digest-45"],'), + lineOf('test/boundary.test.ts', ' [45, "sha45"],'), + lineOf('test/same-line.test.ts', ' [45, "sha45"], expect(PROOF_CHECKS).toBe(45),'), lineOf('test/stale-later.test.ts', 'expect(PROOF_CHECKS).toBe(45);'), - lineOf('test/two-tables.test.ts', ' [45, "digest-45"],'), - lineOf('test/unextended.test.ts', ' [45, "digest-45"],'), + lineOf('test/two-tables.test.ts', ' [45, "sha45"],'), + lineOf('test/unextended.test.ts', ' [45, "sha45"],'), ].sort()); fs.writeFileSync(path.join(root, 'test/versions.test.ts'), ["import { SCHEMA } from '../src/schema.ts';", 'const history = [', " ['v3', 'a'],", ' ["v4", \'b\'],', '];', ''].join(nl)); const strings = (rel: string) => rel === 'test/versions.test.ts' ? ["import { SCHEMA } from '../src/schema.ts';", 'const history = [', " ['v3', 'a'],", '];'].join(nl) : null; @@ -253,6 +255,8 @@ describe('literals (pure)', () => { fs.writeFileSync(path.join(root, 'test/semver.test.ts'), ["import { RELEASE } from '../src/release.ts';", 'const history = [', " ['1.2.3', 'a'],", " ['1.2.4', 'b'],", '];', ''].join(nl)); expect(consumersOf(root, ['test/semver.test.ts'], { name: 'RELEASE', old: "'1.2.3'", new: "'1.2.4'", file: 'src/release.ts' }, () => null)).toEqual([]); expect(consumersOf(root, ['test/semver.test.ts'], { name: 'RELEASE', old: "'1.2.3'", new: "'1.2.4'", file: 'src/release.ts' }, () => " ['1.2.4', 'b'],")).toEqual(['test/semver.test.ts:3']); + fs.writeFileSync(path.join(root, 'test/big.test.ts'), [head, 'const versions = [', ...Array.from({ length: 20_000 }, () => ' [45, "x"],'), ' [46, "y"],', '];', ...Array.from({ length: 20_000 }, () => ''), ''].join(nl)); + expect(consumersOf(root, ['test/big.test.ts'], { name: 'PROOF_CHECKS', old: '45', new: '46', file: 'src/proof-checks.ts' }, () => ['', ...Array.from({ length: 20_000 }, () => '')].join(nl))).toEqual([]); fs.rmSync(root, { recursive: true, force: true }); }); test('runner summaries: bun, node --test, vitest, jest, pytest, zero-run', () => { @@ -402,7 +406,7 @@ describe('gstack-pregate (bin)', () => { }); test('GST-344: a history table the branch extends passes literals through the merge-base read; one it does not extend fails', () => { - const history = (...ns: number[]) => ["const { MAX_RETRIES } = require('../lib/retry.js');", 'const versions = [', ...ns.map(n => ` [${n}, 'digest-${n}'],`), '];', ''].join('\n'); + const history = (...ns: number[]) => ["const { MAX_RETRIES } = require('../lib/retry.js');", 'const versions = [', ...ns.map(n => ` [${n}, 'sha${n}'],`), '];', ''].join('\n'); const repo = fixture({ baseFiles: { 'test/history.test.js': history(2, 3) } }); const bump = { 'lib/retry.js': "exports.MAX_RETRIES = 4;\nexports.TIMEOUT_MS = 5000;\nexports.retry = (fn) => fn();\n", 'test/retry.test.js': "const test = require('node:test');\nconst assert = require('node:assert');\nconst { MAX_RETRIES } = require('../lib/retry.js');\ntest('retries four times', () => assert.strictEqual(MAX_RETRIES, 4));\n" }; commit(repo, bump);