Skip to content
Merged
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
1 change: 1 addition & 0 deletions apps/oxfmt/src/cli/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
93 changes: 69 additions & 24 deletions apps/oxfmt/src/cli/walk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::{
sync::mpsc,
};

use ignore::gitignore::GitignoreBuilder;
use ignore::gitignore::{Gitignore, GitignoreBuilder};

use crate::core::FormatFileStrategy;

Expand All @@ -17,6 +17,7 @@ impl Walk {
paths: &[PathBuf],
ignore_paths: &[PathBuf],
with_node_modules: bool,
oxfmtrc_path: Option<&Path>,
ignore_patterns: &[String],
) -> Result<Option<Self>, String> {
//
Expand Down Expand Up @@ -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<Gitignore> = 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
Expand All @@ -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`.
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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<PathBuf> {
// If specified, just resolves absolute paths
if !ignore_paths.is_empty() {
Expand Down
4 changes: 3 additions & 1 deletion apps/oxfmt/src/core/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand Down
17 changes: 17 additions & 0 deletions apps/oxfmt/test/__snapshots__/config_ignore_patterns.test.ts.snap
Original file line number Diff line number Diff line change
@@ -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 (<variable>ms)

Format issues found in above 1 files. Run without \`--check\` to fix.
Finished in <variable>ms on 1 files using 1 threads.
--- STDERR ---------

--------------------"
`;

exports[`config_ignore_patterns > should respect ignorePatterns in config 1`] = `
"--------------------
arguments: --check
Expand Down
15 changes: 15 additions & 0 deletions apps/oxfmt/test/__snapshots__/editorconfig.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -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 <variable>ms on 1 files using 1 threads.
--- STDERR ---------

--------------------"
`;

exports[`editorconfig > no root [*] section 1`] = `
"--- FILE -----------
test.js
Expand Down
2 changes: 1 addition & 1 deletion apps/oxfmt/test/__snapshots__/ignore_patterns.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,6 @@ exit code: 1

--- STDERR ---------
Failed to parse target paths or ignore settings.
Failed to add ignore file: <cwd>/nonexistent.ignore
<cwd>/nonexistent.ignore: File not found
--------------------"
`;
23 changes: 23 additions & 0 deletions apps/oxfmt/test/config_ignore_patterns.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
});
21 changes: 20 additions & 1 deletion apps/oxfmt/test/editorconfig.test.ts
Original file line number Diff line number Diff line change
@@ -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");

Expand Down Expand Up @@ -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();
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"ignorePatterns": ["sub/generated/**"]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
// This file should be ignored
const x=1
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
// This file should be formatted
const x=1
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
// This file should be ignored
const x=1
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
// This file should be formatted
const x=1
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
root = true

[*]
indent_style = space
indent_size = 2

[sub/*.ts]
indent_size = 8
5 changes: 5 additions & 0 deletions apps/oxfmt/test/fixtures/editorconfig/nested_cwd/sub/test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
function foo() {
if (true) {
console.log("should be indented with 8 spaces");
}
}
Loading