refactor(cli,linter): move path processing logic from cli to linter (#766)

Cargo.lock

@@ -1525,7 +1525,6 @@ version = "0.0.0"
 dependencies = [
  "bpaf",
  "codespan-reporting",
- "dashmap",
  "ignore",
  "jemallocator",
  "miette",
@@ -1535,11 +1534,9 @@ dependencies = [
  "oxc_index",
  "oxc_linter",
  "oxc_parser",
- "oxc_semantic",
  "oxc_span",
  "oxc_type_synthesis",
  "rayon",
- "rustc-hash",
 ]
 
 [[package]]
@@ -1641,6 +1638,7 @@ dependencies = [
  "oxc_span",
  "oxc_syntax",
  "phf",
+ "rayon",
  "regex",
  "rust-lapper",
  "rustc-hash",

crates/oxc_cli/Cargo.toml

@@ -28,17 +28,14 @@ oxc_diagnostics    = { workspace = true }
 oxc_index          = { workspace = true }
 oxc_linter         = { workspace = true }
 oxc_parser         = { workspace = true }
-oxc_semantic       = { workspace = true }
 oxc_span           = { workspace = true }
 oxc_type_synthesis = { workspace = true }
 
 # TODO temp, for type check output, replace with Miette
 codespan-reporting = "0.11.1"
 
-dashmap    = { workspace = true }
 ignore     = { workspace = true, features = ["simd-accel"] }
 miette     = { workspace = true, features = ["fancy-no-backtrace"] }
 rayon      = { workspace = true }
-rustc-hash = { workspace = true }
 bpaf       = { workspace = true, features = ["derive", "autocomplete", "bright-color"] }
 # git2 = { version = "0.16.1", default_features = false }

crates/oxc_cli/src/lint/mod.rs

@@ -1,7 +1,6 @@
 mod error;
 
 use std::{
-    fs,
     io::BufWriter,
     path::Path,
     sync::{
@@ -48,10 +47,10 @@ impl Runner for LintRunner {
 
         let now = std::time::Instant::now();
 
-        let filter = if filter.is_empty() { LintOptions::default().filter } else { filter };
+        let lint_options = LintOptions::default()
-
+            .with_filter(filter)
-        let lint_options =
+            .with_fix(fix_options.fix)
-            LintOptions { filter, fix: fix_options.fix, timing: misc_options.timing };
+            .with_timing(misc_options.timing);
 
         let linter = Arc::new(Linter::from_options(lint_options));
 
@@ -60,6 +59,7 @@ impl Runner for LintRunner {
             .with_max_warnings(warning_options.max_warnings);
 
         let number_of_files = Arc::new(AtomicUsize::new(0));
+
         let (tx_path, rx_path) = mpsc::channel::<Box<Path>>();
 
         rayon::spawn({
@@ -75,35 +75,12 @@ impl Runner for LintRunner {
             }
         });
 
-        let processing = Arc::new(AtomicUsize::new(0));
         rayon::spawn({
-            let linter = Arc::clone(&linter);
+            let lint_service = oxc_linter::LintService::new(Arc::clone(&linter));
             let tx_error = diagnostic_service.sender().clone();
-            let processing = Arc::clone(&processing);
             move || {
                 while let Ok(path) = rx_path.recv() {
-                    processing.fetch_add(1, Ordering::Relaxed);
+                    lint_service.run_path(path, &tx_error);
-                    let tx_error = tx_error.clone();
-                    let linter = Arc::clone(&linter);
-                    let processing = Arc::clone(&processing);
-                    rayon::spawn(move || {
-                        let source_text = fs::read_to_string(&path)
-                            .unwrap_or_else(|_| panic!("Failed to read {path:?}"));
-
-                        let diagnostics = oxc_linter::LintService::new(linter)
-                            .run(&path, &source_text)
-                            .map(|errors| {
-                                DiagnosticService::wrap_diagnostics(&path, &source_text, errors)
-                            });
-
-                        if let Some(diagnostics) = diagnostics {
-                            tx_error.send(Some(diagnostics)).unwrap();
-                        }
-                        processing.fetch_sub(1, Ordering::Relaxed);
-                        if processing.load(Ordering::Relaxed) == 0 {
-                            tx_error.send(None).unwrap();
-                        }
-                    });
                 }
             }
         });

crates/oxc_diagnostics/src/lib.rs

@@ -7,7 +7,7 @@ mod service;
 
 use std::path::PathBuf;
 
-pub use crate::service::DiagnosticService;
+pub use crate::service::{DiagnosticSender, DiagnosticService, DiagnosticTuple};
 pub use graphic_reporter::{GraphicalReportHandler, GraphicalTheme};
 pub use miette;
 pub use thiserror;

crates/oxc_linter/Cargo.toml

@@ -23,6 +23,7 @@ oxc_semantic    = { workspace = true }
 oxc_syntax      = { workspace = true }
 oxc_formatter   = { workspace = true }
 
+rayon       = { workspace = true }
 lazy_static = { workspace = true }                        # used in oxc_macros
 serde_json  = { workspace = true }
 regex       = { workspace = true }

crates/oxc_linter/src/lib.rs

@@ -42,10 +42,6 @@ impl Linter {
             .cloned()
             .filter(|rule| rule.category() == RuleCategory::Correctness)
             .collect::<Vec<_>>();
-        Self::from_rules(rules)
-    }
-
-    pub fn from_rules(rules: Vec<RuleEnum>) -> Self {
         Self { rules, options: LintOptions::default() }
     }
 
@@ -54,6 +50,12 @@ impl Linter {
         Self { rules, options }
     }
 
+    #[must_use]
+    pub fn with_rules(mut self, rules: Vec<RuleEnum>) -> Self {
+        self.rules = rules;
+        self
+    }
+
     pub fn rules(&self) -> &Vec<RuleEnum> {
         &self.rules
     }
@@ -78,27 +80,6 @@ impl Linter {
         self
     }
 
-    pub fn from_json_str(s: &str) -> Self {
-        let rules = serde_json::from_str(s)
-            .ok()
-            .and_then(|v: serde_json::Value| v.get("rules").cloned())
-            .and_then(|v| v.as_object().cloned())
-            .map_or_else(
-                || RULES.to_vec(),
-                |rules_config| {
-                    RULES
-                        .iter()
-                        .map(|rule| {
-                            let value = rules_config.get(rule.name());
-                            rule.read_json(value.cloned())
-                        })
-                        .collect()
-                },
-            );
-
-        Self::from_rules(rules)
-    }
-
     pub fn run<'a>(&self, ctx: LintContext<'a>) -> Vec<Message<'a>> {
         let timing = self.options.timing;
         let semantic = Rc::clone(ctx.semantic());

crates/oxc_linter/src/options.rs

@@ -21,6 +21,28 @@ impl Default for LintOptions {
     }
 }
 
+impl LintOptions {
+    #[must_use]
+    pub fn with_filter(mut self, filter: Vec<(AllowWarnDeny, String)>) -> Self {
+        if !filter.is_empty() {
+            self.filter = filter;
+        }
+        self
+    }
+
+    #[must_use]
+    pub fn with_fix(mut self, yes: bool) -> Self {
+        self.fix = yes;
+        self
+    }
+
+    #[must_use]
+    pub fn with_timing(mut self, yes: bool) -> Self {
+        self.timing = yes;
+        self
+    }
+}
+
 #[derive(Debug, Clone, Copy, Eq, PartialEq)]
 pub enum AllowWarnDeny {
     Allow,

crates/oxc_linter/src/service.rs

@@ -1,60 +1,93 @@
-use std::{fs, path::Path, rc::Rc, sync::Arc};
+use std::{
+    fs,
+    path::Path,
+    rc::Rc,
+    sync::{
+        atomic::{AtomicUsize, Ordering},
+        Arc,
+    },
+};
 
 use oxc_allocator::Allocator;
-use oxc_diagnostics::Error;
+use oxc_diagnostics::{DiagnosticSender, DiagnosticService};
 use oxc_parser::Parser;
 use oxc_semantic::SemanticBuilder;
 use oxc_span::SourceType;
 
-use crate::{Fixer, LintContext, Linter};
+use crate::{Fixer, LintContext, Linter, Message};
 
 pub struct LintService {
     linter: Arc<Linter>,
+    processing: Arc<AtomicUsize>,
 }
 
 impl LintService {
     pub fn new(linter: Arc<Linter>) -> Self {
-        Self { linter }
+        Self { linter, processing: Arc::new(AtomicUsize::new(0)) }
     }
 
     /// # Panics
-    pub fn run(&self, path: &Path, source_text: &str) -> Option<Vec<Error>> {
+    pub fn run_path(&self, path: Box<Path>, tx_error: &DiagnosticSender) {
-        let allocator = Allocator::default();
+        self.processing.fetch_add(1, Ordering::Relaxed);
+        let linter = Arc::clone(&self.linter);
+        let tx_error = tx_error.clone();
+        let processing = Arc::clone(&self.processing);
+        rayon::spawn(move || {
+            let allocator = Allocator::default();
+            let source_text =
+                fs::read_to_string(&path).unwrap_or_else(|_| panic!("Failed to read {path:?}"));
+
+            let mut messages = Self::run_source(&linter, &path, &allocator, &source_text, true);
+
+            if linter.options().fix {
+                let fix_result = Fixer::new(&source_text, messages).fix();
+                fs::write(&path, fix_result.fixed_code.as_bytes()).unwrap();
+                messages = fix_result.messages;
+            }
+
+            let errors = messages.into_iter().map(|m| m.error).collect();
+            let diagnostics = DiagnosticService::wrap_diagnostics(&path, &source_text, errors);
+
+            if !diagnostics.1.is_empty() {
+                tx_error.send(Some(diagnostics)).unwrap();
+            }
+
+            processing.fetch_sub(1, Ordering::Relaxed);
+            if processing.load(Ordering::Relaxed) == 0 {
+                tx_error.send(None).unwrap();
+            }
+        });
+    }
+
+    pub(crate) fn run_source<'a>(
+        linter: &Linter,
+        path: &Path,
+        allocator: &'a Allocator,
+        source_text: &'a str,
+        check_syntax_errors: bool,
+    ) -> Vec<Message<'a>> {
         let source_type =
             SourceType::from_path(path).unwrap_or_else(|_| panic!("Incorrect {path:?}"));
-        let ret = Parser::new(&allocator, source_text, source_type)
+        let ret = Parser::new(allocator, source_text, source_type)
             .allow_return_outside_function(true)
             .parse();
 
         if !ret.errors.is_empty() {
-            return Some(ret.errors);
+            return ret.errors.into_iter().map(|err| Message::new(err, None)).collect();
         };
 
         let program = allocator.alloc(ret.program);
         let semantic_ret = SemanticBuilder::new(source_text, source_type)
             .with_trivias(ret.trivias)
-            .with_check_syntax_error(true)
+            .with_check_syntax_error(check_syntax_errors)
             .with_module_record_builder(true)
             .build(program);
 
         if !semantic_ret.errors.is_empty() {
-            return Some(semantic_ret.errors);
+            return semantic_ret.errors.into_iter().map(|err| Message::new(err, None)).collect();
         };
 
         let lint_ctx = LintContext::new(&Rc::new(semantic_ret.semantic));
-        let result = self.linter.run(lint_ctx);
+        linter.run(lint_ctx)
-
-        if result.is_empty() {
-            return None;
-        }
-
-        if self.linter.options().fix {
-            let fix_result = Fixer::new(source_text, result).fix();
-            fs::write(path, fix_result.fixed_code.as_bytes()).unwrap();
-            let errors = fix_result.messages.into_iter().map(|m| m.error).collect();
-            return Some(errors);
-        }
-
-        Some(result.into_iter().map(|diagnostic| diagnostic.error).collect())
     }
 }

crates/oxc_linter/src/tester.rs

@@ -1,13 +1,14 @@
-use std::{borrow::Cow, path::PathBuf, rc::Rc};
+use std::{
+    borrow::Cow,
+    path::{Path, PathBuf},
+    sync::Arc,
+};
 
 use oxc_allocator::Allocator;
 use oxc_diagnostics::miette::{GraphicalReportHandler, GraphicalTheme, NamedSource};
-use oxc_parser::Parser;
-use oxc_semantic::SemanticBuilder;
-use oxc_span::SourceType;
 use serde_json::Value;
 
-use crate::{rules::RULES, Fixer, LintContext, Linter, Message};
+use crate::{rules::RULES, Fixer, LintOptions, LintService, Linter, Message};
 
 pub struct Tester {
     rule_name: &'static str,
@@ -123,28 +124,18 @@ impl Tester {
     fn run_rules<'a>(
         &mut self,
         allocator: &'a Allocator,
-        path: &PathBuf,
+        path: &Path,
         source_text: &'a str,
         config: Option<Value>,
         is_fix: bool,
     ) -> Vec<Message<'a>> {
-        let source_type = SourceType::from_path(path).expect("incorrect {path:?}");
-        let ret = Parser::new(allocator, source_text, source_type)
-            .allow_return_outside_function(true)
-            .parse();
-        assert!(ret.errors.is_empty(), "{:?}", &ret.errors);
-        let program = allocator.alloc(ret.program);
-        let semantic_ret = SemanticBuilder::new(source_text, source_type)
-            .with_trivias(ret.trivias)
-            .with_module_record_builder(true)
-            .build(program);
-        assert!(semantic_ret.errors.is_empty(), "{:?}", &semantic_ret.errors);
         let rule = RULES
             .iter()
             .find(|rule| rule.name() == self.rule_name)
             .unwrap_or_else(|| panic!("Rule not found: {}", &self.rule_name));
         let rule = rule.read_json(config);
-        let lint_context = LintContext::new(&Rc::new(semantic_ret.semantic));
+        let options = LintOptions::default().with_fix(is_fix);
-        Linter::from_rules(vec![rule]).with_fix(is_fix).run(lint_context)
+        let linter = Arc::new(Linter::from_options(options).with_rules(vec![rule]));
+        LintService::run_source(&linter, path, allocator, source_text, false)
     }
 }

justfile

@@ -32,7 +32,7 @@ update:
 # --no-vcs-ignores: cargo-watch has a bug loading all .gitignores, including the ones listed in .gitignore
 # use .ignore file getting the ignore list
 watch command:
-  cargo watch --no-vcs-ignores -x '{{command}}'
+  cargo watch --no-vcs-ignores -i '*snap*' -x '{{command}}'
 
 # Format all files
 fmt: