diff --git a/packages/compiler/src/frontend/lowering/builtins/fs-paths.ts b/packages/compiler/src/frontend/lowering/builtins/fs-paths.ts index fb9176b53..181e27e34 100644 --- a/packages/compiler/src/frontend/lowering/builtins/fs-paths.ts +++ b/packages/compiler/src/frontend/lowering/builtins/fs-paths.ts @@ -2,9 +2,10 @@ * fileURLToPath inside each function (getValidatedPath), after every * argument has been evaluated, and validates some arguments before the * path and others after it. A URL-typed path lowers to the same - * conversion (url.fileURLToPathUrl). Later arguments with side effects are - * evaluated into hidden locals before the conversion, so they run even - * when it throws. The conversion keeps Node's error order when every + * conversion (url.fileURLToPathUrl). The later arguments through the last + * one with side effects are evaluated into hidden locals before the + * conversion, in source order, so they run even when it throws and each + * reads what it reads in Node. The conversion keeps Node's error order when every * earlier path is a URL too and each argument Node validates before the * path is absent or a literal that passes that validation; other URL * forms fence with SC2020, as do the fs.promises functions, whose @@ -134,16 +135,26 @@ function checkUrlPaths( // path Node checks first cannot precede the URL. if (row.paths.some((i) => i < first && args[i] !== undefined)) lowerer.noLowering(`${name} with a string path before a URL path`, call, hint); + // Every argument from the URL through the last one with effects is + // evaluated ahead of the conversion, in source order: a later argument's + // effect (an assignment, say) must not change what an earlier one read. + let last = first; + for (let i = first + 1; i < args.length; i++) if (!sideEffectFreeOptionValue(args[i]!)) last = i; const staged: ts.Expression[] = []; - for (let i = first + 1; i < args.length; i++) { - const arg = args[i]!; - if (sideEffectFreeOptionValue(arg)) continue; + for (let i = first + 1; i <= last; i++) { + let e = args[i]!; + while (ts.isParenthesizedExpression(e)) e = e.expression; // Literals are read by their members, so a whole-value slot cannot // hold them. - let e = arg; - while (ts.isParenthesizedExpression(e)) e = e.expression; - if (ts.isObjectLiteralExpression(e) || ts.isArrayLiteralExpression(e)) - lowerer.noLowering(`${name} with a URL path and a later literal that has effects`, arg, hint); + if (ts.isObjectLiteralExpression(e) || ts.isArrayLiteralExpression(e)) { + lowerer.noLowering( + sideEffectFreeOptionValue(e) + ? `${name} with a URL path and a literal before a later argument that has effects` + : `${name} with a URL path and a later literal that has effects`, + e, + hint, + ); + } staged.push(e); } if (staged.length > 0) { diff --git a/tests/corpus/6931-fs-url-paths.ts b/tests/corpus/6931-fs-url-paths.ts index 91a630d1b..cf6889066 100644 --- a/tests/corpus/6931-fs-url-paths.ts +++ b/tests/corpus/6931-fs-url-paths.ts @@ -18,6 +18,13 @@ function mode(): number { console.log("evaluated mode"); return 0o600; } +// A later argument that reassigns an earlier argument's variable runs after +// that argument was read. +let uid = -1; +function bumpUid(): number { + uid = 123456; + return -1; +} function clean(text: string): string { return text.split(dir).join(""); @@ -91,6 +98,11 @@ attempt("mkdir", () => { attempt("later effects", () => fs.copyFileSync(new URL("http://a/"), logged("dest.txt"))); attempt("later mode", () => fs.chmodSync(new URL("http://a/"), mode())); attempt("chmod", () => fs.chmodSync(new URL("data:text/plain,a"), 0o600)); +attempt("chown order", () => fs.chownSync(file, uid, bumpUid())); +console.log("uid", uid); +uid = -1; +attempt("chown order scheme", () => fs.chownSync(new URL("http://a/"), uid, bumpUid())); +console.log("uid", uid); fs.rmSync(dir, { recursive: true, force: true }); console.log("done", fs.existsSync(dir)); diff --git a/tests/harness/npm.test.ts b/tests/harness/npm.test.ts index 9ab93f287..2df0d87ef 100644 --- a/tests/harness/npm.test.ts +++ b/tests/harness/npm.test.ts @@ -72,7 +72,7 @@ const KNOWN: Record = { "node24-esm-metadata": { run: "SC1090" }, "node24-event-emitter": { run: "SC2020" }, "node24-file-url-conversion": { build: "SC1090" }, - "node24-fs-descriptors": { run: "SC1090" }, + "node24-fs-descriptors": { run: "SC2020" }, "node24-mime": { run: "SC2020" }, "node24-mime-errors": { run: "SC1090" }, "node24-no-error-assertions": { run: "SC2020" },