Skip to content

bug: lookupFiles() returns a string instead of string[] when given a direct file path #5940

@maniktyagi04

Description

@maniktyagi04

Summary

lookupFiles() in lib/cli/lookup-files.mjs has a documented return type of string[], but when given a direct file path (i.e., a file that exists on disk, not a glob or directory), it returns a bare string instead of a string[].

Environment

  • Mocha version: latest (main branch)
    • Node.js version: v20+ / v22+
      • Platform: Any

Reproduction

// lib/cli/lookup-files.mjs (lines 107-115)
try {
  stat = fs.statSync(filepath);
  if (stat.isFile() || stat.isFIFO()) {
    return filepath; // <- returns a bare string, not an array
  }
} catch {
  return;
}

Minimal reproduction (programmatic API):

const { lookupFiles } = require('mocha/lib/cli/lookup-files');

const result = lookupFiles('./test/mytest.spec.js', ['js'], false);
// Expected: ['./test/mytest.spec.js']  (string[])
// Actual:   './test/mytest.spec.js'    (string)

// This breaks spread:
const files = [...result]; // iterates over characters, not file paths!

Impact

The JSDoc on lookupFiles documents the return type as {string[]}:

 * @return {string[]} An array of paths.

However, when a direct file path is passed (not a glob, not a directory), the function short-circuits and returns a plain string. This creates a type inconsistency that can silently break downstream consumers, including:

  • Any programmatic API user who spreads the result: [...lookupFiles(file)] would iterate over characters.
    • Any consumer who calls .map() or .filter() on the result - these would act on characters of the string, not file paths.
      • Plugins or third-party integrations that rely on the documented interface.
        Currently, Mocha's own internal call site in run-helpers.js happens to use [].concat(lookupFiles(...)), which works around the inconsistency since [].concat("string") produces ["string"]. However, this is fragile and relies on undocumented behavior.

Suggested Fix

Wrap the file path in an array before returning, to match the documented return type:

if (stat.isFile() || stat.isFIFO()) {
-   return filepath;
+   return [filepath];
}

This is a 1-line change in lib/cli/lookup-files.mjs and makes the actual return type consistent with the documentation.

A corresponding test should also verify that a direct file path argument returns an array with exactly one element.

Additional Context

This was discovered while auditing the lib/cli/lookup-files.mjs module. The inconsistency is subtle because the internal call site in run-helpers.js uses [].concat(...), which silently handles both strings and arrays - masking the bug from the existing test suite.

Metadata

Metadata

Assignees

No one assigned

    Labels

    good first issuenew contributors should look here!status: accepting prsMocha can use your help with this one!type: buga defect, confirmed by a maintainer

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions