From 8004f719d63d9661e6e3a48af25972a36de58abe Mon Sep 17 00:00:00 2001 From: Paul Roebuck Date: Fri, 8 Feb 2019 14:51:26 -0600 Subject: [PATCH 01/12] refactor(lib/utils.js): Extract lookupFile conditions into functions --- lib/utils.js | 42 ++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 40 insertions(+), 2 deletions(-) diff --git a/lib/utils.js b/lib/utils.js index c6e7245bc2..b09fde4701 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -506,6 +506,40 @@ exports.canonicalize = function canonicalize(value, stack, typeHint) { return canonicalizedObj; }; +/** + * Determines if pathname has a matching file extension. + * + * @private + * @param {string} pathname - Pathname to check for match. + * @param {string[]} exts - List of file extensions (sans period). + * @return {boolean} whether file extension matches. + * @example + * hasMatchingExtname('foo.html', ['js', 'css']); // => false + */ +function hasMatchingExtname(pathname, exts) { + var re = new RegExp('\\.(?:' + exts.join('|') + ')$'); + return re.test(path.extname(pathname)); +} + +/** + * Determines if pathname would be a "hidden" file (or directory) on UN*X. + * + * @description + * On UN*X, pathnames beginning with a full stop (aka dot) are hidden during + * typical usage. Dotfiles, plain-text configuration files, are prime examples. + * + * @see {@link http://xahlee.info/UnixResource_dir/writ/unix_origin_of_dot_filename.html|Origin of Dot File Names} + * + * @private + * @param {string} pathname - Pathname to check for match. + * @return {boolean} whether pathname would be considered a dotfile. + * @example + * isHiddenOnUnix('.profile'); // => true + */ +function isHiddenOnUnix(pathname) { + return path.basename(pathname)[0] === '.'; +} + /** * Lookup file names at the given `path`. * @@ -570,8 +604,12 @@ exports.lookupFiles = function lookupFiles(filepath, extensions, recursive) { 'array' ); } - var re = new RegExp('\\.(?:' + extensions.join('|') + ')$'); - if (!stat.isFile() || !re.test(file) || path.basename(file)[0] === '.') { + + if ( + !stat.isFile() || + !hasMatchingExtname(file, extensions) || + isHiddenOnUnix(file) + ) { return; } files.push(file); From c6b16e9a20c24f21938b5bd84d111666b4850b75 Mon Sep 17 00:00:00 2001 From: Paul Roebuck Date: Fri, 8 Feb 2019 15:16:46 -0600 Subject: [PATCH 02/12] refactor(lib/utils.js): Rename variables to match intent --- lib/utils.js | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/lib/utils.js b/lib/utils.js index b09fde4701..7a6c1930d0 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -583,13 +583,14 @@ exports.lookupFiles = function lookupFiles(filepath, extensions, recursive) { return; } - fs.readdirSync(filepath).forEach(function(file) { - file = path.join(filepath, file); + fs.readdirSync(filepath).forEach(function(dirent) { + var pathname = path.join(filepath, dirent); + try { - var stat = fs.statSync(file); + var stat = fs.statSync(pathname); if (stat.isDirectory()) { if (recursive) { - files = files.concat(lookupFiles(file, extensions, recursive)); + files = files.concat(lookupFiles(pathname, extensions, recursive)); } return; } @@ -607,12 +608,12 @@ exports.lookupFiles = function lookupFiles(filepath, extensions, recursive) { if ( !stat.isFile() || - !hasMatchingExtname(file, extensions) || - isHiddenOnUnix(file) + !hasMatchingExtname(pathname, extensions) || + isHiddenOnUnix(pathname) ) { return; } - files.push(file); + files.push(pathname); }); return files; From 3ab1f248a96b3d0f637a66948e9c250fbc10570f Mon Sep 17 00:00:00 2001 From: Paul Roebuck Date: Fri, 8 Feb 2019 15:18:26 -0600 Subject: [PATCH 03/12] refactor(lib/utils.js): Make variable scope more apparent --- lib/utils.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/utils.js b/lib/utils.js index 7a6c1930d0..4514765284 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -557,6 +557,7 @@ function isHiddenOnUnix(pathname) { */ exports.lookupFiles = function lookupFiles(filepath, extensions, recursive) { var files = []; + var stat; if (!fs.existsSync(filepath)) { if (fs.existsSync(filepath + '.js')) { @@ -574,7 +575,7 @@ exports.lookupFiles = function lookupFiles(filepath, extensions, recursive) { } try { - var stat = fs.statSync(filepath); + stat = fs.statSync(filepath); if (stat.isFile()) { return filepath; } @@ -585,9 +586,10 @@ exports.lookupFiles = function lookupFiles(filepath, extensions, recursive) { fs.readdirSync(filepath).forEach(function(dirent) { var pathname = path.join(filepath, dirent); + var stat; try { - var stat = fs.statSync(pathname); + stat = fs.statSync(pathname); if (stat.isDirectory()) { if (recursive) { files = files.concat(lookupFiles(pathname, extensions, recursive)); From 5546786a126006a252fba1529c354877ae598a23 Mon Sep 17 00:00:00 2001 From: Paul Roebuck Date: Fri, 8 Feb 2019 15:25:35 -0600 Subject: [PATCH 04/12] refactor(lib/utils.js): Rename variables to match intent (exports.files) --- lib/utils.js | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/lib/utils.js b/lib/utils.js index 4514765284..587fcf6200 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -92,24 +92,24 @@ function ignored(path) { * * @private * @param {string} dir - * @param {string[]} [ext=['.js']] + * @param {string[]} [exts=['.js']] * @param {Array} [ret=[]] * @return {Array} */ -exports.files = function(dir, ext, ret) { +exports.files = function(dir, exts, ret) { ret = ret || []; - ext = ext || ['js']; + exts = exts || ['js']; - var re = new RegExp('\\.(' + ext.join('|') + ')$'); + var re = new RegExp('\\.(' + exts.join('|') + ')$'); fs.readdirSync(dir) .filter(ignored) - .forEach(function(path) { - path = join(dir, path); - if (fs.lstatSync(path).isDirectory()) { - exports.files(path, ext, ret); - } else if (path.match(re)) { - ret.push(path); + .forEach(function(dirent) { + var pathname = join(dir, dirent); + if (fs.lstatSync(pathname).isDirectory()) { + exports.files(pathname, exts, ret); + } else if (pathname.match(re)) { + ret.push(pathname); } }); From 206b2d6e253275a9608757d9ba5788d0539a7bdc Mon Sep 17 00:00:00 2001 From: Paul Roebuck Date: Fri, 8 Feb 2019 15:29:20 -0600 Subject: [PATCH 05/12] refactor(lib/utils.js): Replace with `path.join`, fix @param documentation --- lib/utils.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/utils.js b/lib/utils.js index 587fcf6200..ee383b8706 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -13,7 +13,6 @@ var debug = require('debug')('mocha:watch'); var fs = require('fs'); var glob = require('glob'); var path = require('path'); -var join = path.join; var he = require('he'); var errors = require('./errors'); var createNoFilesMatchPatternError = errors.createNoFilesMatchPatternError; @@ -92,7 +91,7 @@ function ignored(path) { * * @private * @param {string} dir - * @param {string[]} [exts=['.js']] + * @param {string[]} [exts=['js']] * @param {Array} [ret=[]] * @return {Array} */ @@ -105,7 +104,7 @@ exports.files = function(dir, exts, ret) { fs.readdirSync(dir) .filter(ignored) .forEach(function(dirent) { - var pathname = join(dir, dirent); + var pathname = path.join(dir, dirent); if (fs.lstatSync(pathname).isDirectory()) { exports.files(pathname, exts, ret); } else if (pathname.match(re)) { From 75a45093267b72b91d3c234b4ff74cc06f957d4a Mon Sep 17 00:00:00 2001 From: Paul Roebuck Date: Fri, 8 Feb 2019 15:33:46 -0600 Subject: [PATCH 06/12] refactor(lib/utils.js): Replace `export.files` RE with `hasMatchingExtname` Replacement should work faster (re.test() faster than string.match()). --- lib/utils.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/utils.js b/lib/utils.js index ee383b8706..be85f9a957 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -99,15 +99,13 @@ exports.files = function(dir, exts, ret) { ret = ret || []; exts = exts || ['js']; - var re = new RegExp('\\.(' + exts.join('|') + ')$'); - fs.readdirSync(dir) .filter(ignored) .forEach(function(dirent) { var pathname = path.join(dir, dirent); if (fs.lstatSync(pathname).isDirectory()) { exports.files(pathname, exts, ret); - } else if (pathname.match(re)) { + } else if (hasMatchingExtname(pathname, exts)) { ret.push(pathname); } }); From 9b5413df560613a4afb9fc2f8aa4cb05acb45e1b Mon Sep 17 00:00:00 2001 From: Paul Roebuck Date: Sat, 9 Feb 2019 06:14:21 -0600 Subject: [PATCH 07/12] refactor(lib/utils.js): Minor tweaks Ordered requires (node,third party,internal). Fixed docstring. Added comments. --- lib/utils.js | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/lib/utils.js b/lib/utils.js index be85f9a957..b6d2b7b98c 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -9,10 +9,10 @@ * Module dependencies. */ -var debug = require('debug')('mocha:watch'); var fs = require('fs'); -var glob = require('glob'); var path = require('path'); +var debug = require('debug')('mocha:watch'); +var glob = require('glob'); var he = require('he'); var errors = require('./errors'); var createNoFilesMatchPatternError = errors.createNoFilesMatchPatternError; @@ -529,7 +529,7 @@ function hasMatchingExtname(pathname, exts) { * * @private * @param {string} pathname - Pathname to check for match. - * @return {boolean} whether pathname would be considered a dotfile. + * @return {boolean} whether pathname would be considered a hidden file. * @example * isHiddenOnUnix('.profile'); // => true */ @@ -560,6 +560,7 @@ exports.lookupFiles = function lookupFiles(filepath, extensions, recursive) { if (fs.existsSync(filepath + '.js')) { filepath += '.js'; } else { + // Handle glob files = glob.sync(filepath); if (!files.length) { throw createNoFilesMatchPatternError( @@ -571,6 +572,7 @@ exports.lookupFiles = function lookupFiles(filepath, extensions, recursive) { } } + // Handle file try { stat = fs.statSync(filepath); if (stat.isFile()) { @@ -581,6 +583,7 @@ exports.lookupFiles = function lookupFiles(filepath, extensions, recursive) { return; } + // Handle directory fs.readdirSync(filepath).forEach(function(dirent) { var pathname = path.join(filepath, dirent); var stat; From 8094b6392fc2f7cdc102c3c30d7dc31052b4942a Mon Sep 17 00:00:00 2001 From: Paul Roebuck Date: Tue, 12 Feb 2019 10:18:27 -0600 Subject: [PATCH 08/12] docs(lib/utils.js): Add JSDoc for `inherits`. --- lib/utils.js | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/lib/utils.js b/lib/utils.js index 79ca974ce6..49efa6a5d5 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -18,10 +18,19 @@ var errors = require('./errors'); var createNoFilesMatchPatternError = errors.createNoFilesMatchPatternError; var createMissingArgumentError = errors.createMissingArgumentError; -exports.inherits = util.inherits; - var assign = (exports.assign = require('object.assign').getPolyfill()); +/** + * Inherit the prototype methods from one constructor into another. + * + * @param {function} ctor - Constructor function which needs to inherit the + * prototype. + * @param {function} superCtor - Constructor function to inherit prototype from. + * @throws {TypeError} Will error if either constructor is null, or if + * the super constructor lacks a prototype. + */ +exports.inherits = util.inherits; + /** * Escape special characters in the given string of html. * From 895862c1b6fd277ff4f8a7175f132c993faff7e4 Mon Sep 17 00:00:00 2001 From: Paul Roebuck Date: Tue, 12 Feb 2019 10:28:38 -0600 Subject: [PATCH 09/12] docs(lib/utils.js): Fixup JSDoc for `createMap` and `defineConstants` --- lib/utils.js | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/lib/utils.js b/lib/utils.js index 49efa6a5d5..9a0ec035d9 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -846,14 +846,19 @@ exports.ngettext = function(n, msg1, msg2) { exports.noop = function() {}; /** - * @summary Creates a map-like object. - * @desc A "map" is an object with no prototype, for our purposes. In some cases this would be more appropriate than a `Map`, especially if your environment doesn't support it. Recommended for use in Mocha's public APIs. - * @param {...*} [obj] - Arguments to `Object.assign()` - * @returns {Object} An object with no prototype, having `...obj` properties + * Creates a map-like object. + * + * @description + * A "map" is an object with no prototype, for our purposes. In some cases + * this would be more appropriate than a `Map`, especially if your environment + * doesn't support it. Recommended for use in Mocha's public APIs. + * * @public - * @see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map - * @see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/create#Custom_and_Null_objects - * @see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/assign + * @see {@link https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map|MDN:Map} + * @see {@link https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/create#Custom_and_Null_objects|MDN:Object.create - Custom objects} + * @see {@link https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/assign|MDN:Object.assign} + * @param {...*} [obj] - Arguments to `Object.assign()`. + * @returns {Object} An object with no prototype, having `...obj` properties */ exports.createMap = function(obj) { return assign.apply( @@ -863,9 +868,14 @@ exports.createMap = function(obj) { }; /** - * @summary Create a read-only map-like object. - * This differs from {@link module:utils.createMap createMap} only in that the argument must be non-empty, because the result is frozen. + * Creates a read-only map-like object. + * + * @description + * This differs from {@link module:utils.createMap createMap} only in that + * the argument must be non-empty, because the result is frozen. + * * @see {@link module:utils.createMap createMap} + * @param {...*} [obj] - Arguments to `Object.assign()`. * @returns {Object} A frozen object with no prototype, having `...obj` properties */ exports.defineConstants = function(obj) { From a1599d9d2c554f5d95b607dfeaa0519c39743681 Mon Sep 17 00:00:00 2001 From: Paul Roebuck Date: Tue, 12 Feb 2019 10:35:50 -0600 Subject: [PATCH 10/12] refactor(lib/utils.js): Refactor `ignored` Renamed as `considerFurther` since result was opposite of existing name. Added appropriate JSDoc for function. Made `ignore` variable function scope. --- lib/utils.js | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/lib/utils.js b/lib/utils.js index 9a0ec035d9..fc61c668ed 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -75,19 +75,25 @@ exports.watch = function(files, fn) { }; /** - * Ignored directories. - */ -var ignore = ['node_modules', '.git']; - -/** - * Ignored files. + * Predicate to screen `pathname` for further consideration. + * + * @description + * Returns false for pathname referencing: + *
    + *
  • 'npm' package installation directory + *
  • 'git' version control directory + *
* * @private - * @param {string} path - * @return {boolean} + * @param {string} pathname - File or directory name to screen + * @return {boolean} whether pathname should be further considered + * @example + * ['node_modules', 'test.js'].filter(considerFurther); // => ['test.js'] */ -function ignored(path) { - return !~ignore.indexOf(path); +function considerFurther(pathname) { + var ignore = ['node_modules', '.git']; + + return !~ignore.indexOf(pathname); } /** @@ -108,7 +114,7 @@ exports.files = function(dir, exts, ret) { exts = exts || ['js']; fs.readdirSync(dir) - .filter(ignored) + .filter(considerFurther) .forEach(function(dirent) { var pathname = path.join(dir, dirent); if (fs.lstatSync(pathname).isDirectory()) { From b82d65d07cf1f7c3bb1aee35575c4690f25f6222 Mon Sep 17 00:00:00 2001 From: Paul Roebuck Date: Tue, 12 Feb 2019 11:12:43 -0600 Subject: [PATCH 11/12] docs(lib/utils.js): Document functions that throw --- lib/utils.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/utils.js b/lib/utils.js index fc61c668ed..d6faf6dfd8 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -26,8 +26,8 @@ var assign = (exports.assign = require('object.assign').getPolyfill()); * @param {function} ctor - Constructor function which needs to inherit the * prototype. * @param {function} superCtor - Constructor function to inherit prototype from. - * @throws {TypeError} Will error if either constructor is null, or if - * the super constructor lacks a prototype. + * @throws {TypeError} if either constructor is null, or if super constructor + * lacks a prototype. */ exports.inherits = util.inherits; @@ -565,6 +565,8 @@ function isHiddenOnUnix(pathname) { * @param {string[]} extensions - File extensions to look for. * @param {boolean} recursive - Whether to recurse into subdirectories. * @return {string[]} An array of paths. + * @throws {Error} if no files match pattern. + * @throws {TypeError} if `filepath` is directory and `extensions` not provided. */ exports.lookupFiles = function lookupFiles(filepath, extensions, recursive) { var files = []; @@ -883,6 +885,7 @@ exports.createMap = function(obj) { * @see {@link module:utils.createMap createMap} * @param {...*} [obj] - Arguments to `Object.assign()`. * @returns {Object} A frozen object with no prototype, having `...obj` properties + * @throws {TypeError} if argument is not a non-empty object. */ exports.defineConstants = function(obj) { if (type(obj) !== 'object' || !Object.keys(obj).length) { From 6f965949f988c7228a8679f52c720a72850f2118 Mon Sep 17 00:00:00 2001 From: Paul Roebuck Date: Tue, 12 Feb 2019 18:44:20 -0600 Subject: [PATCH 12/12] refactor(lib/utils.js): Replace `hasMatchingExtname` implementation Swapped out the `RegExp.test` implementation for `Array.some` one, providing ~3.5x speedup. --- lib/utils.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/utils.js b/lib/utils.js index d6faf6dfd8..86ba8f0376 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -528,8 +528,10 @@ exports.canonicalize = function canonicalize(value, stack, typeHint) { * hasMatchingExtname('foo.html', ['js', 'css']); // => false */ function hasMatchingExtname(pathname, exts) { - var re = new RegExp('\\.(?:' + exts.join('|') + ')$'); - return re.test(path.extname(pathname)); + var suffix = path.extname(pathname).slice(1); + return exts.some(function(element) { + return suffix === element; + }); } /**