path: Clean up path.parse/format and add tests

+ Remove unused variable and error triggered if
  that variable isn't the right type
+ Remove unreachable code (checking return value of *SplitPath)
+ Remove unnecessary `|| ''` (because what's on the left of that is
  guaranteed to be a string)

Author: nwoltman

lib/path.js

@@ -90,7 +90,7 @@ function win32SplitPath(filename) {
   // Separate device+slash from tail
   var result = splitDeviceRe.exec(filename),
       device = (result[1] || '') + (result[2] || ''),
-      tail = result[3] || '';
+      tail = result[3];
   // Split the tail into dir, basename and extension
   var result2 = splitTailRe.exec(tail),
       dir = result2[1],
@@ -364,42 +364,31 @@ win32.extname = function(path) {
 win32.format = function(pathObject) {
   if (!util.isObject(pathObject)) {
     throw new TypeError(
-        "Parameter 'pathObject' must be an object, not " + typeof pathObject
-    );
-  }
-
-  var root = pathObject.root || '';
-
-  if (!util.isString(root)) {
-    throw new TypeError(
-        "'pathObject.root' must be a string or undefined, not " +
-        typeof pathObject.root
+      "Parameter 'pathObject' must be an object, not " + typeof pathObject
     );
   }
 
   var dir = pathObject.dir;
   var base = pathObject.base || '';
-  if (dir) {
-    if (dir[dir.length - 1] === win32.sep) {
-      return dir + base;
-    }
-    return dir + win32.sep + base;
-  }
 
-  return base;
+  if (!dir) {
+    return base;
+  }
+  if (dir[dir.length - 1] === win32.sep) {
+    return dir + base;
+  }
+  return dir + win32.sep + base;
 };
 
 
 win32.parse = function(pathString) {
   if (!util.isString(pathString)) {
     throw new TypeError(
-        "Parameter 'pathString' must be a string, not " + typeof pathString
+      "Parameter 'pathString' must be a string, not " + typeof pathString
     );
   }
+
   var allParts = win32SplitPath(pathString);
-  if (!allParts || allParts.length !== 4) {
-    throw new TypeError("Invalid path '" + pathString + "'");
-  }
   return {
     root: allParts[0],
     dir: allParts[0] + allParts[1].slice(0, -1),
@@ -571,16 +560,7 @@ posix.extname = function(path) {
 posix.format = function(pathObject) {
   if (!util.isObject(pathObject)) {
     throw new TypeError(
-        "Parameter 'pathObject' must be an object, not " + typeof pathObject
-    );
-  }
-
-  var root = pathObject.root || '';
-
-  if (!util.isString(root)) {
-    throw new TypeError(
-        "'pathObject.root' must be a string or undefined, not " +
-        typeof pathObject.root
+      "Parameter 'pathObject' must be an object, not " + typeof pathObject
     );
   }
 
@@ -593,17 +573,11 @@ posix.format = function(pathObject) {
 posix.parse = function(pathString) {
   if (!util.isString(pathString)) {
     throw new TypeError(
-        "Parameter 'pathString' must be a string, not " + typeof pathString
+      "Parameter 'pathString' must be a string, not " + typeof pathString
     );
   }
-  var allParts = posixSplitPath(pathString);
-  if (!allParts || allParts.length !== 4) {
-    throw new TypeError("Invalid path '" + pathString + "'");
-  }
-  allParts[1] = allParts[1] || '';
-  allParts[2] = allParts[2] || '';
-  allParts[3] = allParts[3] || '';
 
+  var allParts = posixSplitPath(pathString);
   return {
     root: allParts[0],
     dir: allParts[0] + allParts[1].slice(0, -1),

test/simple/test-path-parse-format.js

@@ -35,7 +35,12 @@ var winPaths = [
   '\\\\server two\\shared folder\\file path.zip',
   '\\\\teela\\admin$\\system32',
   '\\\\?\\UNC\\server\\share'
+];
 
+var winSpecialCaseFormatTests = [
+  [{dir: 'some\\dir'}, 'some\\dir\\'],
+  [{base: 'index.html'}, 'index.html'],
+  [{}, '']
 ];
 
 var unixPaths = [
@@ -50,25 +55,30 @@ var unixPaths = [
   'C:\\foo'
 ];
 
+var unixSpecialCaseFormatTests = [
+  [{dir: 'some/dir'}, 'some/dir/'],
+  [{base: 'index.html'}, 'index.html'],
+  [{}, '']
+];
+
 var errors = [
   {method: 'parse', input: [null], message: /Parameter 'pathString' must be a string, not/},
   {method: 'parse', input: [{}], message: /Parameter 'pathString' must be a string, not object/},
   {method: 'parse', input: [true], message: /Parameter 'pathString' must be a string, not boolean/},
   {method: 'parse', input: [1], message: /Parameter 'pathString' must be a string, not number/},
   {method: 'parse', input: [], message: /Parameter 'pathString' must be a string, not undefined/},
-  // {method: 'parse', input: [''], message: /Invalid path/}, // omitted because it's hard to trigger!
   {method: 'format', input: [null], message: /Parameter 'pathObject' must be an object, not/},
   {method: 'format', input: [''], message: /Parameter 'pathObject' must be an object, not string/},
   {method: 'format', input: [true], message: /Parameter 'pathObject' must be an object, not boolean/},
   {method: 'format', input: [1], message: /Parameter 'pathObject' must be an object, not number/},
-  {method: 'format', input: [{root: true}], message: /'pathObject.root' must be a string or undefined, not boolean/},
-  {method: 'format', input: [{root: 12}], message: /'pathObject.root' must be a string or undefined, not number/},
 ];
 
-check(path.win32, winPaths);
-check(path.posix, unixPaths);
+checkParseFormat(path.win32, winPaths);
+checkParseFormat(path.posix, unixPaths);
 checkErrors(path.win32);
 checkErrors(path.posix);
+checkFormat(path.win32, winSpecialCaseFormatTests);
+checkFormat(path.posix, unixSpecialCaseFormatTests);
 
 function checkErrors(path) {
   errors.forEach(function(errorCase) {
@@ -87,8 +97,7 @@ function checkErrors(path) {
   });
 }
 
-
-function check(path, paths) {
+function checkParseFormat(path, paths) {
   paths.forEach(function(element, index, array) {
     var output = path.parse(element);
     assert.strictEqual(path.format(output), element);
@@ -97,3 +106,9 @@ function check(path, paths) {
     assert.strictEqual(output.ext, path.extname(element));
   });
 }
+
+function checkFormat(path, testCases) {
+  testCases.forEach(function(testCase) {
+    assert.strictEqual(path.format(testCase[0]), testCase[1]);
+  });
+}