Skip to content

Commit

Permalink
feat(linter): add DiagnosticResult to the Reporters for receiving a s…
Browse files Browse the repository at this point in the history
…ub part result (#8666)

We are currently outputting only for the default-outputter some extra
information:


https://github.com/oxc-project/oxc/blob/3be03926e8a5097e5c1fe249b8bff0009ec4468d/apps/oxlint/src/result.rs#L61-L87

My goal is that all information will be passed to our new
DiagnosticReporter / OutputFormatter.
This will break the output format in the next PR. **Merging this PR is
the OK for me to make this change** ⚠️
The only breaking point:
`"Found {number_of_warnings} warning{} and {number_of_errors} error{}."`
will still be outputted when `max_warnings_exceeded` is true.

Because this is something the `DiagnosticReporter` should do and not the
`OutputFormatter`.

The end goal is:
- no `println!`, our `OutputFormatter` and his `DiagnosticReporter` will
return `Option<String>` and we output it the our `stdout`
- `LintResult` will only handle `ExitCode` result and nothing more
- `stdout` can be changed from outside (for the next part)
- add snapshots with a custom `stdout`

I do not know if all goals can be done easily. Last two parts should be
a bit tricky for me, never did test setups in rust.
But we do never stop to learn ;)
  • Loading branch information
Sysix authored Jan 23, 2025
1 parent db863a3 commit 4ae568e
Show file tree
Hide file tree
Showing 9 changed files with 114 additions and 66 deletions.
9 changes: 5 additions & 4 deletions apps/oxlint/src/lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -208,15 +208,16 @@ impl Runner for LintRunner {
lint_service.run(&tx_error);
}
});
diagnostic_service.run(&mut stdout);

let diagnostic_result = diagnostic_service.run(&mut stdout);

CliRunResult::LintResult(LintResult {
duration: now.elapsed(),
number_of_rules: lint_service.linter().number_of_rules(),
number_of_files,
number_of_warnings: diagnostic_service.warnings_count(),
number_of_errors: diagnostic_service.errors_count(),
max_warnings_exceeded: diagnostic_service.max_warnings_exceeded(),
number_of_warnings: diagnostic_result.warnings_count(),
number_of_errors: diagnostic_result.errors_count(),
max_warnings_exceeded: diagnostic_result.max_warnings_exceeded(),
deny_warnings: warning_options.deny_warnings,
print_summary: matches!(output_options.format, OutputFormat::Default),
})
Expand Down
11 changes: 7 additions & 4 deletions apps/oxlint/src/output_formatter/checkstyle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::{borrow::Cow, io::Write};
use rustc_hash::FxHashMap;

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

Expand Down Expand Up @@ -31,7 +31,7 @@ struct CheckstyleReporter {
}

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

Expand Down Expand Up @@ -123,7 +123,10 @@ fn xml_escape_impl<F: Fn(u8) -> bool>(raw: &str, escape_chars: F) -> Cow<str> {

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

use super::CheckstyleReporter;
Expand All @@ -142,7 +145,7 @@ mod test {
assert!(first_result.is_none());

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

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>");
Expand Down
14 changes: 10 additions & 4 deletions apps/oxlint/src/output_formatter/default.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
use std::io::Write;

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

use crate::output_formatter::InternalFormatter;
Expand Down Expand Up @@ -37,7 +40,7 @@ impl Default for GraphicalReporter {
}

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

Expand All @@ -62,7 +65,10 @@ mod test {
InternalFormatter,
};
use miette::{GraphicalReportHandler, GraphicalTheme, NamedSource};
use oxc_diagnostics::{reporter::DiagnosticReporter, OxcDiagnostic};
use oxc_diagnostics::{
reporter::{DiagnosticReporter, DiagnosticResult},
OxcDiagnostic,
};
use oxc_span::Span;

#[test]
Expand All @@ -78,7 +84,7 @@ mod test {
fn reporter_finish() {
let mut reporter = GraphicalReporter::default();

let result = reporter.finish();
let result = reporter.finish(&DiagnosticResult::default());

assert!(result.is_none());
}
Expand Down
11 changes: 7 additions & 4 deletions apps/oxlint/src/output_formatter/github.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use std::{borrow::Cow, io::Write};

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

Expand All @@ -25,7 +25,7 @@ impl InternalFormatter for GithubOutputFormatter {
struct GithubReporter;

impl DiagnosticReporter for GithubReporter {
fn finish(&mut self) -> Option<String> {
fn finish(&mut self, _: &DiagnosticResult) -> Option<String> {
None
}

Expand Down Expand Up @@ -88,7 +88,10 @@ fn escape_property(value: &str) -> String {

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

use super::GithubReporter;
Expand All @@ -97,7 +100,7 @@ mod test {
fn reporter_finish() {
let mut reporter = GithubReporter;

let result = reporter.finish();
let result = reporter.finish(&DiagnosticResult::default());

assert!(result.is_none());
}
Expand Down
10 changes: 7 additions & 3 deletions apps/oxlint/src/output_formatter/json.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use std::io::Write;

use oxc_diagnostics::reporter::DiagnosticResult;
use oxc_diagnostics::{reporter::DiagnosticReporter, Error};
use oxc_linter::rules::RULES;
use oxc_linter::RuleCategory;
Expand Down Expand Up @@ -52,7 +53,7 @@ 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) -> Option<String> {
fn finish(&mut self, _: &DiagnosticResult) -> Option<String> {
Some(format_json(&mut self.diagnostics))
}

Expand Down Expand Up @@ -80,7 +81,10 @@ fn format_json(diagnostics: &mut Vec<Error>) -> String {

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

use super::JsonReporter;
Expand All @@ -99,7 +103,7 @@ mod test {
assert!(first_result.is_none());

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

assert!(second_result.is_some());
assert_eq!(
Expand Down
8 changes: 4 additions & 4 deletions apps/oxlint/src/output_formatter/stylish.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use std::io::Write;

use oxc_diagnostics::{
reporter::{DiagnosticReporter, Info},
reporter::{DiagnosticReporter, DiagnosticResult, Info},
Error, Severity,
};
use rustc_hash::FxHashMap;
Expand All @@ -27,7 +27,7 @@ struct StylishReporter {
}

impl DiagnosticReporter for StylishReporter {
fn finish(&mut self) -> Option<String> {
fn finish(&mut self, _: &DiagnosticResult) -> Option<String> {
Some(format_stylish(&self.diagnostics))
}

Expand Down Expand Up @@ -116,7 +116,7 @@ fn format_stylish(diagnostics: &[Error]) -> String {
#[cfg(test)]
mod test {
use super::*;
use oxc_diagnostics::{NamedSource, OxcDiagnostic};
use oxc_diagnostics::{reporter::DiagnosticResult, NamedSource, OxcDiagnostic};
use oxc_span::Span;

#[test]
Expand All @@ -134,7 +134,7 @@ mod test {
reporter.render_error(error);
reporter.render_error(warning);

let output = reporter.finish().unwrap();
let output = reporter.finish(&DiagnosticResult::default()).unwrap();

assert!(output.contains("error message"), "Output should contain 'error message'");
assert!(output.contains("warning message"), "Output should contain 'warning message'");
Expand Down
13 changes: 8 additions & 5 deletions apps/oxlint/src/output_formatter/unix.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use std::{borrow::Cow, io::Write};

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

Expand All @@ -28,7 +28,7 @@ struct UnixReporter {
}

impl DiagnosticReporter for UnixReporter {
fn finish(&mut self) -> Option<String> {
fn finish(&mut self, _: &DiagnosticResult) -> Option<String> {
let total = self.total;
if total > 0 {
return Some(format!("\n{total} problem{}\n", if total > 1 { "s" } else { "" }));
Expand Down Expand Up @@ -57,7 +57,10 @@ fn format_unix(diagnostic: &Error) -> String {

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

use super::UnixReporter;
Expand All @@ -66,7 +69,7 @@ mod test {
fn reporter_finish_empty() {
let mut reporter = UnixReporter::default();

let result = reporter.finish();
let result = reporter.finish(&DiagnosticResult::default());

assert!(result.is_none());
}
Expand All @@ -80,7 +83,7 @@ mod test {
.with_source_code(NamedSource::new("file://test.ts", "debugger;"));

let _ = reporter.render_error(error);
let result = reporter.finish();
let result = reporter.finish(&DiagnosticResult::default());

assert!(result.is_some());
assert_eq!(result.unwrap(), "\n1 problem\n");
Expand Down
38 changes: 37 additions & 1 deletion crates/oxc_diagnostics/src/reporter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,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) -> Option<String>;
fn finish(&mut self, result: &DiagnosticResult) -> 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
Expand All @@ -55,6 +55,42 @@ pub trait DiagnosticReporter {
fn render_error(&mut self, error: Error) -> Option<String>;
}

/// DiagnosticResult will be submitted to the Reporter when the [`DiagnosticService`](crate::service::DiagnosticService)
/// is finished receiving all files
#[derive(Default)]
pub struct DiagnosticResult {
/// Total number of warnings received
warnings_count: usize,

/// Total number of errors received
errors_count: usize,

/// Did the threshold for warnings exceeded the max_warnings?
/// ToDo: We giving the input from outside, let the owner calculate the result
max_warnings_exceeded: bool,
}

impl DiagnosticResult {
pub fn new(warnings_count: usize, errors_count: usize, max_warnings_exceeded: bool) -> Self {
Self { warnings_count, errors_count, max_warnings_exceeded }
}

/// Get the number of warning-level diagnostics received.
pub fn warnings_count(&self) -> usize {
self.warnings_count
}

/// Get the number of error-level diagnostics received.
pub fn errors_count(&self) -> usize {
self.errors_count
}

/// Did the threshold for warnings exceeded the max_warnings?
pub fn max_warnings_exceeded(&self) -> bool {
self.max_warnings_exceeded
}
}

pub struct Info {
pub start: InfoPosition,
pub end: InfoPosition,
Expand Down
Loading

0 comments on commit 4ae568e

Please sign in to comment.