refactor(linter): move DiagnosticsReporters to oxlint

oxc-project · Jan 12, 2025 · 6a3ff9e · 6a3ff9e
1 parent ae23100
commit 6a3ff9e
Show file tree

Hide file tree

Showing 7 changed files with 52 additions and 60 deletions.
diff --git a/apps/oxlint/src/output_formatter/checkstyle.rs b/apps/oxlint/src/output_formatter/checkstyle.rs
@@ -28,13 +28,10 @@ struct CheckstyleReporter {
 }
 
 impl DiagnosticReporter for CheckstyleReporter {
-    fn finish(&mut self, writer: &mut dyn Write) {
-        writer.write_all(format_checkstyle(&self.diagnostics).as_bytes()).unwrap();
-        writer.flush().unwrap();
+    fn finish(&mut self) -> Option<String> {
+        Some(format_checkstyle(&self.diagnostics))
     }
 
-    fn render_diagnostics(&mut self, _writer: &mut dyn Write, _s: &[u8]) {}
-
     fn render_error(&mut self, error: Error) -> Option<String> {
         self.diagnostics.push(error);
         None

diff --git a/apps/oxlint/src/output_formatter/default.rs b/apps/oxlint/src/output_formatter/default.rs
@@ -1,4 +1,4 @@
-use std::io::{ErrorKind, Write};
+use std::io::Write;
 
 use oxc_diagnostics::{reporter::DiagnosticReporter, Error, GraphicalReportHandler};
 use oxc_linter::table::RuleTable;
@@ -37,32 +37,8 @@ impl Default for GraphicalReporter {
 }
 
 impl DiagnosticReporter for GraphicalReporter {
-    fn finish(&mut self, writer: &mut dyn Write) {
-        writer
-            .flush()
-            .or_else(|e| {
-                // Do not panic when the process is skill (e.g. piping into `less`).
-                if matches!(e.kind(), ErrorKind::Interrupted | ErrorKind::BrokenPipe) {
-                    Ok(())
-                } else {
-                    Err(e)
-                }
-            })
-            .unwrap();
-    }
-
-    fn render_diagnostics(&mut self, writer: &mut dyn Write, s: &[u8]) {
-        writer
-            .write_all(s)
-            .or_else(|e| {
-                // Do not panic when the process is skill (e.g. piping into `less`).
-                if matches!(e.kind(), ErrorKind::Interrupted | ErrorKind::BrokenPipe) {
-                    Ok(())
-                } else {
-                    Err(e)
-                }
-            })
-            .unwrap();
+    fn finish(&mut self) -> Option<String> {
+        None
     }
 
     fn render_error(&mut self, error: Error) -> Option<String> {

diff --git a/apps/oxlint/src/output_formatter/github.rs b/apps/oxlint/src/output_formatter/github.rs
@@ -25,12 +25,8 @@ impl InternalFormatter for GithubOutputFormatter {
 struct GithubReporter;
 
 impl DiagnosticReporter for GithubReporter {
-    fn finish(&mut self, writer: &mut dyn Write) {
-        writer.flush().unwrap();
-    }
-
-    fn render_diagnostics(&mut self, writer: &mut dyn Write, s: &[u8]) {
-        writer.write_all(s).unwrap();
+    fn finish(&mut self) -> Option<String> {
+        None
     }
 
     fn render_error(&mut self, error: Error) -> Option<String> {

diff --git a/apps/oxlint/src/output_formatter/json.rs b/apps/oxlint/src/output_formatter/json.rs
@@ -52,13 +52,10 @@ struct JsonReporter {
 impl DiagnosticReporter for JsonReporter {
     // NOTE: this output does not conform to eslint json format yet
     // https://eslint.org/docs/latest/use/formatters/#json
-    fn finish(&mut self, writer: &mut dyn Write) {
-        writer.write_all(format_json(&mut self.diagnostics).as_bytes()).unwrap();
-        writer.flush().unwrap();
+    fn finish(&mut self) -> Option<String> {
+        Some(format_json(&mut self.diagnostics))
     }
 
-    fn render_diagnostics(&mut self, _writer: &mut dyn Write, _s: &[u8]) {}
-
     fn render_error(&mut self, error: Error) -> Option<String> {
         self.diagnostics.push(error);
         None

diff --git a/apps/oxlint/src/output_formatter/unix.rs b/apps/oxlint/src/output_formatter/unix.rs
@@ -26,17 +26,13 @@ struct UnixReporter {
 }
 
 impl DiagnosticReporter for UnixReporter {
-    fn finish(&mut self, writer: &mut dyn Write) {
+    fn finish(&mut self) -> Option<String> {
         let total = self.total;
         if total > 0 {
-            let line = format!("\n{total} problem{}\n", if total > 1 { "s" } else { "" });
-            writer.write_all(line.as_bytes()).unwrap();
+            return Some(format!("\n{total} problem{}\n", if total > 1 { "s" } else { "" }));
         }
-        writer.flush().unwrap();
-    }
 
-    fn render_diagnostics(&mut self, writer: &mut dyn Write, s: &[u8]) {
-        writer.write_all(s).unwrap();
+        None
     }
 
     fn render_error(&mut self, error: Error) -> Option<String> {

diff --git a/crates/oxc_diagnostics/src/reporter/mod.rs b/crates/oxc_diagnostics/src/reporter/mod.rs
@@ -1,7 +1,5 @@
 //! [Reporters](DiagnosticReporter) for rendering and writing diagnostics.
 
-use std::io::Write;
-
 use crate::{Error, Severity};
 
 /// Reporters are responsible for rendering diagnostics to some format and writing them to some
@@ -58,10 +56,7 @@ pub trait DiagnosticReporter {
     ///
     /// While this method _should_ only ever be called a single time, this is not a guarantee
     /// upheld in Oxc's API. Do not rely on this behavior.
-    fn finish(&mut self, writer: &mut dyn Write);
-
-    /// Write a rendered collection of diagnostics to this reporter's output stream.
-    fn render_diagnostics(&mut self, writer: &mut dyn Write, s: &[u8]);
+    fn finish(&mut self) -> Option<String>;
 
     /// Render a diagnostic into this reporter's desired format. For example, a JSONLinesReporter
     /// might return a stringified JSON object on a single line. Returns [`None`] to skip reporting

diff --git a/crates/oxc_diagnostics/src/service.rs b/crates/oxc_diagnostics/src/service.rs
@@ -1,6 +1,6 @@
 use std::{
     cell::Cell,
-    io::{BufWriter, Stdout},
+    io::{ErrorKind, Write},
     path::{Path, PathBuf},
     sync::{mpsc, Arc},
 };
@@ -165,7 +165,7 @@ impl DiagnosticService {
     /// # Panics
     ///
     /// * When the writer fails to write
-    pub fn run(&mut self, writer: &mut BufWriter<Stdout>) {
+    pub fn run(&mut self, writer: &mut dyn Write) {
         while let Ok(Some((path, diagnostics))) = self.receiver.recv() {
             let mut output = String::new();
             for diagnostic in diagnostics {
@@ -207,9 +207,44 @@ impl DiagnosticService {
                     output.push_str(&err_str);
                 }
             }
-            self.reporter.render_diagnostics(writer, output.as_bytes());
+
+            writer
+                .write_all(output.as_bytes())
+                .or_else(|e| {
+                    // Do not panic when the process is skill (e.g. piping into `less`).
+                    if matches!(e.kind(), ErrorKind::Interrupted | ErrorKind::BrokenPipe) {
+                        Ok(())
+                    } else {
+                        Err(e)
+                    }
+                })
+                .unwrap();
+        }
+
+        if let Some(finish_output) = self.reporter.finish() {
+            writer
+                .write_all(finish_output.as_bytes())
+                .or_else(|e| {
+                    // Do not panic when the process is skill (e.g. piping into `less`).
+                    if matches!(e.kind(), ErrorKind::Interrupted | ErrorKind::BrokenPipe) {
+                        Ok(())
+                    } else {
+                        Err(e)
+                    }
+                })
+                .unwrap();
         }
 
-        self.reporter.finish(writer);
+        writer
+            .flush()
+            .or_else(|e| {
+                // Do not panic when the process is skill (e.g. piping into `less`).
+                if matches!(e.kind(), ErrorKind::Interrupted | ErrorKind::BrokenPipe) {
+                    Ok(())
+                } else {
+                    Err(e)
+                }
+            })
+            .unwrap();
     }
 }