From 2b657e6e3cca7bb12f60ef1cb199ce79e73ac198 Mon Sep 17 00:00:00 2001 From: Chris Tate Date: Thu, 22 Oct 2026 11:12:02 -0500 Subject: [PATCH] Format the split tools' output with the tools checkout's oxfmt configuration The split tools ran oxfmt in the checkout they rewrite. oxfmt looks up its configuration from that directory and from the formatted files' directories, and runs an oxfmt.config.ts it finds there, so a branch that rehome-split rewrites could run code with the maintainer's permissions. The tools now format through format.mjs, which passes the tools checkout's .oxfmtrc.jsonc with --config and disables the nested lookup, so no configuration file of the rewritten checkout is read or run. With no files to format it does nothing, where oxfmt would format the whole directory. The split-tools tests cover split-methods and move-state on a checkout whose root and src directory commit an oxfmt.config.ts that records it ran. --- scripts/split/README.md | 4 ++- scripts/split/format.mjs | 20 +++++++++++ scripts/split/move-state.mjs | 5 ++- scripts/split/rehome-split.mjs | 6 ++-- scripts/split/split-methods.mjs | 7 ++-- scripts/split/split-module.mjs | 7 ++-- tests/harness/split-tools.test.ts | 57 +++++++++++++++++++++++++++++++ 7 files changed, 88 insertions(+), 18 deletions(-) create mode 100644 scripts/split/format.mjs diff --git a/scripts/split/README.md b/scripts/split/README.md index e3cdfd077..be8c948ac 100644 --- a/scripts/split/README.md +++ b/scripts/split/README.md @@ -5,13 +5,15 @@ These scripts split a large source file into smaller files without changing beha Run them from a checkout with `node_modules` installed. That checkout is the tools directory (`--tools`) and supplies `typescript` and `oxfmt`. ## Transforms + - `split-module.mjs plan|apply ` moves top-level declarations into new files. `plan` computes a manifest from seed declarations, and `apply` does the move. - `split-methods.mjs apply ` moves class methods into exported functions that take the instance as a parameter (`emitter`). The class keeps a one-line stub with the same signature, so callers don't change. - `move-state.mjs apply ` moves a group of class fields into a feature's state object, defined in the feature's file with `defineModuleState` (`packages/compiler/src/backend/llvm/emitter-state.ts`). It rewrites `this.` and `emitter.` to `KEY.of(this).` and creates files from templates. It also applies the change's other edits as literal replacements, in order. A replacement whose text isn't found the expected number of times is reported and skipped. -Each transform takes `--repo ` (the tree to rewrite) and `--tools `, and formats what it writes with oxfmt. The tools read and write the tree's files only at real paths inside `--repo`, and a config's templates and manifest only inside the config's directory. A path that resolves elsewhere, through a symlink or `..`, stops the tool with an error (`confine.mjs`). `configs/` holds the configs of splits that have landed. For example, `configs/loop-state/` moves the loop optimizations' state out of `LlEmitter`. +Each transform takes `--repo ` (the tree to rewrite) and `--tools `, and formats what it writes with the tools checkout's oxfmt and its `.oxfmtrc.jsonc` (`format.mjs`). The formatter reads no configuration from `--repo`, so an `oxfmt.config.ts` a branch commits is never run. The tools read and write the tree's files only at real paths inside `--repo`, and a config's templates and manifest only inside the config's directory. A path that resolves elsewhere, through a symlink or `..`, stops the tool with an error (`confine.mjs`). `configs/` holds the configs of splits that have landed. For example, `configs/loop-state/` moves the loop optimizations' state out of `LlEmitter`. ## Rehoming an open branch + ```sh git fetch origin pull//head:pr- node scripts/split/rehome-split.mjs --branch pr- --onto origin/main --config scripts/split/configs//.json diff --git a/scripts/split/format.mjs b/scripts/split/format.mjs new file mode 100644 index 000000000..10c1fbc8a --- /dev/null +++ b/scripts/split/format.mjs @@ -0,0 +1,20 @@ +/* Formatting for the split tools. oxfmt reads its configuration from the + * directory it runs in, its parents and the formatted files' directories, + * and an oxfmt.config.ts there is code it runs. The tools format files in + * a checkout they rewrite, which can be a pull request's, so they pass the + * tools checkout's own configuration and turn the nested lookup off: no + * configuration file of the rewritten checkout is read or run. */ +import { execFileSync } from "node:child_process"; +import { join } from "node:path"; + +/** Formats `files` (paths relative to `cwd`) with the tools checkout's + * oxfmt and configuration. */ +export function formatFiles(toolsDir, files, { cwd, stdio }) { + // With no file arguments oxfmt would format the whole directory. + if (files.length === 0) return; + execFileSync( + join(toolsDir, "node_modules/.bin/oxfmt"), + ["--config", join(toolsDir, ".oxfmtrc.jsonc"), "--disable-nested-config", ...files], + { cwd, stdio }, + ); +} diff --git a/scripts/split/move-state.mjs b/scripts/split/move-state.mjs index b70023c67..71306f5d1 100644 --- a/scripts/split/move-state.mjs +++ b/scripts/split/move-state.mjs @@ -26,8 +26,8 @@ import { readFileSync } from "node:fs"; import { dirname, join, resolve } from "node:path"; import { createRequire } from "node:module"; -import { execFileSync } from "node:child_process"; import { confine } from "./confine.mjs"; +import { formatFiles } from "./format.mjs"; const [command, configPath, ...flags] = process.argv.slice(2); if (command !== "apply") throw new Error("usage: move-state.mjs apply "); @@ -199,8 +199,7 @@ for (const r of config.replacements ?? []) { checkout.write(path, text.split(r.find).join(r.replace)); touched.add(path); } -const oxfmt = join(toolsDir, "node_modules/.bin/oxfmt"); -execFileSync(oxfmt, [...touched], { cwd: repo, stdio: "inherit" }); +formatFiles(toolsDir, [...touched], { cwd: repo, stdio: "inherit" }); if (skipped) console.warn(`${skipped} replacement(s) skipped`); console.log( `moved ${config.groups.map((g) => g.present.length).reduce((a, b) => a + b, 0)} fields; rewrote ${touched.size} files`, diff --git a/scripts/split/rehome-split.mjs b/scripts/split/rehome-split.mjs index 89c32d6b7..6533301f8 100644 --- a/scripts/split/rehome-split.mjs +++ b/scripts/split/rehome-split.mjs @@ -29,6 +29,7 @@ import { createRequire } from "node:module"; import { dirname, join, resolve } from "node:path"; import { fileURLToPath } from "node:url"; import { confine } from "./confine.mjs"; +import { formatFiles } from "./format.mjs"; const args = process.argv.slice(2); const opt = (name, fallback) => { @@ -173,10 +174,7 @@ try { splitFiles.has(file) && resolveImportConflicts(confine(resultDir), join(resultDir, file)) ) { - execFileSync(join(tools, "node_modules/.bin/oxfmt"), [file], { - cwd: resultDir, - stdio: ["ignore", "ignore", "inherit"], - }); + formatFiles(tools, [file], { cwd: resultDir, stdio: ["ignore", "ignore", "inherit"] }); gitIn(resultDir, "add", file); console.log(`${file}: import conflicts resolved to both sides' imports`); } diff --git a/scripts/split/split-methods.mjs b/scripts/split/split-methods.mjs index 05d72c4be..4d57644af 100644 --- a/scripts/split/split-methods.mjs +++ b/scripts/split/split-methods.mjs @@ -23,8 +23,8 @@ import { readFileSync } from "node:fs"; import { dirname, join, relative, resolve, posix } from "node:path"; import { createRequire } from "node:module"; -import { execFileSync } from "node:child_process"; import { confine, confineHost } from "./confine.mjs"; +import { formatFiles } from "./format.mjs"; const [command, configPath, ...flags] = process.argv.slice(2); if (command !== "apply") throw new Error("usage: split-methods.mjs apply "); @@ -413,8 +413,5 @@ const touched = [ modulePath, ...[...files].filter(([, e]) => e.decls.length).map(([f]) => join(moduleDir, f)), ]; -execFileSync(join(toolsDir, "node_modules/.bin/oxfmt"), touched, { - cwd: repo, - stdio: ["ignore", "ignore", "inherit"], -}); +formatFiles(toolsDir, touched, { cwd: repo, stdio: ["ignore", "ignore", "inherit"] }); console.log(`${moved} methods moved; ${publicize.size} members reached through host`); diff --git a/scripts/split/split-module.mjs b/scripts/split/split-module.mjs index ba340df40..4b8764d80 100644 --- a/scripts/split/split-module.mjs +++ b/scripts/split/split-module.mjs @@ -39,8 +39,8 @@ import { readFileSync } from "node:fs"; import { dirname, join, relative, resolve, posix } from "node:path"; import { createRequire } from "node:module"; -import { execFileSync } from "node:child_process"; import { confine, confineHost } from "./confine.mjs"; +import { formatFiles } from "./format.mjs"; const [command, configPath, ...flags] = process.argv.slice(2); const config = JSON.parse(readFileSync(configPath, "utf8")); @@ -533,10 +533,7 @@ function apply() { // Format what changed with the tools checkout's oxfmt, so the split and // every rehome of it are formatted the same way. const touched = [modulePath, ...[...files.keys()].map((f) => join(moduleDir, f))]; - execFileSync(join(toolsDir, "node_modules/.bin/oxfmt"), touched, { - cwd: repo, - stdio: ["ignore", "ignore", "inherit"], - }); + formatFiles(toolsDir, touched, { cwd: repo, stdio: ["ignore", "ignore", "inherit"] }); for (const [file, entry] of files) console.log(`${file}: ${entry.decls.length} declarations`); } diff --git a/tests/harness/split-tools.test.ts b/tests/harness/split-tools.test.ts index 08d89e850..8bdefdf0e 100644 --- a/tests/harness/split-tools.test.ts +++ b/tests/harness/split-tools.test.ts @@ -97,6 +97,63 @@ test("split-methods keeps an import a staying method uses as a shorthand propert expect(diagnostics(dir)).toEqual([]); }); +/** An oxfmt.config.ts that records it ran by writing `marker`. */ +const recordingConfig = (marker: string): string => + [ + 'import { writeFileSync } from "node:fs";', + `writeFileSync(${JSON.stringify(marker)}, "ran");`, + "export default {};", + "", + ].join("\n"); + +test("split-methods formats with the tools checkout's configuration, not the branch's", () => { + const dir = checkout({ + "src/host.ts": [ + "export class Host {", + " value = 1;", + " moved(): number {", + " return this.value + 1;", + " }", + "}", + "", + ].join("\n"), + "split.json": JSON.stringify({ + module: "src/host.ts", + class: "Host", + methods: { "moved.ts": ["moved"] }, + }), + }); + const markers = [join(dir, "root-config-ran"), join(dir, "nested-config-ran")]; + writeFileSync(join(dir, "oxfmt.config.ts"), recordingConfig(markers[0]!)); + writeFileSync(join(dir, "src/oxfmt.config.ts"), recordingConfig(markers[1]!)); + run("split-methods.mjs", ["apply", join(dir, "split.json")], dir); + expect(readFileSync(join(dir, "src/moved.ts"), "utf8")).toContain("export function moved"); + for (const marker of markers) expect(existsSync(marker)).toBe(false); +}); + +test("move-state formats with the tools checkout's configuration, not the branch's", () => { + const dir = checkout({ + "src/host.ts": + "export class Host {\n count = 0;\n moved(): number {\n return this.count + 1;\n }\n}\n", + "state.template": "", + "split.json": JSON.stringify({ + module: "src/host.ts", + class: "Host", + dir: "src", + create: { "host-state.ts": "state.template" }, + groups: [ + { file: "host-state.ts", key: "COUNTS", interfaceName: "Counts", fields: ["count"] }, + ], + }), + }); + const markers = [join(dir, "root-config-ran"), join(dir, "nested-config-ran")]; + writeFileSync(join(dir, "oxfmt.config.ts"), recordingConfig(markers[0]!)); + writeFileSync(join(dir, "src/oxfmt.config.ts"), recordingConfig(markers[1]!)); + run("move-state.mjs", ["apply", join(dir, "split.json")], dir); + expect(readFileSync(join(dir, "src/host.ts"), "utf8")).toContain("COUNTS.of(this).count"); + for (const marker of markers) expect(existsSync(marker)).toBe(false); +}); + test("split-module imports a moved class's local base class as a value", () => { const dir = checkout({ "src/shapes.ts": [