fs: allow 'withFileTypes' to be used with globs

avivkeller · targos · commit 7d7a762156ad · 2024-05-08T07:12:44.000+02:00
PR-URL: #52837 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
diff --git a/doc/api/fs.md b/doc/api/fs.md
@@ -1073,6 +1073,10 @@ behavior is similar to `cp dir1/ dir2/`.
 
 <!-- YAML
 added: v22.0.0
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/52837
+    description: Add support for `withFileTypes` as an option.
 -->
 
 > Stability: 1 - Experimental
@@ -1082,6 +1086,8 @@ added: v22.0.0
   * `cwd` {string} current working directory. **Default:** `process.cwd()`
   * `exclude` {Function} Function to filter out files/directories. Return
     `true` to exclude the item, `false` to include it. **Default:** `undefined`.
+  * `withFileTypes` {boolean} `true` if the glob should return paths as Dirents,
+    `false` otherwise. **Default:** `false`.
 * Returns: {AsyncIterator} An AsyncIterator that yields the paths of files
   that match the pattern.
 
@@ -3109,6 +3115,10 @@ descriptor. See [`fs.utimes()`][].
 
 <!-- YAML
 added: v22.0.0
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/52837
+    description: Add support for `withFileTypes` as an option.
 -->
 
 > Stability: 1 - Experimental
@@ -3119,6 +3129,8 @@ added: v22.0.0
   * `cwd` {string} current working directory. **Default:** `process.cwd()`
   * `exclude` {Function} Function to filter out files/directories. Return
     `true` to exclude the item, `false` to include it. **Default:** `undefined`.
+  * `withFileTypes` {boolean} `true` if the glob should return paths as Dirents,
+    `false` otherwise. **Default:** `false`.
 
 * `callback` {Function}
   * `err` {Error}
@@ -5603,6 +5615,10 @@ Synchronous version of [`fs.futimes()`][]. Returns `undefined`.
 
 <!-- YAML
 added: v22.0.0
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/52837
+    description: Add support for `withFileTypes` as an option.
 -->
 
 > Stability: 1 - Experimental
@@ -5612,6 +5628,8 @@ added: v22.0.0
   * `cwd` {string} current working directory. **Default:** `process.cwd()`
   * `exclude` {Function} Function to filter out files/directories. Return
     `true` to exclude the item, `false` to include it. **Default:** `undefined`.
+  * `withFileTypes` {boolean} `true` if the glob should return paths as Dirents,
+    `false` otherwise. **Default:** `false`.
 * Returns: {string\[]} paths of files that match the pattern.
 
 ```mjs
diff --git a/lib/internal/fs/glob.js b/lib/internal/fs/glob.js
@@ -16,7 +16,7 @@ const {
 
 const { lstatSync, readdirSync } = require('fs');
 const { lstat, readdir } = require('fs/promises');
-const { join, resolve } = require('path');
+const { join, resolve, basename, isAbsolute } = require('path');
 
 const {
   kEmptyObject,
@@ -27,6 +27,7 @@ const {
   validateString,
   validateStringArray,
 } = require('internal/validators');
+const { DirentFromStats } = require('internal/fs/utils');
 
 let minimatch;
 function lazyMinimatch() {
@@ -37,6 +38,14 @@ function lazyMinimatch() {
 const isWindows = process.platform === 'win32';
 const isOSX = process.platform === 'darwin';
 
+async function getDirent(path) {
+  return new DirentFromStats(basename(path), await lstat(path), path);
+}
+
+function getDirentSync(path) {
+  return new DirentFromStats(basename(path), lstatSync(path), path);
+}
+
 class Cache {
   #cache = new SafeMap();
   #statsCache = new SafeMap();
@@ -47,7 +56,7 @@ class Cache {
     if (cached) {
       return cached;
     }
-    const promise = PromisePrototypeThen(lstat(path), null, () => null);
+    const promise = PromisePrototypeThen(getDirent(path), null, () => null);
     this.#statsCache.set(path, promise);
     return promise;
   }
@@ -58,7 +67,7 @@ class Cache {
     }
     let val;
     try {
-      val = lstatSync(path);
+      val = getDirentSync(path);
     } catch {
       val = null;
     }
@@ -175,14 +184,16 @@ class Glob {
   #queue = [];
   #subpatterns = new SafeMap();
   #patterns;
+  #withFileTypes;
   constructor(pattern, options = kEmptyObject) {
     validateObject(options, 'options');
-    const { exclude, cwd } = options;
+    const { exclude, cwd, withFileTypes } = options;
     if (exclude != null) {
       validateFunction(exclude, 'options.exclude');
     }
     this.#root = cwd ?? '.';
     this.#exclude = exclude;
+    this.#withFileTypes = !!withFileTypes;
     let patterns;
     if (typeof pattern === 'object') {
       validateStringArray(pattern, 'patterns');
@@ -222,7 +233,14 @@ class Glob {
         .forEach((patterns, path) => ArrayPrototypePush(this.#queue, { __proto__: null, path, patterns }));
       this.#subpatterns.clear();
     }
-    return ArrayFrom(this.#results);
+    return ArrayFrom(
+      this.#results,
+      this.#withFileTypes ? (path) => this.#cache.statSync(
+        isAbsolute(path) ?
+          path :
+          join(this.#root, path),
+      ) : undefined,
+    );
   }
   #addSubpattern(path, pattern) {
     if (!this.#subpatterns.has(path)) {
@@ -317,7 +335,7 @@ class Glob {
         const fromSymlink = pattern.symlinks.has(index);
 
         if (current === lazyMinimatch().GLOBSTAR) {
-          if (entry.name[0] === '.' || (this.#exclude && this.#exclude(entry.name))) {
+          if (entry.name[0] === '.' || (this.#exclude && this.#exclude(this.#withFileTypes ? entry : entry.name))) {
             continue;
           }
           if (!fromSymlink && entry.isDirectory()) {
@@ -460,7 +478,7 @@ class Glob {
         const result = join(path, p);
         if (!this.#results.has(result)) {
           this.#results.add(result);
-          yield result;
+          yield this.#withFileTypes ? stat : result;
         }
       }
       if (pattern.indexes.size === 1 && pattern.indexes.has(last)) {
@@ -472,7 +490,7 @@ class Glob {
       // if path is ".", add it only if pattern starts with "." or pattern is exactly "**"
       if (!this.#results.has(path)) {
         this.#results.add(path);
-        yield path;
+        yield this.#withFileTypes ? stat : path;
       }
     }
 
@@ -522,7 +540,7 @@ class Glob {
             // If ** is last, add to results
             if (!this.#results.has(entryPath)) {
               this.#results.add(entryPath);
-              yield entryPath;
+              yield this.#withFileTypes ? entry : entryPath;
             }
           }
 
@@ -533,7 +551,7 @@ class Glob {
             // If next pattern is the last one, add to results
             if (!this.#results.has(entryPath)) {
               this.#results.add(entryPath);
-              yield entryPath;
+              yield this.#withFileTypes ? entry : entryPath;
             }
           } else if (nextMatches && entry.isDirectory()) {
             // Pattern mached, meaning two patterns forward
@@ -569,14 +587,14 @@ class Glob {
                 this.#cache.add(path, pattern.child(new SafeSet().add(nextIndex)));
                 if (!this.#results.has(path)) {
                   this.#results.add(path);
-                  yield path;
+                  yield this.#withFileTypes ? this.#cache.statSync(fullpath) : path;
                 }
               }
               if (!this.#cache.seen(path, pattern, nextIndex) || !this.#cache.seen(parent, pattern, nextIndex)) {
                 this.#cache.add(parent, pattern.child(new SafeSet().add(nextIndex)));
                 if (!this.#results.has(parent)) {
                   this.#results.add(parent);
-                  yield parent;
+                  yield this.#withFileTypes ? this.#cache.statSync(join(this.#root, parent)) : parent;
                 }
               }
             }
@@ -592,7 +610,7 @@ class Glob {
             if (nextIndex === last) {
               if (!this.#results.has(entryPath)) {
                 this.#results.add(entryPath);
-                yield entryPath;
+                yield this.#withFileTypes ? entry : entryPath;
               }
             } else {
               subPatterns.add(nextIndex + 1);
@@ -605,7 +623,7 @@ class Glob {
           if (index === last) {
             if (!this.#results.has(entryPath)) {
               this.#results.add(entryPath);
-              yield entryPath;
+              yield this.#withFileTypes ? entry : entryPath;
             }
           } else if (entry.isDirectory()) {
             subPatterns.add(nextIndex);
diff --git a/lib/internal/fs/utils.js b/lib/internal/fs/utils.js
@@ -961,6 +961,7 @@ module.exports = {
   BigIntStats,  // for testing
   copyObject,
   Dirent,
+  DirentFromStats,
   emitRecursiveRmdirWarning,
   getDirent,
   getDirents,
diff --git a/test/parallel/test-fs-glob.mjs b/test/parallel/test-fs-glob.mjs
@@ -1,12 +1,16 @@
 import * as common from '../common/index.mjs';
 import tmpdir from '../common/tmpdir.js';
-import { resolve, dirname, sep } from 'node:path';
+import { resolve, dirname, sep, basename } from 'node:path';
 import { mkdir, writeFile, symlink, glob as asyncGlob } from 'node:fs/promises';
-import { glob, globSync } from 'node:fs';
+import { glob, globSync, Dirent } from 'node:fs';
 import { test, describe } from 'node:test';
 import { promisify } from 'node:util';
 import assert from 'node:assert';
 
+function assertDirents(dirents) {
+  assert.ok(dirents.every((dirent) => dirent instanceof Dirent));
+}
+
 tmpdir.refresh();
 
 const fixtureDir = tmpdir.resolve('fixtures');
@@ -333,3 +337,38 @@ describe('fsPromises glob', function() {
     });
   }
 });
+
+describe('glob - withFileTypes', function() {
+  const promisified = promisify(glob);
+  for (const [pattern, expected] of Object.entries(patterns)) {
+    test(pattern, async () => {
+      const actual = await promisified(pattern, { cwd: fixtureDir, withFileTypes: true });
+      assertDirents(actual);
+      const normalized = expected.filter(Boolean).map((item) => basename(item)).sort();
+      assert.deepStrictEqual(actual.map((dirent) => dirent.name).sort(), normalized.sort());
+    });
+  }
+});
+
+describe('globSync - withFileTypes', function() {
+  for (const [pattern, expected] of Object.entries(patterns)) {
+    test(pattern, () => {
+      const actual = globSync(pattern, { cwd: fixtureDir, withFileTypes: true });
+      assertDirents(actual);
+      const normalized = expected.filter(Boolean).map((item) => basename(item)).sort();
+      assert.deepStrictEqual(actual.map((dirent) => dirent.name).sort(), normalized.sort());
+    });
+  }
+});
+
+describe('fsPromises glob - withFileTypes', function() {
+  for (const [pattern, expected] of Object.entries(patterns)) {
+    test(pattern, async () => {
+      const actual = [];
+      for await (const item of asyncGlob(pattern, { cwd: fixtureDir, withFileTypes: true })) actual.push(item);
+      assertDirents(actual);
+      const normalized = expected.filter(Boolean).map((item) => basename(item)).sort();
+      assert.deepStrictEqual(actual.map((dirent) => dirent.name).sort(), normalized.sort());
+    });
+  }
+});
