Repository: denoland/deno
Pull request: fix(ext/node): `fs.rename` and `fs.renameSync` compatibility

Improves compatibility with Node.js by validating the input parameters, accepting Buffer type paths, and converting thrown errors correctly. This PR also addresses `prefer-primordials` lint rule. These changes allow [parallel/test-fs-rename-type-check.js.js](https://github.com/denoland/node_test/blob/8846b5392f4ebd3d4e995b9bed61c23c0192d806/test/parallel/test-fs-rename-type-check.js) test to pass.

Towards https://github.com/denoland/deno/issues/29972, https://github.com/denoland/deno/issues/24236.


Given this code:
```js
import { rename } from "node:fs/promises";

const main = async () => {
  try {
    await rename("doesnt-exists", "doesnt-exists-renamed");
  } catch (error) {
    console.error("Error code:", error.code);
    console.error("Error syscall:", error.syscall);
    console.error("Error path:", error.path);
    console.error("Error dest:", error.dest);
  }
}

main()
```

These are the outputs:
```bash
# Node.js v24.4.1
node foo.js
Error code: ENOENT
Error syscall: rename
Error path: doesnt-exists
Error dest: doesnt-exists-renamed

# Deno 2.4.2
deno -A foo.js
Error code: ENOENT
Error syscall: undefined
Error path: undefined
Error dest: undefined

# Deno (this PR)
./target/debug/deno -A foo.js
Error code: ENOENT
Error syscall: rename
Error path: doesnt-exists
Error dest: doesnt-exists-renamed
```

<!--
Before submitting a PR, please read https://docs.deno.com/runtime/manual/references/contributing

1. Give the PR a descriptive title.

  Examples of good title:
    - fix(std/http): Fix race condition in server
    - docs(console): Update docstrings
    - feat(doc): Handle nested reexports

  Examples of bad title:
    - fix #7123
    - update docs
    - fix bugs

2. Ensure there is a related issue and it is referenced in the PR text.
3. Ensure there are tests that cover the changes.
4. Ensure `cargo test` passes.
5. Ensure `./tools/format.js` passes without changing files.
6. Ensure `./tools/lint.js` passes.
7. Open as a draft PR if your work is still in progress. The CI won't run
   all steps, but you can add '[ci]' to a commit message to force it to.
8. If you would like to run the benchmarks on the CI, add the 'ci-bench' label.
-->


Changed files:
filename: ext/node/polyfills/_fs/_fs_rename.ts
patch: @@ -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;
-
-  if (!callback) throw new Error("No callback function supplied");
-
-  Deno.rename(oldPath, newPath).then((_) => callback(), callback);
+  oldPath = getValidatedPathToString(oldPath, "oldPath");
+  newPath = getValidatedPathToString(newPath, "newPath");
+  validateFunction(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<void>;
 
-export function renameSync(oldPath: string | URL, newPath: string | URL) {
-  oldPath = oldPath instanceof URL ? pathFromURL(oldPath) : oldPath;
-  newPath = newPath instanceof URL ? pathFromURL(newPath) : newPath;
-
-  Deno.renameSync(oldPath, newPath);
+export function renameSync(
+  oldPath: string | Buffer | URL,
+  newPath: string | Buffer | URL,
+) {
+  oldPath = getValidatedPathToString(oldPath, "oldPath");
+  newPath = getValidatedPathToString(newPath, "newPath");
+
+  try {
+    Deno.renameSync(
+      pathModule.toNamespacedPath(oldPath),
+      pathModule.toNamespacedPath(newPath),
+    );
+  } catch (err) {
+    throw denoErrorToNodeError(err as Error, {
+      syscall: "rename",
+      path: oldPath,
+      dest: newPath,
+    });
+  }
 }

filename: ext/node/polyfills/internal/errors.ts
patch: @@ -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);

filename: tests/node_compat/config.toml
patch: @@ -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" = {}

filename: tests/unit_node/_fs/_fs_rename_test.ts
patch: @@ -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<void>((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<void>((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);
+  }
+});