feat(oxfmt): Support --with-node-modules option

Author: leaysgur

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<PathBuf>,
 }
 
+/// 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 {

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();

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<Override>,
+        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<WalkEntry> {
+        let (sender, receiver) = mpsc::channel::<WalkEntry>();
+        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<WalkEntry>,
+    with_node_modules: bool,
 }
 
 impl<'s> ignore::ParallelVisitorBuilder<'s> for WalkBuilder {
     fn build(&mut self) -> Box<dyn ignore::ParallelVisitor + 's> {
-        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<WalkEntry>,
+    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<Override>) -> 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<WalkEntry> {
-        let (sender, receiver) = mpsc::channel::<WalkEntry>();
-
-        // 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
-    }
-}

apps/oxfmt/tests/fixtures/node_modules_dirs/node_modules/test.js

@@ -0,0 +1 @@
+  const       bar    =    2;

apps/oxfmt/tests/fixtures/node_modules_dirs/regular_dir/node_modules/test.js

@@ -0,0 +1 @@
+  const       foo    =    1;

apps/oxfmt/tests/fixtures/node_modules_dirs/regular_dir/test.js

@@ -0,0 +1,2 @@
+// This file should be ignored
+const   x   =   1;

apps/oxfmt/tests/fixtures/node_modules_dirs/root.js

@@ -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

apps/oxfmt/tests/fixtures/vcs_dirs/regular_dir/.jj/test.js

@@ -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 (<variable>ms)
+root.js (<variable>ms)
+
+Format issues found in above 2 files. Run without `--check` to fix.
+Finished in <variable>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 (<variable>ms)
+regular_dir/node_modules/test.js (<variable>ms)
+regular_dir/test.js (<variable>ms)
+root.js (<variable>ms)
+
+Format issues found in above 4 files. Run without `--check` to fix.
+Finished in <variable>ms on 4 files using 1 threads.
+----------
+CLI result: FormatMismatch
+----------