From c66f8c21f0646aaffd59ddc332381c6824df3e5a Mon Sep 17 00:00:00 2001 From: Nathan Woltman Date: Wed, 25 Feb 2015 23:51:10 -0800 Subject: [PATCH] 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 Reviewed-By: Julien Gilli PR-URL: https://github.com/joyent/node/pull/9289 --- lib/path.js | 136 +++++++++++++------------- test/simple/test-path-parse-format.js | 26 ++++- 2 files changed, 91 insertions(+), 71 deletions(-) diff --git a/lib/path.js b/lib/path.js index 27c148ab535ff1..d91feff305d136 100644 --- a/lib/path.js +++ b/lib/path.js @@ -51,6 +51,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 var splitDeviceRe = @@ -76,9 +99,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: device, + isUnc: 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() { @@ -113,11 +148,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 && @@ -159,11 +194,11 @@ win32.resolve = function() { win32.normalize = function(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 @@ -187,22 +222,21 @@ win32.normalize = function(path) { win32.isAbsolute = function(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 (!util.isString(p)) { + var paths = []; + for (var i = 0; i < arguments.length; i++) { + var arg = arguments[i]; + if (!util.isString(arg)) { throw new TypeError('Arguments to path.join must be strings'); } - return p; + if (arg) { + paths.push(arg); + } } - 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 @@ -239,25 +273,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 = trim(to.split('\\')); + var toParts = trimArray(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; @@ -360,15 +379,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; }; @@ -384,7 +401,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) @@ -425,7 +442,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 @@ -442,7 +459,7 @@ posix.resolve = function() { // posix version posix.normalize = function(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('/'); @@ -488,23 +505,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; @@ -603,7 +605,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) diff --git a/test/simple/test-path-parse-format.js b/test/simple/test-path-parse-format.js index 4f6e5af45c06e8..8fab0fc33cfa0b 100644 --- a/test/simple/test-path-parse-format.js +++ b/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,6 +55,12 @@ 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/}, @@ -65,10 +76,12 @@ var errors = [ {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 +100,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 +109,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]); + }); +}