Evaluate fs arguments up to the last one with effects before a URL path conversion

A synchronous fs call with a URL path evaluated only the later arguments
with effects ahead of the path conversion. When such an argument
reassigned the variable an earlier argument reads, as in
fs.chownSync(url, uid, next()), the earlier argument was read after the
reassignment. Every argument from the URL through the last one with
effects is now evaluated in source order before the conversion.

The npm fixture node24-fs-descriptors now reaches its SC2020 refusal for
writeSync with a hex encoding, since openSync accepts its file URL.
This commit is contained in:
Chris Tate
2026-10-21 14:22:51 -05:00
parent d858d840f9
commit 644f423734
3 changed files with 34 additions and 11 deletions
@@ -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) {
+12
View File
@@ -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("<dir>");
@@ -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));
+1 -1
View File
@@ -72,7 +72,7 @@ const KNOWN: Record<string, Known> = {
"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" },