Skip to content

Commit 5006627

Browse files
Trottaduh95
authored andcommitted
fs: apply exclude function to root path
In at least some situations, the root path was being added to the results of globbing even if it matched the optional exclude function. In the relevant test file, a variable was renamed for consistency. (There was a patterns and pattern2, rather than patterns2.) The test case is added to patterns2. Fixes: #56260 PR-URL: #57420 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Jason Zhang <[email protected]> Reviewed-By: LiviaMedeiros <[email protected]> Reviewed-By: James M Snell <[email protected]>
1 parent 6ee15c6 commit 5006627

File tree

2 files changed

+33
-5
lines changed

2 files changed

+33
-5
lines changed

lib/internal/fs/glob.js

+25-1
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ const {
99
ArrayPrototypePop,
1010
ArrayPrototypePush,
1111
ArrayPrototypeSome,
12+
Promise,
1213
PromisePrototypeThen,
1314
SafeMap,
1415
SafeSet,
@@ -125,7 +126,8 @@ class Cache {
125126
}
126127
statSync(path) {
127128
const cached = this.#statsCache.get(path);
128-
if (cached) {
129+
// Do not return a promise from a sync function.
130+
if (cached && !(cached instanceof Promise)) {
129131
return cached;
130132
}
131133
const val = getDirentSync(path);
@@ -326,6 +328,28 @@ class Glob {
326328
if (this.#isExcluded(path)) {
327329
return;
328330
}
331+
const fullpath = resolve(this.#root, path);
332+
333+
// If path is a directory, add trailing slash and test patterns again.
334+
// TODO(Trott): Would running #isExcluded() first and checking isDirectory() only
335+
// if it matches be more performant in the typical use case? #isExcluded()
336+
// is often ()=>false which is about as optimizable as a function gets.
337+
if (this.#cache.statSync(fullpath).isDirectory() && this.#isExcluded(`${fullpath}/`)) {
338+
return;
339+
}
340+
341+
if (this.#exclude) {
342+
if (this.#withFileTypes) {
343+
const stat = this.#cache.statSync(path);
344+
if (stat !== null) {
345+
if (this.#exclude(stat)) {
346+
return;
347+
}
348+
}
349+
} else if (this.#exclude(path)) {
350+
return;
351+
}
352+
}
329353
if (!this.#subpatterns.has(path)) {
330354
this.#subpatterns.set(path, [pattern]);
331355
} else {

test/parallel/test-fs-glob.mjs

+8-4
Original file line numberDiff line numberDiff line change
@@ -388,7 +388,7 @@ describe('fsPromises glob - withFileTypes', function() {
388388
});
389389

390390
// [pattern, exclude option, expected result]
391-
const pattern2 = [
391+
const patterns2 = [
392392
['a/{b,c}*', ['a/*c'], ['a/b', 'a/cb']],
393393
['a/{a,b,c}*', ['a/*bc*', 'a/cb'], ['a/b', 'a/c']],
394394
['a/**/[cg]', ['**/c'], ['a/abcdef/g', 'a/abcfed/g']],
@@ -427,6 +427,10 @@ const pattern2 = [
427427
[`${absDir}/*{a,q}*`, './a/*{c,b}*/*'],
428428
[`${absDir}/foo`, 'a/c', ...(common.isWindows ? [] : ['a/symlink/a/b/c'])],
429429
],
430+
[ 'a/**', () => true, [] ],
431+
[ 'a/**', [ '*' ], [] ],
432+
[ 'a/**', [ '**' ], [] ],
433+
[ 'a/**', [ 'a/**' ], [] ],
430434
];
431435

432436
describe('globSync - exclude', function() {
@@ -436,7 +440,7 @@ describe('globSync - exclude', function() {
436440
assert.strictEqual(actual.length, 0);
437441
});
438442
}
439-
for (const [pattern, exclude, expected] of pattern2) {
443+
for (const [pattern, exclude, expected] of patterns2) {
440444
test(`${pattern} - exclude: ${exclude}`, () => {
441445
const actual = globSync(pattern, { cwd: fixtureDir, exclude }).sort();
442446
const normalized = expected.filter(Boolean).map((item) => item.replaceAll('/', sep)).sort();
@@ -453,7 +457,7 @@ describe('glob - exclude', function() {
453457
assert.strictEqual(actual.length, 0);
454458
});
455459
}
456-
for (const [pattern, exclude, expected] of pattern2) {
460+
for (const [pattern, exclude, expected] of patterns2) {
457461
test(`${pattern} - exclude: ${exclude}`, async () => {
458462
const actual = (await promisified(pattern, { cwd: fixtureDir, exclude })).sort();
459463
const normalized = expected.filter(Boolean).map((item) => item.replaceAll('/', sep)).sort();
@@ -471,7 +475,7 @@ describe('fsPromises glob - exclude', function() {
471475
assert.strictEqual(actual.length, 0);
472476
});
473477
}
474-
for (const [pattern, exclude, expected] of pattern2) {
478+
for (const [pattern, exclude, expected] of patterns2) {
475479
test(`${pattern} - exclude: ${exclude}`, async () => {
476480
const actual = [];
477481
for await (const item of asyncGlob(pattern, { cwd: fixtureDir, exclude })) actual.push(item);

0 commit comments

Comments
 (0)