Skip to content

Commit

Permalink
refactor(linter): move finishing default diagnostic message to `Graph…
Browse files Browse the repository at this point in the history
…icalReporter`
  • Loading branch information
Sysix committed Jan 23, 2025
1 parent 0c5bb30 commit d787cf5
Show file tree
Hide file tree
Showing 10 changed files with 192 additions and 89 deletions.
43 changes: 27 additions & 16 deletions apps/oxlint/src/lint.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use std::{
env, fs,
io::{BufWriter, Write},
io::{BufWriter, ErrorKind, Write},
path::{Path, PathBuf},
time::Instant,
};
Expand All @@ -16,7 +16,7 @@ use serde_json::Value;

use crate::{
cli::{CliRunResult, LintCommand, LintResult, MiscOptions, Runner, WarningOptions},
output_formatter::{OutputFormat, OutputFormatter},
output_formatter::{LintCommandInfo, OutputFormatter},
walk::{Extensions, Walk},
};

Expand Down Expand Up @@ -55,7 +55,6 @@ impl Runner for LintRunner {
ignore_options,
fix_options,
enable_plugins,
output_options,
misc_options,
..
} = self.options;
Expand All @@ -81,11 +80,8 @@ impl Runner for LintRunner {
// If explicit paths were provided, but all have been
// filtered, return early.
if provided_path_count > 0 {
return CliRunResult::LintResult(LintResult {
duration: now.elapsed(),
deny_warnings: warning_options.deny_warnings,
..LintResult::default()
});
// ToDo: when oxc_linter (config) validates the configuration, we can use exit_code = 1 to fail
return CliRunResult::LintResult(LintResult::default());
}

paths.push(self.cwd.clone());
Expand Down Expand Up @@ -211,15 +207,24 @@ impl Runner for LintRunner {

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

CliRunResult::LintResult(LintResult {
duration: now.elapsed(),
let diagnostic_failed = diagnostic_result.max_warnings_exceeded()
|| diagnostic_result.errors_count() > 0
|| (warning_options.deny_warnings && diagnostic_result.warnings_count() > 0);

if let Some(end) = output_formatter.lint_command_info(&LintCommandInfo {
number_of_files,
number_of_rules: lint_service.linter().number_of_rules(),
threads_count: rayon::current_num_threads(),
start_time: now.elapsed(),
}) {
stdout.write_all(end.as_bytes()).or_else(Self::check_for_writer_error).unwrap();
};

CliRunResult::LintResult(LintResult {
number_of_files,
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),
exit_code: u8::from(diagnostic_failed),
})
}
}
Expand Down Expand Up @@ -306,6 +311,15 @@ impl LintRunner {
let config_path = cwd.join(Self::DEFAULT_OXLINTRC);
Oxlintrc::from_file(&config_path).or_else(|_| Ok(Oxlintrc::default()))
}

fn check_for_writer_error(error: std::io::Error) -> Result<(), std::io::Error> {
// Do not panic when the process is skill (e.g. piping into `less`).
if matches!(error.kind(), ErrorKind::Interrupted | ErrorKind::BrokenPipe) {
Ok(())
} else {
Err(error)
}
}
}

#[cfg(test)]
Expand Down Expand Up @@ -364,7 +378,6 @@ mod test {
fn no_arg() {
let args = &[];
let result = test(args);
assert!(result.number_of_rules > 0);
assert!(result.number_of_warnings > 0);
assert_eq!(result.number_of_errors, 0);
}
Expand All @@ -373,7 +386,6 @@ mod test {
fn dir() {
let args = &["fixtures/linter"];
let result = test(args);
assert!(result.number_of_rules > 0);
assert_eq!(result.number_of_files, 3);
assert_eq!(result.number_of_warnings, 3);
assert_eq!(result.number_of_errors, 0);
Expand All @@ -383,7 +395,6 @@ mod test {
fn cwd() {
let args = &["debugger.js"];
let result = test_with_cwd("fixtures/linter", args);
assert!(result.number_of_rules > 0);
assert_eq!(result.number_of_files, 1);
assert_eq!(result.number_of_warnings, 1);
assert_eq!(result.number_of_errors, 0);
Expand Down
4 changes: 4 additions & 0 deletions apps/oxlint/src/output_formatter/checkstyle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ impl InternalFormatter for CheckStyleOutputFormatter {
writeln!(writer, "flag --rules with flag --format=checkstyle is not allowed").unwrap();
}

fn lint_command_info(&self, _: &super::LintCommandInfo) -> Option<String> {
None
}

fn get_diagnostic_reporter(&self) -> Box<dyn DiagnosticReporter> {
Box::new(CheckstyleReporter::default())
}
Expand Down
94 changes: 89 additions & 5 deletions apps/oxlint/src/output_formatter/default.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::io::Write;
use std::{io::Write, time::Duration};

use oxc_diagnostics::{
reporter::{DiagnosticReporter, DiagnosticResult},
Expand All @@ -21,11 +21,34 @@ impl InternalFormatter for DefaultOutputFormatter {
writeln!(writer, "Total: {}", table.total).unwrap();
}

fn lint_command_info(&self, lint_command_info: &super::LintCommandInfo) -> Option<String> {
let time = Self::get_execution_time(&lint_command_info.start_time);
let s = if lint_command_info.number_of_files == 1 { "" } else { "s" };

Some(format!(
"Finished in {time} on {} file{s} with {} rules using {} threads.\n",
lint_command_info.number_of_files,
lint_command_info.number_of_rules,
lint_command_info.threads_count
))
}

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

impl DefaultOutputFormatter {
fn get_execution_time(duration: &Duration) -> String {
let ms = duration.as_millis();
if ms < 1000 {
format!("{ms}ms")
} else {
format!("{:.1}s", duration.as_secs_f64())
}
}
}

/// Pretty-prints diagnostics. Primarily meant for human-readable output in a terminal.
///
/// See [`GraphicalReportHandler`] for how to configure colors, context lines, etc.
Expand All @@ -40,8 +63,35 @@ impl Default for GraphicalReporter {
}

impl DiagnosticReporter for GraphicalReporter {
fn finish(&mut self, _: &DiagnosticResult) -> Option<String> {
None
fn finish(&mut self, result: &DiagnosticResult) -> Option<String> {
let mut output = String::new();

if result.warnings_count() + result.errors_count() > 0 {
output.push('\n');
}

output.push_str(
format!(
"Found {} warning{} and {} error{}.\n",
result.warnings_count(),
if result.warnings_count() == 1 { "" } else { "s" },
result.errors_count(),
if result.errors_count() == 1 { "" } else { "s" },
)
.as_str(),
);

if result.max_warnings_exceeded() {
output.push_str(
format!(
"Exceeded maximum number of warnings. Found {}.\n",
result.warnings_count()
)
.as_str(),
);
}

Some(output)
}

fn render_error(&mut self, error: Error) -> Option<String> {
Expand Down Expand Up @@ -81,12 +131,46 @@ mod test {
}

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

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

assert!(result.is_none());
assert!(result.is_some());
assert_eq!(result.unwrap(), "Found 0 warnings and 0 errors.\n");
}

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

let result = reporter.finish(&DiagnosticResult::new(1, 1, false));

assert!(result.is_some());
assert_eq!(result.unwrap(), "\nFound 1 warning and 1 error.\n");
}

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

let result = reporter.finish(&DiagnosticResult::new(6, 4, false));

assert!(result.is_some());
assert_eq!(result.unwrap(), "\nFound 6 warnings and 4 errors.\n");
}

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

let result = reporter.finish(&DiagnosticResult::new(6, 4, true));

assert!(result.is_some());
assert_eq!(
result.unwrap(),
"\nFound 6 warnings and 4 errors.\nExceeded maximum number of warnings. Found 6.\n"
);
}

#[test]
Expand Down
4 changes: 4 additions & 0 deletions apps/oxlint/src/output_formatter/github.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ impl InternalFormatter for GithubOutputFormatter {
writeln!(writer, "flag --rules with flag --format=github is not allowed").unwrap();
}

fn lint_command_info(&self, _: &super::LintCommandInfo) -> Option<String> {
None
}

fn get_diagnostic_reporter(&self) -> Box<dyn DiagnosticReporter> {
Box::new(GithubReporter)
}
Expand Down
4 changes: 4 additions & 0 deletions apps/oxlint/src/output_formatter/json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ impl InternalFormatter for JsonOutputFormatter {
.unwrap();
}

fn lint_command_info(&self, _: &super::LintCommandInfo) -> Option<String> {
None
}

fn get_diagnostic_reporter(&self) -> Box<dyn DiagnosticReporter> {
Box::new(JsonReporter::default())
}
Expand Down
26 changes: 22 additions & 4 deletions apps/oxlint/src/output_formatter/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ mod unix;

use std::io::{BufWriter, Stdout, Write};
use std::str::FromStr;
use std::time::Duration;

use checkstyle::CheckStyleOutputFormatter;
use github::GithubOutputFormatter;
Expand Down Expand Up @@ -45,19 +46,32 @@ impl FromStr for OutputFormat {
}
}

pub struct LintCommandInfo {
pub number_of_files: usize,
pub number_of_rules: usize,

pub threads_count: usize,

/// The start of the current command.
/// Some reporters want to output the duration it took to finished the task
pub start_time: Duration,
}

trait InternalFormatter {
fn all_rules(&mut self, writer: &mut dyn Write);

fn lint_command_info(&self, lint_command_info: &LintCommandInfo) -> Option<String>;

fn get_diagnostic_reporter(&self) -> Box<dyn DiagnosticReporter>;
}

pub struct OutputFormatter {
internal_formatter: Box<dyn InternalFormatter>,
internal: Box<dyn InternalFormatter>,
}

impl OutputFormatter {
pub fn new(format: OutputFormat) -> Self {
Self { internal_formatter: Self::get_internal_formatter(format) }
Self { internal: Self::get_internal_formatter(format) }
}

fn get_internal_formatter(format: OutputFormat) -> Box<dyn InternalFormatter> {
Expand All @@ -72,10 +86,14 @@ impl OutputFormatter {
}

pub fn all_rules(&mut self, writer: &mut BufWriter<Stdout>) {
self.internal_formatter.all_rules(writer);
self.internal.all_rules(writer);
}

pub fn lint_command_info(&self, lint_command_info: &LintCommandInfo) -> Option<String> {
self.internal.lint_command_info(lint_command_info)
}

pub fn get_diagnostic_reporter(&self) -> Box<dyn DiagnosticReporter> {
self.internal_formatter.get_diagnostic_reporter()
self.internal.get_diagnostic_reporter()
}
}
4 changes: 4 additions & 0 deletions apps/oxlint/src/output_formatter/stylish.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ impl InternalFormatter for StylishOutputFormatter {
writeln!(writer, "flag --rules with flag --format=stylish is not allowed").unwrap();
}

fn lint_command_info(&self, _: &super::LintCommandInfo) -> Option<String> {
None
}

fn get_diagnostic_reporter(&self) -> Box<dyn DiagnosticReporter> {
Box::new(StylishReporter::default())
}
Expand Down
4 changes: 4 additions & 0 deletions apps/oxlint/src/output_formatter/unix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ impl InternalFormatter for UnixOutputFormatter {
writeln!(writer, "flag --rules with flag --format=unix is not allowed").unwrap();
}

fn lint_command_info(&self, _: &super::LintCommandInfo) -> Option<String> {
None
}

fn get_diagnostic_reporter(&self) -> Box<dyn DiagnosticReporter> {
Box::new(UnixReporter::default())
}
Expand Down
Loading

0 comments on commit d787cf5

Please sign in to comment.