Merge pull request #8180 from NomicFoundation/optimize-get-all-files-matching

Optimize `getAllFilesMatching` and `getAllDirectoriesMatching`

Author: alcuadrado

.changeset/tough-lines-begin.md

@@ -0,0 +1,6 @@
+---
+"@nomicfoundation/hardhat-utils": patch
+"hardhat": patch
+---
+
+Optimize file system traversal helpers

cspell.dictionary.txt

@@ -29,6 +29,7 @@ dedup
 defi
 Defi
 dessant
+dirents
 distros
 DISTROS
 dorny

packages/hardhat-utils/src/fs.ts

@@ -20,6 +20,10 @@ import {
   IsDirectoryError,
   DirectoryNotEmptyError,
 } from "./errors/fs.js";
+import {
+  isDirectoryDirentAware,
+  readdirWithFileTypesOrEmpty,
+} from "./internal/fs.js";
 
 /**
  * Determines the canonical pathname for a given path, resolving any symbolic
@@ -59,12 +63,12 @@ export async function getAllFilesMatching(
   matches?: (absolutePathToFile: string) => Promise<boolean> | boolean,
   directoryFilter?: (absolutePathToDir: string) => Promise<boolean> | boolean,
 ): Promise<string[]> {
-  const dirContent = await readdirOrEmpty(dirFrom);
+  const dirContent = await readdirWithFileTypesOrEmpty(dirFrom);
 
   const results = await Promise.all(
-    dirContent.map(async (file) => {
-      const absolutePathToFile = path.join(dirFrom, file);
-      if (await isDirectory(absolutePathToFile)) {
+    dirContent.map(async (dirent) => {
+      const absolutePathToFile = path.join(dirFrom, dirent.name);
+      if (await isDirectoryDirentAware(absolutePathToFile, dirent)) {
         if (
           directoryFilter === undefined ||
           (await directoryFilter(absolutePathToFile))
@@ -107,12 +111,12 @@ export async function getAllDirectoriesMatching(
   dirFrom: string,
   matches?: (absolutePathToDir: string) => Promise<boolean> | boolean,
 ): Promise<string[]> {
-  const dirContent = await readdirOrEmpty(dirFrom);
+  const dirContent = await readdirWithFileTypesOrEmpty(dirFrom);
 
   const results = await Promise.all(
-    dirContent.map(async (file) => {
-      const absolutePathToFile = path.join(dirFrom, file);
-      if (!(await isDirectory(absolutePathToFile))) {
+    dirContent.map(async (dirent) => {
+      const absolutePathToFile = path.join(dirFrom, dirent.name);
+      if (!(await isDirectoryDirentAware(absolutePathToFile, dirent))) {
         return [];
       }
 
@@ -494,17 +498,24 @@ export async function readdir(absolutePathToDir: string): Promise<string[]> {
 }
 
 /**
- * Wrapper around `readdir` that returns an empty array if the directory doesn't exist.
+ * Reads a directory and returns its content as an array of strings, returning
+ * an empty array if the directory doesn't exist.
  *
- * @see readdir
+ * @param absolutePathToDir The path to the directory.
+ * @returns An array of strings with the names of the files and directories in the directory, and an empty array if the directory doesn't exist.
+ * @throws NotADirectoryError if the path is not a directory.
+ * @throws FileSystemAccessError for any other error.
  */
-async function readdirOrEmpty(dirFrom: string): Promise<string[]> {
+export async function readdirOrEmpty(
+  absolutePathToDir: string,
+): Promise<string[]> {
   try {
-    return await readdir(dirFrom);
+    return await readdir(absolutePathToDir);
   } catch (error) {
     if (error instanceof FileNotFoundError) {
       return [];
     }
+
     throw error;
   }
 }

packages/hardhat-utils/src/internal/fs.ts

@@ -0,0 +1,57 @@
+import type { Dirent } from "node:fs";
+
+import fsPromises from "node:fs/promises";
+
+import { ensureNodeErrnoExceptionError } from "../error.js";
+import { FileSystemAccessError, NotADirectoryError } from "../errors/fs.js";
+import { isDirectory } from "../fs.js";
+
+/**
+ * Like `readdirOrEmpty`, but returns `Dirent` entries to know if an entry is a
+ * directory or not without an extra `lstat` syscall.
+ */
+export async function readdirWithFileTypesOrEmpty(
+  dirFrom: string,
+): Promise<Dirent[]> {
+  try {
+    return await fsPromises.readdir(dirFrom, { withFileTypes: true });
+  } catch (e) {
+    ensureNodeErrnoExceptionError(e);
+
+    if (e.code === "ENOENT") {
+      return [];
+    }
+
+    if (e.code === "ENOTDIR") {
+      throw new NotADirectoryError(dirFrom, e);
+    }
+
+    throw new FileSystemAccessError(e.message, e);
+  }
+}
+
+/**
+ * Determines if a dirent refers to a directory, falling back to `lstat` only
+ * when the dirent type is unknown.
+ */
+export async function isDirectoryDirentAware(
+  absolutePath: string,
+  dirent: Dirent,
+): Promise<boolean> {
+  if (dirent.isDirectory()) {
+    return true;
+  }
+
+  if (
+    dirent.isFile() ||
+    dirent.isSymbolicLink() ||
+    dirent.isBlockDevice() ||
+    dirent.isCharacterDevice() ||
+    dirent.isFIFO() ||
+    dirent.isSocket()
+  ) {
+    return false;
+  }
+
+  return await isDirectory(absolutePath);
+}

packages/hardhat-utils/test/fs.ts

@@ -1,3 +1,5 @@
+import type { Dirent } from "node:fs";
+
 import assert from "node:assert/strict";
 import fsPromises from "node:fs/promises";
 import path from "node:path";
@@ -32,10 +34,35 @@ import {
   readJsonFileAsStream,
   writeJsonFileAsStream,
   mkdtemp,
+  readdirOrEmpty,
 } from "../src/fs.js";
 
 import { useTmpDir } from "./helpers/fs.js";
 
+function withUnknownDirentType(dirent: Dirent): Dirent {
+  const cloned: Dirent = Object.create(dirent);
+  cloned.isDirectory = () => false;
+  cloned.isFile = () => false;
+  cloned.isSymbolicLink = () => false;
+  cloned.isBlockDevice = () => false;
+  cloned.isCharacterDevice = () => false;
+  cloned.isFIFO = () => false;
+  cloned.isSocket = () => false;
+  return cloned;
+}
+
+function withKnownFileDirentType(dirent: Dirent): Dirent {
+  const cloned: Dirent = Object.create(dirent);
+  cloned.isDirectory = () => false;
+  cloned.isFile = () => true;
+  cloned.isSymbolicLink = () => false;
+  cloned.isBlockDevice = () => false;
+  cloned.isCharacterDevice = () => false;
+  cloned.isFIFO = () => false;
+  cloned.isSocket = () => false;
+  return cloned;
+}
+
 describe("File system utils", () => {
   const getTmpDir = useTmpDir("fs");
 
@@ -209,6 +236,78 @@ describe("File system utils", () => {
       );
     });
 
+    it("Should recurse into directories when the dirent type is unknown", async (t) => {
+      const originalReaddir = fsPromises.readdir;
+      const originalLstat = fsPromises.lstat;
+
+      const unknownDirPath = path.join(getTmpDir(), "dir-with-files");
+      const knownFilePath = path.join(getTmpDir(), "file-1.txt");
+      const lstatPaths: string[] = [];
+
+      t.mock.method(fsPromises, "readdir", async (...args: any[]) => {
+        const [absolutePathToDir, options] = args;
+        const dirents: Dirent[] = await Reflect.apply(
+          originalReaddir,
+          fsPromises,
+          args,
+        );
+
+        if (
+          absolutePathToDir !== getTmpDir() ||
+          options?.withFileTypes !== true
+        ) {
+          return dirents;
+        }
+
+        return dirents.map((dirent) => {
+          if (dirent.name === path.basename(unknownDirPath)) {
+            return withUnknownDirentType(dirent);
+          }
+
+          if (dirent.name === path.basename(knownFilePath)) {
+            return withKnownFileDirentType(dirent);
+          }
+
+          return dirent;
+        });
+      });
+
+      t.mock.method(fsPromises, "lstat", async (...args: any[]) => {
+        lstatPaths.push(String(args[0]));
+        return await Reflect.apply(originalLstat, fsPromises, args);
+      });
+
+      const files = await getAllFilesMatching(getTmpDir());
+
+      assert.deepEqual(
+        new Set(files),
+        new Set([
+          path.join(getTmpDir(), "file-1.txt"),
+          path.join(getTmpDir(), "file-2.txt"),
+          path.join(getTmpDir(), "file-3.json"),
+          path.join(getTmpDir(), "dir-with-files", "inner-file-1.json"),
+          path.join(getTmpDir(), "dir-with-files", "inner-file-2.txt"),
+          path.join(getTmpDir(), "dir-with-extension.txt", "inner-file-3.txt"),
+          path.join(getTmpDir(), "dir-with-extension.txt", "inner-file-4.json"),
+          path.join(getTmpDir(), "dir-WithCasing", "file-WithCASING"),
+          path.join(
+            getTmpDir(),
+            "dir-with-files",
+            "dir-within-dir",
+            "file-deep",
+          ),
+        ]),
+      );
+      assert.ok(
+        lstatPaths.includes(unknownDirPath),
+        `expected lstat to be used for unknown dirent path ${unknownDirPath}`,
+      );
+      assert.ok(
+        !lstatPaths.includes(knownFilePath),
+        `expected lstat to not be used for known file path ${knownFilePath}`,
+      );
+    });
+
     it("Should throw NotADirectoryError if the path is not a directory", async () => {
       const dirPath = path.join(getTmpDir(), "file-1.txt");
 
@@ -361,6 +460,58 @@ describe("File system utils", () => {
         [path.join(getTmpDir(), "dir-with-files")],
       );
     });
+
+    it("Should match directories when the dirent type is unknown", async (t) => {
+      const originalReaddir = fsPromises.readdir;
+      const originalLstat = fsPromises.lstat;
+
+      const unknownDirPath = path.join(getTmpDir(), "dir-with-files");
+      const lstatPaths: string[] = [];
+
+      t.mock.method(fsPromises, "readdir", async (...args: any[]) => {
+        const [absolutePathToDir, options] = args;
+        const dirents: Dirent[] = await Reflect.apply(
+          originalReaddir,
+          fsPromises,
+          args,
+        );
+
+        if (
+          absolutePathToDir !== getTmpDir() ||
+          options?.withFileTypes !== true
+        ) {
+          return dirents;
+        }
+
+        return dirents.map((dirent) =>
+          dirent.name === path.basename(unknownDirPath)
+            ? withUnknownDirentType(dirent)
+            : dirent,
+        );
+      });
+
+      t.mock.method(fsPromises, "lstat", async (...args: any[]) => {
+        lstatPaths.push(String(args[0]));
+        return await Reflect.apply(originalLstat, fsPromises, args);
+      });
+
+      const dirs = await getAllDirectoriesMatching(getTmpDir());
+
+      assert.deepEqual(
+        new Set(dirs),
+        new Set([
+          path.join(getTmpDir(), "dir-empty"),
+          path.join(getTmpDir(), "dir-with-files"),
+          path.join(getTmpDir(), "dir-with-subdir"),
+          path.join(getTmpDir(), "dir-with-extension.txt"),
+          path.join(getTmpDir(), "dir-WithCasing"),
+        ]),
+      );
+      assert.ok(
+        lstatPaths.includes(unknownDirPath),
+        `expected lstat to be used for unknown dirent path ${unknownDirPath}`,
+      );
+    });
   });
 
   describe("getFileTrueCase", () => {
@@ -935,6 +1086,52 @@ describe("File system utils", () => {
     });
   });
 
+  describe("readdirOrEmpty", () => {
+    it("Should return the files in a directory", async () => {
+      const dirPath = path.join(getTmpDir(), "dir");
+      await mkdir(dirPath);
+
+      const files = ["file1.txt", "file2.txt", "file3.json"];
+      for (const file of files) {
+        await createFile(path.join(dirPath, file));
+      }
+
+      const subDirPath = path.join(dirPath, "subdir");
+      await mkdir(subDirPath);
+      await createFile(path.join(subDirPath, "file4.txt"));
+
+      // should include the subdir but not its content as it's not recursive
+      assert.deepEqual(
+        new Set(await readdirOrEmpty(dirPath)),
+        new Set(["file1.txt", "file2.txt", "file3.json", "subdir"]),
+      );
+    });
+
+    it("Should return an empty array if the directory doesn't exist", async () => {
+      const dirPath = path.join(getTmpDir(), "not-exists");
+
+      assert.deepEqual(await readdirOrEmpty(dirPath), []);
+    });
+
+    it("Should throw NotADirectoryError if the path is not a directory", async () => {
+      const filePath = path.join(getTmpDir(), "file");
+      await createFile(filePath);
+
+      await assert.rejects(readdirOrEmpty(filePath), {
+        name: "NotADirectoryError",
+        message: `Path ${filePath} is not a directory`,
+      });
+    });
+
+    it("Should throw FileSystemAccessError if a different error is thrown", async () => {
+      const invalidPath = "\0";
+
+      await assert.rejects(readdirOrEmpty(invalidPath), {
+        name: "FileSystemAccessError",
+      });
+    });
+  });
+
   describe("mkdir", () => {
     it("Should create a directory", async () => {
       const dirPath = path.join(getTmpDir(), "dir");