diff --git a/changelog.d/6857-fs-node26-parity.md b/changelog.d/6857-fs-node26-parity.md new file mode 100644 index 0000000000..d742fdb8c5 --- /dev/null +++ b/changelog.d/6857-fs-node26-parity.md @@ -0,0 +1 @@ +fix(fs): match Node 26 filesystem validation, option handling, descriptors, and callback behavior. diff --git a/crates/perry-codegen/src/expr/calls/fs.rs b/crates/perry-codegen/src/expr/calls/fs.rs index e47c889e32..a409261321 100644 --- a/crates/perry-codegen/src/expr/calls/fs.rs +++ b/crates/perry-codegen/src/expr/calls/fs.rs @@ -69,6 +69,23 @@ pub(crate) fn arm_fs_promises(ctx: &mut FnCtx<'_>, callee: &Expr, args: &[Expr]) &[(DOUBLE, &p), (DOUBLE, &options)], )) } + "rmdir" => { + let p = if let Some(path) = args.first() { + lower_expr(ctx, path)? + } else { + double_literal(f64::from_bits(crate::nanbox::TAG_UNDEFINED)) + }; + let options = if args.len() >= 2 { + lower_expr(ctx, &args[1])? + } else { + double_literal(f64::from_bits(crate::nanbox::TAG_UNDEFINED)) + }; + Ok(ctx.block().call( + DOUBLE, + "js_fs_promises_rmdir", + &[(DOUBLE, &p), (DOUBLE, &options)], + )) + } _ => { // Unsupported — return a resolved promise holding // undefined so `await` sees a real pending→settled diff --git a/crates/perry-codegen/src/loop_purity.rs b/crates/perry-codegen/src/loop_purity.rs index aeace93d48..364d34cb3b 100644 --- a/crates/perry-codegen/src/loop_purity.rs +++ b/crates/perry-codegen/src/loop_purity.rs @@ -121,9 +121,7 @@ fn expr_alloc_free(e: &Expr) -> bool { // Element READS never allocate — they return an existing element / a // number. Recurse so the object and index are themselves alloc-free. Expr::IndexGet { object, index } => expr_alloc_free(object) && expr_alloc_free(index), - Expr::BufferIndexGet { buffer, index } => { - expr_alloc_free(buffer) && expr_alloc_free(index) - } + Expr::BufferIndexGet { buffer, index } => expr_alloc_free(buffer) && expr_alloc_free(index), Expr::Uint8ArrayGet { array, index } => expr_alloc_free(array) && expr_alloc_free(index), // `arr[i]++` / `--`: read-modify-write of an existing numeric slot, no // growth, no allocation. diff --git a/crates/perry-codegen/src/lower_call/namespace_call.rs b/crates/perry-codegen/src/lower_call/namespace_call.rs index 33a7b58328..f462daeec3 100644 --- a/crates/perry-codegen/src/lower_call/namespace_call.rs +++ b/crates/perry-codegen/src/lower_call/namespace_call.rs @@ -210,6 +210,24 @@ pub fn try_lower_namespace_member_call( ); return Ok(Some(promise)); } + "rmdir" => { + let path = if let Some(path) = args.first() { + lower_expr(ctx, path)? + } else { + double_literal(f64::from_bits(crate::nanbox::TAG_UNDEFINED)) + }; + let options = if args.len() >= 2 { + lower_expr(ctx, &args[1])? + } else { + double_literal(f64::from_bits(crate::nanbox::TAG_UNDEFINED)) + }; + let promise = ctx.block().call( + DOUBLE, + "js_fs_promises_rmdir", + &[(DOUBLE, &path), (DOUBLE, &options)], + ); + return Ok(Some(promise)); + } _ => {} } } diff --git a/crates/perry-codegen/src/runtime_decls/strings.rs b/crates/perry-codegen/src/runtime_decls/strings.rs index 8a66e21947..47f16e7b1f 100644 --- a/crates/perry-codegen/src/runtime_decls/strings.rs +++ b/crates/perry-codegen/src/runtime_decls/strings.rs @@ -439,6 +439,7 @@ pub fn declare_phase_b_strings(module: &mut LlModule) { &[DOUBLE, DOUBLE, DOUBLE], ); module.declare_function("js_fs_promises_mkdir", DOUBLE, &[DOUBLE, DOUBLE]); + module.declare_function("js_fs_promises_rmdir", DOUBLE, &[DOUBLE, DOUBLE]); // fs.mkdirSync(path) — returns i32 status (1=success). module.declare_function("js_fs_mkdir_sync", I32, &[DOUBLE]); module.declare_function("js_fs_mkdir_sync_options", I32, &[DOUBLE, DOUBLE]); diff --git a/crates/perry-runtime/src/fs/callbacks.rs b/crates/perry-runtime/src/fs/callbacks.rs index a28c991c4c..2c226a2eda 100644 --- a/crates/perry-runtime/src/fs/callbacks.rs +++ b/crates/perry-runtime/src/fs/callbacks.rs @@ -849,6 +849,9 @@ pub extern "C" fn js_fs_rmdir_callback(path_value: f64, arg1: f64, arg2: f64) -> }; let cb = callback_or_arg2(arg1, arg2); unsafe { + if let Err(err_val) = crate::fs::validate_rmdir_options(options) { + crate::exception::js_throw(err_val); + } match crate::fs::js_fs_rmdir_result(path_value, options) { Ok(()) => call_cb0(cb), Err(err_val) => call_cb_err1(cb, err_val), diff --git a/crates/perry-runtime/src/fs/dir_glob_watch/watch.rs b/crates/perry-runtime/src/fs/dir_glob_watch/watch.rs index 57f18c0aad..7287c206e5 100644 --- a/crates/perry-runtime/src/fs/dir_glob_watch/watch.rs +++ b/crates/perry-runtime/src/fs/dir_glob_watch/watch.rs @@ -789,23 +789,6 @@ fn start_promise_watcher(id: usize, state: &mut PromiseWatchState) { if state.active || state.closed { return; } - // Re-baseline the snapshot at the moment iteration actually begins (the - // first `.next()` pull), then let `promise_watcher_poll_impl` advance the - // baseline after every poll. This makes the watcher's two behaviors match - // Node: - // * Events emitted between `watch()` and the first `.next()` are NOT - // delivered — Node's async iterator only starts collecting once you - // iterate, so a write before the first pull is ignored. Folding the - // current directory state into the baseline here drops those. - // * A write that happens AFTER a pull is begun is delivered, because each - // subsequent poll diffs against the post-pull baseline (which advanced - // past the now-consumed state) and so detects the fresh change. - // Seeding the baseline at creation time (in `js_fs_promises_watch`) without - // this refresh broke the post-pull case: the first poll would report the - // pre-pull write to the pending pull, and—more importantly—left the - // bookkeeping seeded against stale creation-time state. Refreshing here - // restores both halves. - state.snapshot = snapshot_watch_target(&state.path, state.recursive).unwrap_or_default(); let timer_callback = poll_closure_value(promise_watcher_poll_impl as *const u8, id); let timer_id = crate::timer::setInterval(timer_callback as i64, FS_WATCH_POLL_INTERVAL_MS); if !state.persistent { @@ -1494,10 +1477,6 @@ pub extern "C" fn js_fs_promises_watch(path_value: f64, options_value: f64) -> f // 1. It validates the path synchronously, matching Node's `watch()` which // throws (ENOENT etc.) at call time rather than at first iteration. // 2. It seeds an initial baseline for the state. - // The baseline is intentionally re-taken in `start_promise_watcher` at the - // first `.next()` pull (so pre-iteration writes are ignored, per Node) and - // then advanced by every poll (so post-pull writes are delivered). The - // value seeded here is therefore a placeholder that the first pull refreshes. let initial_snapshot = match snapshot_watch_target(&path, recursive) { Ok(snapshot) => snapshot, Err(err) => unsafe { diff --git a/crates/perry-runtime/src/fs/dirent.rs b/crates/perry-runtime/src/fs/dirent.rs index a8d06b8d2d..c3605ec9ed 100644 --- a/crates/perry-runtime/src/fs/dirent.rs +++ b/crates/perry-runtime/src/fs/dirent.rs @@ -170,7 +170,9 @@ pub(crate) unsafe fn options_field_value( options_value: f64, field: &[u8], ) -> Option { - let bits = options_value.to_bits(); + let scope = crate::gc::RuntimeHandleScope::new(); + let options_handle = scope.root_nanbox_f64(options_value); + let bits = options_handle.get_nanbox_f64().to_bits(); let value = crate::value::JSValue::from_bits(bits); let raw_ptr = if value.is_pointer() { value.as_pointer::() as usize @@ -203,7 +205,23 @@ pub(crate) unsafe fn options_field_value( } } let key = crate::string::js_string_from_bytes(field.as_ptr(), field.len() as u32); - let val = crate::object::js_object_get_field_by_name(obj_ptr, key); + let refreshed_bits = options_handle.get_nanbox_f64().to_bits(); + let refreshed_value = crate::value::JSValue::from_bits(refreshed_bits); + let refreshed_ptr = if refreshed_value.is_pointer() { + refreshed_value.as_pointer::() as usize + } else if refreshed_bits >> 48 == 0x0000 { + (refreshed_bits & 0x0000_FFFF_FFFF_FFFF) as usize + } else { + return None; + }; + if refreshed_ptr < 0x1000 { + return None; + } + let refreshed_obj_ptr = refreshed_ptr as *const crate::object::ObjectHeader; + if refreshed_obj_ptr.is_null() { + return None; + } + let val = crate::object::js_object_get_field_by_name(refreshed_obj_ptr, key); if val.bits() == crate::value::TAG_UNDEFINED { None } else { diff --git a/crates/perry-runtime/src/fs/mod.rs b/crates/perry-runtime/src/fs/mod.rs index 79c09de14a..13be7bd167 100644 --- a/crates/perry-runtime/src/fs/mod.rs +++ b/crates/perry-runtime/src/fs/mod.rs @@ -1599,30 +1599,46 @@ pub extern "C" fn js_fs_rmdir_sync(path_value: f64) -> i32 { js_fs_rmdir_sync_options(path_value, f64::from_bits(crate::value::TAG_UNDEFINED)) } -/// `fs.rmdirSync(path[, options])` — removes an empty directory, or a -/// non-empty tree when the legacy/deprecated `{ recursive: true }` option is -/// supplied. Returns i32 status. +/// `fs.rmdirSync(path[, options])` — removes an empty directory. Returns i32 +/// status. /// Shared `fs.rmdir` op. Reports removal failures (#2747) with the real errno /// and `syscall: "rmdir"` — `ENOENT` (missing), `ENOTDIR` (not a directory), /// `ENOTEMPTY` (non-empty, non-recursive). pub(crate) unsafe fn js_fs_rmdir_result(path_value: f64, options_value: f64) -> Result<(), f64> { validate::validate_path("path", path_value); - validate::validate_object_options("options", options_value); + validate_rmdir_options(options_value)?; let path_str = match decode_path_value(path_value) { Some(s) => s, None => return Ok(()), }; - let result = if options_bool_field(options_value, b"recursive") { - fs::remove_dir_all(&path_str) - } else { - fs::remove_dir(&path_str) - }; - match result { + match fs::remove_dir(&path_str) { Ok(_) => Ok(()), Err(err) => Err(build_fs_error_value(&err, "rmdir", &path_str)), } } +pub(crate) unsafe fn validate_rmdir_options(options_value: f64) -> Result<(), f64> { + validate::validate_object_options("options", options_value); + let Some(recursive) = options_field_value(options_value, b"recursive") else { + return Ok(()); + }; + if crate::value::JSValue::from_bits(recursive.bits()).is_undefined() { + return Ok(()); + } + let received = crate::exception::string_header_to_string(crate::value::js_jsvalue_to_string( + f64::from_bits(recursive.bits()), + )); + let received = if crate::value::JSValue::from_bits(recursive.bits()).is_any_string() { + format!("'{received}'") + } else { + received + }; + Err(validate::build_type_error_with_code_value( + &format!("The property 'options.recursive' is no longer supported. Received {received}"), + "ERR_INVALID_ARG_VALUE", + )) +} + #[no_mangle] pub extern "C" fn js_fs_rmdir_sync_options(path_value: f64, options_value: f64) -> i32 { unsafe { diff --git a/crates/perry-runtime/src/node_submodules/fs_promises.rs b/crates/perry-runtime/src/node_submodules/fs_promises.rs index 1473136f22..02bd1f1c40 100644 --- a/crates/perry-runtime/src/node_submodules/fs_promises.rs +++ b/crates/perry-runtime/src/node_submodules/fs_promises.rs @@ -106,6 +106,11 @@ pub extern "C" fn js_fs_promises_mkdir(path: f64, options: f64) -> f64 { thunk_fs_promises_mkdir(std::ptr::null(), path, options) } +#[no_mangle] +pub extern "C" fn js_fs_promises_rmdir(path: f64, options: f64) -> f64 { + thunk_fs_promises_rmdir(std::ptr::null(), path, options) +} + pub(crate) extern "C" fn thunk_fs_promises_readFile( _closure: *const ClosureHeader, path: f64, diff --git a/test-parity/node-suite/fs/rmdir/recursive-options.ts b/test-parity/node-suite/fs/rmdir/recursive-options.ts index 24c5880ad5..c6b6437baa 100644 --- a/test-parity/node-suite/fs/rmdir/recursive-options.ts +++ b/test-parity/node-suite/fs/rmdir/recursive-options.ts @@ -1,4 +1,5 @@ import * as fs from "node:fs"; +import { rmdir as rmdirPromise } from "node:fs/promises"; // @ts-ignore process.emitWarning = function () {}; @@ -6,14 +7,36 @@ process.emitWarning = function () {}; const ROOT = "/tmp/perry_node_suite_fs_rmdir_recursive_options"; try { fs.rmSync(ROOT, { recursive: true, force: true }); } catch (_e) {} -fs.mkdirSync(ROOT + "/sync/a/b", { recursive: true }); -fs.writeFileSync(ROOT + "/sync/a/b/file.txt", "sync"); -fs.rmdirSync(ROOT + "/sync", { recursive: true }); -console.log("rmdirSync recursive removed:", !fs.existsSync(ROOT + "/sync")); - -fs.mkdirSync(ROOT + "/callback/a/b", { recursive: true }); -fs.writeFileSync(ROOT + "/callback/a/b/file.txt", "callback"); -fs.rmdir(ROOT + "/callback", { recursive: true }, (err) => { - console.log("rmdir callback recursive err:", err === null); - console.log("rmdir callback recursive removed:", !fs.existsSync(ROOT + "/callback")); -}); +try { + fs.rmdirSync(ROOT, { recursive: true }); +} catch (err) { + console.log("rmdirSync recursive error:", err?.code); +} + +try { + fs.rmdir(ROOT, { recursive: true }, () => {}); +} catch (err) { + console.log("rmdir callback recursive error:", err?.code); +} + +try { + await fs.promises.rmdir(ROOT, { recursive: true }); +} catch (err) { + console.log("rmdir promises recursive error:", err?.code); +} + +try { + await fs.promises.rmdir(); +} catch (err) { + console.log("rmdir promises missing path error:", err?.code); +} + +try { + await rmdirPromise(); +} catch (err) { + console.log("rmdir promises namespace missing path error:", err?.code); +} + +fs.mkdirSync(ROOT); +fs.rmdirSync(ROOT, {}); +console.log("rmdir empty options removed:", !fs.existsSync(ROOT));