vfs: address review feedback on path utilities and providers

- Remove redundant #getBaseName/#getParentPath from MemoryProvider,
  use pathPosix.basename/dirname directly
- Remove redundant getBaseName/getParentPath/splitPath from router.js,
  keep only functions with non-trivial VFS-specific logic
- Convert RealFSProvider constant getters (readonly, supportsSymlinks)
  to readonly properties via ObjectDefineProperty
- Fix joinVFSPath/normalizeVFSPath to detect Windows drive-letter paths
  by checking for ':' at position 1 instead of checking for leading '/',
  so bare '/' is always treated as a POSIX VFS path
- Update test-vfs-internals.js to match router.js export changes

Author: mcollina

lib/internal/vfs/file_system.js

@@ -18,31 +18,39 @@ const pathPosix = path.posix;
 const { isAbsolute, resolve: resolvePath } = path;
 
 /**
- * Normalizes a VFS path. Uses POSIX normalization for Unix-style paths (starting with /)
- * and platform normalization for Windows drive letter paths.
+ * Normalizes a VFS path. Uses POSIX normalization for all paths except
+ * Windows drive-letter paths (e.g. C:\), which use platform normalization.
+ *
+ * On Windows, a bare '/' prefix is treated as a POSIX VFS path, not as the
+ * current-drive root.
  * @param {string} inputPath The path to normalize
  * @returns {string} The normalized path
  */
 function normalizeVFSPath(inputPath) {
-  // If path starts with / (Unix-style), use posix normalization to preserve forward slashes
-  if (inputPath.startsWith('/')) {
-    return pathPosix.normalize(inputPath);
+  if (inputPath.length >= 3 && inputPath[1] === ':') {
+    // Windows drive-letter path (e.g. C:\app)
+    return path.normalize(inputPath);
   }
-  // Otherwise use platform normalization (for Windows drive letters like C:\)
-  return path.normalize(inputPath);
+  return pathPosix.normalize(inputPath);
 }
 
 /**
- * Joins VFS paths. Uses POSIX join for Unix-style base paths.
+ * Joins VFS paths. Uses POSIX join for Unix-style base paths and platform
+ * join for Windows drive-letter paths (e.g. C:\).
+ *
+ * On Windows, a bare '/' prefix is treated as a POSIX VFS path, not as the
+ * current-drive root.  Only explicit drive-letter paths like 'C:\app' use
+ * platform path.join.
  * @param {string} base The base path
  * @param {string} part The path part to join
  * @returns {string} The joined path
  */
 function joinVFSPath(base, part) {
-  if (base.startsWith('/')) {
-    return pathPosix.join(base, part);
+  if (base.length >= 3 && base[1] === ':') {
+    // Windows drive-letter path (e.g. C:\app)
+    return path.join(base, part);
   }
-  return path.join(base, part);
+  return pathPosix.join(base, part);
 }
 const {
   isUnderMountPoint,
@@ -602,23 +610,6 @@ class VirtualFileSystem {
     this[kProvider].accessSync(providerPath, mode);
   }
 
-  /**
-   * Returns the stat result code for module resolution.
-   * @param {string} filePath The path to check
-   * @returns {number} 0 for file, 1 for directory, -2 for not found
-   */
-  internalModuleStat(filePath) {
-    try {
-      const stats = this.statSync(filePath);
-      if (stats.isDirectory()) {
-        return 1;
-      }
-      return 0;
-    } catch {
-      return -2;
-    }
-  }
-
   // ==================== File Descriptor Operations ====================
 
   /**

lib/internal/vfs/module_hooks.js

@@ -101,6 +101,24 @@ function deregisterVFS(vfs) {
   // as another VFS might be registered later.
 }
 
+/**
+ * Returns the stat result code for a VFS path.
+ * @param {VirtualFileSystem} vfs The VFS instance
+ * @param {string} filePath The path to check
+ * @returns {number} 0 for file, 1 for directory, -2 for not found
+ */
+function vfsStat(vfs, filePath) {
+  try {
+    const stats = vfs.statSync(filePath);
+    if (stats.isDirectory()) {
+      return 1;
+    }
+    return 0;
+  } catch {
+    return -2;
+  }
+}
+
 /**
  * Checks all active VFS instances for a file/directory.
  * @param {string} filename The absolute path to check
@@ -111,7 +129,7 @@ function findVFSForStat(filename) {
   for (let i = 0; i < activeVFSList.length; i++) {
     const vfs = activeVFSList[i];
     if (vfs.shouldHandle(normalized)) {
-      const result = vfs.internalModuleStat(normalized);
+      const result = vfsStat(vfs, normalized);
       // For mounted VFS, always return result (even -2 for ENOENT within mount)
       // For overlay VFS, only return if found
       if (vfs.mounted || result >= 0) {
@@ -136,7 +154,7 @@ function findVFSForRead(filename, options) {
       // Check if the file actually exists in VFS
       if (vfs.existsSync(normalized)) {
         // Only read files, not directories
-        const statResult = vfs.internalModuleStat(normalized);
+        const statResult = vfsStat(vfs, normalized);
         if (statResult !== 0) {
           // Not a file (1 = dir, -2 = not found)
           // Let the real fs handle it (will throw appropriate error)
@@ -377,7 +395,7 @@ function getVFSPackageType(vfs, filePath) {
       break;
     }
     const pjsonPath = normalizeVFSPath(resolve(currentDir, 'package.json'));
-    if (vfs.shouldHandle(pjsonPath) && vfs.internalModuleStat(pjsonPath) === 0) {
+    if (vfs.shouldHandle(pjsonPath) && vfsStat(vfs, pjsonPath) === 0) {
       try {
         const content = vfs.readFileSync(pjsonPath, 'utf8');
         const parsed = JSONParse(content);
@@ -420,7 +438,7 @@ function tryExtensions(vfs, basePath) {
   const extensions = ['.js', '.json', '.node', '.mjs', '.cjs'];
   for (let i = 0; i < extensions.length; i++) {
     const candidate = basePath + extensions[i];
-    if (vfs.internalModuleStat(candidate) === 0) {
+    if (vfsStat(vfs, candidate) === 0) {
       return candidate;
     }
   }
@@ -437,7 +455,7 @@ function tryIndexFiles(vfs, dirPath) {
   const indexFiles = ['index.js', 'index.mjs', 'index.cjs', 'index.json'];
   for (let i = 0; i < indexFiles.length; i++) {
     const candidate = normalizeVFSPath(resolve(dirPath, indexFiles[i]));
-    if (vfs.internalModuleStat(candidate) === 0) {
+    if (vfsStat(vfs, candidate) === 0) {
       const url = pathToFileURL(candidate).href;
       const format = getVFSFormat(vfs, candidate);
       return { url, format, shortCircuit: true };
@@ -462,7 +480,7 @@ function resolveConditions(vfs, pkgDir, condMap, conditions) {
       const value = condMap[key];
       if (typeof value === 'string') {
         const resolved = normalizeVFSPath(resolve(pkgDir, value));
-        if (vfs.internalModuleStat(resolved) === 0) {
+        if (vfsStat(vfs, resolved) === 0) {
           const url = pathToFileURL(resolved).href;
           const format = getVFSFormat(vfs, resolved);
           return { url, format, shortCircuit: true };
@@ -497,7 +515,7 @@ function resolvePackageExports(vfs, pkgDir, packageSubpath, exports, context) {
   if (typeof exports === 'string') {
     if (packageSubpath === '.') {
       const resolved = normalizeVFSPath(resolve(pkgDir, exports));
-      if (vfs.internalModuleStat(resolved) === 0) {
+      if (vfsStat(vfs, resolved) === 0) {
         const url = pathToFileURL(resolved).href;
         const format = getVFSFormat(vfs, resolved);
         return { url, format, shortCircuit: true };
@@ -527,7 +545,7 @@ function resolvePackageExports(vfs, pkgDir, packageSubpath, exports, context) {
 
   if (typeof target === 'string') {
     const resolved = normalizeVFSPath(resolve(pkgDir, target));
-    if (vfs.internalModuleStat(resolved) === 0) {
+    if (vfsStat(vfs, resolved) === 0) {
       const url = pathToFileURL(resolved).href;
       const format = getVFSFormat(vfs, resolved);
       return { url, format, shortCircuit: true };
@@ -552,7 +570,7 @@ function resolvePackageExports(vfs, pkgDir, packageSubpath, exports, context) {
  */
 function resolveDirectoryEntry(vfs, dirPath, context) {
   const pjsonPath = normalizeVFSPath(resolve(dirPath, 'package.json'));
-  if (vfs.internalModuleStat(pjsonPath) === 0) {
+  if (vfsStat(vfs, pjsonPath) === 0) {
     try {
       const content = vfs.readFileSync(pjsonPath, 'utf8');
       const parsed = JSONParse(content);
@@ -567,7 +585,7 @@ function resolveDirectoryEntry(vfs, dirPath, context) {
       // Try main
       if (parsed.main) {
         const mainPath = normalizeVFSPath(resolve(dirPath, parsed.main));
-        if (vfs.internalModuleStat(mainPath) === 0) {
+        if (vfsStat(vfs, mainPath) === 0) {
           const url = pathToFileURL(mainPath).href;
           const format = getVFSFormat(vfs, mainPath);
           return { url, format, shortCircuit: true };
@@ -623,7 +641,7 @@ function resolveVFSPath(checkPath, context, nextResolve, specifier) {
     const vfs = activeVFSList[i];
     if (!vfs.shouldHandle(normalized)) continue;
 
-    const stat = vfs.internalModuleStat(normalized);
+    const stat = vfsStat(vfs, normalized);
 
     if (stat === 0) {
       // It's a file
@@ -701,10 +719,10 @@ function resolveBareSpecifier(specifier, context, nextResolve) {
       resolve(currentDir, 'node_modules', packageName));
 
     if (parentVfs.shouldHandle(pkgDir) &&
-        parentVfs.internalModuleStat(pkgDir) === 1) {
+        vfsStat(parentVfs, pkgDir) === 1) {
       // Found the package directory
       const pjsonPath = normalizeVFSPath(resolve(pkgDir, 'package.json'));
-      if (parentVfs.internalModuleStat(pjsonPath) === 0) {
+      if (vfsStat(parentVfs, pjsonPath) === 0) {
         try {
           const content = parentVfs.readFileSync(pjsonPath, 'utf8');
           const parsed = JSONParse(content);
@@ -720,7 +738,7 @@ function resolveBareSpecifier(specifier, context, nextResolve) {
           if (packageSubpath === '.') {
             if (parsed.main) {
               const mainPath = normalizeVFSPath(resolve(pkgDir, parsed.main));
-              if (parentVfs.internalModuleStat(mainPath) === 0) {
+              if (vfsStat(parentVfs, mainPath) === 0) {
                 const url = pathToFileURL(mainPath).href;
                 const format = getVFSFormat(parentVfs, mainPath);
                 return { url, format, shortCircuit: true };
@@ -738,7 +756,7 @@ function resolveBareSpecifier(specifier, context, nextResolve) {
             // Resolve subpath like './feature'
             const subResolved = normalizeVFSPath(
               resolve(pkgDir, packageSubpath));
-            if (parentVfs.internalModuleStat(subResolved) === 0) {
+            if (vfsStat(parentVfs, subResolved) === 0) {
               const url = pathToFileURL(subResolved).href;
               const format = getVFSFormat(parentVfs, subResolved);
               return { url, format, shortCircuit: true };
@@ -840,7 +858,7 @@ function vfsLoadHook(url, context, nextLoad) {
     const vfs = activeVFSList[i];
     if (vfs.shouldHandle(normalized) && vfs.existsSync(normalized)) {
       // Only load files, not directories
-      const statResult = vfs.internalModuleStat(normalized);
+      const statResult = vfsStat(vfs, normalized);
       if (statResult !== 0) {
         return nextLoad(url, context);
       }

lib/internal/vfs/providers/memory.js

@@ -195,26 +195,6 @@ class MemoryProvider extends VirtualProvider {
     return path.slice(1).split('/');
   }
 
-  /**
-   * Gets the parent path.
-   * @param {string} path Normalized path
-   * @returns {string|null} Parent path or null for root
-   */
-  #getParentPath(path) {
-    if (path === '/') {
-      return null;
-    }
-    return pathPosix.dirname(path);
-  }
-
-  /**
-   * Gets the base name.
-   * @param {string} path Normalized path
-   * @returns {string} Base name
-   */
-  #getBaseName(path) {
-    return pathPosix.basename(path);
-  }
 
   /**
    * Resolves a symlink target to an absolute path.
@@ -227,7 +207,7 @@ class MemoryProvider extends VirtualProvider {
       return this.#normalizePath(target);
     }
     // Relative target: resolve against symlink's parent directory
-    const parentPath = this.#getParentPath(symlinkPath) || '/';
+    const parentPath = pathPosix.dirname(symlinkPath);
     return this.#normalizePath(pathPosix.join(parentPath, target));
   }
 
@@ -323,10 +303,10 @@ class MemoryProvider extends VirtualProvider {
    * @returns {MemoryEntry} The parent directory entry
    */
   #ensureParent(path, create, syscall) {
-    const parentPath = this.#getParentPath(path);
-    if (parentPath === null) {
+    if (path === '/') {
       return this[kRoot];
     }
+    const parentPath = pathPosix.dirname(path);
 
     const segments = this.#splitPath(parentPath);
     let current = this[kRoot];
@@ -458,7 +438,7 @@ class MemoryProvider extends VirtualProvider {
       if (err.code !== 'ENOENT' || !isCreate) throw err;
       // Create the file
       const parent = this.#ensureParent(normalized, true, 'open');
-      const name = this.#getBaseName(normalized);
+      const name = pathPosix.basename(normalized);
       entry = new MemoryEntry(TYPE_FILE, { mode });
       entry.content = Buffer.alloc(0);
       parent.children.set(name, entry);
@@ -570,7 +550,7 @@ class MemoryProvider extends VirtualProvider {
       }
     } else {
       const parent = this.#ensureParent(normalized, false, 'mkdir');
-      const name = this.#getBaseName(normalized);
+      const name = pathPosix.basename(normalized);
       const entry = new MemoryEntry(TYPE_DIR, { mode: options?.mode });
       entry.children = new SafeMap();
       parent.children.set(name, entry);
@@ -600,7 +580,7 @@ class MemoryProvider extends VirtualProvider {
     }
 
     const parent = this.#ensureParent(normalized, false, 'rmdir');
-    const name = this.#getBaseName(normalized);
+    const name = pathPosix.basename(normalized);
     parent.children.delete(name);
   }
 
@@ -621,7 +601,7 @@ class MemoryProvider extends VirtualProvider {
     }
 
     const parent = this.#ensureParent(normalized, false, 'unlink');
-    const name = this.#getBaseName(normalized);
+    const name = pathPosix.basename(normalized);
     parent.children.delete(name);
   }
 
@@ -642,12 +622,12 @@ class MemoryProvider extends VirtualProvider {
 
     // Remove from old location
     const oldParent = this.#ensureParent(normalizedOld, false, 'rename');
-    const oldName = this.#getBaseName(normalizedOld);
+    const oldName = pathPosix.basename(normalizedOld);
     oldParent.children.delete(oldName);
 
     // Add to new location
     const newParent = this.#ensureParent(normalizedNew, true, 'rename');
-    const newName = this.#getBaseName(normalizedNew);
+    const newName = pathPosix.basename(normalizedNew);
     newParent.children.set(newName, entry);
   }
 
@@ -684,7 +664,7 @@ class MemoryProvider extends VirtualProvider {
     }
 
     const parent = this.#ensureParent(normalized, true, 'symlink');
-    const name = this.#getBaseName(normalized);
+    const name = pathPosix.basename(normalized);
     const entry = new MemoryEntry(TYPE_SYMLINK);
     entry.target = target;
     parent.children.set(name, entry);

lib/internal/vfs/providers/real.js

@@ -1,6 +1,7 @@
 'use strict';
 
 const {
+  ObjectDefineProperty,
   Promise,
   StringPrototypeStartsWith,
 } = primordials;
@@ -172,6 +173,8 @@ class RealFSProvider extends VirtualProvider {
     }
     // Resolve to absolute path and normalize
     this.#rootPath = path.resolve(rootPath);
+    ObjectDefineProperty(this, 'readonly', { __proto__: null, value: false });
+    ObjectDefineProperty(this, 'supportsSymlinks', { __proto__: null, value: true });
   }
 
   /**
@@ -182,14 +185,6 @@ class RealFSProvider extends VirtualProvider {
     return this.#rootPath;
   }
 
-  get readonly() {
-    return false;
-  }
-
-  get supportsSymlinks() {
-    return true;
-  }
-
   /**
    * Resolves a VFS path to a real filesystem path.
    * Ensures the path doesn't escape the root directory.