diff --git a/src/invisible-chars.test.ts b/src/invisible-chars.test.ts index 2110825..066f1d7 100644 --- a/src/invisible-chars.test.ts +++ b/src/invisible-chars.test.ts @@ -1,6 +1,6 @@ import { describe, it, expect } from "vitest"; -import { findInvisibleChars } from "./invisible-chars.js"; +import { describeInvisibleChars, findInvisibleChars } from "./invisible-chars.js"; import { validateLesson } from "./validate.js"; /** @@ -212,6 +212,32 @@ describe("W-INVISIBLE-CHAR: control characters", () => { expect(messageFor(lesson)).toContain("U+009B"); }); + it("flags the bidi isolates, which modern editors emit instead of the embeddings", () => { + for (const codepoint of [0x2066, 0x2067, 0x2068, 0x2069]) { + const lesson = lessonWith({ title: `A${String.fromCharCode(codepoint)}B` }); + expect(warningsFor(lesson), `U+${codepoint.toString(16)}`).toHaveLength(1); + } + }); + + it("flags the invisible math operators a formula editor pastes", () => { + for (const codepoint of [0x2061, 0x2062, 0x2063, 0x2064]) { + const lesson = lessonWith({ title: `A${String.fromCharCode(codepoint)}B` }); + expect(warningsFor(lesson), `U+${codepoint.toString(16)}`).toHaveLength(1); + } + }); + + it("sorts reported codepoints numerically, not as text", () => { + // Once a codepoint needs five hex digits the two orders disagree: + // "U+10000" precedes "U+FEFF" as text and follows it as a number. + const described = describeInvisibleChars([ + { path: "/a", codepoint: "U+10000", name: "SUPPLEMENTARY" }, + { path: "/a", codepoint: "U+FEFF", name: "BYTE ORDER MARK" }, + { path: "/a", codepoint: "U+00AD", name: "SOFT HYPHEN" }, + ])!; + const order = ["U+00AD", "U+FEFF", "U+10000"].map((label) => described.indexOf(label)); + expect(order).toEqual([...order].sort((left, right) => left - right)); + }); + it("survives a circular reference instead of blowing the stack", () => { // validateLesson takes `unknown`, so a caller can hand it a hand-built // object rather than JSON.parse output. Before the guard this threw diff --git a/src/invisible-chars.ts b/src/invisible-chars.ts index c178ad8..bb2dc5e 100644 --- a/src/invisible-chars.ts +++ b/src/invisible-chars.ts @@ -21,6 +21,20 @@ * characters themselves made the source unreadable: a reviewer saw an empty * string and had to take the label on trust, in the one file where that is * least acceptable. + * + * Codepoints and names follow the Unicode Character Database; the format + * ranges are those of general category Cf. The starting set was taken from + * the sanitizer of `manuscript-tools` + * (https://github.com/astrapi69/manuscript-tools), whose checker had the same + * gap this module closed. + * + * Scope note: the walker below reads plain JSON shapes only. ``Map`` and + * ``Set`` values would be walked as empty objects, which sounds like a silent + * false negative but cannot happen here. ``validateLesson`` returns early when + * the structural check fails, so the lint only ever sees a schema-valid + * lesson, and this module is not exported from the package entry. Both would + * have to change before the gap is reachable; if this is ever made public, + * handle those two types before doing so. */ /** A codepoint, or an inclusive range of them, that renders as nothing. */ @@ -56,7 +70,11 @@ const INVISIBLE_RANGES: readonly InvisibleRange[] = [ { from: 0x202c, to: 0x202c, name: "POP DIRECTIONAL FORMATTING" }, { from: 0x202d, to: 0x202d, name: "LEFT-TO-RIGHT OVERRIDE" }, { from: 0x202e, to: 0x202e, name: "RIGHT-TO-LEFT OVERRIDE" }, - { from: 0x2060, to: 0x2060, name: "WORD JOINER" }, + // The whole U+2060-206F format block: word joiner, the invisible math + // operators a formula editor pastes, the bidi isolates that modern editors + // emit in place of the older embeddings, and the deprecated format + // characters. All are Unicode general category Cf and all render as nothing. + { from: 0x2060, to: 0x206f, name: "INVISIBLE FORMAT CHARACTER" }, { from: 0xfeff, to: 0xfeff, name: "BYTE ORDER MARK" }, ]; @@ -123,27 +141,40 @@ function findingsInString(value: string, path: string): InvisibleCharFinding[] { * what lets this cover ``ext_payload`` (whose shape the engine does not know) * and any field a later schema version adds. */ -export function findInvisibleChars( - value: unknown, - path = "", - ancestors: ReadonlySet = new Set(), -): InvisibleCharFinding[] { +export function findInvisibleChars(value: unknown, path = ""): InvisibleCharFinding[] { + return walk(value, path, new Set()); +} + +/** + * The recursion behind {@link findInvisibleChars}. + * + * ``ancestors`` holds the current PATH, not everything seen, so a node reached + * twice without a cycle is still reported at each path it appears at. It is + * one mutable set that is extended on the way down and restored on the way + * back up: copying it per node would allocate one set per object in the + * lesson, which for a pasted chapter is thousands of short-lived sets. The + * ``finally`` keeps the set honest even if a callee throws. + */ +function walk(value: unknown, path: string, ancestors: Set): InvisibleCharFinding[] { if (typeof value === "string") return findingsInString(value, path); if (typeof value !== "object" || value === null) return []; // A lesson from JSON.parse cannot be cyclic, but this API takes `unknown`, // so a hand-built object can be. Unguarded that is a stack overflow rather - // than a diagnosis. Tracking the ANCESTORS rather than everything seen keeps - // a shared (but acyclic) node reported at each path it appears at. + // than a diagnosis. if (ancestors.has(value)) return []; - const nested = new Set(ancestors).add(value); - if (Array.isArray(value)) { - return value.flatMap((entry, index) => - findInvisibleChars(entry, `${path}/${index}`, nested), + ancestors.add(value); + try { + if (Array.isArray(value)) { + return value.flatMap((entry, index) => walk(entry, `${path}/${index}`, ancestors)); + } + // Object.entries, not for..in: only own enumerable keys, so nothing + // inherited from a polluted prototype is ever walked. + return Object.entries(value).flatMap(([key, entry]) => + walk(entry, `${path}/${key}`, ancestors), ); + } finally { + ancestors.delete(value); } - return Object.entries(value).flatMap(([key, entry]) => - findInvisibleChars(entry, `${path}/${key}`, nested), - ); } /** How many distinct paths to name before trailing off. Enough to start