From 7b810f490fba1e98344f84454949ea2b2a424df3 Mon Sep 17 00:00:00 2001 From: leaysgur <6259812+leaysgur@users.noreply.github.com> Date: Mon, 22 Dec 2025 08:55:37 +0000 Subject: [PATCH] fix(oxfmt): Use correct root dir with ignore and overrides for nested cwd (#17244) In the `oxfmt`-related settings, paths are specified in the following: - `.editorconfig` `[src/*.json]` part - `.(prettier|git)ignore`, `--ignore-path`ed file - `.oxfmtrc` `.ignorePatterns` - CLI positional paths These paths are often written as relative paths, but path resolution is currently based on the `cwd`. This is generally not a problem in most common use cases, as follows. ``` - project/ # root == cwd - .editorconfig - .prettierignore - .oxfmtrc.json ``` However, `oxfmt` can also find `.oxfmtrc` files by traversing upward from nested subdirectories toward the parent directory. ``` - project/ # root - .editorconfig - .prettierignore - .oxfmtrc.json - nested/ # cwd ``` At this case, relative path resolution should be based on each individual setting file. --- apps/oxfmt/src/cli/format.rs | 1 + apps/oxfmt/src/cli/walk.rs | 93 ++++++++++++++----- apps/oxfmt/src/core/config.rs | 4 +- .../config_ignore_patterns.test.ts.snap | 17 ++++ .../__snapshots__/editorconfig.test.ts.snap | 15 +++ .../ignore_patterns.test.ts.snap | 2 +- .../oxfmt/test/config_ignore_patterns.test.ts | 23 +++++ apps/oxfmt/test/editorconfig.test.ts | 21 ++++- .../.oxfmtrc.json | 3 + .../generated/ignored.js | 2 + .../src/test.js | 2 + .../sub/generated/ignored.js | 2 + .../sub/src/test.js | 2 + .../editorconfig/nested_cwd/.editorconfig | 8 ++ .../editorconfig/nested_cwd/sub/test.ts | 5 + 15 files changed, 173 insertions(+), 27 deletions(-) create mode 100644 apps/oxfmt/test/fixtures/config_ignore_patterns_nested_cwd/.oxfmtrc.json create mode 100644 apps/oxfmt/test/fixtures/config_ignore_patterns_nested_cwd/generated/ignored.js create mode 100644 apps/oxfmt/test/fixtures/config_ignore_patterns_nested_cwd/src/test.js create mode 100644 apps/oxfmt/test/fixtures/config_ignore_patterns_nested_cwd/sub/generated/ignored.js create mode 100644 apps/oxfmt/test/fixtures/config_ignore_patterns_nested_cwd/sub/src/test.js create mode 100644 apps/oxfmt/test/fixtures/editorconfig/nested_cwd/.editorconfig create mode 100644 apps/oxfmt/test/fixtures/editorconfig/nested_cwd/sub/test.ts diff --git a/apps/oxfmt/src/cli/format.rs b/apps/oxfmt/src/cli/format.rs index 9e94803a5788d..fcb4b53a71dbb 100644 --- a/apps/oxfmt/src/cli/format.rs +++ b/apps/oxfmt/src/cli/format.rs @@ -120,6 +120,7 @@ impl FormatRunner { &paths, &ignore_options.ignore_path, ignore_options.with_node_modules, + oxfmtrc_path.as_deref(), &ignore_patterns, ) { Ok(Some(walker)) => walker, diff --git a/apps/oxfmt/src/cli/walk.rs b/apps/oxfmt/src/cli/walk.rs index 373a9a1961546..fbdca11a8abcc 100644 --- a/apps/oxfmt/src/cli/walk.rs +++ b/apps/oxfmt/src/cli/walk.rs @@ -3,7 +3,7 @@ use std::{ sync::mpsc, }; -use ignore::gitignore::GitignoreBuilder; +use ignore::gitignore::{Gitignore, GitignoreBuilder}; use crate::core::FormatFileStrategy; @@ -17,6 +17,7 @@ impl Walk { paths: &[PathBuf], ignore_paths: &[PathBuf], with_node_modules: bool, + oxfmtrc_path: Option<&Path>, ignore_patterns: &[String], ) -> Result, String> { // @@ -59,31 +60,65 @@ impl Walk { // // Build ignores // - let mut builder = GitignoreBuilder::new(cwd); - // Handle ignore files + // Use multiple matchers, each with correct root for pattern resolution: + // - Ignore files: root = parent directory of the ignore file + // - `.ignorePatterns`: root = parent directory of `.oxfmtrc.json` + // - Exclude paths (`!` prefix): root = cwd + let mut matchers: Vec = vec![]; + + // 1. Handle ignore files (`.gitignore`, `.prettierignore`, or `--ignore-path`) + // Patterns are relative to the ignore file location 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())); + if !ignore_path.exists() { + return Err(format!("{}: File not found", ignore_path.display())); } - } - // Handle `config.ignorePatterns` - for pattern in ignore_patterns { - if builder.add_line(None, pattern).is_err() { + let (gitignore, err) = Gitignore::new(ignore_path); + if let Some(err) = err { return Err(format!( - "Failed to add ignore pattern `{pattern}` from `.ignorePatterns`" + "Failed to parse ignore file {}: {err}", + ignore_path.display() )); } + matchers.push(gitignore); } - // 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")); + + // 2. Handle `oxfmtrc.ignorePatterns` + // Patterns are relative to the config file location + if !ignore_patterns.is_empty() + && let Some(oxfmtrc_path) = oxfmtrc_path + { + let mut builder = GitignoreBuilder::new( + oxfmtrc_path.parent().expect("`oxfmtrc_path` should have a parent directory"), + ); + for pattern in ignore_patterns { + if builder.add_line(None, pattern).is_err() { + return Err(format!( + "Failed to add ignore pattern `{pattern}` from `.ignorePatterns`" + )); + } + } + let gitignore = builder.build().map_err(|_| "Failed to build ignores".to_string())?; + matchers.push(gitignore); + } + + // 3. Handle `!` prefixed paths + // These are relative to cwd + if !exclude_patterns.is_empty() { + let mut builder = GitignoreBuilder::new(cwd); + 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 gitignore = builder.build().map_err(|_| "Failed to build ignores".to_string())?; + matchers.push(gitignore); } - let ignores = builder.build().map_err(|_| "Failed to build ignores".to_string())?; // // Filter paths by ignores @@ -92,10 +127,7 @@ impl Walk { // 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() - }) + .filter(|path| !is_ignored(&matchers, path, path.is_dir())) .collect(); // If no target paths remain after filtering, return `None`. @@ -137,8 +169,7 @@ impl Walk { } // Check ignore files, patterns - let matched = ignores.matched(entry.path(), is_dir); - if matched.is_ignore() && !matched.is_whitelist() { + if is_ignored(&matchers, entry.path(), is_dir) { return false; } @@ -180,6 +211,20 @@ impl Walk { } } +// --- + +/// Check if a path should be ignored by any of the matchers. +/// A path is ignored if any matcher says it's ignored (and not whitelisted in that same matcher). +fn is_ignored(matchers: &[Gitignore], path: &Path, is_dir: bool) -> bool { + for matcher in matchers { + let matched = matcher.matched(path, is_dir); + if matched.is_ignore() && !matched.is_whitelist() { + return true; + } + } + false +} + fn load_ignore_paths(cwd: &Path, ignore_paths: &[PathBuf]) -> Vec { // If specified, just resolves absolute paths if !ignore_paths.is_empty() { diff --git a/apps/oxfmt/src/core/config.rs b/apps/oxfmt/src/core/config.rs index 5aba33fa6991d..e17d5b1fe2640 100644 --- a/apps/oxfmt/src/core/config.rs +++ b/apps/oxfmt/src/core/config.rs @@ -124,7 +124,9 @@ impl ConfigResolver { let str = utils::read_to_string(path) .map_err(|_| format!("Failed to read {}: File not found", path.display()))?; - Some(EditorConfig::parse(&str).with_cwd(cwd)) + // Use the directory containing `.editorconfig` as the base, not the CLI's cwd. + // This ensures patterns like `[src/*.ts]` are resolved relative to where `.editorconfig` is located. + Some(EditorConfig::parse(&str).with_cwd(path.parent().unwrap_or(cwd))) } None => None, }; diff --git a/apps/oxfmt/test/__snapshots__/config_ignore_patterns.test.ts.snap b/apps/oxfmt/test/__snapshots__/config_ignore_patterns.test.ts.snap index e05989afd9f4b..556c5b05386b7 100644 --- a/apps/oxfmt/test/__snapshots__/config_ignore_patterns.test.ts.snap +++ b/apps/oxfmt/test/__snapshots__/config_ignore_patterns.test.ts.snap @@ -1,5 +1,22 @@ // Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html +exports[`config_ignore_patterns > nested cwd - ignorePatterns resolved relative to .oxfmtrc.json location 1`] = ` +"-------------------- +arguments: --check . +working directory: fixtures/config_ignore_patterns_nested_cwd/sub +exit code: 1 +--- STDOUT --------- +Checking formatting... + +src/test.js (ms) + +Format issues found in above 1 files. Run without \`--check\` to fix. +Finished in ms on 1 files using 1 threads. +--- STDERR --------- + +--------------------" +`; + exports[`config_ignore_patterns > should respect ignorePatterns in config 1`] = ` "-------------------- arguments: --check diff --git a/apps/oxfmt/test/__snapshots__/editorconfig.test.ts.snap b/apps/oxfmt/test/__snapshots__/editorconfig.test.ts.snap index 35a3493b0000d..b79e5d7a9fa3a 100644 --- a/apps/oxfmt/test/__snapshots__/editorconfig.test.ts.snap +++ b/apps/oxfmt/test/__snapshots__/editorconfig.test.ts.snap @@ -42,6 +42,21 @@ if (true) { --------------------" `; +exports[`editorconfig > nested cwd - patterns resolved relative to .editorconfig location 1`] = ` +"-------------------- +arguments: --check . +working directory: fixtures/editorconfig/nested_cwd/sub +exit code: 0 +--- STDOUT --------- +Checking formatting... + +All matched files use the correct format. +Finished in ms on 1 files using 1 threads. +--- STDERR --------- + +--------------------" +`; + exports[`editorconfig > no root [*] section 1`] = ` "--- FILE ----------- test.js diff --git a/apps/oxfmt/test/__snapshots__/ignore_patterns.test.ts.snap b/apps/oxfmt/test/__snapshots__/ignore_patterns.test.ts.snap index 899c0303086cb..bcb7aea536269 100644 --- a/apps/oxfmt/test/__snapshots__/ignore_patterns.test.ts.snap +++ b/apps/oxfmt/test/__snapshots__/ignore_patterns.test.ts.snap @@ -51,6 +51,6 @@ exit code: 1 --- STDERR --------- Failed to parse target paths or ignore settings. -Failed to add ignore file: /nonexistent.ignore +/nonexistent.ignore: File not found --------------------" `; diff --git a/apps/oxfmt/test/config_ignore_patterns.test.ts b/apps/oxfmt/test/config_ignore_patterns.test.ts index d193987adf430..5f796561922b3 100644 --- a/apps/oxfmt/test/config_ignore_patterns.test.ts +++ b/apps/oxfmt/test/config_ignore_patterns.test.ts @@ -12,4 +12,27 @@ describe("config_ignore_patterns", () => { const snapshot = await runAndSnapshot(cwd, testCases); expect(snapshot).toMatchSnapshot(); }); + + // Structure: + // config_ignore_patterns_nested_cwd/ + // .oxfmtrc.json # ignorePatterns: ["sub/generated/**"] + // sub/ # <- cwd (nested directory) + // generated/ + // ignored.js # Should be ignored + // src/ + // test.js # Should be formatted + // + // When running from `sub/` directory with target `.`: + // - .oxfmtrc.json is found in parent directory + // - ignorePatterns should resolve relative to .oxfmtrc.json location + // - Pattern "sub/generated/**" should match "sub/generated/ignored.js" + // relative to .oxfmtrc.json location, NOT "sub/sub/generated/**" from cwd + // + // Expected: generated/ignored.js should be ignored, src/test.js should be listed + it("nested cwd - ignorePatterns resolved relative to .oxfmtrc.json location", async () => { + // Run from nested `sub/` directory - check mode doesn't need temp copy + const nestedCwd = join(fixturesDir, "config_ignore_patterns_nested_cwd", "sub"); + const snapshot = await runAndSnapshot(nestedCwd, [["--check", "."]]); + expect(snapshot).toMatchSnapshot(); + }); }); diff --git a/apps/oxfmt/test/editorconfig.test.ts b/apps/oxfmt/test/editorconfig.test.ts index b70ffd241af3a..c6df317824ca1 100644 --- a/apps/oxfmt/test/editorconfig.test.ts +++ b/apps/oxfmt/test/editorconfig.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from "vitest"; import { join } from "node:path"; -import { runWriteModeAndSnapshot } from "./utils"; +import { runAndSnapshot, runWriteModeAndSnapshot } from "./utils"; const fixturesDir = join(__dirname, "fixtures", "editorconfig"); @@ -87,4 +87,23 @@ describe("editorconfig", () => { const snapshot = await runWriteModeAndSnapshot(cwd, ["test.js"]); expect(snapshot).toMatchSnapshot(); }); + + // Structure: + // nested_cwd/ + // .editorconfig # [*] indent_size=2, [sub/*.ts] indent_size=8 + // sub/ # <- cwd (nested directory) + // test.ts # Pre-formatted with 8-space indentation + // + // When running from `sub/` directory: + // - .editorconfig is found in parent directory + // - [sub/*.ts] pattern should match `sub/test.ts` relative to .editorconfig location + // - NOT relative to cwd (sub/) - which would be `sub/sub/*.ts` + // + // Expected: --check should pass (exit 0) because file is already formatted with indent_size=8 + // If pattern resolution was wrong, it would use indent_size=2 and fail + it("nested cwd - patterns resolved relative to .editorconfig location", async () => { + const nestedCwd = join(fixturesDir, "nested_cwd", "sub"); + const snapshot = await runAndSnapshot(nestedCwd, [["--check", "."]]); + expect(snapshot).toMatchSnapshot(); + }); }); diff --git a/apps/oxfmt/test/fixtures/config_ignore_patterns_nested_cwd/.oxfmtrc.json b/apps/oxfmt/test/fixtures/config_ignore_patterns_nested_cwd/.oxfmtrc.json new file mode 100644 index 0000000000000..751122ed6e87d --- /dev/null +++ b/apps/oxfmt/test/fixtures/config_ignore_patterns_nested_cwd/.oxfmtrc.json @@ -0,0 +1,3 @@ +{ + "ignorePatterns": ["sub/generated/**"] +} diff --git a/apps/oxfmt/test/fixtures/config_ignore_patterns_nested_cwd/generated/ignored.js b/apps/oxfmt/test/fixtures/config_ignore_patterns_nested_cwd/generated/ignored.js new file mode 100644 index 0000000000000..6dd369d986dfb --- /dev/null +++ b/apps/oxfmt/test/fixtures/config_ignore_patterns_nested_cwd/generated/ignored.js @@ -0,0 +1,2 @@ +// This file should be ignored +const x=1 diff --git a/apps/oxfmt/test/fixtures/config_ignore_patterns_nested_cwd/src/test.js b/apps/oxfmt/test/fixtures/config_ignore_patterns_nested_cwd/src/test.js new file mode 100644 index 0000000000000..3bb2c9c5fd491 --- /dev/null +++ b/apps/oxfmt/test/fixtures/config_ignore_patterns_nested_cwd/src/test.js @@ -0,0 +1,2 @@ +// This file should be formatted +const x=1 diff --git a/apps/oxfmt/test/fixtures/config_ignore_patterns_nested_cwd/sub/generated/ignored.js b/apps/oxfmt/test/fixtures/config_ignore_patterns_nested_cwd/sub/generated/ignored.js new file mode 100644 index 0000000000000..6dd369d986dfb --- /dev/null +++ b/apps/oxfmt/test/fixtures/config_ignore_patterns_nested_cwd/sub/generated/ignored.js @@ -0,0 +1,2 @@ +// This file should be ignored +const x=1 diff --git a/apps/oxfmt/test/fixtures/config_ignore_patterns_nested_cwd/sub/src/test.js b/apps/oxfmt/test/fixtures/config_ignore_patterns_nested_cwd/sub/src/test.js new file mode 100644 index 0000000000000..3bb2c9c5fd491 --- /dev/null +++ b/apps/oxfmt/test/fixtures/config_ignore_patterns_nested_cwd/sub/src/test.js @@ -0,0 +1,2 @@ +// This file should be formatted +const x=1 diff --git a/apps/oxfmt/test/fixtures/editorconfig/nested_cwd/.editorconfig b/apps/oxfmt/test/fixtures/editorconfig/nested_cwd/.editorconfig new file mode 100644 index 0000000000000..a96601c66ba48 --- /dev/null +++ b/apps/oxfmt/test/fixtures/editorconfig/nested_cwd/.editorconfig @@ -0,0 +1,8 @@ +root = true + +[*] +indent_style = space +indent_size = 2 + +[sub/*.ts] +indent_size = 8 diff --git a/apps/oxfmt/test/fixtures/editorconfig/nested_cwd/sub/test.ts b/apps/oxfmt/test/fixtures/editorconfig/nested_cwd/sub/test.ts new file mode 100644 index 0000000000000..f0a2a4b816ced --- /dev/null +++ b/apps/oxfmt/test/fixtures/editorconfig/nested_cwd/sub/test.ts @@ -0,0 +1,5 @@ +function foo() { + if (true) { + console.log("should be indented with 8 spaces"); + } +}