diff --git a/lib/pregate/checks/literals.ts b/lib/pregate/checks/literals.ts index 53567dfe9..7a02f0ef5 100644 --- a/lib/pregate/checks/literals.ts +++ b/lib/pregate/checks/literals.ts @@ -17,7 +17,7 @@ export interface ChangedLiteral { name: string; old: string; new: string; file: /** A consumer line a `pregate-literal:` marker waived; `added` when the merge-base file lacks the marker. */ export interface LiteralWaiver { at: string; reason: string; added: boolean } -const MARKER = /pregate-literal:[ \t]*([A-Za-z_]\w*)=(\S+)[ \t]+(\S.*?)[ \t]*(?:\*\/[ \t]*)?$/; +const MARKER = /pregate-literal:[ \t]*([A-Za-z_]\w*)=(\S+)[ \t]+((?!\*\/)\S.*?)[ \t]*(?:\*\/[ \t]*)?$/; /** The marker in this line's trailing comment (`//`, `#` or `/*` outside a string), or null. */ function markerIn(line: string): { marker: string; name: string; value: string; reason: string } | null { @@ -153,8 +153,8 @@ export function consumersOf(repoRoot: string, testFiles: readonly string[], chan return extended; }; const waiver = (i: number) => { - const m = markerIn(lines[i]!) ?? (i > 0 && comment(lines[i - 1]!) ? markerIn(lines[i - 1]!) : null); - if (!m || m.name !== change.name || unquote(m.value) !== value) return false; + const m = [markerIn(lines[i]!), i > 0 && comment(lines[i - 1]!) ? markerIn(lines[i - 1]!) : null].find(c => c && c.name === change.name && unquote(c.value) === value); + if (!m) return false; const before = baseText?.(rel); waived?.push({ at: `${rel}:${i + 1}`, reason: m.reason, added: typeof before !== 'string' || !before.includes(m.marker) }); return true; diff --git a/test/pregate-selection.test.ts b/test/pregate-selection.test.ts index 17376df1b..2277398c0 100644 --- a/test/pregate-selection.test.ts +++ b/test/pregate-selection.test.ts @@ -300,7 +300,8 @@ describe('literals (pure)', () => { }); // Value: protects=a test that keeps an old value on purpose (the refused input after a minimum-version bump) can waive that one line // with a `pregate-literal: NAME=value reason` comment, while every other old-value line, a marker for another constant or value, - // a marker with no reason and marker text inside a string still count; fails_when=the waiver widens past its line or a stray + // a marker with no reason (a bare `*/` closer is not a reason) and marker text inside a string still count, and a line marked + // for one constant can still be waived for another by the comment line above; fails_when=the waiver widens past its line or a stray // marker clears a real consumer (#3490, gbrain#7690); why_new=the only escape was warn: ["literals"] for every constant; // seam=none; no_claim=the policy file (no key exists; see the config test) test('consumersOf: a pregate-literal marker waives only its own line, for its own constant and old value, with a reason (#3490)', () => { @@ -320,18 +321,21 @@ describe('literals (pure)', () => { 'test/in-string.test.ts': ["import { MIN_RUNTIME } from '../src/runtime';", "const src = \"expect(v('1.4.0')); // pregate-literal: MIN_RUNTIME='1.4.0' looks waived\"; check('1.4.0');", ''].join(nl), 'test/two-above.test.ts': ["import { MIN_RUNTIME } from '../src/runtime';", "// pregate-literal: MIN_RUNTIME='1.4.0' too far above", '', "expect(checkRuntime('1.4.0')).toBe('refused');", ''].join(nl), 'test/all-waived.test.ts': ["import { MIN_RUNTIME } from '../src/runtime';", "expect(checkRuntime('1.4.0')).toBe('refused'); # pregate-literal: MIN_RUNTIME='1.4.0' refused input", ''].join(nl), + 'test/block-no-reason.test.ts': ["import { MIN_RUNTIME } from '../src/runtime';", "expect(checkRuntime('1.4.0')).toBe('refused'); /* pregate-literal: MIN_RUNTIME='1.4.0' */", ''].join(nl), + 'test/two-constants.test.ts': ["import { MIN_RUNTIME } from '../src/runtime';", "// pregate-literal: MIN_RUNTIME='1.4.0' lower bound of the refused range", "expect(inRange('1.4.0', '2.0.0')).toBe(false); /* pregate-literal: MAX_RUNTIME='2.0.0' upper bound */", ''].join(nl), }; for (const [rel, text] of Object.entries(files)) { fs.mkdirSync(path.dirname(path.join(root, rel)), { recursive: true }); fs.writeFileSync(path.join(root, rel), text); } const change = { name: 'MIN_RUNTIME', old: "'1.4.0'", new: "'1.4.4'", file: 'src/runtime.ts' }; const waived: LiteralWaiver[] = []; const base = (rel: string) => rel === 'test/runtime.test.ts' ? files[rel]!.split(nl).slice(0, 2).join(nl) : null; expect(consumersOf(root, Object.keys(files), change, base, waived)).toEqual([ - 'test/runtime.test.ts:5', 'test/other-name.test.ts:2', 'test/other-value.test.ts:2', 'test/no-reason.test.ts:2', 'test/in-string.test.ts:2', 'test/two-above.test.ts:4', + 'test/runtime.test.ts:5', 'test/other-name.test.ts:2', 'test/other-value.test.ts:2', 'test/no-reason.test.ts:2', 'test/in-string.test.ts:2', 'test/two-above.test.ts:4', 'test/block-no-reason.test.ts:2', ]); expect(waived).toEqual([ { at: 'test/runtime.test.ts:2', reason: 'refused input below the minimum', added: false }, { at: 'test/runtime.test.ts:4', reason: 'the oldest runtime we still name in the error', added: true }, { at: 'test/all-waived.test.ts:2', reason: 'refused input', added: true }, + { at: 'test/two-constants.test.ts:3', reason: 'lower bound of the refused range', added: true }, ]); fs.rmSync(root, { recursive: true, force: true }); });