From 45ae014d0baba97b4b50b37ae526e1b50a9334e9 Mon Sep 17 00:00:00 2001 From: "P. Roebuck" Date: Wed, 13 Feb 2019 07:42:45 -0600 Subject: [PATCH] Refactor `lookupFiles` and `files` (#3722) * Extract `lookupFile` conditions into functions. * Rename functions/variables to match intent; various scope reductions * Ordered requires (node->third party->project). * Add/Correct JSDoc for various functions. * Replaced `hasMatchingExtname` implementation, providing ~3.5x speedup. --- lib/utils.js | 158 +++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 114 insertions(+), 44 deletions(-) diff --git a/lib/utils.js b/lib/utils.js index 82d4017603..86ba8f0376 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -9,26 +9,27 @@ * Module dependencies. */ -var debug = require('debug')('mocha:watch'); var fs = require('fs'); -var glob = require('glob'); var path = require('path'); var util = require('util'); -var join = path.join; +var glob = require('glob'); var he = require('he'); var errors = require('./errors'); var createNoFilesMatchPatternError = errors.createNoFilesMatchPatternError; var createMissingArgumentError = errors.createMissingArgumentError; +var assign = (exports.assign = require('object.assign').getPolyfill()); + /** - * Ignored directories. + * 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} if either constructor is null, or if super constructor + * lacks a prototype. */ - -var ignore = ['node_modules', '.git']; - -exports.inherits = require('util').inherits; - -var assign = (exports.assign = require('object.assign').getPolyfill()); +exports.inherits = util.inherits; /** * Escape special characters in the given string of html. @@ -62,6 +63,7 @@ exports.isString = function(obj) { */ exports.watch = function(files, fn) { var options = {interval: 100}; + var debug = require('debug')('mocha:watch'); files.forEach(function(file) { debug('file %s', file); fs.watchFile(file, options, function(curr, prev) { @@ -73,14 +75,25 @@ exports.watch = function(files, fn) { }; /** - * Ignored files. + * Predicate to screen `pathname` for further consideration. + * + * @description + * Returns false for pathname referencing: + * * * @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); } /** @@ -92,24 +105,22 @@ 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']; - - var re = new RegExp('\\.(' + ext.join('|') + ')$'); + exts = exts || ['js']; 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); + .filter(considerFurther) + .forEach(function(dirent) { + var pathname = path.join(dir, dirent); + if (fs.lstatSync(pathname).isDirectory()) { + exports.files(pathname, exts, ret); + } else if (hasMatchingExtname(pathname, exts)) { + ret.push(pathname); } }); @@ -506,6 +517,42 @@ 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 suffix = path.extname(pathname).slice(1); + return exts.some(function(element) { + return suffix === element; + }); +} + +/** + * 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 hidden file. + * @example + * isHiddenOnUnix('.profile'); // => true + */ +function isHiddenOnUnix(pathname) { + return path.basename(pathname)[0] === '.'; +} + /** * Lookup file names at the given `path`. * @@ -520,14 +567,18 @@ exports.canonicalize = function canonicalize(value, stack, typeHint) { * @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 = []; + var stat; if (!fs.existsSync(filepath)) { if (fs.existsSync(filepath + '.js')) { filepath += '.js'; } else { + // Handle glob files = glob.sync(filepath); if (!files.length) { throw createNoFilesMatchPatternError( @@ -539,8 +590,9 @@ exports.lookupFiles = function lookupFiles(filepath, extensions, recursive) { } } + // Handle file try { - var stat = fs.statSync(filepath); + stat = fs.statSync(filepath); if (stat.isFile()) { return filepath; } @@ -549,13 +601,16 @@ exports.lookupFiles = function lookupFiles(filepath, extensions, recursive) { return; } - fs.readdirSync(filepath).forEach(function(file) { - file = path.join(filepath, file); + // Handle directory + fs.readdirSync(filepath).forEach(function(dirent) { + var pathname = path.join(filepath, dirent); + var stat; + try { - var stat = fs.statSync(file); + stat = fs.statSync(pathname); if (stat.isDirectory()) { if (recursive) { - files = files.concat(lookupFiles(file, extensions, recursive)); + files = files.concat(lookupFiles(pathname, extensions, recursive)); } return; } @@ -574,11 +629,15 @@ 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(pathname, extensions) || + isHiddenOnUnix(pathname) + ) { return; } - files.push(file); + files.push(pathname); }); return files; @@ -797,14 +856,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( @@ -814,10 +878,16 @@ 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 + * @throws {TypeError} if argument is not a non-empty object. */ exports.defineConstants = function(obj) { if (type(obj) !== 'object' || !Object.keys(obj).length) {