From 89e2b97895d2251a33167627eeaf5698c85e148d Mon Sep 17 00:00:00 2001 From: mintaka Date: Wed, 16 Sep 2026 22:25:04 -0400 Subject: [PATCH] fix(tools): parse UI source with the TypeScript compiler in the font gate (RIG-3855) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The font-coverage gate located rendered characters with a hand-rolled comment/string lexer. It had no JSX or regex-literal awareness, so an apostrophe in JSX text opened a string state that never closed on that line and swallowed everything after it until the next matching quote. Measured on `apps/ui/src`: three files and 42 lines sat inside such a fake quote state, invisible to the scan. `RightSidebar.tsx` shows both directions of the bug — the apostrophe in `{handle}'s workspace` hid the following lines, and when the state closed on a later quote the em-dash inside a doc comment was reported as a rendered character. The sibling gap had the same cause: a `//` inside a regex literal blanked its line. The gate is WARN-only today, so the false positive is harmless. The suppression is not: RIG-3742 makes the gate fail-closed, at which point an uncovered glyph written into any of those lines passes green, which is the failure the gate exists to prevent. The file's own header claimed these gaps did not fire on the current tree; that claim was wrong and is removed. `scanSource` now parses with the TypeScript compiler and reads only leaf tokens, so comment bodies drop out as trivia and JSX text, regex literals, and template spans are located as the parser tokenizes them. JSDoc subtrees are skipped explicitly: they are the one comment form surfaced as real nodes, and without the skip every doc-comment em-dash becomes a finding. `typescript` was already a declared devDependency of this tool, so no dependency is added. `stripComments` is deleted; it had no remaining references. Verification. Old and new scanners were compared over 163 files keyed `path:line:codepoint`: 94 findings before, 93 after, the single dropped one being the doc-comment false positive, and nothing newly missed. A span oracle marking every byte reached at a leaf or inside a skipped JSDoc subtree, against comment trivia from `getLeadingCommentRanges`, found no non-ASCII byte in neither set — no fail-open path. Every finding's reported line and column round-trips back to its own character. Malformed input (a hard parse error, an unterminated string, JSX in a `.ts` file) still yields findings rather than silently returning none. Three tests pin the defect classes and genuinely fail against the old implementation. Three more pin positions on tokens with leading trivia: mutating the offset base to `getFullStart` previously survived the whole suite, so a class of location bug was unpinned on a gate whose entire output is `path:line:column`. `Finding.column` is now documented as a UTF-16 code-unit offset and pinned by a test, since the old loop counted codepoints. One assumption is recorded rather than fixed: a `.ts` file containing JSX would be parsed as non-JSX and its JSX text unscanned. It cannot fire while JSX lives in `.tsx`, and parsing everything as TSX would change how type-assertion casts parse. RIG-3855 Co-authored-by: Matt Wilkinson --- tools/font-coverage-gate/index.test.ts | 85 +++++++++++++--- tools/font-coverage-gate/index.ts | 136 +++++++++++-------------- 2 files changed, 128 insertions(+), 93 deletions(-) diff --git a/tools/font-coverage-gate/index.test.ts b/tools/font-coverage-gate/index.test.ts index bf0bd8e5..cfe8750b 100644 --- a/tools/font-coverage-gate/index.test.ts +++ b/tools/font-coverage-gate/index.test.ts @@ -15,7 +15,6 @@ import { type Finding, resolveMode, scanSource, - stripComments, } from "./index.ts"; const FONTS = `${import.meta.dir}/../../apps/eng-docs/public/fonts`; @@ -51,7 +50,7 @@ describe("cmapCodepoints", () => { }); // --------------------------------------------------------------------------- -// stripComments / scanSource — comment awareness + line preservation. +// scanSource — token awareness: comments excluded, rendered chars located. // --------------------------------------------------------------------------- describe("scanSource", () => { @@ -74,9 +73,9 @@ describe("scanSource", () => { expect(found.map((f) => f.codepoint)).toEqual([0x27e9]); }); - test("reports the correct line AFTER stripping a multi-line block comment", () => { - // A naive strip that DELETES comment lines would report the ■ on line 2, - // not 4. The space-preserving strip keeps it on line 4. + test("reports the correct line after a multi-line block comment", () => { + // Positions come from the source file's own line map, so the ■ is on + // line 4 — not line 2, as a scan of comment-free text would report. const text = ["/* a", " b", "*/", 'const t = "■";', ""].join("\n"); const found = scanSource("apps/ui/src/a.ts", text); expect(found).toHaveLength(1); @@ -84,7 +83,7 @@ describe("scanSource", () => { expect(found[0]?.codepoint).toBe(0x25a0); }); - test("column is 1-based within the (stripped) line", () => { + test("column is 1-based within the line", () => { const found = scanSource("apps/ui/src/a.ts", 'x="▸"\n'); expect(found[0]?.column).toBe(4); }); @@ -96,21 +95,75 @@ describe("scanSource", () => { ); }); - test("an escaped quote does not end a string, so // inside stays a string", () => { - // The \" does not close the string; the // and the ▸ are still inside it. + test("a // inside a string literal is not a comment", () => { const found = scanSource("apps/ui/src/a.ts", 'const s = "a\\" // ▸";\n'); expect(found.map((f) => f.codepoint)).toEqual([0x25b8]); }); -}); -// --------------------------------------------------------------------------- -// stripComments — direct: line count is invariant. -// --------------------------------------------------------------------------- + test("locates a glyph inside a multi-line template literal", () => { + const found = scanSource( + "apps/ui/src/a.ts", + "const s = `intro\n body ▸ tail`;\n", + ); + expect(found).toHaveLength(1); + expect(found[0]?.line).toBe(2); + expect(found[0]?.column).toBe(8); + }); + + test("a leading comment does not shift the reported column", () => { + const found = scanSource("apps/ui/src/a.ts", '// note\nconst s = "▸";\n'); + expect(found).toHaveLength(1); + expect(found[0]?.line).toBe(2); + expect(found[0]?.column).toBe(12); + }); + + test("locates a glyph in multi-line JSX text", () => { + const text = "const e = (\n

\n go ▸\n

\n);\n"; + const found = scanSource("apps/ui/src/a.tsx", text); + expect(found).toHaveLength(1); + expect(found[0]?.line).toBe(3); + expect(found[0]?.column).toBe(8); + }); + + test("an apostrophe in JSX text does not hide a glyph on a later line", () => { + // An apostrophe in JSX text is prose, not a string delimiter, so it must + // not affect how later lines are scanned. + const text = [ + "

Matt's row

;", + "go ▸;", + "", + ].join("\n"); + const found = scanSource("apps/ui/src/a.tsx", text); + expect(found).toHaveLength(1); + expect(found[0]?.line).toBe(2); + expect(found[0]?.codepoint).toBe(0x25b8); + }); + + test("an apostrophe in JSX text does not turn a later doc comment into a finding", () => { + // A doc comment renders nothing, so its em-dash is not a finding. + const text = [ + "

Matt's row

;", + "/** A pane — see notes. */", + "const x = 1;", + "", + ].join("\n"); + expect(scanSource("apps/ui/src/a.tsx", text)).toEqual([]); + }); + test("a // inside a regex literal does not blank the rest of the line", () => { + // The `//` is regex syntax, not a comment, so the ▸ after it is rendered. + const found = scanSource( + "apps/ui/src/a.ts", + 'const re = /https?:\\/\\//; const s = "▸";\n', + ); + expect(found).toHaveLength(1); + expect(found[0]?.line).toBe(1); + expect(found[0]?.codepoint).toBe(0x25b8); + }); -describe("stripComments", () => { - test("preserves the line count of a block comment", () => { - const text = "/* a\nb\nc */\nx"; - expect(stripComments(text).split("\n")).toHaveLength(4); + test("an astral character is one finding, not two surrogate halves", () => { + const found = scanSource("apps/ui/src/a.ts", 'const s = "𝄞 ▸";\n'); + expect(found.map((f) => f.codepoint)).toEqual([0x1d11e, 0x25b8]); + expect(found[1]?.column).toBe(15); }); }); diff --git a/tools/font-coverage-gate/index.ts b/tools/font-coverage-gate/index.ts index 42f46983..f411f022 100644 --- a/tools/font-coverage-gate/index.ts +++ b/tools/font-coverage-gate/index.ts @@ -28,11 +28,9 @@ // * Only LITERAL non-ASCII bytes are seen. A "\u25b8" escape, a ▸ // entity, or String.fromCodePoint renders the glyph invisibly to the scan. // The tree writes literal glyphs throughout, which is what makes this safe. -// * stripComments has no regex-literal state, so a `//` inside a regex blanks -// the rest of that line; and an apostrophe in JSX text ("don't") opens a -// string state that can suppress a later comment strip. Neither fires on -// the current tree (verified against a whole-tree oracle sweep). // * Only UI_SRC_DIR is scanned; apps/ui/e2e authors baselines too. +// * A .ts file containing JSX is parsed as non-JSX, so JSX text in it is not +// scanned. The tree keeps JSX in .tsx, which is what makes this safe. // // Inputs (env): // GATE_ROOT - directory to scan (default: git toplevel). @@ -43,6 +41,7 @@ // 2 - usage / internal error (font missing, truncated, or implausibly small) import { $ } from "bun"; +import ts from "typescript"; /** The UI source root whose rendered characters the gate governs. */ export const UI_SRC_DIR = "apps/ui/src"; @@ -63,6 +62,7 @@ export type Mode = "warn" | "error"; export interface Finding { path: string; line: number; + /** 1-based UTF-16 code-unit offset within the line. */ column: number; char: string; codepoint: number; @@ -188,91 +188,73 @@ function readFormat12( /** * Find every non-ASCII character in a rendered position in one UI source file. - * Comment bodies (line + block) are stripped preserving line numbers; test - * files (*.test.ts / *.test.tsx) are skipped entirely. The strip is - * string-aware: a `//` inside a string literal is not a comment, and an escaped - * quote does not end the string. + * The text is parsed by the TypeScript compiler and only leaf TOKENS are read, + * so comment bodies are excluded as trivia and JSX text, regex literals, and + * template spans are located as the parser tokenizes them. JSDoc subtrees are + * skipped. Test files (*.test.ts / *.test.tsx) are skipped entirely. */ export function scanSource(relPath: string, text: string): Finding[] { if (relPath.endsWith(".test.ts") || relPath.endsWith(".test.tsx")) return []; - const stripped = stripComments(text); + const sourceFile = ts.createSourceFile( + relPath, + text, + ts.ScriptTarget.Latest, + // No parent pointers: the walk only descends. + false, + relPath.endsWith(".tsx") ? ts.ScriptKind.TSX : ts.ScriptKind.TS, + ); const findings: Finding[] = []; - let line = 0; - for (const raw of stripped.split("\n")) { - line++; - let column = 0; - for (const ch of raw) { - column++; - const cp = ch.codePointAt(0); - if (cp !== undefined && cp > 127) { - findings.push({ path: relPath, line, column, char: ch, codepoint: cp }); - } - } - } + collectFindings(sourceFile, sourceFile, relPath, findings); return findings; } /** - * Replace comment bodies with spaces, PRESERVING newlines (so line numbers of - * later code are unchanged). String literals ('...', "...", `...`) are skipped - * with escape handling, so a `//` or `/*` inside a string is not a comment. + * Recurse to leaf tokens, appending one Finding per non-ASCII codepoint in a + * leaf's text. Iteration is by codepoint so an astral character is one finding, + * not two surrogate halves. */ -export function stripComments(text: string): string { - const out: string[] = []; - let i = 0; - const n = text.length; - let state: "normal" | "line" | "block" = "normal"; - let quote: string | null = null; - while (i < n) { - const c = text[i] ?? ""; - const next = i + 1 < n ? (text[i + 1] ?? "") : ""; - if (quote !== null) { - if (c === "\\") { - out.push(c, next); - i += 2; - continue; - } - out.push(c); - if (c === quote) quote = null; - i++; - continue; - } - if (state === "line") { - out.push(c === "\n" ? "\n" : " "); - if (c === "\n") state = "normal"; - i++; - continue; - } - if (state === "block") { - if (c === "*" && next === "/") { - out.push(" ", " "); - i += 2; - state = "normal"; - continue; - } - out.push(c === "\n" ? "\n" : " "); - i++; - continue; - } - if (c === "/" && next === "/") { - out.push(" ", " "); - i += 2; - state = "line"; - continue; - } - if (c === "/" && next === "*") { - out.push(" ", " "); - i += 2; - state = "block"; - continue; +function collectFindings( + node: ts.Node, + sourceFile: ts.SourceFile, + relPath: string, + out: Finding[], +): void { + // JSDoc is the one comment form the parser surfaces as real nodes rather + // than trivia, so its subtree is skipped: a doc comment renders nothing. + if (isJSDocNode(node)) return; + const children = node.getChildren(sourceFile); + if (children.length > 0) { + for (const child of children) { + collectFindings(child, sourceFile, relPath, out); } - if (c === '"' || c === "'" || c === "`") { - quote = c; + return; + } + const start = node.getStart(sourceFile); + const leaf = node.getText(sourceFile); + for (let offset = 0; offset < leaf.length; ) { + const codepoint = leaf.codePointAt(offset); + if (codepoint === undefined) break; + const char = String.fromCodePoint(codepoint); + if (codepoint > 127) { + const at = sourceFile.getLineAndCharacterOfPosition(start + offset); + out.push({ + path: relPath, + line: at.line + 1, + column: at.character + 1, + char, + codepoint, + }); } - out.push(c); - i++; + offset += char.length; } - return out.join(""); +} + +/** True for a JSDoc node or anything inside one (tags, type expressions). */ +function isJSDocNode(node: ts.Node): boolean { + return ( + node.kind >= ts.SyntaxKind.FirstJSDocNode && + node.kind <= ts.SyntaxKind.LastJSDocNode + ); } // ---------------------------------------------------------------------------