diff --git a/ext/node/polyfills/_fs/_fs_rename.ts b/ext/node/polyfills/_fs/_fs_rename.ts index 7a84cf8d98..6f9e0af334 100644 --- a/ext/node/polyfills/_fs/_fs_rename.ts +++ b/ext/node/polyfills/_fs/_fs_rename.ts @@ -1,32 +1,63 @@ // Copyright 2018-2025 the Deno authors. MIT license. -// TODO(petamoriken): enable prefer-primordials for node polyfills -// deno-lint-ignore-file prefer-primordials - -import { pathFromURL } from "ext:deno_web/00_infra.js"; import { promisify } from "ext:deno_node/internal/util.mjs"; +import { getValidatedPathToString } from "ext:deno_node/internal/fs/utils.mjs"; +import { validateFunction } from "ext:deno_node/internal/validators.mjs"; +import { primordials } from "ext:core/mod.js"; +import type { Buffer } from "node:buffer"; +import * as pathModule from "node:path"; +import { denoErrorToNodeError } from "ext:deno_node/internal/errors.ts"; + +const { + PromisePrototypeThen, +} = primordials; export function rename( - oldPath: string | URL, - newPath: string | URL, + oldPath: string | Buffer | URL, + newPath: string | Buffer | URL, callback: (err?: Error) => void, ) { - oldPath = oldPath instanceof URL ? pathFromURL(oldPath) : oldPath; - newPath = newPath instanceof URL ? pathFromURL(newPath) : newPath; + oldPath = getValidatedPathToString(oldPath, "oldPath"); + newPath = getValidatedPathToString(newPath, "newPath"); + validateFunction(callback, "callback"); - if (!callback) throw new Error("No callback function supplied"); - - Deno.rename(oldPath, newPath).then((_) => callback(), callback); + PromisePrototypeThen( + Deno.rename( + pathModule.toNamespacedPath(oldPath), + pathModule.toNamespacedPath(newPath), + ), + () => callback(), + (err: Error) => + callback(denoErrorToNodeError(err, { + syscall: "rename", + path: oldPath, + dest: newPath, + })), + ); } export const renamePromise = promisify(rename) as ( - oldPath: string | URL, - newPath: string | URL, + oldPath: string | Buffer | URL, + newPath: string | Buffer | URL, ) => Promise; -export function renameSync(oldPath: string | URL, newPath: string | URL) { - oldPath = oldPath instanceof URL ? pathFromURL(oldPath) : oldPath; - newPath = newPath instanceof URL ? pathFromURL(newPath) : newPath; +export function renameSync( + oldPath: string | Buffer | URL, + newPath: string | Buffer | URL, +) { + oldPath = getValidatedPathToString(oldPath, "oldPath"); + newPath = getValidatedPathToString(newPath, "newPath"); - Deno.renameSync(oldPath, newPath); + try { + Deno.renameSync( + pathModule.toNamespacedPath(oldPath), + pathModule.toNamespacedPath(newPath), + ); + } catch (err) { + throw denoErrorToNodeError(err as Error, { + syscall: "rename", + path: oldPath, + dest: newPath, + }); + } } diff --git a/ext/node/polyfills/internal/errors.ts b/ext/node/polyfills/internal/errors.ts index ec23af78c5..db32cd2d9a 100644 --- a/ext/node/polyfills/internal/errors.ts +++ b/ext/node/polyfills/internal/errors.ts @@ -2666,6 +2666,7 @@ export class ERR_INVALID_STATE extends NodeError { interface UvExceptionContext { syscall: string; path?: string; + dest?: string; } export function denoErrorToNodeError(e: Error, ctx: UvExceptionContext) { const errno = extractOsErrorNumberFromErrorMessage(e); diff --git a/tests/node_compat/config.toml b/tests/node_compat/config.toml index a05b716565..d95999654b 100644 --- a/tests/node_compat/config.toml +++ b/tests/node_compat/config.toml @@ -452,6 +452,7 @@ "parallel/test-fs-readfilesync-enoent.js" = {} "parallel/test-fs-readv-promisify.js" = {} "parallel/test-fs-ready-event-stream.js" = {} +"parallel/test-fs-rename-type-check.js" = {} "parallel/test-fs-rmdir-type-check.js" = {} "parallel/test-fs-stream-fs-options.js" = {} "parallel/test-fs-stream-options.js" = {} diff --git a/tests/unit_node/_fs/_fs_rename_test.ts b/tests/unit_node/_fs/_fs_rename_test.ts index 91185e02a2..05686ef6bf 100644 --- a/tests/unit_node/_fs/_fs_rename_test.ts +++ b/tests/unit_node/_fs/_fs_rename_test.ts @@ -4,6 +4,7 @@ import { assertCallbackErrorUncaught } from "../_test_utils.ts"; import { rename, renameSync } from "node:fs"; import { existsSync } from "node:fs"; import { join, parse } from "@std/path"; +import { Buffer } from "node:buffer"; Deno.test({ name: "ASYNC: renaming a file", @@ -50,3 +51,72 @@ Deno.test("[std/node/fs] rename callback isn't called twice if error is thrown", }, }); }); + +Deno.test("[std/node/fs] rename: accepts Buffer paths", async () => { + const file = Deno.makeTempFileSync(); + const bufferOldPath = Buffer.from(file, "utf-8"); + const newPath = join(parse(file).dir, `${parse(file).base}_renamed`); + const bufferNewPath = Buffer.from(newPath, "utf-8"); + + await new Promise((resolve, reject) => { + rename(bufferOldPath, bufferNewPath, (err) => { + if (err) reject(err); + resolve(); + }); + }) + .then(() => { + assertEquals(existsSync(newPath), true); + assertEquals(existsSync(file), false); + }, () => fail()); +}); + +Deno.test("[std/node/fs] rename: convert Deno errors to Node.js errors", async () => { + const dir = Deno.makeTempDirSync(); + const oldPath = join(dir, "non_existent_file"); + const newPath = join(dir, "new_file"); + + await new Promise((resolve, reject) => { + rename(oldPath, newPath, (err) => { + if (err) reject(err); + resolve(); + }); + }) + .then(() => fail()) + .catch((err) => { + assertEquals(err.code, "ENOENT"); + assertEquals(err.syscall, "rename"); + assertEquals(err.path, oldPath); + assertEquals(err.dest, newPath); + }); +}); + +Deno.test("[std/node/fs] renameSync: accepts Buffer paths", () => { + const file = Deno.makeTempFileSync(); + const bufferOldPath = Buffer.from(file, "utf-8"); + const newPath = join(parse(file).dir, `${parse(file).base}_renamed`); + const bufferNewPath = Buffer.from(newPath, "utf-8"); + + renameSync(bufferOldPath, bufferNewPath); + assertEquals(existsSync(newPath), true); + assertEquals(existsSync(file), false); +}); + +Deno.test("[std/node/fs] renameSync: convert Deno errors to Node.js errors", () => { + const dir = Deno.makeTempDirSync(); + const oldPath = join(dir, "non_existent_file"); + const newPath = join(dir, "new_file"); + + try { + renameSync(oldPath, newPath); + fail(); + } catch (err) { + // deno-lint-ignore no-explicit-any + assertEquals((err as any).code, "ENOENT"); + // deno-lint-ignore no-explicit-any + assertEquals((err as any).syscall, "rename"); + // deno-lint-ignore no-explicit-any + assertEquals((err as any).path, oldPath); + // deno-lint-ignore no-explicit-any + assertEquals((err as any).dest, newPath); + } +});