From 038c5afeec32c7d5ff746983f692d218c93b69e9 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 6 Sep 2026 17:47:46 +0000 Subject: [PATCH] chore(scripts): retire the deJsxClosingTags workaround in check-vi-mock-inherit The shared masker `scripts/js-comment-mask.mjs` has known since objectui#6891 (PR #7880) that a `/` whose immediately preceding byte is `<` opens no regex, so this gate's local length-preserving rewrite of JSX closing tags no longer changes any reading. Re-measured on this base rather than trusting the card's older figure: walking all 1763 `vi.mock` call sites in 4380 tracked sources with every specifier judged, the RAW mask (rewrite NOT applied) yields 0 sites whose argument list does not balance; the same walk against a copy of the masker with objectui#6891's rule reverted yields 6 in 6 files, one of them the covered `@object-ui/react` site the header names, so the 0 is a live reading and not a collapsed walk. `pnpm check:vi-mock-inherit` is byte-identical before and after this commit. Removed: `JSX_CLOSING_TAG`, `deJsxClosingTags`, its call in `findCallSites` (a pure substitution -- the rewritten source was only ever the input to `scanSource`/`blank`), the test's import of it, and its two unit cases. The two behavioural cases stay and pass with no rewrite in the gate at all; they are the evidence the removal changed nothing. The header section keeps its measured history in the past tense and now records objectui#6891 as closed -- and states explicitly that only the `<` `/` half is closed, the other half being objectui#7882, still open. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01FhBNJcLRZLe8M87VcUgpKr --- .../__tests__/check-vi-mock-inherit.test.ts | 58 ++++++++--------- scripts/check-vi-mock-inherit.mjs | 62 ++++++++----------- 2 files changed, 53 insertions(+), 67 deletions(-) diff --git a/scripts/__tests__/check-vi-mock-inherit.test.ts b/scripts/__tests__/check-vi-mock-inherit.test.ts index 3fbf5252d6..9ce41ec797 100644 --- a/scripts/__tests__/check-vi-mock-inherit.test.ts +++ b/scripts/__tests__/check-vi-mock-inherit.test.ts @@ -11,7 +11,6 @@ import { fileURLToPath } from 'node:url'; import { COVERED_SPECIFIERS, FLOORS, - deJsxClosingTags, findCallSites, scan, summarise, @@ -335,10 +334,10 @@ describe('only text the language would execute', () => { }); // --------------------------------------------------------------------------- -// The JSX mask — a real mis-mask in the shared scanner, measured +// The JSX mask — a real mis-mask in the shared scanner, since fixed // --------------------------------------------------------------------------- -describe('deJsxClosingTags — the shared masker USED to read `` as a regex literal', () => { +describe('a JSX closing tag — the shared masker USED to read `` as a regex literal', () => { /** * `js-comment-mask` opened a regex when a `/` followed something that is not * a value. In `` that something is `<`, so a PHANTOM regex opened and @@ -346,11 +345,22 @@ describe('deJsxClosingTags — the shared masker USED to read `` as a rege * `)` that closes a `vi.mock` call. Measured on this tree at the time: SEVEN * call sites could not be delimited at all, one of them a covered site. * - * Fixed in the shared module by objectui#6891, whose own pin - * (`scripts/__tests__/js-comment-mask-jsx-6891.test.ts`) now holds the - * scanner. `deJsxClosingTags` stays: it is still correct, still length- - * preserving, and removing it belongs to whoever owns THIS gate's source. - * The first case below is what makes that a decision rather than a guess. + * That is history. The shared masker handles `` itself since + * objectui#6891, whose own pin + * (`scripts/__tests__/js-comment-mask-jsx-6891.test.ts`) holds the scanner, + * and THIS gate rewrites nothing before masking any more — objectui#7883 + * retired the local `deJsxClosingTags` workaround and its two unit cases + * with it. + * + * The three cases below are what say the retirement changed nothing: the + * first reads the mask directly on the RAW source, and the two behavioural + * ones drive the gate end to end on a factory that returns JSX. They pass + * with no rewrite in the gate at all. + * + * ⛔ Not a claim that the masker is correct on JSX: objectui#6891 closed + * only the `<` `/` half, and a `/` after `}` or `>` still opens a phantom + * (objectui#7882, still open). The retired rewrite never covered that half + * either, so nothing was lost with it. */ const jsxFactory = `({ open, children }: any) => (open ?
{children}
: null)`; @@ -363,10 +373,11 @@ describe('deJsxClosingTags — the shared masker USED to read `` as a rege // whose immediately preceding byte is `<` opens nothing, and this case // has been turned over to pin the fix instead. // - // `deJsxClosingTags` is deliberately NOT removed in that change — it is a - // second gate's source, outside that card's file surface. It is now a - // no-op-in-effect on this shape, and the assertions below are what say so: - // the raw source, WITHOUT the rewrite, already masks correctly. + // `deJsxClosingTags` was deliberately NOT removed in that change — it was + // a second gate's source, outside that card's file surface. objectui#7883 + // then retired it, and these assertions are what made that a decision + // rather than a guess: the raw source, with no rewrite anywhere, already + // masks correctly. const src = `const C = ${jsxFactory};\n`; const { literal } = scanSource(src); const inside = src.indexOf('') + 2; @@ -377,26 +388,11 @@ describe('deJsxClosingTags — the shared masker USED to read `` as a rege expect(literal[src.lastIndexOf(')')]).toBe(0); }); - it('neutralises the tag while PRESERVING LENGTH, so every offset still holds', () => { - const src = 'abcz'; - const out = deJsxClosingTags(src); - expect(out).toHaveLength(src.length); - expect(out).toBe('a<____>b<_>c<________>z'); - // Every offset past the rewrite still indexes the same byte, which is what - // lets the mask's flags be read against the ORIGINAL source. - expect(out.indexOf('z')).toBe(src.indexOf('z')); - }); - - it('leaves a `/` that is not a closing tag alone — a regex, a path, a division', () => { - for (const src of ['const re = //;', 'const p = "a/b";', 'const q = a / b;', 'x.replace(/ { - // Without the workaround this call site is `unreadable`. `unreadable` fails - // the gate, so the mis-mask would not have been silent — but it would have - // reddened five innocent files instead of judging them. + // Under the mis-mask this call site was `unreadable`. `unreadable` fails + // the gate, so the defect would not have been silent — but it would have + // reddened five innocent files instead of judging them. This case is now + // the load-bearing half: it goes red if the shared masker ever regresses. const site = verdictOf(`async (importOriginal) => ({ ...(await importOriginal()), C: ${jsxFactory} })`); expect(site.verdict).toBe('inherits'); }); diff --git a/scripts/check-vi-mock-inherit.mjs b/scripts/check-vi-mock-inherit.mjs index 19c16c2521..0a999c085f 100644 --- a/scripts/check-vi-mock-inherit.mjs +++ b/scripts/check-vi-mock-inherit.mjs @@ -302,32 +302,35 @@ * the shared `js-comment-mask.mjs`, exactly as the sibling gate does it, and * for the same reasons (this file's own header quotes the defect in prose). * - * ## `js-comment-mask` reads a JSX closing tag as a regex literal + * ## `js-comment-mask` USED to read a JSX closing tag as a regex literal * - * The shared masker decides a `/` opens a regex when the preceding character is - * not a value. In `` the preceding character is `<`, so it opens a - * PHANTOM regex that runs to the end of the line and swallows whatever is + * Kept as measured history: it is why the shared module was fixed, and the two + * behavioural cases in this gate's test still pin the outcome. + * + * The shared masker decided a `/` opens a regex when the preceding character is + * not a value. In `` the preceding character is `<`, so it opened a + * PHANTOM regex that ran to the end of the line and swallowed whatever was * there -- including the `)` that closes a `vi.mock` call. * - * That is not hypothetical here: measured on this tree, SEVEN `vi.mock` call - * sites in five files could not have their argument list delimited at all - * because of it, one of them a covered `@object-ui/react` site + * That was not hypothetical here: measured on this tree at the time, SEVEN + * `vi.mock` call sites in five files could not have their argument list + * delimited at all because of it, one of them a covered `@object-ui/react` site * (`plugin-dashboard/src/__tests__/ObjectDataTable.cells.test.tsx`). The sibling * gate never noticed because it only reads the specifier; this gate reads the - * factory BODY, so it cannot. - * - * `deJsxClosingTags` neutralises it, and the shape of the fix is what keeps it - * safe: a JSX closing tag is rewritten to the SAME NUMBER OF BYTES - * (`` -> `<____>`) before masking, so every offset the mask returns still - * indexes the original source, and the only bytes that change are slashes that - * cannot be part of a spread, an identifier, or a specifier. A `` -- a self-closing tag, a `/` in JSX text + * -- still opens a phantom; that half is objectui#7882 and is still open. The + * retired rewrite never covered it either (its pattern matched closing tags + * only), which is why removing it lost no coverage. */ import { execFileSync } from 'node:child_process'; @@ -382,18 +385,6 @@ export const FLOORS = Object.freeze({ covered: 50, }); -/** A JSX closing tag: ``, ``, ``. */ -const JSX_CLOSING_TAG = /<\/([A-Za-z_$][\w$.:-]*)?\s*>/g; - -/** - * `source` with the slash of every JSX closing tag replaced, PRESERVING LENGTH, - * so offsets from the mask still index the original. See the header section on - * `js-comment-mask` for the measurement that made this necessary. - */ -export function deJsxClosingTags(source) { - return source.replace(JSX_CLOSING_TAG, (m) => `<${'_'.repeat(m.length - 2)}>`); -} - /** 1-based line number of `offset` in `source`. */ function lineOf(source, offset) { let line = 1; @@ -682,9 +673,8 @@ export function classifyFactory(masked, literal, start, end, specifier) { * for the instance that made this distinction necessary). */ export function findCallSites(source, { covered = COVERED_SPECIFIERS } = {}) { - const dejsxed = deJsxClosingTags(source); - const { comment, literal } = scanSource(dejsxed); - const masked = blank(dejsxed, comment); + const { comment, literal } = scanSource(source); + const masked = blank(source, comment); const coveredSet = new Set(covered); const sites = [];