fix(oxfmt): Apply is_ignored_dir for glob paths too

apps/oxfmt/src/cli/walk.rs

@@ -1,4 +1,5 @@
 use std::{
+    ffi::OsStr,
     path::{Path, PathBuf},
     sync::{Mutex, mpsc},
 };
@@ -66,7 +67,7 @@ impl Walk {
         // Expand glob patterns and add to target paths
         // NOTE: See `expand_glob_patterns()` for why we pre-expand globs here
         if !glob_patterns.is_empty() {
-            target_paths.extend(expand_glob_patterns(cwd, &glob_patterns)?);
+            target_paths.extend(expand_glob_patterns(cwd, &glob_patterns, with_node_modules)?);
         }
 
         // Default to `cwd` if no positive paths were specified.
@@ -174,16 +175,7 @@ impl Walk {
                 // it means we want to include hidden files and directories.
                 // However, we (and also Prettier) still skip traversing certain directories.
                 // https://prettier.io/docs/ignore#ignoring-files-prettierignore
-                let is_ignored_dir = {
-                    let dir_name = entry.file_name();
-                    dir_name == ".git"
-                        || dir_name == ".jj"
-                        || dir_name == ".sl"
-                        || dir_name == ".svn"
-                        || dir_name == ".hg"
-                        || (!with_node_modules && dir_name == "node_modules")
-                };
-                if is_ignored_dir {
+                if is_ignored_dir(entry.file_name(), with_node_modules) {
                     return false;
                 }
             }
@@ -246,6 +238,17 @@ fn is_ignored(matchers: &[Gitignore], path: &Path, is_dir: bool, check_ancestors
     false
 }
 
+/// Check if a directory should be skipped during walking.
+/// VCS internal directories are always skipped, and `node_modules` is skipped by default.
+fn is_ignored_dir(dir_name: &OsStr, with_node_modules: bool) -> bool {
+    dir_name == ".git"
+        || dir_name == ".jj"
+        || dir_name == ".sl"
+        || dir_name == ".svn"
+        || dir_name == ".hg"
+        || (!with_node_modules && dir_name == "node_modules")
+}
+
 /// Check if a path string looks like a glob pattern.
 /// Glob-like characters are also valid path characters on some environments.
 /// If the path actually exists on disk, it is treated as a concrete path.
@@ -270,7 +273,11 @@ fn is_glob_pattern(s: &str, cwd: &Path) -> bool {
 // `ignore::Overrides` have the highest priority in the `ignore` crate,
 // so files matching the glob would be collected even if they're in `.gitignore`!
 /// Expand glob patterns to concrete file paths.
-fn expand_glob_patterns(cwd: &Path, patterns: &[String]) -> Result<Vec<PathBuf>, String> {
+fn expand_glob_patterns(
+    cwd: &Path,
+    patterns: &[String],
+    with_node_modules: bool,
+) -> Result<Vec<PathBuf>, String> {
     let mut ob = OverrideBuilder::new(cwd);
     for pattern in patterns {
         ob.add(pattern).map_err(|e| format!("Invalid glob pattern `{pattern}`: {e}"))?;
@@ -279,6 +286,11 @@ fn expand_glob_patterns(cwd: &Path, patterns: &[String]) -> Result<Vec<PathBuf>,
 
     let mut builder = ignore::WalkBuilder::new(cwd);
     builder.overrides(overrides);
+    // Skip ignored directories to align with the main walk behavior.
+    builder.filter_entry(move |entry| {
+        !(entry.file_type().is_some_and(|ft| ft.is_dir())
+            && is_ignored_dir(entry.file_name(), with_node_modules))
+    });
 
     let paths = Mutex::new(vec![]);
     apply_walk_settings(&mut builder).build_parallel().run(|| {

apps/oxfmt/test/cli/node_modules_dirs/__snapshots__/node_modules_dirs.test.ts.snap

@@ -34,3 +34,38 @@ Finished in <variable>ms on 4 files using 1 threads.
 
 --------------------"
 `;
+
+exports[`node_modules_dirs > should ignore node_modules when expanding globs but include with flag 1`] = `
+"--------------------
+arguments: --check **/*.js
+working directory: node_modules_dirs/fixtures
+exit code: 1
+--- STDOUT ---------
+Checking formatting...
+
+regular_dir/test.js (<variable>ms)
+root.js (<variable>ms)
+
+Format issues found in above 2 files. Run without \`--check\` to fix.
+Finished in <variable>ms on 2 files using 1 threads.
+--- STDERR ---------
+
+--------------------
+--------------------
+arguments: --check --with-node-modules **/*.js
+working directory: node_modules_dirs/fixtures
+exit code: 1
+--- STDOUT ---------
+Checking formatting...
+
+node_modules/test.js (<variable>ms)
+regular_dir/node_modules/test.js (<variable>ms)
+regular_dir/test.js (<variable>ms)
+root.js (<variable>ms)
+
+Format issues found in above 4 files. Run without \`--check\` to fix.
+Finished in <variable>ms on 4 files using 1 threads.
+--- STDERR ---------
+
+--------------------"
+`;

apps/oxfmt/test/cli/node_modules_dirs/node_modules_dirs.test.ts

@@ -11,4 +11,14 @@ describe("node_modules_dirs", () => {
     const snapshot = await runAndSnapshot(fixturesDir, testCases);
     expect(snapshot).toMatchSnapshot();
   });
+
+  it("should ignore node_modules when expanding globs but include with flag", async () => {
+    const testCases = [
+      ["--check", "**/*.js"],
+      ["--check", "--with-node-modules", "**/*.js"],
+    ];
+
+    const snapshot = await runAndSnapshot(fixturesDir, testCases);
+    expect(snapshot).toMatchSnapshot();
+  });
 });

apps/oxfmt/test/cli/vcs_dirs/__snapshots__/vcs_dirs.test.ts.snap

@@ -17,3 +17,21 @@ Finished in <variable>ms on 2 files using 1 threads.
 
 --------------------"
 `;
+
+exports[`vcs_dirs > should ignore VCS directories when expanding globs 1`] = `
+"--------------------
+arguments: --check **/*.js
+working directory: vcs_dirs/fixtures
+exit code: 1
+--- STDOUT ---------
+Checking formatting...
+
+regular_dir/test.js (<variable>ms)
+root.js (<variable>ms)
+
+Format issues found in above 2 files. Run without \`--check\` to fix.
+Finished in <variable>ms on 2 files using 1 threads.
+--- STDERR ---------
+
+--------------------"
+`;

apps/oxfmt/test/cli/vcs_dirs/vcs_dirs.test.ts

@@ -11,4 +11,11 @@ describe("vcs_dirs", () => {
     const snapshot = await runAndSnapshot(fixturesDir, testCases);
     expect(snapshot).toMatchSnapshot();
   });
+
+  it("should ignore VCS directories when expanding globs", async () => {
+    const testCases = [["--check", "**/*.js"]];
+
+    const snapshot = await runAndSnapshot(fixturesDir, testCases);
+    expect(snapshot).toMatchSnapshot();
+  });
 });