diff --git a/apps/oxlint/src/lint.rs b/apps/oxlint/src/lint.rs index 5773c6876240ec..ddb771ea7ccd5c 100644 --- a/apps/oxlint/src/lint.rs +++ b/apps/oxlint/src/lint.rs @@ -1,6 +1,6 @@ use std::{ env, fs, - io::{BufWriter, Write}, + io::{BufWriter, ErrorKind, Write}, path::{Path, PathBuf}, time::Instant, }; @@ -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}, }; @@ -55,7 +55,6 @@ impl Runner for LintRunner { ignore_options, fix_options, enable_plugins, - output_options, misc_options, .. } = self.options; @@ -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()); @@ -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), }) } } @@ -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)] @@ -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); } @@ -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); @@ -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); diff --git a/apps/oxlint/src/output_formatter/checkstyle.rs b/apps/oxlint/src/output_formatter/checkstyle.rs index ac5bfd1e32e3d7..f28b0af93815b7 100644 --- a/apps/oxlint/src/output_formatter/checkstyle.rs +++ b/apps/oxlint/src/output_formatter/checkstyle.rs @@ -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 { + None + } + fn get_diagnostic_reporter(&self) -> Box { Box::new(CheckstyleReporter::default()) } diff --git a/apps/oxlint/src/output_formatter/default.rs b/apps/oxlint/src/output_formatter/default.rs index d2704837d4097d..c00e0c2da823f6 100644 --- a/apps/oxlint/src/output_formatter/default.rs +++ b/apps/oxlint/src/output_formatter/default.rs @@ -1,4 +1,4 @@ -use std::io::Write; +use std::{io::Write, time::Duration}; use oxc_diagnostics::{ reporter::{DiagnosticReporter, DiagnosticResult}, @@ -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 { + 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 { 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. @@ -40,8 +63,35 @@ impl Default for GraphicalReporter { } impl DiagnosticReporter for GraphicalReporter { - fn finish(&mut self, _: &DiagnosticResult) -> Option { - None + fn finish(&mut self, result: &DiagnosticResult) -> Option { + 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 { @@ -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] diff --git a/apps/oxlint/src/output_formatter/github.rs b/apps/oxlint/src/output_formatter/github.rs index 897bffcb43685b..150d0383a6dfae 100644 --- a/apps/oxlint/src/output_formatter/github.rs +++ b/apps/oxlint/src/output_formatter/github.rs @@ -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 { + None + } + fn get_diagnostic_reporter(&self) -> Box { Box::new(GithubReporter) } diff --git a/apps/oxlint/src/output_formatter/json.rs b/apps/oxlint/src/output_formatter/json.rs index 3dd6637aaa7806..e43ea783bd6fa8 100644 --- a/apps/oxlint/src/output_formatter/json.rs +++ b/apps/oxlint/src/output_formatter/json.rs @@ -36,6 +36,10 @@ impl InternalFormatter for JsonOutputFormatter { .unwrap(); } + fn lint_command_info(&self, _: &super::LintCommandInfo) -> Option { + None + } + fn get_diagnostic_reporter(&self) -> Box { Box::new(JsonReporter::default()) } diff --git a/apps/oxlint/src/output_formatter/mod.rs b/apps/oxlint/src/output_formatter/mod.rs index 31d87f02de956e..be9aa9350ad0e5 100644 --- a/apps/oxlint/src/output_formatter/mod.rs +++ b/apps/oxlint/src/output_formatter/mod.rs @@ -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; @@ -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; + fn get_diagnostic_reporter(&self) -> Box; } pub struct OutputFormatter { - internal_formatter: Box, + internal: Box, } 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 { @@ -72,10 +86,14 @@ impl OutputFormatter { } pub fn all_rules(&mut self, writer: &mut BufWriter) { - self.internal_formatter.all_rules(writer); + self.internal.all_rules(writer); + } + + pub fn lint_command_info(&self, lint_command_info: &LintCommandInfo) -> Option { + self.internal.lint_command_info(lint_command_info) } pub fn get_diagnostic_reporter(&self) -> Box { - self.internal_formatter.get_diagnostic_reporter() + self.internal.get_diagnostic_reporter() } } diff --git a/apps/oxlint/src/output_formatter/stylish.rs b/apps/oxlint/src/output_formatter/stylish.rs index c046fae484685e..a122bba592cd94 100644 --- a/apps/oxlint/src/output_formatter/stylish.rs +++ b/apps/oxlint/src/output_formatter/stylish.rs @@ -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 { + None + } + fn get_diagnostic_reporter(&self) -> Box { Box::new(StylishReporter::default()) } diff --git a/apps/oxlint/src/output_formatter/unix.rs b/apps/oxlint/src/output_formatter/unix.rs index e1f0607c6db184..783f4b21d0ddff 100644 --- a/apps/oxlint/src/output_formatter/unix.rs +++ b/apps/oxlint/src/output_formatter/unix.rs @@ -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 { + None + } + fn get_diagnostic_reporter(&self) -> Box { Box::new(UnixReporter::default()) } diff --git a/apps/oxlint/src/result.rs b/apps/oxlint/src/result.rs index 78c7eddb55fc50..a9e3ba93441228 100644 --- a/apps/oxlint/src/result.rs +++ b/apps/oxlint/src/result.rs @@ -1,7 +1,6 @@ use std::{ path::PathBuf, process::{ExitCode, Termination}, - time::Duration, }; #[derive(Debug)] @@ -17,22 +16,14 @@ pub enum CliRunResult { /// A summary of a complete linter run. #[derive(Debug, Default)] pub struct LintResult { - /// The total time it took to run the linter. - pub duration: Duration, - /// The number of lint rules that were run. - pub number_of_rules: usize, /// The number of files that were linted. pub number_of_files: usize, /// The number of warnings that were found. pub number_of_warnings: usize, /// The number of errors that were found. pub number_of_errors: usize, - /// Whether or not the maximum number of warnings was exceeded. - pub max_warnings_exceeded: bool, - /// Whether or not warnings should be treated as errors (from `--deny-warnings` for example) - pub deny_warnings: bool, - /// Whether or not to print a summary of the results - pub print_summary: bool, + /// The exit unix code for, in general 0 or 1 (from `--deny-warnings` or `--max-warnings` for example) + pub exit_code: u8, } impl Termination for CliRunResult { @@ -49,47 +40,11 @@ impl Termination for CliRunResult { ExitCode::from(1) } Self::LintResult(LintResult { - duration, - number_of_rules, - number_of_files, - number_of_warnings, - number_of_errors, - max_warnings_exceeded, - deny_warnings, - print_summary, - }) => { - if print_summary { - let threads = rayon::current_num_threads(); - let number_of_diagnostics = number_of_warnings + number_of_errors; - - if number_of_diagnostics > 0 { - println!(); - } - - let time = Self::get_execution_time(&duration); - let s = if number_of_files == 1 { "" } else { "s" }; - println!( - "Finished in {time} on {number_of_files} file{s} with {number_of_rules} rules using {threads} threads." - ); - - if max_warnings_exceeded { - println!( - "Exceeded maximum number of warnings. Found {number_of_warnings}." - ); - return ExitCode::from(1); - } - - println!( - "Found {number_of_warnings} warning{} and {number_of_errors} error{}.", - if number_of_warnings == 1 { "" } else { "s" }, - if number_of_errors == 1 { "" } else { "s" } - ); - } - - let exit_code = - u8::from((number_of_warnings > 0 && deny_warnings) || number_of_errors > 0); - ExitCode::from(exit_code) - } + number_of_files: _, // ToDo: only for tests, make snapshots + number_of_warnings: _, // ToDo: only for tests, make snapshots + number_of_errors: _, // ToDo: only for tests, make snapshots + exit_code, + }) => ExitCode::from(exit_code), Self::PrintConfigResult { config_file } => { println!("{config_file}"); ExitCode::from(0) @@ -101,14 +56,3 @@ impl Termination for CliRunResult { } } } - -impl CliRunResult { - 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()) - } - } -} diff --git a/crates/oxc_diagnostics/src/service.rs b/crates/oxc_diagnostics/src/service.rs index 16ec5a098a234e..d8b8843cb3ab0f 100644 --- a/crates/oxc_diagnostics/src/service.rs +++ b/crates/oxc_diagnostics/src/service.rs @@ -58,6 +58,11 @@ pub struct DiagnosticService { /// which can be used to force exit with an error status if there are too many warning-level rule violations in your project max_warnings: Option, + /// The number of files that were linted. + number_of_files: usize, + /// The number of lint rules that were run. + number_of_rules: usize, + sender: DiagnosticSender, receiver: DiagnosticReceiver, } @@ -67,7 +72,16 @@ impl DiagnosticService { /// provided [`DiagnosticReporter`]. pub fn new(reporter: Box) -> Self { let (sender, receiver) = mpsc::channel(); - Self { reporter, quiet: false, silent: false, max_warnings: None, sender, receiver } + Self { + reporter, + quiet: false, + silent: false, + max_warnings: None, + sender, + receiver, + number_of_files: 0, + number_of_rules: 0, + } } /// Set to `true` to only report errors and ignore warnings. @@ -106,6 +120,18 @@ impl DiagnosticService { self } + #[must_use] + pub fn with_number_of_files(mut self, number_of_files: usize) -> Self { + self.number_of_files = number_of_files; + self + } + + #[must_use] + pub fn with_number_of_rules(mut self, number_of_rules: usize) -> Self { + self.number_of_rules = number_of_rules; + self + } + /// Channel for sending [diagnostic messages] to the service. /// /// The service will only start processing diagnostics after [`run`](DiagnosticService::run)