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.

Author: plroebuck

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 <code>false</code> for pathname referencing:
+ * <ul>
+ *   <li>'npm' package installation directory
+ *   <li>'git' version control directory
+ * </ul>
  *
  * @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) {