fix(oxfmt): Always respect ignored files even specified (#16632)

Fixes #16621

Author: leaysgur

apps/oxfmt/src/cli/format.rs

@@ -109,7 +109,16 @@ impl FormatRunner {
             ignore_options.with_node_modules,
             &ignore_patterns,
         ) {
-            Ok(walker) => walker,
+            Ok(Some(walker)) => walker,
+            // All target paths are ignored
+            Ok(None) => {
+                if misc_options.no_error_on_unmatched_pattern {
+                    print_and_flush(stderr, "No files found matching the given patterns.\n");
+                    return CliRunResult::None;
+                }
+                print_and_flush(stderr, "Expected at least one target file\n");
+                return CliRunResult::NoFilesFound;
+            }
             Err(err) => {
                 print_and_flush(
                     stderr,

apps/oxfmt/src/cli/walk.rs

@@ -3,7 +3,7 @@ use std::{
     sync::mpsc,
 };
 
-use ignore::{gitignore::GitignoreBuilder, overrides::OverrideBuilder};
+use ignore::gitignore::GitignoreBuilder;
 
 use crate::core::FormatFileSource;
 
@@ -18,38 +18,49 @@ impl Walk {
         ignore_paths: &[PathBuf],
         with_node_modules: bool,
         ignore_patterns: &[String],
-    ) -> Result<Self, String> {
-        let (target_paths, exclude_patterns) = normalize_paths(cwd, paths);
-
-        // Add all non-`!` prefixed paths to the walker base
-        let mut inner = ignore::WalkBuilder::new(
-            target_paths
-                .first()
-                .expect("`target_paths` never be empty, should have at least `cwd`"),
-        );
-        if let Some(paths) = target_paths.get(1..) {
-            for path in paths {
-                inner.add(path);
+    ) -> Result<Option<Self>, String> {
+        //
+        // Classify and normalize specified paths
+        //
+        let mut target_paths = vec![];
+        let mut exclude_patterns = vec![];
+        for path in paths {
+            let path_str = path.to_string_lossy();
+
+            // Instead of `oxlint`'s `--ignore-pattern=PAT`,
+            // `oxfmt` supports `!` prefix in paths like Prettier.
+            if path_str.starts_with('!') {
+                exclude_patterns.push(path_str.to_string());
+                continue;
             }
-        }
 
-        // NOTE: We are using `OverrideBuilder` only for exclusion.
-        // This means there is no way to "re-include" a file once ignored.
+            // Otherwise, treat as target path
 
-        // Treat all `!` prefixed patterns as overrides to exclude
-        if !exclude_patterns.is_empty() {
-            let mut builder = OverrideBuilder::new(cwd);
-            for pattern_str in exclude_patterns {
-                builder
-                    .add(&pattern_str)
-                    .map_err(|_| format!("{pattern_str} is not a valid glob for override."))?;
+            if path.is_absolute() {
+                target_paths.push(path.clone());
+                continue;
             }
-            let overrides = builder.build().map_err(|_| "Failed to build overrides".to_string())?;
-            inner.overrides(overrides);
+
+            // NOTE: `.` and cwd behave differently, need to normalize
+            let path = if path_str == "." {
+                cwd.to_path_buf()
+            } else if let Some(stripped) = path_str.strip_prefix("./") {
+                cwd.join(stripped)
+            } else {
+                cwd.join(path)
+            };
+            target_paths.push(path);
+        }
+        // Default to cwd if no target paths are provided
+        if target_paths.is_empty() {
+            target_paths.push(cwd.to_path_buf());
         }
 
-        // Handle ignore files
+        //
+        // Build ignores
+        //
         let mut builder = GitignoreBuilder::new(cwd);
+        // Handle ignore files
         for ignore_path in &load_ignore_paths(cwd, ignore_paths) {
             if builder.add(ignore_path).is_some() {
                 return Err(format!("Failed to add ignore file: {}", ignore_path.display()));
@@ -58,11 +69,47 @@ impl Walk {
         // Handle `config.ignorePatterns`
         for pattern in ignore_patterns {
             if builder.add_line(None, pattern).is_err() {
-                return Err(format!("Failed to add ignore pattern `{pattern}`"));
+                return Err(format!(
+                    "Failed to add ignore pattern `{pattern}` from `.ignorePatterns`"
+                ));
+            }
+        }
+        // Handle `!` prefixed paths as ignore patterns too
+        for pattern in &exclude_patterns {
+            // Remove the leading `!` because `GitignoreBuilder` uses `!` as negation
+            let pattern =
+                pattern.strip_prefix('!').expect("There should be a `!` prefix, already checked");
+            if builder.add_line(None, pattern).is_err() {
+                return Err(format!("Failed to add ignore pattern `{pattern}` from `!` prefix"));
             }
         }
         let ignores = builder.build().map_err(|_| "Failed to build ignores".to_string())?;
 
+        //
+        // Filter paths by ignores
+        //
+        // NOTE: Base paths passed to `WalkBuilder` are not filtered by `filter_entry()`,
+        // so we need to filter them here before passing to the walker.
+        let target_paths: Vec<_> = target_paths
+            .into_iter()
+            .filter(|path| {
+                let matched = ignores.matched(path, path.is_dir());
+                !matched.is_ignore() || matched.is_whitelist()
+            })
+            .collect();
+
+        // If no target paths remain after filtering, return `None`.
+        // Not an error, but nothing to format, leave it to the caller how to handle.
+        let Some(first_path) = target_paths.first() else {
+            return Ok(None);
+        };
+
+        // Add all non-`!` prefixed paths to the walker base
+        let mut inner = ignore::WalkBuilder::new(first_path);
+        for path in target_paths.iter().skip(1) {
+            inner.add(path);
+        }
+
         // NOTE: If return `false` here, it will not be `visit()`ed at all
         inner.filter_entry(move |entry| {
             let Some(file_type) = entry.file_type() else {
@@ -115,7 +162,7 @@ impl Walk {
             .git_ignore(false)
             .git_exclude(false)
             .build_parallel();
-        Ok(Self { inner })
+        Ok(Some(Self { inner }))
     }
 
     /// Stream entries through a channel as they are discovered
@@ -133,49 +180,6 @@ impl Walk {
     }
 }
 
-/// Normalize user input paths into `target_paths` and `exclude_patterns`.
-/// - `target_paths`: Absolute paths to format
-/// - `exclude_patterns`: Pattern strings to exclude (with `!` prefix)
-fn normalize_paths(cwd: &Path, input_paths: &[PathBuf]) -> (Vec<PathBuf>, Vec<String>) {
-    let mut target_paths = vec![];
-    let mut exclude_patterns = vec![];
-
-    for path in input_paths {
-        let path_str = path.to_string_lossy();
-
-        // Instead of `oxlint`'s `--ignore-pattern=PAT`,
-        // `oxfmt` supports `!` prefix in paths like Prettier.
-        if path_str.starts_with('!') {
-            exclude_patterns.push(path_str.to_string());
-            continue;
-        }
-
-        // Otherwise, treat as target path
-
-        if path.is_absolute() {
-            target_paths.push(path.clone());
-            continue;
-        }
-
-        // NOTE: `.` and cwd behaves differently, need to normalize
-        let path = if path_str == "." {
-            cwd.to_path_buf()
-        } else if let Some(stripped) = path_str.strip_prefix("./") {
-            cwd.join(stripped)
-        } else {
-            cwd.join(path)
-        };
-        target_paths.push(path);
-    }
-
-    // Default to cwd if no `target_paths` are provided
-    if target_paths.is_empty() {
-        target_paths.push(cwd.into());
-    }
-
-    (target_paths, exclude_patterns)
-}
-
 fn load_ignore_paths(cwd: &Path, ignore_paths: &[PathBuf]) -> Vec<PathBuf> {
     // If specified, just resolves absolute paths
     if !ignore_paths.is_empty() {
@@ -187,12 +191,12 @@ fn load_ignore_paths(cwd: &Path, ignore_paths: &[PathBuf]) -> Vec<PathBuf> {
 
     // Else, search for default ignore files in cwd
     [".gitignore", ".prettierignore"]
-        .iter()
+        .into_iter()
         .filter_map(|file_name| {
             let path = cwd.join(file_name);
-            if path.exists() { Some(path) } else { None }
+            path.exists().then_some(path)
         })
-        .collect::<Vec<_>>()
+        .collect()
 }
 
 // ---

apps/oxfmt/test/__snapshots__/ignore_and_override.test.ts.snap

@@ -30,6 +30,24 @@ arguments: --check --ignore-path ignore1 should_format/ok.js
 working directory: fixtures/ignore_and_override
 exit code: 1
 --- STDOUT ---------
+
+--- STDERR ---------
+Expected at least one target file
+--------------------
+--------------------
+arguments: --check --ignore-path ignore1 should_format/ok.js --no-error-on-unmatched-pattern
+working directory: fixtures/ignore_and_override
+exit code: 0
+--- STDOUT ---------
+
+--- STDERR ---------
+No files found matching the given patterns.
+--------------------
+--------------------
+arguments: --check --ignore-path ignore2
+working directory: fixtures/ignore_and_override
+exit code: 1
+--- STDOUT ---------
 Checking formatting...
 
 should_format/ok.js (<variable>ms)
@@ -40,7 +58,7 @@ Finished in <variable>ms on 1 files using 1 threads.
 
 --------------------
 --------------------
-arguments: --check --ignore-path ignore2
+arguments: --check --ignore-path ignore2 should_format/ok.js
 working directory: fixtures/ignore_and_override
 exit code: 1
 --- STDOUT ---------

apps/oxfmt/test/ignore_and_override.test.ts

@@ -8,10 +8,24 @@ describe("ignore_and_override", () => {
   it("should handle ignore files with overrides", async () => {
     const cwd = join(fixturesDir, "ignore_and_override");
     const testCases = [
+      // Exclude `err.js` via `!` pattern, format `ok.js` only
       ["--check", "!**/err.js"],
+      // `ignore1` excludes all files -> no files found error
       ["--check", "--ignore-path", "ignore1"],
+      // Explicitly specified file is also excluded by `ignore1` -> no files found error
       ["--check", "--ignore-path", "ignore1", "should_format/ok.js"],
+      // Same as above, but suppress error with `--no-error-on-unmatched-pattern`
+      [
+        "--check",
+        "--ignore-path",
+        "ignore1",
+        "should_format/ok.js",
+        "--no-error-on-unmatched-pattern",
+      ],
+      // `ignore2` has `!should_format/ok.js` (whitelist), so `ok.js` is formatted
       ["--check", "--ignore-path", "ignore2"],
+      // Whitelist + explicit file: `ok.js` is whitelisted in `ignore2` and explicitly specified
+      ["--check", "--ignore-path", "ignore2", "should_format/ok.js"],
     ];
 
     const snapshot = await runAndSnapshot(cwd, testCases);