path: refactor for performance and consistency

Improve performance by:
+ Not leaking the `arguments` object!
+ Getting the last character of a string by index, instead of
  with `.substr()` or `.slice()`

Improve code consistency by:
+ Using `[]` instead of `.charAt()` where possible
+ Using a function declaration instead of a var declaration
+ Using `.slice()` with clearer arguments
+ Checking if `dir` is truthy in `win32.format`
  (added tests for this)

Improve both by:
+ Making the reusable `trimArray()` function
+ Standardizing getting certain path statistics with
  the new `win32StatPath()` function

PR-URL: https://github.com/nodejs/io.js/pull/1778
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>

1 files changed, 69 insertions, 70 deletions

diff --git a/lib/path.js b/lib/path.js
index b7e28b2225..69e7992c01 100644
--- a/lib/path.js
+++ b/lib/path.js
@@ -37,6 +37,29 @@ function normalizeArray(parts, allowAboveRoot) {
   return res;
 }
 
+// Returns an array with empty elements removed from either end of the input
+// array or the original array if no elements need to be removed
+function trimArray(arr) {
+  var lastIndex = arr.length - 1;
+  var start = 0;
+  for (; start <= lastIndex; start++) {
+    if (arr[start])
+      break;
+  }
+
+  var end = lastIndex;
+  for (; end >= 0; end--) {
+    if (arr[end])
+      break;
+  }
+
+  if (start === 0 && end === lastIndex)
+    return arr;
+  if (start > end)
+    return [];
+  return arr.slice(start, end + 1);
+}
+
 // Regex to split a windows path into three parts: [*, device, slash,
 // tail] windows-only
 const splitDeviceRe =
@@ -62,9 +85,21 @@ function win32SplitPath(filename) {
   return [device, dir, basename, ext];
 }
 
-var normalizeUNCRoot = function(device) {
+function win32StatPath(path) {
+  var result = splitDeviceRe.exec(path),
+      device = result[1] || '',
+      isUnc = !!device && device[1] !== ':';
+  return {
+    device,
+    isUnc,
+    isAbsolute: isUnc || !!result[2], // UNC paths are always absolute
+    tail: result[3]
+  };
+}
+
+function normalizeUNCRoot(device) {
   return '\\\\' + device.replace(/^[\\\/]+/, '').replace(/[\\\/]+/g, '\\');
-};
+}
 
 // path.resolve([from ...], to)
 win32.resolve = function() {
@@ -99,11 +134,11 @@ win32.resolve = function() {
       continue;
     }
 
-    var result = splitDeviceRe.exec(path),
-        device = result[1] || '',
-        isUnc = device && device.charAt(1) !== ':',
-        isAbsolute = win32.isAbsolute(path),
-        tail = result[3];
+    var result = win32StatPath(path),
+        device = result.device,
+        isUnc = result.isUnc,
+        isAbsolute = result.isAbsolute,
+        tail = result.tail;
 
     if (device &&
         resolvedDevice &&
@@ -147,11 +182,11 @@ win32.resolve = function() {
 win32.normalize = function(path) {
   assertPath(path);
 
-  var result = splitDeviceRe.exec(path),
-      device = result[1] || '',
-      isUnc = device && device.charAt(1) !== ':',
-      isAbsolute = win32.isAbsolute(path),
-      tail = result[3],
+  var result = win32StatPath(path),
+      device = result.device,
+      isUnc = result.isUnc,
+      isAbsolute = result.isAbsolute,
+      tail = result.tail,
       trailingSlash = /[\\\/]$/.test(tail);
 
   // Normalize the tail path
@@ -176,23 +211,19 @@ win32.normalize = function(path) {
 
 win32.isAbsolute = function(path) {
   assertPath(path);
-
-  var result = splitDeviceRe.exec(path),
-      device = result[1] || '',
-      isUnc = !!device && device.charAt(1) !== ':';
-  // UNC paths are always absolute
-  return !!result[2] || isUnc;
+  return win32StatPath(path).isAbsolute;
 };
 
 win32.join = function() {
-  function f(p) {
-    if (typeof p !== 'string') {
-      throw new TypeError('Arguments to path.join must be strings');
+  var paths = [];
+  for (var i = 0; i < arguments.length; i++) {
+    var arg = arguments[i];
+    assertPath(arg);
+    if (arg) {
+      paths.push(arg);
     }
-    return p;
   }
 
-  var paths = Array.prototype.filter.call(arguments, f);
   var joined = paths.join('\\');
 
   // Make sure that the joined path doesn't start with two slashes, because
@@ -232,25 +263,10 @@ win32.relative = function(from, to) {
   var lowerFrom = from.toLowerCase();
   var lowerTo = to.toLowerCase();
 
-  function trim(arr) {
-    var start = 0;
-    for (; start < arr.length; start++) {
-      if (arr[start] !== '') break;
-    }
-
-    var end = arr.length - 1;
-    for (; end >= 0; end--) {
-      if (arr[end] !== '') break;
-    }
-
-    if (start > end) return [];
-    return arr.slice(start, end + 1);
-  }
+  var toParts = trimArray(to.split('\\'));
 
-  var toParts = trim(to.split('\\'));
-
-  var lowerFromParts = trim(lowerFrom.split('\\'));
-  var lowerToParts = trim(lowerTo.split('\\'));
+  var lowerFromParts = trimArray(lowerFrom.split('\\'));
+  var lowerToParts = trimArray(lowerTo.split('\\'));
 
   var length = Math.min(lowerFromParts.length, lowerToParts.length);
   var samePartsLength = length;
@@ -356,15 +372,13 @@ win32.format = function(pathObject) {
 
   var dir = pathObject.dir;
   var base = pathObject.base || '';
-  if (dir.slice(dir.length - 1, dir.length) === win32.sep) {
-    return dir + base;
+  if (!dir) {
+    return base;
   }
-
-  if (dir) {
-    return dir + win32.sep + base;
+  if (dir[dir.length - 1] === win32.sep) {
+    return dir + base;
   }
-
-  return base;
+  return dir + win32.sep + base;
 };
 
 
@@ -377,7 +391,7 @@ win32.parse = function(pathString) {
   }
   return {
     root: allParts[0],
-    dir: allParts[0] + allParts[1].slice(0, allParts[1].length - 1),
+    dir: allParts[0] + allParts[1].slice(0, -1),
     base: allParts[2],
     ext: allParts[3],
     name: allParts[2].slice(0, allParts[2].length - allParts[3].length)
@@ -418,7 +432,7 @@ posix.resolve = function() {
     }
 
     resolvedPath = path + '/' + resolvedPath;
-    resolvedAbsolute = path.charAt(0) === '/';
+    resolvedAbsolute = path[0] === '/';
   }
 
   // At this point the path should be resolved to a full absolute path, but
@@ -437,7 +451,7 @@ posix.normalize = function(path) {
   assertPath(path);
 
   var isAbsolute = posix.isAbsolute(path),
-      trailingSlash = path.substr(-1) === '/';
+      trailingSlash = path && path[path.length - 1] === '/';
 
   // Normalize the path
   path = normalizeArray(path.split('/'), !isAbsolute).join('/');
@@ -455,7 +469,7 @@ posix.normalize = function(path) {
 // posix version
 posix.isAbsolute = function(path) {
   assertPath(path);
-  return path.charAt(0) === '/';
+  return !!path && path[0] === '/';
 };
 
 // posix version
@@ -487,23 +501,8 @@ posix.relative = function(from, to) {
   from = posix.resolve(from).substr(1);
   to = posix.resolve(to).substr(1);
 
-  function trim(arr) {
-    var start = 0;
-    for (; start < arr.length; start++) {
-      if (arr[start] !== '') break;
-    }
-
-    var end = arr.length - 1;
-    for (; end >= 0; end--) {
-      if (arr[end] !== '') break;
-    }
-
-    if (start > end) return [];
-    return arr.slice(start, end + 1);
-  }
-
-  var fromParts = trim(from.split('/'));
-  var toParts = trim(to.split('/'));
+  var fromParts = trimArray(from.split('/'));
+  var toParts = trimArray(to.split('/'));
 
   var length = Math.min(fromParts.length, toParts.length);
   var samePartsLength = length;
@@ -602,7 +601,7 @@ posix.parse = function(pathString) {
 
   return {
     root: allParts[0],
-    dir: allParts[0] + allParts[1].slice(0, allParts[1].length - 1),
+    dir: allParts[0] + allParts[1].slice(0, -1),
     base: allParts[2],
     ext: allParts[3],
     name: allParts[2].slice(0, allParts[2].length - allParts[3].length)