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.
This commit is contained in:
Chris Tate
2026-10-22 11:12:02 -05:00
parent 76d7d29b13
commit 2b657e6e3c
7 changed files with 88 additions and 18 deletions
+3 -1
View File
@@ -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 <config>` moves top-level declarations into new files. `plan` computes a manifest from seed declarations, and `apply` does the move.
- `split-methods.mjs apply <config>` 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 <config>` 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.<field>` and `emitter.<field>` to `KEY.of(this).<field>` 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 <dir>` (the tree to rewrite) and `--tools <dir>`, 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 <dir>` (the tree to rewrite) and `--tools <dir>`, 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/<n>/head:pr-<n>
node scripts/split/rehome-split.mjs --branch pr-<n> --onto origin/main --config scripts/split/configs/<split>/<config>.json
+20
View File
@@ -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 },
);
}
+2 -3
View File
@@ -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 <config.json>");
@@ -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`,
+2 -4
View File
@@ -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`);
}
+2 -5
View File
@@ -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 <config.json>");
@@ -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`);
+2 -5
View File
@@ -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`);
}
+57
View File
@@ -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": [