Skip to content

Commit

Permalink
refactor(linter): move DiagnosticsReporters to oxlint (#8454)
Browse files Browse the repository at this point in the history
it was never the plan that oxc_diagnostics should be the
`std::io::stdout` writer:

https://github.com/oxc-project/oxc/blob/8fc238ac34462945cac66dfa3c5c5f7993cc7df4/crates/oxc_diagnostics/src/service.rs#L84-L85

This PR does refactor all reporters to `oxlint` crate and make the
current implementation of `oxc_diagnostics` public for others to
consume.

I added some tests to reflect to expected output (and found some bugs^^)

For the future I think the BufWriter for `std::io::stdout` should come
from outside.
Or maybe `std::fmt::stdout`? I do not know^^"

I could not test 100% of the code, I hope I can fix this with the next
PR which will include a own Tester for oxlint (like `oxc_linter`).
Please be extra careful when reviewing it.

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
  • Loading branch information
Sysix and autofix-ci[bot] authored Jan 16, 2025
1 parent 9ec4e24 commit b4c87e2
Show file tree
Hide file tree
Showing 17 changed files with 484 additions and 326 deletions.
2 changes: 1 addition & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions apps/oxlint/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ bpaf = { workspace = true, features = ["autocomplete", "bright-color", "derive"]
ignore = { workspace = true, features = ["simd-accel"] }
miette = { workspace = true }
rayon = { workspace = true }
rustc-hash = { workspace = true }
serde = { workspace = true }
serde_json = { workspace = true }
tempfile = { workspace = true }
Expand Down
34 changes: 13 additions & 21 deletions apps/oxlint/src/lint.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
use std::{
env, fs,
io::BufWriter,
io::{BufWriter, Write},
path::{Path, PathBuf},
time::Instant,
};

use ignore::gitignore::Gitignore;

use oxc_diagnostics::{DiagnosticService, GraphicalReportHandler};
use oxc_linter::{
loader::LINT_PARTIAL_LOADER_EXT, AllowWarnDeny, ConfigStoreBuilder, InvalidFilterKind,
Expand All @@ -15,9 +14,7 @@ use oxc_linter::{
use oxc_span::VALID_EXTENSIONS;

use crate::{
cli::{
CliRunResult, LintCommand, LintResult, MiscOptions, OutputOptions, Runner, WarningOptions,
},
cli::{CliRunResult, LintCommand, LintResult, MiscOptions, Runner, WarningOptions},
output_formatter::{OutputFormat, OutputFormatter},
walk::{Extensions, Walk},
};
Expand All @@ -37,11 +34,15 @@ impl Runner for LintRunner {

fn run(self) -> CliRunResult {
let format_str = self.options.output_options.format;
let output_formatter = OutputFormatter::new(format_str);
let mut output_formatter = OutputFormatter::new(format_str);

// stdio is blocked by LineWriter, use a BufWriter to reduce syscalls.
// See `https://github.com/rust-lang/rust/issues/60673`.
let mut stdout = BufWriter::new(std::io::stdout());

if self.options.list_rules {
let mut stdout = BufWriter::new(std::io::stdout());
output_formatter.all_rules(&mut stdout);
stdout.flush().unwrap();
return CliRunResult::None;
}

Expand Down Expand Up @@ -180,7 +181,7 @@ impl Runner for LintRunner {

let lint_service = LintService::new(linter, options);
let mut diagnostic_service =
Self::get_diagnostic_service(&warning_options, &output_options, &misc_options);
Self::get_diagnostic_service(&output_formatter, &warning_options, &misc_options);

// Spawn linting in another thread so diagnostics can be printed immediately from diagnostic_service.run.
rayon::spawn({
Expand All @@ -190,7 +191,7 @@ impl Runner for LintRunner {
lint_service.run(&tx_error);
}
});
diagnostic_service.run();
diagnostic_service.run(&mut stdout);

CliRunResult::LintResult(LintResult {
duration: now.elapsed(),
Expand All @@ -215,23 +216,14 @@ impl LintRunner {
}

fn get_diagnostic_service(
reporter: &OutputFormatter,
warning_options: &WarningOptions,
output_options: &OutputOptions,
misc_options: &MiscOptions,
) -> DiagnosticService {
let mut diagnostic_service = DiagnosticService::default()
DiagnosticService::new(reporter.get_diagnostic_reporter())
.with_quiet(warning_options.quiet)
.with_silent(misc_options.silent)
.with_max_warnings(warning_options.max_warnings);

match output_options.format {
OutputFormat::Default => {}
OutputFormat::Json => diagnostic_service.set_json_reporter(),
OutputFormat::Unix => diagnostic_service.set_unix_reporter(),
OutputFormat::Checkstyle => diagnostic_service.set_checkstyle_reporter(),
OutputFormat::Github => diagnostic_service.set_github_reporter(),
}
diagnostic_service
.with_max_warnings(warning_options.max_warnings)
}

// moved into a separate function for readability, but it's only ever used
Expand Down
Original file line number Diff line number Diff line change
@@ -1,30 +1,47 @@
use std::borrow::Cow;
use std::{borrow::Cow, io::Write};

use rustc_hash::FxHashMap;

use super::{DiagnosticReporter, Info};
use crate::{Error, Severity};
use oxc_diagnostics::{
reporter::{DiagnosticReporter, Info},
Error, Severity,
};

use crate::output_formatter::InternalFormatter;

#[derive(Debug, Default)]
pub struct CheckStyleOutputFormatter;

impl InternalFormatter for CheckStyleOutputFormatter {
fn all_rules(&mut self, writer: &mut dyn Write) {
writeln!(writer, "flag --rules with flag --format=checkstyle is not allowed").unwrap();
}

fn get_diagnostic_reporter(&self) -> Box<dyn DiagnosticReporter> {
Box::new(CheckstyleReporter::default())
}
}

/// Reporter to output diagnostics in checkstyle format
///
/// Checkstyle Format Documentation: <https://checkstyle.sourceforge.io/>
#[derive(Default)]
pub struct CheckstyleReporter {
struct CheckstyleReporter {
diagnostics: Vec<Error>,
}

impl DiagnosticReporter for CheckstyleReporter {
fn finish(&mut self) {
format_checkstyle(&self.diagnostics);
fn finish(&mut self) -> Option<String> {
Some(format_checkstyle(&self.diagnostics))
}

fn render_diagnostics(&mut self, _s: &[u8]) {}

fn render_error(&mut self, error: Error) -> Option<String> {
self.diagnostics.push(error);
None
}
}

#[allow(clippy::print_stdout)]
fn format_checkstyle(diagnostics: &[Error]) {
fn format_checkstyle(diagnostics: &[Error]) -> String {
let infos = diagnostics.iter().map(Info::new).collect::<Vec<_>>();
let mut grouped: FxHashMap<String, Vec<Info>> = FxHashMap::default();
for info in infos {
Expand All @@ -48,9 +65,9 @@ fn format_checkstyle(diagnostics: &[Error]) {
let filename = &infos[0].filename;
format!(r#"<file name="{filename}">{messages}</file>"#)
}).collect::<Vec<_>>().join(" ");
println!(
format!(
r#"<?xml version="1.0" encoding="utf-8"?><checkstyle version="4.3">{messages}</checkstyle>"#
);
)
}

/// <https://github.com/tafia/quick-xml/blob/6e34a730853fe295d68dc28460153f08a5a12955/src/escapei.rs#L84-L86>
Expand Down Expand Up @@ -103,3 +120,31 @@ fn xml_escape_impl<F: Fn(u8) -> bool>(raw: &str, escape_chars: F) -> Cow<str> {
Cow::Borrowed(raw)
}
}

#[cfg(test)]
mod test {
use oxc_diagnostics::{reporter::DiagnosticReporter, NamedSource, OxcDiagnostic};
use oxc_span::Span;

use super::CheckstyleReporter;

#[test]
fn reporter() {
let mut reporter = CheckstyleReporter::default();

let error = OxcDiagnostic::warn("error message")
.with_label(Span::new(0, 8))
.with_source_code(NamedSource::new("file://test.ts", "debugger;"));

let first_result = reporter.render_error(error);

// reporter keeps it in memory
assert!(first_result.is_none());

// report not gives us all diagnostics at ones
let second_result = reporter.finish();

assert!(second_result.is_some());
assert_eq!(second_result.unwrap(), "<?xml version=\"1.0\" encoding=\"utf-8\"?><checkstyle version=\"4.3\"><file name=\"file://test.ts\"><error line=\"1\" column=\"1\" severity=\"warning\" message=\"error message\" source=\"\" /></file></checkstyle>");
}
}
83 changes: 79 additions & 4 deletions apps/oxlint/src/output_formatter/default.rs
Original file line number Diff line number Diff line change
@@ -1,29 +1,104 @@
use std::io::Write;

use oxc_diagnostics::{reporter::DiagnosticReporter, Error, GraphicalReportHandler};
use oxc_linter::table::RuleTable;

use crate::output_formatter::InternalFormatter;

#[derive(Debug)]
pub struct DefaultOutputFormatter;

impl DefaultOutputFormatter {
pub fn all_rules<T: Write>(writer: &mut T) {
impl InternalFormatter for DefaultOutputFormatter {
fn all_rules(&mut self, writer: &mut dyn Write) {
let table = RuleTable::new();
for section in table.sections {
writeln!(writer, "{}", section.render_markdown_table(None)).unwrap();
}
writeln!(writer, "Default: {}", table.turned_on_by_default_count).unwrap();
writeln!(writer, "Total: {}", table.total).unwrap();
}

fn get_diagnostic_reporter(&self) -> Box<dyn DiagnosticReporter> {
Box::new(GraphicalReporter::default())
}
}

/// Pretty-prints diagnostics. Primarily meant for human-readable output in a terminal.
///
/// See [`GraphicalReportHandler`] for how to configure colors, context lines, etc.
struct GraphicalReporter {
handler: GraphicalReportHandler,
}

impl Default for GraphicalReporter {
fn default() -> Self {
Self { handler: GraphicalReportHandler::new() }
}
}

impl DiagnosticReporter for GraphicalReporter {
fn finish(&mut self) -> Option<String> {
None
}

fn render_error(&mut self, error: Error) -> Option<String> {
let mut output = String::new();
self.handler.render_report(&mut output, error.as_ref()).unwrap();
Some(output)
}
}
impl GraphicalReporter {
#[cfg(test)]
pub fn with_handler(mut self, handler: GraphicalReportHandler) -> Self {
self.handler = handler;
self
}
}

#[cfg(test)]
mod test {
use crate::output_formatter::default::DefaultOutputFormatter;
use crate::output_formatter::{
default::{DefaultOutputFormatter, GraphicalReporter},
InternalFormatter,
};
use miette::{GraphicalReportHandler, GraphicalTheme, NamedSource};
use oxc_diagnostics::{reporter::DiagnosticReporter, OxcDiagnostic};
use oxc_span::Span;

#[test]
fn all_rules() {
let mut writer = Vec::new();
let mut formatter = DefaultOutputFormatter;

DefaultOutputFormatter::all_rules(&mut writer);
formatter.all_rules(&mut writer);
assert!(!writer.is_empty());
}

#[test]
fn reporter_finish() {
let mut reporter = GraphicalReporter::default();

let result = reporter.finish();

assert!(result.is_none());
}

#[test]
fn reporter_error() {
let mut reporter = GraphicalReporter::default().with_handler(
GraphicalReportHandler::new_themed(GraphicalTheme::none()).with_links(false),
);

let error = OxcDiagnostic::warn("error message")
.with_label(Span::new(0, 8))
.with_source_code(NamedSource::new("file://test.ts", "debugger;"));

let result = reporter.render_error(error);

assert!(result.is_some());
assert_eq!(
result.unwrap(),
"\n ! error message\n ,-[file://test.ts:1:1]\n 1 | debugger;\n : ^^^^^^^^\n `----\n"
);
}
}
Loading

0 comments on commit b4c87e2

Please sign in to comment.