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 Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions apps/oxfmt/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ json-strip-comments = { workspace = true }
miette = { workspace = true }
phf = { workspace = true, features = ["macros"] }
rayon = { workspace = true }
rustc-hash = { workspace = true }
schemars = { workspace = true }
serde = { workspace = true }
serde_json = { workspace = true }
Expand Down
6 changes: 4 additions & 2 deletions apps/oxfmt/src/cli/command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,11 @@ pub struct FormatCommand {
pub ignore_options: IgnoreOptions,
#[bpaf(external)]
pub runtime_options: RuntimeOptions,
/// Single file, single path or list of paths.
/// Single file, path or list of paths.
/// Glob patterns are also supported.
/// (Be sure to quote them, otherwise your shell may expand them before passing.)
/// Exclude patterns with `!` prefix like `'!**/fixtures/*.js'` are also supported.
/// If not provided, current working directory is used.
/// Glob is supported only for exclude patterns like `'!**/fixtures/*.js'`.
// `bpaf(fallback)` seems to have issues with `many` or `positional`,
// so we implement the fallback behavior in code instead.
#[bpaf(positional("PATH"), many, guard(validate_paths, PATHS_ERROR_MESSAGE))]
Expand Down
138 changes: 103 additions & 35 deletions apps/oxfmt/src/cli/walk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,11 @@ use std::{
sync::mpsc,
};

use ignore::gitignore::{Gitignore, GitignoreBuilder};
use ignore::{
gitignore::{Gitignore, GitignoreBuilder},
overrides::OverrideBuilder,
};
use rustc_hash::FxHashSet;

use crate::core::{FormatFileStrategy, utils::normalize_relative_path};

Expand All @@ -23,8 +27,10 @@ impl Walk {
//
// Classify and normalize specified paths
//
let mut target_paths = vec![];
let mut target_paths = FxHashSet::default();
let mut glob_patterns = vec![];
let mut exclude_patterns = vec![];

for path in paths {
let path_str = path.to_string_lossy();

Expand All @@ -35,26 +41,38 @@ impl Walk {
continue;
}

// Otherwise, treat as target path
// Normalize `./` prefix
let normalized =
if let Some(stripped) = path_str.strip_prefix("./") { stripped } else { &path_str };

if path.is_absolute() {
target_paths.push(path.clone());
// Separate glob patterns from concrete paths
if is_glob_pattern(normalized, cwd) {
glob_patterns.push(normalized.to_string());
continue;
}

// NOTE: `.` and cwd behave differently, need to normalize
let path = if path_str == "." {
// Resolve full path for concrete paths
let full_path = if path.is_absolute() {
path.clone()
} else if normalized == "." {
// NOTE: `.` and cwd behave differently, need to normalize
cwd.to_path_buf()
} else if let Some(stripped) = path_str.strip_prefix("./") {
cwd.join(stripped)
} else {
cwd.join(path)
cwd.join(normalized)
};
target_paths.push(path);
target_paths.insert(full_path);
}
// Default to cwd if no target paths are provided
if target_paths.is_empty() {
target_paths.push(cwd.to_path_buf());

// 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)?);
}

// Default to `cwd` if no positive paths were specified.
// Exclude patterns alone should still walk, but unmatched globs should not.
if target_paths.is_empty() && glob_patterns.is_empty() {
target_paths.insert(cwd.to_path_buf());
}

//
Expand Down Expand Up @@ -182,27 +200,7 @@ impl Walk {
true
});

let inner = inner
// Do not follow symlinks like Prettier does.
// See https://github.com/prettier/prettier/pull/14627
.follow_links(false)
// Include hidden files and directories except those we explicitly skip above
.hidden(false)
// 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)
// 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();
let inner = apply_walk_settings(&mut inner).build_parallel();
Ok(Some(Self { inner }))
}

Expand Down Expand Up @@ -248,6 +246,51 @@ fn is_ignored(matchers: &[Gitignore], path: &Path, is_dir: bool, check_ancestors
false
}

/// 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.
/// e.g. `{config}.js`, `[id].tsx`
fn is_glob_pattern(s: &str, cwd: &Path) -> bool {
let has_glob_chars = s.contains('*') || s.contains('?') || s.contains('[') || s.contains('{');
has_glob_chars && !cwd.join(s).exists()
}

// NOTE: Why pre-expand globs?
// An alternative approach would be:
// - to always walk the entire `cwd`
// - and filter by both concrete paths and glob patterns
//
// However, this would be inefficient for common use cases
// like `oxfmt src/a.js` or pre-commit hooks that specify only staged files.
//
// Pre-expanding globs allows us to walk only the necessary paths.
// And this only happens if glob patterns are specified.
//
// NOTE: Why not use `ignore::Overrides` in the main walk?
// `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> {
let mut ob = OverrideBuilder::new(cwd);
for pattern in patterns {
ob.add(pattern).map_err(|e| format!("Invalid glob pattern `{pattern}`: {e}"))?;
}
let overrides = ob.build().map_err(|e| format!("Failed to build glob overrides: {e}"))?;

let mut builder = ignore::WalkBuilder::new(cwd);
builder.overrides(overrides);

let mut paths = vec![];
for entry in apply_walk_settings(&mut builder).build().flatten() {
// Use `!is_dir()` instead of `is_file()` to handle symlinks correctly
if entry.file_type().is_some_and(|ft| !ft.is_dir()) {
paths.push(entry.into_path());
}
}

Ok(paths)
}

fn load_ignore_paths(cwd: &Path, ignore_paths: &[PathBuf]) -> Result<Vec<PathBuf>, String> {
// If specified, resolve absolute paths and check existence
if !ignore_paths.is_empty() {
Expand All @@ -272,6 +315,31 @@ fn load_ignore_paths(cwd: &Path, ignore_paths: &[PathBuf]) -> Result<Vec<PathBuf
.collect())
}

/// Apply common walk settings.
/// This ensures consistent behavior across glob expansion and main walk.
fn apply_walk_settings(builder: &mut ignore::WalkBuilder) -> &mut ignore::WalkBuilder {
builder
Comment on lines +320 to +321
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question - this will walk single-threaded, right?
On large repos that will be fairly slow.

Should we use the parallelised form with maybe with a single thread by default and provide an option to add more threads to a parallel walk for larger repos?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right. Let me consider it as an enhancement.

// Do not follow symlinks like Prettier does.
// See https://github.com/prettier/prettier/pull/14627
.follow_links(false)
// Include hidden files and directories except those we explicitly skip
.hidden(false)
// 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)
// 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)
}

// ---

struct WalkBuilder {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -212,3 +212,30 @@ Finished in <variable>ms on 1 files using 1 threads.

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

exports[`exclude_nested > should handle glob include with glob exclude 1`] = `
"--------------------
arguments: --check * !**/error.js
working directory: exclude_nested/fixtures
exit code: 0
--- STDOUT ---------
Checking formatting...

All matched files use the correct format.
Finished in <variable>ms on 3 files using 1 threads.
--- STDERR ---------

--------------------
--------------------
arguments: --check foo/**/*.js !**/bar/*
working directory: exclude_nested/fixtures
exit code: 0
--- STDOUT ---------
Checking formatting...

All matched files use the correct format.
Finished in <variable>ms on 1 files using 1 threads.
--- STDERR ---------

--------------------"
`;
12 changes: 12 additions & 0 deletions apps/oxfmt/test/cli/exclude_nested/exclude_nested.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,4 +45,16 @@ describe("exclude_nested", () => {
const snapshot = await runAndSnapshot(fixturesDir, testCases);
expect(snapshot).toMatchSnapshot();
});

it("should handle glob include with glob exclude", async () => {
const testCases = [
// Glob include all .js, glob exclude error.js
["--check", "*", "!**/error.js"],
// Glob include foo/**/*.js, glob exclude bar directory
["--check", "foo/**/*.js", "!**/bar/*"],
];

const snapshot = await runAndSnapshot(fixturesDir, testCases);
expect(snapshot).toMatchSnapshot();
});
});
Loading
Loading