From 82bd97d4206215782016d3910157c7b81e042976 Mon Sep 17 00:00:00 2001 From: Boshen Date: Tue, 7 May 2024 18:44:03 +0800 Subject: [PATCH] refactor(diagnostics): use a trait to implement the reporters (#3190) --- crates/oxc_cli/src/lint/mod.rs | 2 +- crates/oxc_diagnostics/src/reporter.rs | 244 ------------------ .../src/reporter/checkstyle.rs | 103 ++++++++ .../oxc_diagnostics/src/reporter/graphical.rs | 32 +++ crates/oxc_diagnostics/src/reporter/json.rs | 38 +++ crates/oxc_diagnostics/src/reporter/mod.rs | 68 +++++ crates/oxc_diagnostics/src/reporter/unix.rs | 51 ++++ crates/oxc_diagnostics/src/service.rs | 20 +- 8 files changed, 303 insertions(+), 255 deletions(-) delete mode 100644 crates/oxc_diagnostics/src/reporter.rs create mode 100644 crates/oxc_diagnostics/src/reporter/checkstyle.rs create mode 100644 crates/oxc_diagnostics/src/reporter/graphical.rs create mode 100644 crates/oxc_diagnostics/src/reporter/json.rs create mode 100644 crates/oxc_diagnostics/src/reporter/mod.rs create mode 100644 crates/oxc_diagnostics/src/reporter/unix.rs diff --git a/crates/oxc_cli/src/lint/mod.rs b/crates/oxc_cli/src/lint/mod.rs index e00b95b2f..30027238b 100644 --- a/crates/oxc_cli/src/lint/mod.rs +++ b/crates/oxc_cli/src/lint/mod.rs @@ -146,7 +146,7 @@ impl Runner for LintRunner { number_of_errors: diagnostic_service.errors_count(), max_warnings_exceeded: diagnostic_service.max_warnings_exceeded(), deny_warnings: warning_options.deny_warnings, - print_summary: diagnostic_service.is_graphical_output(), + print_summary: matches!(output_options.format, OutputFormat::Default), }) } } diff --git a/crates/oxc_diagnostics/src/reporter.rs b/crates/oxc_diagnostics/src/reporter.rs deleted file mode 100644 index 36e047c43..000000000 --- a/crates/oxc_diagnostics/src/reporter.rs +++ /dev/null @@ -1,244 +0,0 @@ -use std::{ - borrow::Cow, - collections::HashMap, - io::{BufWriter, Stdout, Write}, -}; - -use crate::{ - miette::{Error, JSONReportHandler}, - GraphicalReportHandler, Severity, -}; - -/// stdio is blocked by LineWriter, use a BufWriter to reduce syscalls. -/// See `https://github.com/rust-lang/rust/issues/60673`. -fn writer() -> BufWriter { - BufWriter::new(std::io::stdout()) -} - -#[allow(clippy::large_enum_variant)] // Lerge size is fine because this is a singleton -#[derive(Debug)] -#[non_exhaustive] -pub enum DiagnosticReporter { - Graphical { handler: GraphicalReportHandler, writer: BufWriter }, - Json { diagnostics: Vec }, - Unix { total: usize, writer: BufWriter }, - Checkstyle { diagnostics: Vec }, -} - -impl DiagnosticReporter { - pub fn new_graphical() -> Self { - Self::Graphical { handler: GraphicalReportHandler::new(), writer: writer() } - } - - pub fn new_json() -> Self { - Self::Json { diagnostics: vec![] } - } - - pub fn new_unix() -> Self { - Self::Unix { total: 0, writer: writer() } - } - - pub fn new_checkstyle() -> Self { - Self::Checkstyle { diagnostics: vec![] } - } - - pub fn finish(&mut self) { - match self { - Self::Graphical { writer, .. } => { - writer.flush().unwrap(); - } - // NOTE: this output does not conform to eslint json format yet - // https://eslint.org/docs/latest/use/formatters/#json - Self::Json { diagnostics } => { - format_json(diagnostics); - } - Self::Unix { total, writer } => { - if *total > 0 { - let line = format!("\n{total} problem{}\n", if *total > 1 { "s" } else { "" }); - writer.write_all(line.as_bytes()).unwrap(); - } - writer.flush().unwrap(); - } - Self::Checkstyle { diagnostics } => { - format_checkstyle(diagnostics); - } - } - } - - pub fn render_diagnostics(&mut self, s: &[u8]) { - match self { - Self::Graphical { writer, .. } | Self::Unix { writer, .. } => { - writer.write_all(s).unwrap(); - } - Self::Json { .. } | Self::Checkstyle { .. } => {} - } - } - - pub fn render_error(&mut self, error: Error) -> Option { - match self { - Self::Graphical { handler, .. } => { - let mut output = String::new(); - handler.render_report(&mut output, error.as_ref()).unwrap(); - Some(output) - } - Self::Json { diagnostics } | Self::Checkstyle { diagnostics } => { - diagnostics.push(error); - None - } - Self::Unix { total: count, .. } => { - *count += 1; - Some(format_unix(&error)) - } - } - } -} - -struct Info { - line: usize, - column: usize, - filename: String, - message: String, - severity: Severity, - rule_id: Option, -} - -impl Info { - fn new(diagnostic: &Error) -> Self { - let mut line = 0; - let mut column = 0; - let mut filename = String::new(); - let mut message = String::new(); - let mut severity = Severity::Warning; - let mut rule_id = None; - if let Some(mut labels) = diagnostic.labels() { - if let Some(source) = diagnostic.source_code() { - if let Some(label) = labels.next() { - if let Ok(span_content) = source.read_span(label.inner(), 0, 0) { - line = span_content.line() + 1; - column = span_content.column() + 1; - if let Some(name) = span_content.name() { - filename = name.to_string(); - }; - if matches!(diagnostic.severity(), Some(Severity::Error)) { - severity = Severity::Error; - } - let msg = diagnostic.to_string(); - // Our messages usually comes with `eslint(rule): message` - (rule_id, message) = msg.split_once(':').map_or_else( - || (None, msg.to_string()), - |(id, msg)| (Some(id.to_string()), msg.trim().to_string()), - ); - } - } - } - } - Self { line, column, filename, message, severity, rule_id } - } -} - -/// -fn format_json(diagnostics: &mut Vec) { - let handler = JSONReportHandler::new(); - let messages = diagnostics - .drain(..) - .map(|error| { - let mut output = String::from("\t"); - handler.render_report(&mut output, error.as_ref()).unwrap(); - output - }) - .collect::>() - .join(",\n"); - println!("[\n{messages}\n]"); -} - -/// -fn format_unix(diagnostic: &Error) -> String { - let Info { line, column, filename, message, severity, rule_id } = Info::new(diagnostic); - let severity = match severity { - Severity::Error => "Error", - _ => "Warning", - }; - let rule_id = - rule_id.map_or_else(|| Cow::Borrowed(""), |rule_id| Cow::Owned(format!("/{rule_id}"))); - format!("{filename}:{line}:{column}: {message} [{severity}{rule_id}]\n") -} - -fn format_checkstyle(diagnostics: &[Error]) { - let infos = diagnostics.iter().map(Info::new).collect::>(); - let mut grouped: HashMap> = HashMap::new(); - for info in infos { - grouped.entry(info.filename.clone()).or_default().push(info); - } - let messages = grouped.into_values().map(|infos| { - let messages = infos - .iter() - .fold(String::new(), |mut acc, info| { - let Info { line, column, message, severity, rule_id, .. } = info; - let severity = match severity { - Severity::Error => "error", - _ => "warning", - }; - let message = rule_id.as_ref().map_or_else(|| xml_escape(message), |rule_id| Cow::Owned(format!("{} ({rule_id})", xml_escape(message)))); - let source = rule_id.as_ref().map_or_else(|| Cow::Borrowed(""), |rule_id| Cow::Owned(format!("eslint.rules.{rule_id}"))); - let line = format!(r#""#); - acc.push_str(&line); - acc - }); - let filename = &infos[0].filename; - format!(r#"{messages}"#) - }).collect::>().join(" "); - println!( - r#"{messages}"# - ); -} - -/// -fn xml_escape(raw: &str) -> Cow { - xml_escape_impl(raw, |ch| matches!(ch, b'<' | b'>' | b'&' | b'\'' | b'\"')) -} - -fn xml_escape_impl bool>(raw: &str, escape_chars: F) -> Cow { - let bytes = raw.as_bytes(); - let mut escaped = None; - let mut iter = bytes.iter(); - let mut pos = 0; - while let Some(i) = iter.position(|&b| escape_chars(b)) { - if escaped.is_none() { - escaped = Some(Vec::with_capacity(raw.len())); - } - let escaped = escaped.as_mut().expect("initialized"); - let new_pos = pos + i; - escaped.extend_from_slice(&bytes[pos..new_pos]); - match bytes[new_pos] { - b'<' => escaped.extend_from_slice(b"<"), - b'>' => escaped.extend_from_slice(b">"), - b'\'' => escaped.extend_from_slice(b"'"), - b'&' => escaped.extend_from_slice(b"&"), - b'"' => escaped.extend_from_slice(b"""), - - // This set of escapes handles characters that should be escaped - // in elements of xs:lists, because those characters works as - // delimiters of list elements - b'\t' => escaped.extend_from_slice(b" "), - b'\n' => escaped.extend_from_slice(b" "), - b'\r' => escaped.extend_from_slice(b" "), - b' ' => escaped.extend_from_slice(b" "), - _ => unreachable!( - "Only '<', '>','\', '&', '\"', '\\t', '\\r', '\\n', and ' ' are escaped" - ), - } - pos = new_pos + 1; - } - - if let Some(mut escaped) = escaped { - if let Some(raw) = bytes.get(pos..) { - escaped.extend_from_slice(raw); - } - #[allow(unsafe_code)] - // SAFETY: we operate on UTF-8 input and search for an one byte chars only, - // so all slices that was put to the `escaped` is a valid UTF-8 encoded strings - Cow::Owned(unsafe { String::from_utf8_unchecked(escaped) }) - } else { - Cow::Borrowed(raw) - } -} diff --git a/crates/oxc_diagnostics/src/reporter/checkstyle.rs b/crates/oxc_diagnostics/src/reporter/checkstyle.rs new file mode 100644 index 000000000..f4ddb1cb5 --- /dev/null +++ b/crates/oxc_diagnostics/src/reporter/checkstyle.rs @@ -0,0 +1,103 @@ +use std::{borrow::Cow, collections::HashMap}; + +use crate::{miette::Error, Severity}; + +use super::{DiagnosticReporter, Info}; + +#[derive(Default)] +pub struct CheckstyleReporter { + diagnostics: Vec, +} + +impl DiagnosticReporter for CheckstyleReporter { + fn finish(&mut self) { + format_checkstyle(&self.diagnostics); + } + + fn render_diagnostics(&mut self, _s: &[u8]) {} + + fn render_error(&mut self, error: Error) -> Option { + self.diagnostics.push(error); + None + } +} + +fn format_checkstyle(diagnostics: &[Error]) { + let infos = diagnostics.iter().map(Info::new).collect::>(); + let mut grouped: HashMap> = HashMap::new(); + for info in infos { + grouped.entry(info.filename.clone()).or_default().push(info); + } + let messages = grouped.into_values().map(|infos| { + let messages = infos + .iter() + .fold(String::new(), |mut acc, info| { + let Info { line, column, message, severity, rule_id, .. } = info; + let severity = match severity { + Severity::Error => "error", + _ => "warning", + }; + let message = rule_id.as_ref().map_or_else(|| xml_escape(message), |rule_id| Cow::Owned(format!("{} ({rule_id})", xml_escape(message)))); + let source = rule_id.as_ref().map_or_else(|| Cow::Borrowed(""), |rule_id| Cow::Owned(format!("eslint.rules.{rule_id}"))); + let line = format!(r#""#); + acc.push_str(&line); + acc + }); + let filename = &infos[0].filename; + format!(r#"{messages}"#) + }).collect::>().join(" "); + println!( + r#"{messages}"# + ); +} + +/// +fn xml_escape(raw: &str) -> Cow { + xml_escape_impl(raw, |ch| matches!(ch, b'<' | b'>' | b'&' | b'\'' | b'\"')) +} + +fn xml_escape_impl bool>(raw: &str, escape_chars: F) -> Cow { + let bytes = raw.as_bytes(); + let mut escaped = None; + let mut iter = bytes.iter(); + let mut pos = 0; + while let Some(i) = iter.position(|&b| escape_chars(b)) { + if escaped.is_none() { + escaped = Some(Vec::with_capacity(raw.len())); + } + let escaped = escaped.as_mut().expect("initialized"); + let new_pos = pos + i; + escaped.extend_from_slice(&bytes[pos..new_pos]); + match bytes[new_pos] { + b'<' => escaped.extend_from_slice(b"<"), + b'>' => escaped.extend_from_slice(b">"), + b'\'' => escaped.extend_from_slice(b"'"), + b'&' => escaped.extend_from_slice(b"&"), + b'"' => escaped.extend_from_slice(b"""), + + // This set of escapes handles characters that should be escaped + // in elements of xs:lists, because those characters works as + // delimiters of list elements + b'\t' => escaped.extend_from_slice(b" "), + b'\n' => escaped.extend_from_slice(b" "), + b'\r' => escaped.extend_from_slice(b" "), + b' ' => escaped.extend_from_slice(b" "), + _ => unreachable!( + "Only '<', '>','\', '&', '\"', '\\t', '\\r', '\\n', and ' ' are escaped" + ), + } + pos = new_pos + 1; + } + + if let Some(mut escaped) = escaped { + if let Some(raw) = bytes.get(pos..) { + escaped.extend_from_slice(raw); + } + #[allow(unsafe_code)] + // SAFETY: we operate on UTF-8 input and search for an one byte chars only, + // so all slices that was put to the `escaped` is a valid UTF-8 encoded strings + Cow::Owned(unsafe { String::from_utf8_unchecked(escaped) }) + } else { + Cow::Borrowed(raw) + } +} diff --git a/crates/oxc_diagnostics/src/reporter/graphical.rs b/crates/oxc_diagnostics/src/reporter/graphical.rs new file mode 100644 index 000000000..a9b49cdc2 --- /dev/null +++ b/crates/oxc_diagnostics/src/reporter/graphical.rs @@ -0,0 +1,32 @@ +use std::io::{BufWriter, Stdout, Write}; + +use crate::{miette::Error, GraphicalReportHandler}; + +use super::{writer, DiagnosticReporter}; + +pub struct GraphicalReporter { + handler: GraphicalReportHandler, + writer: BufWriter, +} + +impl Default for GraphicalReporter { + fn default() -> Self { + Self { handler: GraphicalReportHandler::new(), writer: writer() } + } +} + +impl DiagnosticReporter for GraphicalReporter { + fn finish(&mut self) { + self.writer.flush().unwrap(); + } + + fn render_diagnostics(&mut self, s: &[u8]) { + self.writer.write_all(s).unwrap(); + } + + fn render_error(&mut self, error: Error) -> Option { + let mut output = String::new(); + self.handler.render_report(&mut output, error.as_ref()).unwrap(); + Some(output) + } +} diff --git a/crates/oxc_diagnostics/src/reporter/json.rs b/crates/oxc_diagnostics/src/reporter/json.rs new file mode 100644 index 000000000..46f6ca00c --- /dev/null +++ b/crates/oxc_diagnostics/src/reporter/json.rs @@ -0,0 +1,38 @@ +use crate::miette::{Error, JSONReportHandler}; + +use super::DiagnosticReporter; + +#[derive(Default)] +pub struct JsonReporter { + diagnostics: Vec, +} + +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) { + format_json(&mut self.diagnostics); + } + + fn render_diagnostics(&mut self, _s: &[u8]) {} + + fn render_error(&mut self, error: Error) -> Option { + self.diagnostics.push(error); + None + } +} + +/// +fn format_json(diagnostics: &mut Vec) { + let handler = JSONReportHandler::new(); + let messages = diagnostics + .drain(..) + .map(|error| { + let mut output = String::from("\t"); + handler.render_report(&mut output, error.as_ref()).unwrap(); + output + }) + .collect::>() + .join(",\n"); + println!("[\n{messages}\n]"); +} diff --git a/crates/oxc_diagnostics/src/reporter/mod.rs b/crates/oxc_diagnostics/src/reporter/mod.rs new file mode 100644 index 000000000..4536bdb8b --- /dev/null +++ b/crates/oxc_diagnostics/src/reporter/mod.rs @@ -0,0 +1,68 @@ +mod checkstyle; +mod graphical; +mod json; +mod unix; + +pub use self::{ + checkstyle::CheckstyleReporter, graphical::GraphicalReporter, json::JsonReporter, + unix::UnixReporter, +}; + +use std::io::{BufWriter, Stdout}; + +use crate::{miette::Error, Severity}; + +/// stdio is blocked by LineWriter, use a BufWriter to reduce syscalls. +/// See `https://github.com/rust-lang/rust/issues/60673`. +fn writer() -> BufWriter { + BufWriter::new(std::io::stdout()) +} + +pub trait DiagnosticReporter { + fn finish(&mut self); + fn render_diagnostics(&mut self, s: &[u8]); + fn render_error(&mut self, error: Error) -> Option; +} + +struct Info { + line: usize, + column: usize, + filename: String, + message: String, + severity: Severity, + rule_id: Option, +} + +impl Info { + fn new(diagnostic: &Error) -> Self { + let mut line = 0; + let mut column = 0; + let mut filename = String::new(); + let mut message = String::new(); + let mut severity = Severity::Warning; + let mut rule_id = None; + if let Some(mut labels) = diagnostic.labels() { + if let Some(source) = diagnostic.source_code() { + if let Some(label) = labels.next() { + if let Ok(span_content) = source.read_span(label.inner(), 0, 0) { + line = span_content.line() + 1; + column = span_content.column() + 1; + if let Some(name) = span_content.name() { + filename = name.to_string(); + }; + if matches!(diagnostic.severity(), Some(Severity::Error)) { + severity = Severity::Error; + } + let msg = diagnostic.to_string(); + // Our messages usually comes with `eslint(rule): message` + (rule_id, message) = msg.split_once(':').map_or_else( + || (None, msg.to_string()), + |(id, msg)| (Some(id.to_string()), msg.trim().to_string()), + ); + } + } + } + } + Self { line, column, filename, message, severity, rule_id } + } +} diff --git a/crates/oxc_diagnostics/src/reporter/unix.rs b/crates/oxc_diagnostics/src/reporter/unix.rs new file mode 100644 index 000000000..8883f4dc9 --- /dev/null +++ b/crates/oxc_diagnostics/src/reporter/unix.rs @@ -0,0 +1,51 @@ +use std::{ + borrow::Cow, + io::{BufWriter, Stdout, Write}, +}; + +use crate::{miette::Error, Severity}; + +use super::{writer, DiagnosticReporter, Info}; + +pub struct UnixReporter { + total: usize, + writer: BufWriter, +} + +impl Default for UnixReporter { + fn default() -> Self { + Self { total: 0, writer: writer() } + } +} + +impl DiagnosticReporter for UnixReporter { + fn finish(&mut self) { + let total = self.total; + if total > 0 { + let line = format!("\n{total} problem{}\n", if total > 1 { "s" } else { "" }); + self.writer.write_all(line.as_bytes()).unwrap(); + } + self.writer.flush().unwrap(); + } + + fn render_diagnostics(&mut self, s: &[u8]) { + self.writer.write_all(s).unwrap(); + } + + fn render_error(&mut self, error: Error) -> Option { + self.total += 1; + Some(format_unix(&error)) + } +} + +/// +fn format_unix(diagnostic: &Error) -> String { + let Info { line, column, filename, message, severity, rule_id } = Info::new(diagnostic); + let severity = match severity { + Severity::Error => "Error", + _ => "Warning", + }; + let rule_id = + rule_id.map_or_else(|| Cow::Borrowed(""), |rule_id| Cow::Owned(format!("/{rule_id}"))); + format!("{filename}:{line}:{column}: {message} [{severity}{rule_id}]\n") +} diff --git a/crates/oxc_diagnostics/src/service.rs b/crates/oxc_diagnostics/src/service.rs index 5d5773009..058fd09e5 100644 --- a/crates/oxc_diagnostics/src/service.rs +++ b/crates/oxc_diagnostics/src/service.rs @@ -5,7 +5,11 @@ use std::{ }; use crate::{ - miette::NamedSource, reporter::DiagnosticReporter, Error, MinifiedFileError, Severity, + miette::NamedSource, + reporter::{ + CheckstyleReporter, DiagnosticReporter, GraphicalReporter, JsonReporter, UnixReporter, + }, + Error, MinifiedFileError, Severity, }; pub type DiagnosticTuple = (PathBuf, Vec); @@ -13,7 +17,7 @@ pub type DiagnosticSender = mpsc::Sender>; pub type DiagnosticReceiver = mpsc::Receiver>; pub struct DiagnosticService { - reporter: DiagnosticReporter, + reporter: Box, /// Disable reporting on warnings, only errors are reported quiet: bool, @@ -36,7 +40,7 @@ impl Default for DiagnosticService { fn default() -> Self { let (sender, receiver) = mpsc::channel(); Self { - reporter: DiagnosticReporter::new_graphical(), + reporter: Box::::default(), quiet: false, max_warnings: None, warnings_count: Cell::new(0), @@ -49,19 +53,15 @@ impl Default for DiagnosticService { impl DiagnosticService { pub fn set_json_reporter(&mut self) { - self.reporter = DiagnosticReporter::new_json(); + self.reporter = Box::::default(); } pub fn set_unix_reporter(&mut self) { - self.reporter = DiagnosticReporter::new_unix(); + self.reporter = Box::::default(); } pub fn set_checkstyle_reporter(&mut self) { - self.reporter = DiagnosticReporter::new_checkstyle(); - } - - pub fn is_graphical_output(&self) -> bool { - matches!(self.reporter, DiagnosticReporter::Graphical { .. }) + self.reporter = Box::::default(); } #[must_use]