refactor(linter): move finishing default diagnostic message to GraphicalReporter (#8683)

Now every lint output is owned by is right OutputFormatter and his DiagnosticReporter 🥳
Next step is to setup a snapshot Tester, so I can remove the ToDos.

Reorded some lines so the outfor is now for: `cargo run -p oxlint -- test.js --max-warnings=2`
```
Found 4 warnings and 0 errors.
Exceeded maximum number of warnings. Found 4.
Finished in 5ms on 1 file with 97 rules using 24 threads.
```

and for `cargo run -p oxlint -- test.js`

```
Found 4 warnings and 0 errors.
Finished in 5ms on 1 file with 97 rules using 24 threads.
```

The output time and warnings/error count wil be always printed.

apps/oxlint/src/lint.rs

@@ -1,7 +1,8 @@
 use std::{
     env, fs,
-    io::{BufWriter, Write},
+    io::{BufWriter, ErrorKind, Write},
     path::{Path, PathBuf},
+    process::ExitCode,
     time::Instant,
 };
 
@@ -16,7 +17,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 +56,6 @@ impl Runner for LintRunner {
             ignore_options,
             fix_options,
             enable_plugins,
-            output_options,
             misc_options,
             ..
         } = self.options;
@@ -81,11 +81,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 +208,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: ExitCode::from(u8::from(diagnostic_failed)),
         })
     }
 }
@@ -306,6 +312,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 killed (e.g. piping into `less`).
+        if matches!(error.kind(), ErrorKind::Interrupted | ErrorKind::BrokenPipe) {
+            Ok(())
+        } else {
+            Err(error)
+        }
+    }
 }
 
 #[cfg(test)]
@@ -364,7 +379,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 +387,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 +396,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);

apps/oxlint/src/output_formatter/checkstyle.rs

@@ -66,7 +66,7 @@ fn format_checkstyle(diagnostics: &[Error]) -> String {
          format!(r#"<file name="{filename}">{messages}</file>"#)
      }).collect::<Vec<_>>().join(" ");
     format!(
-        r#"<?xml version="1.0" encoding="utf-8"?><checkstyle version="4.3">{messages}</checkstyle>"#
+        "<?xml version=\"1.0\" encoding=\"utf-8\"?><checkstyle version=\"4.3\">{messages}</checkstyle>\n"
     )
 }
 
@@ -148,6 +148,6 @@ mod test {
         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>");
+        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>\n");
     }
 }

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<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.
@@ -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> {
@@ -60,9 +110,11 @@ impl GraphicalReporter {
 
 #[cfg(test)]
 mod test {
+    use std::time::Duration;
+
     use crate::output_formatter::{
         default::{DefaultOutputFormatter, GraphicalReporter},
-        InternalFormatter,
+        InternalFormatter, LintCommandInfo,
     };
     use miette::{GraphicalReportHandler, GraphicalTheme, NamedSource};
     use oxc_diagnostics::{
@@ -81,12 +133,63 @@ mod test {
     }
 
     #[test]
-    fn reporter_finish() {
+    fn lint_command_info() {
+        let formatter = DefaultOutputFormatter;
+        let result = formatter.lint_command_info(&LintCommandInfo {
+            number_of_files: 5,
+            number_of_rules: 10,
+            threads_count: 12,
+            start_time: Duration::new(1, 0),
+        });
+
+        assert!(result.is_some());
+        assert_eq!(
+            result.unwrap(),
+            "Finished in 1.0s on 5 files with 10 rules using 12 threads.\n"
+        );
+    }
+
+    #[test]
+    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]

apps/oxlint/src/output_formatter/json.rs

@@ -76,7 +76,7 @@ fn format_json(diagnostics: &mut Vec<Error>) -> String {
         })
         .collect::<Vec<_>>()
         .join(",\n");
-    format!("[\n{messages}\n]")
+    format!("[\n{messages}\n]\n")
 }
 
 #[cfg(test)]
@@ -108,7 +108,7 @@ mod test {
         assert!(second_result.is_some());
         assert_eq!(
             second_result.unwrap(),
-            "[\n\t{\"message\": \"error message\",\"severity\": \"warning\",\"causes\": [],\"filename\": \"file://test.ts\",\"labels\": [{\"span\": {\"offset\": 0,\"length\": 8}}],\"related\": []}\n]"
+            "[\n\t{\"message\": \"error message\",\"severity\": \"warning\",\"causes\": [],\"filename\": \"file://test.ts\",\"labels\": [{\"span\": {\"offset\": 0,\"length\": 8}}],\"related\": []}\n]\n"
         );
     }
 }

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,45 @@ impl FromStr for OutputFormat {
     }
 }
 
+/// Some extra lint information, which can be outputted
+/// at the end of the command
+pub struct LintCommandInfo {
+    /// The number of files that were linted.
+    pub number_of_files: usize,
+    /// The number of lint rules that were run.
+    pub number_of_rules: usize,
+    /// The used CPU threads count
+    pub threads_count: usize,
+    /// Some reporters want to output the duration it took to finished the task
+    pub start_time: Duration,
+}
+
+/// An Interface for the different output formats.
+/// The Formatter is then managed by [`OutputFormatter`].
 trait InternalFormatter {
+    /// Print all available rules by oxlint
+    /// Some Formatter do not know how to output the rules in the style,
+    /// instead you should print out that this combination of flags is not supported.
+    /// Example: "flag --rules with flag --format=checkstyle is not allowed"
     fn all_rules(&mut self, writer: &mut dyn Write);
 
+    /// At the end of the Lint command the Formatter can output extra information.
+    fn lint_command_info(&self, _lint_command_info: &LintCommandInfo) -> Option<String> {
+        None
+    }
+
+    /// oxlint words with [`DiagnosticService`](oxc_diagnostics::DiagnosticService),
+    /// which uses a own reporter to output to stdout.
     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> {
@@ -71,11 +98,20 @@ impl OutputFormatter {
         }
     }
 
+    /// Print all available rules by oxlint
+    /// See [`InternalFormatter::all_rules`] for more details.
     pub fn all_rules(&mut self, writer: &mut BufWriter<Stdout>) {
-        self.internal_formatter.all_rules(writer);
+        self.internal.all_rules(writer);
     }
 
+    /// At the end of the Lint command we may output extra information.
+    pub fn lint_command_info(&self, lint_command_info: &LintCommandInfo) -> Option<String> {
+        self.internal.lint_command_info(lint_command_info)
+    }
+
+    /// Returns the [`DiagnosticReporter`] which then will be used by [`DiagnosticService`](oxc_diagnostics::DiagnosticService)
+    /// See [`InternalFormatter::get_diagnostic_reporter`] for more details.
     pub fn get_diagnostic_reporter(&self) -> Box<dyn DiagnosticReporter> {
-        self.internal_formatter.get_diagnostic_reporter()
+        self.internal.get_diagnostic_reporter()
     }
 }

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: ExitCode,
 }
 
 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: _,
+                exit_code,
+            }) => 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())
-        }
-    }
-}

crates/oxc_diagnostics/src/service.rs

@@ -220,7 +220,7 @@ impl DiagnosticService {
     }
 
     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`).
+        // Do not panic when the process is killed (e.g. piping into `less`).
         if matches!(error.kind(), ErrorKind::Interrupted | ErrorKind::BrokenPipe) {
             Ok(())
         } else {