Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 29 additions & 17 deletions src/glob/GlobWalker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -697,9 +697,14 @@
};
self.close_disallowing_cwd(fd);
let mode = stat_result.st_mode as u32;
let matches = (S::ISDIR(mode) && !self.walker.only_files)
|| S::ISREG(mode)
|| !self.walker.only_files;
let trailing_sep = self.walker.pattern_components[idx as usize].trailing_sep;
let matches = if trailing_sep {
S::ISDIR(mode) && !self.walker.only_files
} else {
(S::ISDIR(mode) && !self.walker.only_files)
|| S::ISREG(mode)
|| !self.walker.only_files
};

Check warning on line 707 in src/glob/GlobWalker.rs

View check run for this annotation

Claude / Claude Code Review

Absolute all-literal pattern with trailing '/' still yields a regular file

One more same-class sibling: the absolute all-literal fast path in `Iterator::init()` still yields a regular file for a trailing-`/` pattern. `new Glob("/abs/foo/").scanSync({onlyFiles:false})` where `/abs/foo` is a regular file — `A::open("/abs/foo/", O_DIRECTORY)` returns `ENOTDIR` and that arm sets `iter_state = Matched(path)` without consulting the final component's `trailing_sep`. The new "excludes files when the final component carries a trailing separator" test block has no absolute-path
Comment thread
robobun marked this conversation as resolved.
if matches {
if let Some(path) = self
.walker
Expand Down Expand Up @@ -1761,10 +1766,12 @@

// Handle case b)
if !is_last {
let next = next_pattern.unwrap();
return pattern.syntax_hint == SyntaxHint::Double
&& (component_idx + 1) as usize == self.pattern_components.len().saturating_sub(1)
&& next_pattern.unwrap().syntax_hint != SyntaxHint::Double
&& self.match_pattern_impl(next_pattern.unwrap(), entry_name);
&& next.syntax_hint != SyntaxHint::Double
&& !next.trailing_sep
&& self.match_pattern_impl(next, entry_name);
}

// Handle case a)
Expand Down Expand Up @@ -2081,9 +2088,24 @@
return None;
}

// The final component of a pattern may carry its trailing `/` inside
// `len`. Classify the syntax hint on the same slice `pattern_slice()`
// returns (i.e. without that separator) so `**/` is recognized as a
// globstar and recurses like `**` during scan().
let last_idx = (component.start + component.len - 1) as usize;
if pattern[last_idx] == b'/' {
component.trailing_sep = true;
} else {
#[cfg(windows)]
{
component.trailing_sep = pattern[last_idx] == b'\\';
}
}
let effective_len = component.len - u32::from(component.trailing_sep);
Comment thread
robobun marked this conversation as resolved.

'out: {
let comp_slice =
&pattern[component.start as usize..(component.start + component.len) as usize];
&pattern[component.start as usize..(component.start + effective_len) as usize];
if comp_slice == b"." {
component.syntax_hint = SyntaxHint::Dot;
break 'out;
Expand All @@ -2098,7 +2120,7 @@
break 'out;
}

match component.len {
match effective_len {
1 => {
if pattern[component.start as usize] == b'*' {
component.syntax_hint = SyntaxHint::Single;
Expand Down Expand Up @@ -2147,16 +2169,6 @@
}
}

let last_idx = (component.start + component.len).saturating_sub(1) as usize;
if pattern[last_idx] == b'/' {
component.trailing_sep = true;
} else {
#[cfg(windows)]
{
component.trailing_sep = pattern[last_idx] == b'\\';
}
}

Some(component)
}

Expand Down
78 changes: 78 additions & 0 deletions test/js/bun/glob/scan.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -637,6 +637,84 @@ describe("trailing directory separator", async () => {
const entries = await Array.fromAsync(glob.scan({ onlyFiles: false, cwd: tmpdir }));
expect(entries.sort()).toEqual(files.slice(2, 3).sort());
});

describe("globstar recurses into nested directories", () => {
const files = {
"d1/d2/d3/deep.txt": "x",
"d1/f.txt": "x",
"e1/g.txt": "x",
};
const j = (...p: string[]) => p.join(path.sep);
const scan = (dir: string, p: string) => [...new Glob(p).scanSync({ cwd: dir, onlyFiles: false })].sort();

test("**/ yields every directory at every depth and no files", () => {
using dir = tempDir("glob-trailing-globstar", files);
expect(scan(String(dir), "**/")).toEqual(["d1", j("d1", "d2"), j("d1", "d2", "d3"), "e1"]);
});

test("prefix/**/ yields every directory under prefix", () => {
using dir = tempDir("glob-trailing-globstar-prefix", files);
expect(scan(String(dir), "d1/**/")).toEqual([j("d1", "d2"), j("d1", "d2", "d3")]);
});

test("**/ and match() agree on nested directory paths", () => {
using dir = tempDir("glob-trailing-globstar-match", files);
const g = new Glob("d1/**/");
const entries = scan(String(dir), "d1/**/");
expect(entries).toEqual([j("d1", "d2"), j("d1", "d2", "d3")]);
for (const e of entries) {
expect(g.match(e.replaceAll(path.sep, "/") + "/")).toBe(true);
}
});

test("async scan agrees with sync", async () => {
using dir = tempDir("glob-trailing-globstar-async", files);
const entries = (await Array.fromAsync(new Glob("**/").scan({ cwd: String(dir), onlyFiles: false }))).sort();
expect(entries).toEqual(["d1", j("d1", "d2"), j("d1", "d2", "d3"), "e1"]);
});
});

describe("excludes files when the final component carries a trailing separator", () => {
const files = {
"foo": "x",
"file.txt": "x",
"sub/foo": "x",
"bar/placeholder": "x",
};
const j = (...p: string[]) => p.join(path.sep);
const scan = (dir: string, p: string) => [...new Glob(p).scanSync({ cwd: dir, onlyFiles: false })].sort();

test("**/foo/ does not yield a file named foo", () => {
using dir = tempDir("glob-trailing-sep-peek", files);
expect(scan(String(dir), "**/foo/")).toEqual([]);
});

test("**/*/ yields only directories", () => {
using dir = tempDir("glob-trailing-sep-star", files);
expect(scan(String(dir), "**/*/")).toEqual(["bar", "sub"]);
});

test("literal/ does not yield a file via the statat fast path", () => {
using dir = tempDir("glob-trailing-sep-literal", files);
expect(scan(String(dir), "foo/")).toEqual([]);
expect(scan(String(dir), "bar/")).toEqual(["bar"]);
});

test("prefix/literal/ does not yield a file", () => {
using dir = tempDir("glob-trailing-sep-prefix-literal", files);
expect(scan(String(dir), "sub/foo/")).toEqual([]);
expect(scan(String(dir), "*/foo/")).toEqual([]);
});

test("**/foo/ yields a directory named foo", () => {
using dir = tempDir("glob-trailing-sep-dir-named-foo", {
"foo/placeholder": "x",
"sub/foo/placeholder": "x",
"sub/bar": "x",
});
expect(scan(String(dir), "**/foo/")).toEqual(["foo", j("sub", "foo")]);
});
});
});

describe("absolute path pattern", async () => {
Expand Down
Loading