Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 27 additions & 31 deletions scripts/__tests__/check-vi-mock-inherit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import { fileURLToPath } from 'node:url';
import {
COVERED_SPECIFIERS,
FLOORS,
deJsxClosingTags,
findCallSites,
scan,
summarise,
Expand Down Expand Up @@ -335,22 +334,33 @@ 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 `</div>` as a regex literal', () => {
describe('a JSX closing tag — the shared masker USED to read `</div>` as a regex literal', () => {
/**
* `js-comment-mask` opened a regex when a `/` followed something that is not
* a value. In `</div>` that something is `<`, so a PHANTOM regex opened and
* ran to the end of the line, swallowing whatever was there — including the
* `)` 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 `</tag>` 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 ? <div>{children}</div> : null)`;
Expand All @@ -363,10 +373,11 @@ describe('deJsxClosingTags — the shared masker USED to read `</div>` 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('</div>') + 2;
Expand All @@ -377,26 +388,11 @@ describe('deJsxClosingTags — the shared masker USED to read `</div>` as a rege
expect(literal[src.lastIndexOf(')')]).toBe(0);
});

it('neutralises the tag while PRESERVING LENGTH, so every offset still holds', () => {
const src = 'a</div>b</>c</Foo.Bar>z';
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 = /<x>/;', 'const p = "a/b";', 'const q = a / b;', 'x.replace(/</g, "&lt;");']) {
expect(deJsxClosingTags(src)).toBe(src);
}
});

it('THE CONSEQUENCE: a covered factory returning JSX is READ, not skipped', () => {
// 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');
});
Expand Down
62 changes: 26 additions & 36 deletions scripts/check-vi-mock-inherit.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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 `</div>` 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 `</div>` 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
* (`</div>` -> `<____>`) 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 `</` inside a
* string or a regex is rewritten too and does not matter: it is literal content
* either way, and its quotes are untouched, so nothing structural moves.
* Measured: the seven unreadable sites become zero, and no site changes verdict.
*
* This is a LOCAL workaround in this gate, not a change to the shared masker --
* that module is used by many gates and its JSX behaviour is filed separately
* as objectui#6891.
* factory BODY, so it could not.
*
* This gate carried a LOCAL workaround for it -- `deJsxClosingTags`, a
* length-preserving rewrite of every closing tag applied before masking. The
* shared masker itself was then fixed by objectui#6891 (CLOSED, PR #7880),
* which taught `scanSource` that a `/` whose immediately preceding byte is `<`
* opens nothing. Re-measured on the fixed masker, the raw source with the
* rewrite NOT applied has ZERO undelimitable sites and the gate's verdict is
* byte-identical either way, so the workaround was retired by objectui#7883.
* ⛔ That is not "the masker is correct on JSX": objectui#6891 closed only the
* `<` `/` half. A `/` after `}` or `>` -- 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';
Expand Down Expand Up @@ -382,18 +385,6 @@ export const FLOORS = Object.freeze({
covered: 50,
});

/** A JSX closing tag: `</div>`, `</Foo.Bar>`, `</>`. */
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;
Expand Down Expand Up @@ -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 = [];
Expand Down
Loading