diff --git a/apps/oxfmt/src/cli/walk.rs b/apps/oxfmt/src/cli/walk.rs index 505ba3b4ce6fb..b0281341a19d9 100644 --- a/apps/oxfmt/src/cli/walk.rs +++ b/apps/oxfmt/src/cli/walk.rs @@ -64,9 +64,11 @@ impl Walk { // - Ignore files: root = parent directory of the ignore file // - `.ignorePatterns`: root = parent directory of `.oxfmtrc.json` // - Exclude paths (`!` prefix): root = cwd + // + // NOTE: Git ignore files are handled by `WalkBuilder` itself let mut matchers: Vec = vec![]; - // 1. Handle ignore files (`.gitignore`, `.prettierignore`, or `--ignore-path`) + // 1. Handle formatter ignore files (`.prettierignore`, or `--ignore-path`) // Patterns are relative to the ignore file location for ignore_path in &load_ignore_paths(cwd, ignore_paths)? { let (gitignore, err) = Gitignore::new(ignore_path); @@ -118,10 +120,12 @@ impl Walk { } // - // Filter paths by ignores + // Filter paths by formatter 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. + // This is needed for cases like `husky`, may specify ignored paths as staged files. + // NOTE: Git ignored paths are not filtered here. let target_paths: Vec<_> = target_paths .into_iter() .filter(|path| !is_ignored(&matchers, path, path.is_dir())) @@ -183,12 +187,20 @@ impl Walk { .follow_links(false) // Include hidden files and directories except those we explicitly skip above .hidden(false) - // Do not respect `.gitignore` automatically, we handle it manually + // Do not respect `.ignore` file .ignore(false) + // Do not search upward + // NOTE: Prettier only searches current working directory .parents(false) + // Also do not respect globals .git_global(false) - .git_ignore(false) + // But respect downward nested `.gitignore` files + // NOTE: Prettier does not: https://github.com/prettier/prettier/issues/4081 + .git_ignore(true) + // Also do not respect `.git/info/exclude` .git_exclude(false) + // Git is not required + .require_git(false) .build_parallel(); Ok(Some(Self { inner })) } @@ -238,8 +250,7 @@ fn load_ignore_paths(cwd: &Path, ignore_paths: &[PathBuf]) -> Result should respect .gitignore in subdirectory 1`] = ` +"-------------------- +arguments: --check +working directory: fixtures/gitignore +exit code: 1 +--- STDOUT --------- +Checking formatting... + +another/another.js (ms) +root.js (ms) +subdir/sub.js (ms) + +Format issues found in above 3 files. Run without \`--check\` to fix. +Finished in ms on 3 files using 1 threads. +--- STDERR --------- + +--------------------" +`; + +exports[`gitignore > should respect root .gitignore 1`] = ` +"-------------------- +arguments: --check +working directory: fixtures/gitignore +exit code: 1 +--- STDOUT --------- +Checking formatting... + +another/another.js (ms) +root.js (ms) + +Format issues found in above 2 files. Run without \`--check\` to fix. +Finished in ms on 2 files using 1 threads. +--- STDERR --------- + +--------------------" +`; diff --git a/apps/oxfmt/test/fixtures/gitignore/another/another.js b/apps/oxfmt/test/fixtures/gitignore/another/another.js new file mode 100644 index 0000000000000..6f5029b0a5922 --- /dev/null +++ b/apps/oxfmt/test/fixtures/gitignore/another/another.js @@ -0,0 +1 @@ +const v=5 diff --git a/apps/oxfmt/test/fixtures/gitignore/ignored-by-root.js b/apps/oxfmt/test/fixtures/gitignore/ignored-by-root.js new file mode 100644 index 0000000000000..47e60c685397e --- /dev/null +++ b/apps/oxfmt/test/fixtures/gitignore/ignored-by-root.js @@ -0,0 +1 @@ +const y=2 diff --git a/apps/oxfmt/test/fixtures/gitignore/root.js b/apps/oxfmt/test/fixtures/gitignore/root.js new file mode 100644 index 0000000000000..a989d9a9b0ea3 --- /dev/null +++ b/apps/oxfmt/test/fixtures/gitignore/root.js @@ -0,0 +1 @@ +const x=1 diff --git a/apps/oxfmt/test/fixtures/gitignore/subdir/ignored-by-subdir.js b/apps/oxfmt/test/fixtures/gitignore/subdir/ignored-by-subdir.js new file mode 100644 index 0000000000000..26ee85d02a481 --- /dev/null +++ b/apps/oxfmt/test/fixtures/gitignore/subdir/ignored-by-subdir.js @@ -0,0 +1 @@ +const w=4 diff --git a/apps/oxfmt/test/fixtures/gitignore/subdir/sub.js b/apps/oxfmt/test/fixtures/gitignore/subdir/sub.js new file mode 100644 index 0000000000000..831e9862b6aa7 --- /dev/null +++ b/apps/oxfmt/test/fixtures/gitignore/subdir/sub.js @@ -0,0 +1 @@ +const z=3 diff --git a/apps/oxfmt/test/gitignore.test.ts b/apps/oxfmt/test/gitignore.test.ts new file mode 100644 index 0000000000000..5200e315c3fd4 --- /dev/null +++ b/apps/oxfmt/test/gitignore.test.ts @@ -0,0 +1,39 @@ +import { describe, expect, it } from "vitest"; +import { join } from "node:path"; +import { writeFile, rm } from "node:fs/promises"; +import { runAndSnapshot } from "./utils"; + +const fixturesDir = join(__dirname, "fixtures"); + +// NOTE: These tests modify `.gitignore` files in the fixtures directory. +// If we commit `.gitignore`, required test fixtures are also ignored by git! + +describe("gitignore", () => { + const cwd = join(fixturesDir, "gitignore"); + const rootGitignore = join(cwd, ".gitignore"); + + it("should respect root .gitignore", async () => { + try { + await writeFile(rootGitignore, ["ignored-by-root.js", "subdir/"].join("\n")); + + const snapshot = await runAndSnapshot(cwd, [["--check"]]); + expect(snapshot).toMatchSnapshot(); + } finally { + await rm(rootGitignore, { force: true }); + } + }); + + it("should respect .gitignore in subdirectory", async () => { + const subdirGitignore = join(cwd, "subdir", ".gitignore"); + try { + await writeFile(rootGitignore, ["ignored-by-root.js"].join("\n")); + await writeFile(subdirGitignore, "ignored-by-subdir.js\n"); + + const snapshot = await runAndSnapshot(cwd, [["--check"]]); + expect(snapshot).toMatchSnapshot(); + } finally { + await rm(rootGitignore, { force: true }); + await rm(subdirGitignore, { force: true }); + } + }); +});