perf: cli/lookup-files.js, refactor, add jsdoc, reduce intermediary variables

lib/cli/lookup-files.js

@@ -1,18 +1,22 @@
 'use strict';
+
 /**
  * Contains `lookupFiles`, which takes some globs/dirs/options and returns a list of files.
  * @module
  * @private
  */
 
-var fs = require('node:fs');
-var path = require('node:path');
-var glob = require('glob');
-var errors = require('../errors');
-var createNoFilesMatchPatternError = errors.createNoFilesMatchPatternError;
-var createMissingArgumentError = errors.createMissingArgumentError;
+const fs = require('node:fs');
+const path = require('node:path');
+const glob = require('glob');
+const {
+  createNoFilesMatchPatternError,
+  createMissingArgumentError
+} = require('../errors');
 const debug = require('debug')('mocha:cli:lookup-files');
 
+/** @typedef {`.${string}`} HiddenUnixPathname */
+
 /**
  * Determines if pathname would be a "hidden" file (or directory) on UN*X.
  *
@@ -24,30 +28,54 @@ const debug = require('debug')('mocha:cli:lookup-files');
  *
  * @private
  * @param {string} pathname - Pathname to check for match.
- * @return {boolean} whether pathname would be considered a hidden file.
+ * @returns {value is HiddenUnixPathname} whether pathname would be considered a hidden file.
  * @example
  * isHiddenOnUnix('.profile'); // => true
  */
-const isHiddenOnUnix = pathname => path.basename(pathname).startsWith('.');
+const isHiddenOnUnix = pathname => path.basename(pathname)[0] === '.';
+
+/** @typedef {`.${string}`} FileExtension */
 
 /**
- * Determines if pathname has a matching file extension.
+ * Normalize file extensions to ensure they have a leading period character.
  *
- * Supports multi-part extensions.
+ * @private
+ * @param {FileExtension[]|string[]|undefined|null} exts
+ * @returns {FileExtension[]}
+ */
+const normalizeFileExtensions = (exts) => {
+  if (!exts) {
+    return [];
+  }
+
+  for (var i = 0; i < exts.length; i++) {
+    if (exts[i][0] !== '.') {
+      exts[i] = `.${exts[i]}`;
+    }
+  }
+  return /** @type {FileExtension[]} */ (exts);
+}
+
+/**
+ * Determines if pathname has a matching file extension.
  *
  * @private
  * @param {string} pathname - Pathname to check for match.
- * @param {string[]} exts - List of file extensions, w/-or-w/o leading period
- * @return {boolean} `true` if file extension matches.
+ * @param {FileExtension[]} fileExtensions - List of file extensions, w/-or-w/o leading period
+ * @returns {boolean} `true` if file extension matches.
  * @example
- * hasMatchingExtname('foo.html', ['js', 'css']); // false
- * hasMatchingExtname('foo.js', ['.js']); // true
- * hasMatchingExtname('foo.js', ['js']); // ture
+ * hasMatchingFileExtension('foo.html', ['js', 'css']); // false
+ * hasMatchingFileExtension('foo.js', ['.js']); // true
+ * hasMatchingFileExtension('foo.js', ['js']); // ture
  */
-const hasMatchingExtname = (pathname, exts = []) =>
-  exts
-    .map(ext => (ext.startsWith('.') ? ext : `.${ext}`))
-    .some(ext => pathname.endsWith(ext));
+const hasMatchingFileExtension = (pathname, fileExtensions) => {
+  for (var i = 0; i < fileExtensions.length; i++) {
+    if (pathname.endsWith(fileExtensions[i])) {
+      return true;
+    }
+  }
+  return false;
+}
 
 /**
  * Lookup file names at the given `path`.
@@ -59,31 +87,30 @@ const hasMatchingExtname = (pathname, exts = []) =>
  * @public
  * @alias module:lib/cli.lookupFiles
  * @param {string} filepath - Base path to start searching from.
- * @param {string[]} [extensions=[]] - File extensions to look for.
+ * @param {string[]} [fileExtensions=[]] - File extensions to look for.
  * @param {boolean} [recursive=false] - Whether to recurse into subdirectories.
- * @return {string[]} An array of paths.
+ * @returns {string[]} An array of paths.
  * @throws {Error} if no files match pattern.
- * @throws {TypeError} if `filepath` is directory and `extensions` not provided.
+ * @throws {TypeError} if `filepath` is directory and `fileExtensions` not 
+ * provided or an empty array.
  */
 module.exports = function lookupFiles(
   filepath,
-  extensions = [],
+  fileExtensions,
   recursive = false
 ) {
   const files = [];
-  let stat;
+  fileExtensions = normalizeFileExtensions(fileExtensions);
 
+  // Detect glob patterns by checking if the path does not exist as-is
   if (!fs.existsSync(filepath)) {
     let pattern;
-    if (glob.hasMagic(filepath, {windowsPathsNoEscape: true})) {
+    if (glob.hasMagic(filepath, { windowsPathsNoEscape: true })) {
       // Handle glob as is without extensions
       pattern = filepath;
     } else {
       // glob pattern e.g. 'filepath+(.js|.ts)'
-      const strExtensions = extensions
-        .map(ext => (ext.startsWith('.') ? ext : `.${ext}`))
-        .join('|');
-      pattern = `${filepath}+(${strExtensions})`;
+      pattern = `${filepath}+(${fileExtensions.join('|')})`;
       debug('looking for files using glob pattern: %s', pattern);
     }
     files.push(
@@ -106,50 +133,51 @@ module.exports = function lookupFiles(
     return files;
   }
 
-  // Handle file
-  try {
-    stat = fs.statSync(filepath);
-    if (stat.isFile()) {
-      return filepath;
-    }
-  } catch (err) {
-    // ignore error
-    return;
-  }
-
-  // Handle directory
-  fs.readdirSync(filepath).forEach(dirent => {
-    const pathname = path.join(filepath, dirent);
-    let stat;
+  const stat = fs.statSync(filepath, {
+    throwIfNoEntry: false
+  });
 
-    try {
-      stat = fs.statSync(pathname);
-      if (stat.isDirectory()) {
-        if (recursive) {
-          files.push(...lookupFiles(pathname, extensions, recursive));
-        }
-        return;
-      }
-    } catch (ignored) {
-      return;
-    }
-    if (!extensions.length) {
+  if (stat === undefined) {
+    // Unreachable because glob check already checks if path exists, but for
+    // completeness...
+  } else if (stat.isFile()) {
+    files.push(filepath);
+  } else if (stat.isDirectory()) {
+    if (fileExtensions.length === 0) {
       throw createMissingArgumentError(
-        `Argument '${extensions}' required when argument '${filepath}' is a directory`,
+        `Argument '${fileExtensions}' required when argument '${filepath}' is a directory`,
         'extensions',
         'array'
-      );
+      )
     }
 
-    if (
-      !stat.isFile() ||
-      !hasMatchingExtname(pathname, extensions) ||
-      isHiddenOnUnix(pathname)
-    ) {
-      return;
+    // Handle directory
+    const dirEnts = fs.readdirSync(filepath, { recursive, withFileTypes: true });
+
+    for (var i = 0; i < dirEnts.length; i++) {
+      const dirEnt = dirEnts[i];
+
+      const pathname = dirEnt.parentPath
+        ? path.join(dirEnt.parentPath, dirEnt.name)
+        : path.join(filepath, dirEnt.name);
+
+      if (dirEnt.isFile() || dirEnt.isSymbolicLink()) {
+        if (dirEnt.isSymbolicLink()) {
+          const stat = fs.statSync(pathname, { throwIfNoEntry: false });
+          if (!stat || !stat.isFile()) {
+            continue; // Skip broken symlinks or symlinks to directories
+          }
+        }
+
+        if (
+          hasMatchingFileExtension(pathname, /** @type {FileExtension[]} */(fileExtensions)) &&
+          !isHiddenOnUnix(pathname)
+        ) {
+          files.push(pathname);
+        }
+      }
     }
-    files.push(pathname);
-  });
+  }
 
   return files;
 };

test/integration/file-utils.spec.js

@@ -17,7 +17,7 @@ describe('file utils', function () {
     tmpDir = result.dirpath;
     removeTempDir = result.removeTempDir;
 
-    tmpFile = filepath => path.join(tmpDir, filepath);
+    tmpFile = (...args) => path.join(...[tmpDir, ...args]);
 
     touchFile(tmpFile('mocha-utils.js'));
     if (SYMLINK_SUPPORT) {
@@ -64,6 +64,22 @@ describe('file utils', function () {
       ex.and('to have length', expectedLength);
     });
 
+    describe('when containing subdirectories', function () {
+      beforeEach(function () {
+        touchFile(tmpFile('types','mocha-utils.d.ts'));
+      });
+
+      it('should find files in subdirectories', function () {
+        expect(
+          lookupFiles(tmpDir, ['.d.ts'], true).map(foundFilepath =>
+            path.normalize(foundFilepath)
+          ),
+          'to contain',
+          tmpFile('types', 'mocha-utils.d.ts')
+        ).and('to have length', 1);
+      });
+    })
+
     describe('when given `extension` option', function () {
       describe('when provided a directory for the filepath', function () {
         let filepath;
@@ -253,6 +269,15 @@ describe('file utils', function () {
       });
     });
 
+    describe('when provided path to non existing file', function () {
+      it('should throw an exception', function () {
+        expect(() => lookupFiles(tmpFile('mocha-utils-faux')), 'to throw', {
+          name: 'Error',
+          code: 'ERR_MOCHA_NO_FILES_MATCH_PATTERN'
+        });
+      });
+    });
+
     describe('when no files match', function () {
       it('should throw an exception', function () {
         expect(() => lookupFiles(tmpFile('mocha-utils')), 'to throw', {