diff --git a/apps/oxfmt/src/command.rs b/apps/oxfmt/src/command.rs index 147d7beec8953..dcd107049b961 100644 --- a/apps/oxfmt/src/command.rs +++ b/apps/oxfmt/src/command.rs @@ -27,6 +27,9 @@ pub struct FormatCommand { #[bpaf(external)] pub basic_options: BasicOptions, + #[bpaf(external)] + pub ignore_options: IgnoreOptions, + #[bpaf(external)] pub misc_options: MiscOptions, @@ -60,6 +63,14 @@ pub struct BasicOptions { pub config: Option, } +/// Ignore Options +#[derive(Debug, Clone, Bpaf)] +pub struct IgnoreOptions { + /// Format code in node_modules directory (disabled by default) + #[bpaf(switch, hide_usage)] + pub with_node_modules: bool, +} + /// Miscellaneous #[derive(Debug, Clone, Bpaf)] pub struct MiscOptions { diff --git a/apps/oxfmt/src/format.rs b/apps/oxfmt/src/format.rs index 648e467a25186..05c5b8b44d2c7 100644 --- a/apps/oxfmt/src/format.rs +++ b/apps/oxfmt/src/format.rs @@ -44,7 +44,8 @@ impl FormatRunner { let start_time = Instant::now(); let cwd = self.cwd; - let FormatCommand { paths, output_options, basic_options, misc_options } = self.options; + let FormatCommand { paths, output_options, basic_options, ignore_options, misc_options } = + self.options; // Find and load config // NOTE: Currently, we only load single config file. @@ -76,7 +77,7 @@ impl FormatRunner { .flatten(); // TODO: Support ignoring files - let walker = Walk::new(&target_paths, override_builder); + let walker = Walk::new(&target_paths, override_builder, ignore_options.with_node_modules); // Get the receiver for streaming entries let rx_entry = walker.stream_entries(); diff --git a/apps/oxfmt/src/walk.rs b/apps/oxfmt/src/walk.rs index e1bf09271feca..3c55a6bfa7e1a 100644 --- a/apps/oxfmt/src/walk.rs +++ b/apps/oxfmt/src/walk.rs @@ -7,6 +7,54 @@ use oxc_span::SourceType; pub struct Walk { inner: ignore::WalkParallel, + with_node_modules: bool, +} + +impl Walk { + /// Will not canonicalize paths. + /// # Panics + pub fn new( + paths: &[PathBuf], + override_builder: Option, + with_node_modules: bool, + ) -> Self { + let mut inner = ignore::WalkBuilder::new( + paths + .iter() + .next() + .expect("Expected paths parameter to Walk::new() to contain at least one path."), + ); + + if let Some(paths) = paths.get(1..) { + for path in paths { + inner.add(path); + } + } + + if let Some(override_builder) = override_builder { + inner.overrides(override_builder); + } + + // Do not follow symlinks like Prettier does. + // See https://github.com/prettier/prettier/pull/14627 + let inner = inner.hidden(false).ignore(false).git_global(false).build_parallel(); + Self { inner, with_node_modules } + } + + /// Stream entries through a channel as they are discovered + pub fn stream_entries(self) -> mpsc::Receiver { + let (sender, receiver) = mpsc::channel::(); + let with_node_modules = self.with_node_modules; + + // Spawn the walk operation in a separate thread + rayon::spawn(move || { + let mut builder = WalkBuilder { sender, with_node_modules }; + self.inner.visit(&mut builder); + // Channel will be closed when builder is dropped + }); + + receiver + } } pub struct WalkEntry { @@ -16,16 +64,36 @@ pub struct WalkEntry { struct WalkBuilder { sender: mpsc::Sender, + with_node_modules: bool, } impl<'s> ignore::ParallelVisitorBuilder<'s> for WalkBuilder { fn build(&mut self) -> Box { - Box::new(WalkVisitor { sender: self.sender.clone() }) + Box::new(WalkVisitor { + sender: self.sender.clone(), + with_node_modules: self.with_node_modules, + }) } } struct WalkVisitor { sender: mpsc::Sender, + with_node_modules: bool, +} + +impl WalkVisitor { + // We are setting `.hidden(false)` on the `WalkBuilder`, + // it means we want to include hidden files and directories. + // However, we (and also Prettier) still skip traversing certain directories. + // https://prettier.io/docs/ignore#ignoring-files-prettierignore + fn is_skipped_dir(&self, dir_name: &OsStr) -> bool { + dir_name == ".git" + || dir_name == ".jj" + || dir_name == ".sl" + || dir_name == ".svn" + || dir_name == ".hg" + || (!self.with_node_modules && dir_name == "node_modules") + } } impl ignore::ParallelVisitor for WalkVisitor { @@ -37,15 +105,13 @@ impl ignore::ParallelVisitor for WalkVisitor { }; if file_type.is_dir() { - // We are setting `.hidden(false)` on the `WalkBuilder` below, - // it means we want to include hidden files and directories. - // However, we (and also Prettier) still skip traversing VCS directories. - // https://prettier.io/docs/ignore#ignoring-files-prettierignore - let dir_name = entry.file_name(); - if matches!(dir_name.to_str(), Some(".git" | ".jj" | ".sl" | ".svn" | ".hg")) { + if self.is_skipped_dir(entry.file_name()) { return ignore::WalkState::Skip; } - } else if let Some(source_type) = get_supported_source_type(entry.path()) { + return ignore::WalkState::Continue; + } + + if let Some(source_type) = get_supported_source_type(entry.path()) { let walk_entry = WalkEntry { path: entry.path().as_os_str().into(), source_type }; // Send each entry immediately through the channel @@ -54,51 +120,10 @@ impl ignore::ParallelVisitor for WalkVisitor { return ignore::WalkState::Quit; } } + ignore::WalkState::Continue } Err(_err) => ignore::WalkState::Skip, } } } - -impl Walk { - /// Will not canonicalize paths. - /// # Panics - pub fn new(paths: &[PathBuf], override_builder: Option) -> Self { - let mut inner = ignore::WalkBuilder::new( - paths - .iter() - .next() - .expect("Expected paths parameter to Walk::new() to contain at least one path."), - ); - - if let Some(paths) = paths.get(1..) { - for path in paths { - inner.add(path); - } - } - - if let Some(override_builder) = override_builder { - inner.overrides(override_builder); - } - - // Do not follow symlinks like Prettier does. - // See https://github.com/prettier/prettier/pull/14627 - let inner = inner.hidden(false).ignore(false).git_global(false).build_parallel(); - Self { inner } - } - - /// Stream entries through a channel as they are discovered - pub fn stream_entries(self) -> mpsc::Receiver { - let (sender, receiver) = mpsc::channel::(); - - // Spawn the walk operation in a separate thread - rayon::spawn(move || { - let mut builder = WalkBuilder { sender }; - self.inner.visit(&mut builder); - // Channel will be closed when builder is dropped - }); - - receiver - } -} diff --git a/apps/oxfmt/tests/fixtures/node_modules_dirs/node_modules/test.js b/apps/oxfmt/tests/fixtures/node_modules_dirs/node_modules/test.js new file mode 100644 index 0000000000000..89f35eb9cdbc6 --- /dev/null +++ b/apps/oxfmt/tests/fixtures/node_modules_dirs/node_modules/test.js @@ -0,0 +1 @@ + const ignored = 3; diff --git a/apps/oxfmt/tests/fixtures/node_modules_dirs/regular_dir/node_modules/test.js b/apps/oxfmt/tests/fixtures/node_modules_dirs/regular_dir/node_modules/test.js new file mode 100644 index 0000000000000..b4e02639ac646 --- /dev/null +++ b/apps/oxfmt/tests/fixtures/node_modules_dirs/regular_dir/node_modules/test.js @@ -0,0 +1 @@ + const also_ignored = 4; diff --git a/apps/oxfmt/tests/fixtures/node_modules_dirs/regular_dir/test.js b/apps/oxfmt/tests/fixtures/node_modules_dirs/regular_dir/test.js new file mode 100644 index 0000000000000..ba586e4fd7c02 --- /dev/null +++ b/apps/oxfmt/tests/fixtures/node_modules_dirs/regular_dir/test.js @@ -0,0 +1 @@ + const bar = 2; diff --git a/apps/oxfmt/tests/fixtures/node_modules_dirs/root.js b/apps/oxfmt/tests/fixtures/node_modules_dirs/root.js new file mode 100644 index 0000000000000..2563e52d823aa --- /dev/null +++ b/apps/oxfmt/tests/fixtures/node_modules_dirs/root.js @@ -0,0 +1 @@ + const foo = 1; diff --git a/apps/oxfmt/tests/fixtures/vcs_dirs/regular_dir/.jj/test.js b/apps/oxfmt/tests/fixtures/vcs_dirs/regular_dir/.jj/test.js new file mode 100644 index 0000000000000..863ce06e44b76 --- /dev/null +++ b/apps/oxfmt/tests/fixtures/vcs_dirs/regular_dir/.jj/test.js @@ -0,0 +1,2 @@ +// This file should be ignored +const x = 1; diff --git a/apps/oxfmt/tests/mod.rs b/apps/oxfmt/tests/mod.rs index f5278438226d7..910302ee90cdf 100644 --- a/apps/oxfmt/tests/mod.rs +++ b/apps/oxfmt/tests/mod.rs @@ -86,6 +86,15 @@ fn vcs_dirs_ignored() { .test_and_snapshot_multiple(&[&["--check"]]); } +#[test] +fn node_modules_ignored() { + // Test that `node_modules` directories are ignored by default + // but can be included with `--with-node-modules` flag + Tester::new() + .with_cwd(PathBuf::from("tests/fixtures/node_modules_dirs")) + .test_and_snapshot_multiple(&[&["--check"], &["--check", "--with-node-modules"]]); +} + #[test] fn exclude_nested_paths() { // Test that nested path exclusion works correctly diff --git a/apps/oxfmt/tests/snapshots/tests__fixtures__node_modules_dirs_--check --check --with-node-modules@oxfmt.snap b/apps/oxfmt/tests/snapshots/tests__fixtures__node_modules_dirs_--check --check --with-node-modules@oxfmt.snap new file mode 100644 index 0000000000000..6a21078cddd7f --- /dev/null +++ b/apps/oxfmt/tests/snapshots/tests__fixtures__node_modules_dirs_--check --check --with-node-modules@oxfmt.snap @@ -0,0 +1,33 @@ +--- +source: apps/oxfmt/tests/tester.rs +assertion_line: 102 +--- +########## +arguments: --check +working directory: tests/fixtures/node_modules_dirs +---------- +Checking formatting... +regular_dir/test.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. +---------- +CLI result: FormatMismatch +---------- + +########## +arguments: --check --with-node-modules +working directory: tests/fixtures/node_modules_dirs +---------- +Checking formatting... +node_modules/test.js (ms) +regular_dir/node_modules/test.js (ms) +regular_dir/test.js (ms) +root.js (ms) + +Format issues found in above 4 files. Run without `--check` to fix. +Finished in ms on 4 files using 1 threads. +---------- +CLI result: FormatMismatch +----------