diff --git a/apps/web/scripts/fix-trans-indices.mjs b/apps/web/scripts/fix-trans-indices.mjs index dc0bce6c1a..bf6b1fe19a 100644 --- a/apps/web/scripts/fix-trans-indices.mjs +++ b/apps/web/scripts/fix-trans-indices.mjs @@ -12,17 +12,18 @@ * right: a different number of tags than element children, or nested tags. Those * need a human to decide which element each phrase belongs to. * - * Usage: node scripts/fix-trans-indices.mjs [--write] [ ...] + * Usage: node scripts/fix-trans-indices.mjs [--write] [ ...] */ import { parseSync } from "@babel/core"; import fs from "node:fs"; import path from "node:path"; const ROOT = path.resolve(import.meta.dirname, ".."); -const EN = path.join(ROOT, "src", "locales", "en"); +/** Overridable so a test can point the remap at a fixture catalog. */ +const EN = process.env.KANDEV_I18N_EN_DIR ?? path.join(ROOT, "src", "locales", "en"); const WRITE = process.argv.includes("--write"); const args = process.argv.slice(2).filter((a) => !a.startsWith("--")); -const DIRS = args.length ? args : ["components", "app"]; +const TARGETS = args.length ? args : ["components", "app"]; const catalogs = {}; for (const file of fs.readdirSync(EN)) { @@ -55,17 +56,68 @@ const runtimeChildren = (element) => (c) => !(c.type === "JSXText" && c.value.trim() === "" && c.value.includes("\n")), ); -function listFiles(dir, out = []) { - for (const entry of fs.readdirSync(dir, { withFileTypes: true })) { +function listFiles(dir, out, errors) { + let entries; + try { + entries = fs.readdirSync(dir, { withFileTypes: true }); + } catch (error) { + errors.push(`${dir}: ${error.code ?? error.message}`); + return out; + } + for (const entry of entries) { const full = path.join(dir, entry.name); if (entry.isDirectory()) { if (["node_modules", "dist", "e2e", "locales"].includes(entry.name)) continue; - listFiles(full, out); + listFiles(full, out, errors); } else if (/\.tsx$/.test(entry.name) && !/\.test\.tsx$/.test(entry.name)) out.push(full); } return out; } +/** + * Resolve arguments to the de-duplicated file list to visit. + * + * A directory is walked as before; a file is taken as named, because filtering a + * path someone typed is how the previous version came to report a clean run over + * input it never opened. A path that is neither is a hard error — this walked + * directories only, so a file argument threw ENOTDIR and a missing one was + * skipped silently, both leaving `{"fixed": 0}` looking like a verdict. + */ +function collectFiles(targets) { + const found = []; + const errors = []; + + for (const target of targets) { + const abs = path.isAbsolute(target) ? target : path.join(ROOT, target); + let stat; + try { + stat = fs.statSync(abs); + } catch (error) { + errors.push(`${target}: ${error.code ?? error.message}`); + continue; + } + if (stat.isDirectory()) listFiles(abs, found, errors); + else if (stat.isFile()) found.push(abs); + else errors.push(`${target}: not a file or directory`); + } + + const seen = new Set(); + const files = []; + for (const file of found) { + const key = path.resolve(file); + if (seen.has(key)) continue; + seen.add(key); + files.push(file); + } + return { files, errors }; +} + +const { files: FILES, errors: PATH_ERRORS } = collectFiles(TARGETS); +if (PATH_ERRORS.length) { + for (const error of PATH_ERRORS) console.error(`fix-trans-indices: cannot read ${error}`); + process.exit(2); +} + const report = { fixed: 0, alreadyOk: 0, declinedCount: 0, declinedNested: 0 }; const declined = []; const touched = new Set(); @@ -110,43 +162,39 @@ function remapMessage(ns, catKey, children, elementIdx, where) { } } -for (const dir of DIRS) { - const abs = path.isAbsolute(dir) ? dir : path.join(ROOT, dir); - if (!fs.existsSync(abs)) continue; - for (const file of listFiles(abs)) { - const src = fs.readFileSync(file, "utf8"); - if (!src.includes(" { - if (node.type !== "JSXElement" || node.openingElement?.name?.name !== "Trans") return; - const keyAttr = node.openingElement.attributes.find( - (a) => a.type === "JSXAttribute" && a.name?.name === "i18nKey", - ); - const key = keyAttr?.value?.type === "StringLiteral" ? keyAttr.value.value : null; - if (!key) return; - const rel = path.relative(ROOT, file); - const line = src.slice(0, node.start).split("\n").length; - const children = runtimeChildren(node); - const elementIdx = children - .map((c, i) => (c.type === "JSXElement" ? i : -1)) - .filter((i) => i >= 0); - - for (const [ns, catKey] of entriesFor(key)) { - remapMessage(ns, catKey, children, elementIdx, `${rel}:${line}`); - } +for (const file of FILES) { + const src = fs.readFileSync(file, "utf8"); + if (!src.includes(" { + if (node.type !== "JSXElement" || node.openingElement?.name?.name !== "Trans") return; + const keyAttr = node.openingElement.attributes.find( + (a) => a.type === "JSXAttribute" && a.name?.name === "i18nKey", + ); + const key = keyAttr?.value?.type === "StringLiteral" ? keyAttr.value.value : null; + if (!key) return; + const rel = path.relative(ROOT, file); + const line = src.slice(0, node.start).split("\n").length; + const children = runtimeChildren(node); + const elementIdx = children + .map((c, i) => (c.type === "JSXElement" ? i : -1)) + .filter((i) => i >= 0); + + for (const [ns, catKey] of entriesFor(key)) { + remapMessage(ns, catKey, children, elementIdx, `${rel}:${line}`); + } + }); } if (WRITE) { @@ -157,9 +205,10 @@ if (WRITE) { fs.writeFileSync(path.join(EN, `${ns}.json`), JSON.stringify(sorted, null, 2) + "\n"); } } +console.log(`scanned ${FILES.length} file(s) from ${TARGETS.length} argument(s)`); if (declined.length) { console.log(`Declined ${declined.length} message(s) — fix these by hand:`); for (const d of declined) console.log(` ${d}`); console.log(""); } -console.log(JSON.stringify({ ...report, wrote: WRITE }, null, 2)); +console.log(JSON.stringify({ scanned: FILES.length, ...report, wrote: WRITE }, null, 2)); diff --git a/apps/web/scripts/fix-trans-indices.test.ts b/apps/web/scripts/fix-trans-indices.test.ts new file mode 100644 index 0000000000..3c84c651a7 --- /dev/null +++ b/apps/web/scripts/fix-trans-indices.test.ts @@ -0,0 +1,135 @@ +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import path from "node:path"; +import { spawnSync } from "node:child_process"; + +import { describe, expect, it } from "vitest"; + +const SCRIPT = path.resolve(import.meta.dirname, "fix-trans-indices.mjs"); + +type Fixture = Record; + +interface Run { + status: number | null; + stdout: string; + stderr: string; + /** The trailing JSON summary, or null when the script exited before printing one. */ + report: Record | null; +} + +/** + * A `` whose message carries two tags but whose JSX has one element child. + * The script cannot know which phrase belongs to the element, so it declines and + * reports — a deterministic finding to assert on. + */ +function declining(key: string): string { + return [ + 'import { Trans } from "react-i18next";', + "", + "export const Line = () => (", + ` `, + " Hi there", + " ", + ");", + ].join("\n"); +} + +const CATALOG = JSON.stringify({ + greetingA: "Hi <0>there and <1>you", + greetingB: "Hi <0>there and <1>you", +}); + +const LOOSE = "loose.tsx"; +const SUBDIR = "nested"; + +const TREE: Fixture = { + "src/locales/en/common.json": CATALOG, + [LOOSE]: declining("greetingA"), + [`${SUBDIR}/other.tsx`]: declining("greetingB"), +}; + +function withFixture(fixture: Fixture, use: (root: string) => T): T { + const root = mkdtempSync(path.join(tmpdir(), "kandev-trans-indices-")); + try { + for (const [file, source] of Object.entries(fixture)) { + const full = path.join(root, file); + mkdirSync(path.dirname(full), { recursive: true }); + writeFileSync(full, source); + } + return use(root); + } finally { + rmSync(root, { recursive: true, force: true }); + } +} + +/** Run against `root`'s fixture catalog so the real `src/locales/en` is never read. */ +function run(root: string | null, ...args: string[]): Run { + const env = { ...process.env }; + if (root) env.KANDEV_I18N_EN_DIR = path.join(root, "src", "locales", "en"); + const result = spawnSync(process.execPath, [SCRIPT, ...args], { encoding: "utf8", env }); + const start = result.stdout.indexOf("{"); + return { + status: result.status, + stdout: result.stdout, + stderr: result.stderr, + report: start === -1 ? null : JSON.parse(result.stdout.slice(start)), + }; +} + +/** + * The regression this suite exists for. The script walked directories only, so a + * file argument threw ENOTDIR mid-walk and a non-existent one was skipped + * silently — both ending at a `{"fixed": 0}` summary that reads as a verdict on + * code the script never opened. + */ +describe("path arguments", () => { + it("reports the findings in a file named directly", () => { + const result = withFixture(TREE, (root) => run(root, path.join(root, LOOSE))); + + expect(result.status).toBe(0); + expect(result.report?.scanned).toBe(1); + expect(result.report?.declinedCount).toBe(1); + expect(result.stdout).toContain(LOOSE); + expect(result.stdout).toContain("common:greetingA"); + }); + + it("reports the union of file and directory arguments", () => { + const result = withFixture(TREE, (root) => + run(root, path.join(root, SUBDIR), path.join(root, LOOSE)), + ); + + expect(result.report?.scanned).toBe(2); + expect(result.report?.declinedCount).toBe(2); + expect(result.stdout).toContain("common:greetingA"); + expect(result.stdout).toContain("common:greetingB"); + }); + + it("counts a file reached through two arguments once", () => { + const result = withFixture(TREE, (root) => run(root, root, path.join(root, LOOSE))); + + expect(result.report?.scanned).toBe(2); + expect(result.report?.declinedCount).toBe(2); + }); + + it("exits non-zero on a path that does not exist rather than reporting clean", () => { + const result = run(null, "does/not/exist"); + + expect(result.status).not.toBe(0); + expect(result.stderr).toContain("does/not/exist"); + expect(result.report).toBeNull(); + }); + + it("prints how many files were scanned, and from how many arguments", () => { + const result = withFixture(TREE, (root) => + run(root, path.join(root, SUBDIR), path.join(root, LOOSE)), + ); + + expect(result.stdout).toContain("scanned 2 file(s) from 2 argument(s)"); + }); + + it("leaves the catalog alone without --write", () => { + const result = withFixture(TREE, (root) => run(root, path.join(root, LOOSE))); + + expect(result.report?.wrote).toBe(false); + }); +}); diff --git a/apps/web/scripts/i18n-sweep.mjs b/apps/web/scripts/i18n-sweep.mjs index ebe493ef19..01ba84ca44 100644 --- a/apps/web/scripts/i18n-sweep.mjs +++ b/apps/web/scripts/i18n-sweep.mjs @@ -14,9 +14,10 @@ * `check-inline-plurals.mjs` only inspects morphemes passed through `t()`. * 2. Prose literals in non-JSX positions — for eye review, not auto-flagged. * - * This is a human tool, not a gate: it always exits 0 and is wired into neither - * CI nor pre-commit. The eye-review half needs judgement, and a check that fires - * on every clean run teaches people to ignore it. + * This is a human tool, not a gate: whatever it finds, it exits 0, and it is + * wired into neither CI nor pre-commit. The eye-review half needs judgement, and + * a check that fires on every clean run teaches people to ignore it. Input it + * cannot read is the one exception — see `collectFiles`. * * Known limits, inherited from the Python original and kept for parity with it: * a line is skipped only when it *starts* with a comment, so a trailing @@ -26,7 +27,7 @@ * `.d.ts` files are scanned like any other `.ts` (there are none in the * directories this is pointed at). See also the note on `NOISE_VAL` below. * - * Usage: node scripts/i18n-sweep.mjs [ ...] + * Usage: node scripts/i18n-sweep.mjs [ ...] */ import fs from "node:fs"; import path from "node:path"; @@ -98,19 +99,23 @@ function repr(value) { /** * Walk without following build output. The tool is pointed at source directories, * but `.` is a plausible mistake and node_modules would take minutes. + * + * A directory that cannot be read is recorded rather than skipped: silently + * scanning less than was asked for is the failure this tool must not have. */ -function listFiles(dir, out = []) { +function listFiles(dir, out, errors) { let entries; try { entries = fs.readdirSync(dir, { withFileTypes: true }); - } catch { + } catch (error) { + errors.push(`${dir}: ${error.code ?? error.message}`); return out; } for (const entry of entries.sort((a, b) => a.name.localeCompare(b.name))) { const full = path.join(dir, entry.name); if (entry.isDirectory()) { if (["node_modules", "dist", ".git"].includes(entry.name)) continue; - listFiles(full, out); + listFiles(full, out, errors); } else if (/\.tsx?$/.test(entry.name) && !entry.name.includes(".test.")) { out.push(full); } @@ -118,28 +123,67 @@ function listFiles(dir, out = []) { return out; } -function sweep(dirs) { +/** + * Resolve arguments to the de-duplicated file list to scan. + * + * A directory is walked as before. A file is scanned as named — the extension + * and `.test.` filters exist to keep a *walk* focused, and re-applying them to a + * path someone typed would put us back in the business of quietly ignoring the + * input. Anything that is neither is an error, never a skip: a detector that + * reports "0 findings" for input it never opened converts a missing check into a + * passed one. + * + * De-duplication keys on the resolved path but keeps the argument's own spelling, + * so findings still print the relative paths the caller passed in. + */ +function collectFiles(args) { + const found = []; + const errors = []; + + for (const arg of args) { + let stat; + try { + stat = fs.statSync(arg); + } catch (error) { + errors.push(`${arg}: ${error.code ?? error.message}`); + continue; + } + if (stat.isDirectory()) listFiles(arg, found, errors); + else if (stat.isFile()) found.push(arg); + else errors.push(`${arg}: not a file or directory`); + } + + const seen = new Set(); + const files = []; + for (const file of found) { + const key = path.resolve(file); + if (seen.has(key)) continue; + seen.add(key); + files.push(file); + } + return { files, errors }; +} + +function sweep(files) { const plurals = []; const hits = []; - for (const dir of dirs) { - for (const file of listFiles(dir)) { - const lines = fs.readFileSync(file, "utf8").split("\n"); - lines.forEach((line, index) => { - const lineNumber = index + 1; - if (SKIP_LINE.test(line.trimStart())) return; - if (PLURAL.test(line)) { - plurals.push(`${file}:${lineNumber}: ${line.trim().slice(0, 100)}`); - } - for (const match of line.matchAll(LITERAL)) { - const value = match[1]; - if (value.length < MIN_LENGTH || !isProse(value)) continue; - if (NOISE_CTX.test(line.slice(0, match.index))) continue; - if (CATALOG_KEY.test(value)) continue; - hits.push(`${file}:${lineNumber}: ${repr(value)}`); - } - }); - } + for (const file of files) { + const lines = fs.readFileSync(file, "utf8").split("\n"); + lines.forEach((line, index) => { + const lineNumber = index + 1; + if (SKIP_LINE.test(line.trimStart())) return; + if (PLURAL.test(line)) { + plurals.push(`${file}:${lineNumber}: ${line.trim().slice(0, 100)}`); + } + for (const match of line.matchAll(LITERAL)) { + const value = match[1]; + if (value.length < MIN_LENGTH || !isProse(value)) continue; + if (NOISE_CTX.test(line.slice(0, match.index))) continue; + if (CATALOG_KEY.test(value)) continue; + hits.push(`${file}:${lineNumber}: ${repr(value)}`); + } + }); } return { plurals, hits }; @@ -175,12 +219,24 @@ function report({ plurals, hits }) { " Judge each hit; nothing in this section is automatically a defect.", ); for (const h of hits) out.push(` ${h}`); + out.push( + "", + " Limit: copy handed back by a plain helper is invisible here and to the lint rule — the pseudo-locale is the completeness check.", + ); return out.join("\n"); } -const DIRS = process.argv.slice(2).filter((a) => !a.startsWith("--")); -if (!DIRS.length) { - console.error("Usage: node scripts/i18n-sweep.mjs [ ...]"); +const ARGS = process.argv.slice(2).filter((a) => !a.startsWith("--")); +if (!ARGS.length) { + console.error("Usage: node scripts/i18n-sweep.mjs [ ...]"); process.exit(2); } -console.log(report(sweep(DIRS))); + +const { files, errors } = collectFiles(ARGS); +if (errors.length) { + for (const error of errors) console.error(`i18n-sweep: cannot read ${error}`); + process.exit(2); +} + +console.log(`scanned ${files.length} file(s) from ${ARGS.length} argument(s)`); +console.log(report(sweep(files))); diff --git a/apps/web/scripts/i18n-sweep.test.ts b/apps/web/scripts/i18n-sweep.test.ts index f9945469a3..85b1e61b07 100644 --- a/apps/web/scripts/i18n-sweep.test.ts +++ b/apps/web/scripts/i18n-sweep.test.ts @@ -12,6 +12,7 @@ type Fixture = Record; interface SweepResult { status: number | null; stdout: string; + stderr: string; /** Findings under "English plural concatenation" — defects. */ plurals: string[]; /** Findings under "literal(s) to review by eye" — judgement calls. */ @@ -29,7 +30,18 @@ function parseSections(stdout: string): Pick(fixture: Fixture, use: (root: string) => T): T { const root = mkdtempSync(path.join(tmpdir(), "kandev-i18n-sweep-")); try { for (const [file, source] of Object.entries(fixture)) { @@ -37,13 +49,16 @@ function sweep(fixture: Fixture): SweepResult { mkdirSync(path.dirname(full), { recursive: true }); writeFileSync(full, source); } - const result = spawnSync(process.execPath, [SCRIPT, root], { encoding: "utf8" }); - return { status: result.status, stdout: result.stdout, ...parseSections(result.stdout) }; + return use(root); } finally { rmSync(root, { recursive: true, force: true }); } } +function sweep(fixture: Fixture): SweepResult { + return withFixture(fixture, (root) => run(root)); +} + describe("English plural concatenation", () => { it("reports a morpheme concatenated onto a count in JSX", () => { const result = sweep({ @@ -205,9 +220,72 @@ describe("eye-review filtering", () => { }); }); +/** + * The regression this suite exists for. The sweep used to walk directories only, + * so a file argument hit ENOTDIR inside the walk, was swallowed, and produced the + * boilerplate footer with no findings and exit 0 — a clean bill of health for + * input it never opened. Briefs had instructed exactly that form + * (`pnpm run i18n:sweep …`), so real batches were signed off on a + * scan of nothing. + */ +describe("path arguments", () => { + const DEFECT = 'export const s = (n: number) => `${n} file${n === 1 ? "" : "s"}`;'; + const LOOSE = "loose.tsx"; + const SUBDIR = "nested"; + const TREE: Fixture = { [LOOSE]: DEFECT, [`${SUBDIR}/counter.ts`]: DEFECT }; + + it("reports the findings in a file named directly", () => { + const result = withFixture(TREE, (root) => run(path.join(root, LOOSE))); + + expect(result.status).toBe(0); + expect(result.plurals).toHaveLength(1); + expect(result.plurals[0]).toContain(`${LOOSE}:1`); + }); + + it("reports the union of file and directory arguments", () => { + const result = withFixture(TREE, (root) => + run(path.join(root, SUBDIR), path.join(root, LOOSE)), + ); + + expect(result.plurals).toHaveLength(2); + expect(result.plurals.join("\n")).toContain("counter.ts:1"); + expect(result.plurals.join("\n")).toContain(`${LOOSE}:1`); + }); + + it("counts a file reached through two arguments once", () => { + const result = withFixture(TREE, (root) => run(root, path.join(root, LOOSE))); + + expect(result.plurals).toHaveLength(2); + }); + + it("exits non-zero on a path that does not exist rather than reporting clean", () => { + const result = run("does/not/exist"); + + expect(result.status).not.toBe(0); + expect(result.stderr).toContain("does/not/exist"); + expect(result.stdout).not.toContain("0 English plural concatenation(s)"); + }); + + /** + * `0 findings` alongside `scanned 0 files` is self-evidently a non-result; + * `0 findings` alone is indistinguishable from a pass. Printing the count is + * the cheap structural guard against the whole failure class. + */ + it("prints how many files were scanned, and from how many arguments", () => { + const result = withFixture(TREE, (root) => + run(path.join(root, SUBDIR), path.join(root, LOOSE)), + ); + + expect(result.stdout).toContain("scanned 2 file(s) from 2 argument(s)"); + }); +}); + describe("report shape", () => { + /** A file with nothing to find, so the report is only its own boilerplate. */ + const CLEAN: Fixture = { "clean.ts": "export const n = 1;" }; + it("always prints both sections and exits 0, even with nothing to report", () => { - const result = sweep({ "clean.ts": "export const n = 1;" }); + const result = sweep(CLEAN); expect(result.status).toBe(0); expect(result.stdout).toContain("0 English plural concatenation(s)"); @@ -223,7 +301,7 @@ describe("report shape", () => { }); it("notes the prompt-builder exclusion in the eye-review section", () => { - const result = sweep({ "clean.ts": "export const n = 1;" }); + const result = sweep(CLEAN); expect(result.stdout).toContain("agent-facing"); }); @@ -251,16 +329,22 @@ describe("report shape", () => { }); it("keeps the plural section bare when it has no findings", () => { - const result = sweep({ "clean.ts": "export const n = 1;" }); + const result = sweep(CLEAN); const pluralSection = result.stdout.slice(0, result.stdout.indexOf("to review by eye")); expect(pluralSection).not.toContain("agent-facing"); }); - it("exits non-zero when given no directory", () => { - const result = spawnSync(process.execPath, [SCRIPT], { encoding: "utf8" }); + it("exits non-zero when given no path", () => { + const result = run(); expect(result.status).toBe(2); expect(result.stderr).toContain("Usage:"); }); + + it("notes that copy from a plain helper is invisible to it", () => { + const result = sweep(CLEAN); + + expect(result.stdout).toContain("plain helper"); + }); });