path: refactor more path code for simplicity

1) Consolidate format to a single function. 2) Move some code that can only be reached in some code branches that was formerly executed in all cases. 3) Explicitly check for the string length of zero instead of converting the string to a boolean. 4) Consolidate nested if statements if possible e.g., if (foo) { if (bar) { /* do stuff */ } } to reduce indentation depth. 5) Simplify checks by removing extra length checks when comparing two strings. 6) Use object shorthand notation where possible. PR-URL: #25278 Reviewed-By: Michaël Zasso <targos@protonmail.com>
nodejs · BridgeAR · Mar 6, 2019 · Feb 12, 2019 · Feb 14, 2019 · Dec 10, 2018
commit cccc44b85446c4387985a3b7faca66f3a831ca50
diff --git a/lib/path.js b/lib/path.js
@@ -110,6 +110,9 @@ function normalizeString(path, allowAboveRoot, separator, isPathSeparator) {
 }
 
 function _format(sep, pathObject) {
+  if (pathObject === null || typeof pathObject !== 'object') {
+    throw new ERR_INVALID_ARG_TYPE('pathObject', 'Object', pathObject);
+  }
   const dir = pathObject.dir || pathObject.root;
   const base = pathObject.base ||
     `${pathObject.name || ''}${pathObject.ext || ''}`;
@@ -121,16 +124,22 @@ function _format(sep, pathObject) {
 
 const win32 = {
   // path.resolve([from ...], to)
-  resolve: function resolve() {
-    var resolvedDevice = '';
-    var resolvedTail = '';
-    var resolvedAbsolute = false;
+  resolve(...args) {
+    let resolvedDevice = '';
+    let resolvedTail = '';
+    let resolvedAbsolute = false;
 
-    for (var i = arguments.length - 1; i >= -1; i--) {
-      var path;
+    for (var i = args.length - 1; i >= -1; i--) {
+      let path;
       if (i >= 0) {
-        path = arguments[i];
-      } else if (!resolvedDevice) {
+        path = args[i];
+        validateString(path, 'path');
+
+        // Skip empty entries
+        if (path.length === 0) {
+          continue;
+        }
+      } else if (resolvedDevice.length === 0) {
         path = process.cwd();
       } else {
         // Windows has the concept of drive-specific current working
@@ -149,17 +158,10 @@ const win32 = {
         }
       }
 
-      validateString(path, 'path');
-
-      // Skip empty entries
-      if (path.length === 0) {
-        continue;
-      }
-
-      var len = path.length;
-      var rootEnd = 0;
-      var device = '';
-      var isAbsolute = false;
+      const len = path.length;
+      let rootEnd = 0;
+      let device = '';
+      let isAbsolute = false;
       const code = path.charCodeAt(0);
 
       // Try to match a root
@@ -409,16 +411,14 @@ const win32 = {
     if (isPathSeparator(firstPart.charCodeAt(0))) {
       ++slashCount;
       const firstLen = firstPart.length;
-      if (firstLen > 1) {
-        if (isPathSeparator(firstPart.charCodeAt(1))) {
-          ++slashCount;
-          if (firstLen > 2) {
-            if (isPathSeparator(firstPart.charCodeAt(2)))
-              ++slashCount;
-            else {
-              // We matched a UNC path in the first part
-              needsReplace = false;
-            }
+      if (firstLen > 1 && isPathSeparator(firstPart.charCodeAt(1))) {
+        ++slashCount;
+        if (firstLen > 2) {
+          if (isPathSeparator(firstPart.charCodeAt(2)))
+            ++slashCount;
+          else {
+            // We matched a UNC path in the first part
+            needsReplace = false;
           }
         }
       }
@@ -699,16 +699,14 @@ const win32 = {
     // Check for a drive letter prefix so as not to mistake the following
     // path separator as an extra separator at the end of the path that can be
     // disregarded
-    if (path.length >= 2) {
-      const drive = path.charCodeAt(0);
-      if (isWindowsDeviceRoot(drive)) {
-        if (path.charCodeAt(1) === CHAR_COLON)
-          start = 2;
-      }
+    if (path.length >= 2 &&
+        isWindowsDeviceRoot(path.charCodeAt(0)) &&
+        path.charCodeAt(1) === CHAR_COLON) {
+      start = 2;
     }
 
     if (ext !== undefined && ext.length > 0 && ext.length <= path.length) {
-      if (ext.length === path.length && ext === path)
+      if (ext === path)
         return '';
       var extIdx = ext.length - 1;
       var firstNonSlashEnd = -1;
@@ -839,16 +837,9 @@ const win32 = {
     return path.slice(startDot, end);
   },
 
+  format: _format.bind(null, '\\'),
 
-  format: function format(pathObject) {
-    if (pathObject === null || typeof pathObject !== 'object') {
-      throw new ERR_INVALID_ARG_TYPE('pathObject', 'Object', pathObject);
-    }
-    return _format('\\', pathObject);
-  },
-
-
-  parse: function parse(path) {
+  parse(path) {
     validateString(path, 'path');
 
     const ret = { root: '', dir: '', base: '', ext: '', name: '' };
@@ -1056,9 +1047,12 @@ const posix = {
     // Normalize the path
     path = normalizeString(path, !isAbsolute, '/', isPosixPathSeparator);
 
-    if (path.length === 0 && !isAbsolute)
-      path = '.';
-    if (path.length > 0 && trailingSeparator)
+    if (path.length === 0) {
+      if (isAbsolute)
+        return '/';
+      return trailingSeparator ? './' : '.';
+    }
+    if (trailingSeparator)
       path += '/';
 
     return isAbsolute ? `/${path}` : path;
@@ -1219,7 +1213,7 @@ const posix = {
     var i;
 
     if (ext !== undefined && ext.length > 0 && ext.length <= path.length) {
-      if (ext.length === path.length && ext === path)
+      if (ext === path)
         return '';
       var extIdx = ext.length - 1;
       var firstNonSlashEnd = -1;
@@ -1338,16 +1332,9 @@ const posix = {
     return path.slice(startDot, end);
   },
 
+  format: _format.bind(null, '/'),
 
-  format: function format(pathObject) {
-    if (pathObject === null || typeof pathObject !== 'object') {
-      throw new ERR_INVALID_ARG_TYPE('pathObject', 'Object', pathObject);
-    }
-    return _format('/', pathObject);
-  },
-
-
-  parse: function parse(path) {
+  parse(path) {
     validateString(path, 'path');
 
     const ret = { root: '', dir: '', base: '', ext: '', name: '' };