Skip to content

Commit

Permalink
refactor(linter): move stdout outside LintRunner (#8694)
Browse files Browse the repository at this point in the history
This is needed so we can use a custom `Write` implementation (or just `[u8]`) to make snapshots.
In this step I also updated `OutputFormatter::all_rules` so the Formatter does not need to handle the write-error.
  • Loading branch information
Sysix committed Jan 24, 2025
1 parent b7f13e6 commit 741fb40
Show file tree
Hide file tree
Showing 10 changed files with 65 additions and 60 deletions.
36 changes: 21 additions & 15 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, ErrorKind, Write},
io::{ErrorKind, Write},
path::{Path, PathBuf},
process::ExitCode,
time::Instant,
Expand Down Expand Up @@ -34,16 +34,14 @@ impl Runner for LintRunner {
Self { options, cwd: env::current_dir().expect("Failed to get current working directory") }
}

fn run(self) -> CliRunResult {
fn run(self, stdout: &mut dyn Write) -> CliRunResult {
let format_str = self.options.output_options.format;
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());
let output_formatter = OutputFormatter::new(format_str);

if self.options.list_rules {
output_formatter.all_rules(&mut stdout);
if let Some(output) = output_formatter.all_rules() {
stdout.write_all(output.as_bytes()).or_else(Self::check_for_writer_error).unwrap();
}
stdout.flush().unwrap();
return CliRunResult::None;
}
Expand Down Expand Up @@ -206,7 +204,7 @@ impl Runner for LintRunner {
}
});

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

let diagnostic_failed = diagnostic_result.max_warnings_exceeded()
|| diagnostic_result.errors_count() > 0
Expand Down Expand Up @@ -334,7 +332,9 @@ mod test {
let mut new_args = vec!["--silent"];
new_args.extend(args);
let options = lint_command().run_inner(new_args.as_slice()).unwrap();
match LintRunner::new(options).run() {
let mut output = Vec::new();

match LintRunner::new(options).run(&mut output) {
CliRunResult::LintResult(lint_result) => lint_result,
other => panic!("{other:?}"),
}
Expand All @@ -356,8 +356,9 @@ mod test {
};

current_cwd.push(part_cwd);
let mut output = Vec::new();

match LintRunner::new(options).with_cwd(current_cwd).run() {
match LintRunner::new(options).with_cwd(current_cwd).run(&mut output) {
CliRunResult::LintResult(lint_result) => lint_result,
other => panic!("{other:?}"),
}
Expand All @@ -367,7 +368,9 @@ mod test {
let mut new_args = vec!["--quiet"];
new_args.extend(args);
let options = lint_command().run_inner(new_args.as_slice()).unwrap();
match LintRunner::new(options).run() {
let mut output = Vec::new();

match LintRunner::new(options).run(&mut output) {
CliRunResult::InvalidOptions { message } => message,
other => {
panic!("Expected InvalidOptions, got {other:?}");
Expand Down Expand Up @@ -752,7 +755,8 @@ mod test {
fn test_print_config_ban_all_rules() {
let args = &["-A", "all", "--print-config"];
let options = lint_command().run_inner(args).unwrap();
let ret = LintRunner::new(options).run();
let mut output = Vec::new();
let ret = LintRunner::new(options).run(&mut output);
let CliRunResult::PrintConfigResult { config_file: config } = ret else {
panic!("Expected PrintConfigResult, got {ret:?}")
};
Expand All @@ -776,7 +780,8 @@ mod test {
"--print-config",
];
let options = lint_command().run_inner(args).unwrap();
let ret = LintRunner::new(options).run();
let mut output = Vec::new();
let ret = LintRunner::new(options).run(&mut output);
let CliRunResult::PrintConfigResult { config_file: config } = ret else {
panic!("Expected PrintConfigResult, got {ret:?}")
};
Expand All @@ -793,7 +798,8 @@ mod test {
fn test_init_config() {
let args = &["--init"];
let options = lint_command().run_inner(args).unwrap();
let ret = LintRunner::new(options).run();
let mut output = Vec::new();
let ret = LintRunner::new(options).run(&mut output);
let CliRunResult::ConfigFileInitResult { message } = ret else {
panic!("Expected configuration file to be created, got {ret:?}")
};
Expand Down
7 changes: 6 additions & 1 deletion apps/oxlint/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,19 @@ static GLOBAL: jemallocator::Jemalloc = jemallocator::Jemalloc;
static GLOBAL: mimalloc::MiMalloc = mimalloc::MiMalloc;

use oxlint::cli::{CliRunResult, LintRunner, Runner};
use std::io::BufWriter;

fn main() -> CliRunResult {
init_tracing();
init_miette();

let command = oxlint::cli::lint_command().run();
command.handle_threads();
LintRunner::new(command).run()
// 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());

LintRunner::new(command).run(&mut stdout)
}

// Initialize the data which relies on `is_atty` system calls so they don't block subsequent threads.
Expand Down
6 changes: 3 additions & 3 deletions apps/oxlint/src/output_formatter/checkstyle.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::{borrow::Cow, io::Write};
use std::borrow::Cow;

use rustc_hash::FxHashMap;

Expand All @@ -13,8 +13,8 @@ use crate::output_formatter::InternalFormatter;
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 all_rules(&self) -> Option<String> {
None
}

fn get_diagnostic_reporter(&self) -> Box<dyn DiagnosticReporter> {
Expand Down
20 changes: 11 additions & 9 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, time::Duration};
use std::time::Duration;

use oxc_diagnostics::{
reporter::{DiagnosticReporter, DiagnosticResult},
Expand All @@ -12,13 +12,16 @@ use crate::output_formatter::InternalFormatter;
pub struct DefaultOutputFormatter;

impl InternalFormatter for DefaultOutputFormatter {
fn all_rules(&mut self, writer: &mut dyn Write) {
fn all_rules(&self) -> Option<String> {
let mut output = String::new();
let table = RuleTable::new();
for section in table.sections {
writeln!(writer, "{}", section.render_markdown_table(None)).unwrap();
output.push_str(section.render_markdown_table(None).as_str());
output.push('\n');
}
writeln!(writer, "Default: {}", table.turned_on_by_default_count).unwrap();
writeln!(writer, "Total: {}", table.total).unwrap();
output.push_str(format!("Default: {}\n", table.turned_on_by_default_count).as_str());
output.push_str(format!("Total: {}\n", table.total).as_str());
Some(output)
}

fn lint_command_info(&self, lint_command_info: &super::LintCommandInfo) -> Option<String> {
Expand Down Expand Up @@ -125,11 +128,10 @@ mod test {

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

formatter.all_rules(&mut writer);
assert!(!writer.is_empty());
assert!(result.is_some());
}

#[test]
Expand Down
6 changes: 3 additions & 3 deletions apps/oxlint/src/output_formatter/github.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::{borrow::Cow, io::Write};
use std::borrow::Cow;

use oxc_diagnostics::{
reporter::{DiagnosticReporter, DiagnosticResult, Info},
Expand All @@ -11,8 +11,8 @@ use crate::output_formatter::InternalFormatter;
pub struct GithubOutputFormatter;

impl InternalFormatter for GithubOutputFormatter {
fn all_rules(&mut self, writer: &mut dyn Write) {
writeln!(writer, "flag --rules with flag --format=github is not allowed").unwrap();
fn all_rules(&self) -> Option<String> {
None
}

fn get_diagnostic_reporter(&self) -> Box<dyn DiagnosticReporter> {
Expand Down
24 changes: 10 additions & 14 deletions apps/oxlint/src/output_formatter/json.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
use std::io::Write;

use oxc_diagnostics::reporter::DiagnosticResult;
use oxc_diagnostics::{reporter::DiagnosticReporter, Error};
use oxc_linter::rules::RULES;
use oxc_linter::RuleCategory;
use oxc_diagnostics::{
reporter::{DiagnosticReporter, DiagnosticResult},
Error,
};
use oxc_linter::{rules::RULES, RuleCategory};

use miette::JSONReportHandler;

Expand All @@ -13,7 +12,7 @@ use crate::output_formatter::InternalFormatter;
pub struct JsonOutputFormatter;

impl InternalFormatter for JsonOutputFormatter {
fn all_rules(&mut self, writer: &mut dyn Write) {
fn all_rules(&self) -> Option<String> {
#[derive(Debug, serde::Serialize)]
struct RuleInfoJson<'a> {
scope: &'a str,
Expand All @@ -27,13 +26,10 @@ impl InternalFormatter for JsonOutputFormatter {
category: rule.category(),
});

writer
.write_all(
serde_json::to_string_pretty(&rules_info.collect::<Vec<_>>())
.expect("Failed to serialize")
.as_bytes(),
)
.unwrap();
Some(
serde_json::to_string_pretty(&rules_info.collect::<Vec<_>>())
.expect("Failed to serialize"),
)
}

fn get_diagnostic_reporter(&self) -> Box<dyn DiagnosticReporter> {
Expand Down
10 changes: 3 additions & 7 deletions apps/oxlint/src/output_formatter/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ mod json;
mod stylish;
mod unix;

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

Expand Down Expand Up @@ -63,10 +62,7 @@ pub struct LintCommandInfo {
/// 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);
fn all_rules(&self) -> Option<String>;

/// At the end of the Lint command the Formatter can output extra information.
fn lint_command_info(&self, _lint_command_info: &LintCommandInfo) -> Option<String> {
Expand Down Expand Up @@ -100,8 +96,8 @@ 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.all_rules(writer);
pub fn all_rules(&self) -> Option<String> {
self.internal.all_rules()
}

/// At the end of the Lint command we may output extra information.
Expand Down
6 changes: 2 additions & 4 deletions apps/oxlint/src/output_formatter/stylish.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
use std::io::Write;

use oxc_diagnostics::{
reporter::{DiagnosticReporter, DiagnosticResult, Info},
Error, Severity,
Expand All @@ -12,8 +10,8 @@ use crate::output_formatter::InternalFormatter;
pub struct StylishOutputFormatter;

impl InternalFormatter for StylishOutputFormatter {
fn all_rules(&mut self, writer: &mut dyn Write) {
writeln!(writer, "flag --rules with flag --format=stylish is not allowed").unwrap();
fn all_rules(&self) -> Option<String> {
None
}

fn get_diagnostic_reporter(&self) -> Box<dyn DiagnosticReporter> {
Expand Down
6 changes: 3 additions & 3 deletions apps/oxlint/src/output_formatter/unix.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::{borrow::Cow, io::Write};
use std::borrow::Cow;

use oxc_diagnostics::{
reporter::{DiagnosticReporter, DiagnosticResult, Info},
Expand All @@ -11,8 +11,8 @@ use crate::output_formatter::InternalFormatter;
pub struct UnixOutputFormatter;

impl InternalFormatter for UnixOutputFormatter {
fn all_rules(&mut self, writer: &mut dyn Write) {
writeln!(writer, "flag --rules with flag --format=unix is not allowed").unwrap();
fn all_rules(&self) -> Option<String> {
None
}

fn get_diagnostic_reporter(&self) -> Box<dyn DiagnosticReporter> {
Expand Down
4 changes: 3 additions & 1 deletion apps/oxlint/src/runner.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use std::io::Write;

use crate::cli::CliRunResult;

/// A trait for exposing functionality to the CLI.
Expand All @@ -7,5 +9,5 @@ pub trait Runner {
fn new(matches: Self::Options) -> Self;

/// Executes the runner, providing some result to the CLI.
fn run(self) -> CliRunResult;
fn run(self, stdout: &mut dyn Write) -> CliRunResult;
}

0 comments on commit 741fb40

Please sign in to comment.