From 6f7d3cc79848a2b0d831775a7ecba27130fd2979 Mon Sep 17 00:00:00 2001 From: Dhruv Manilawala Date: Tue, 20 Jun 2023 22:16:49 +0530 Subject: [PATCH] Add option (`-o`/`--output-file`) to write output to a file (#4950) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary A new CLI option (`-o`/`--output-file`) to write output to a file instead of stdout. Major change is to remove the lock acquired on stdout. The argument is that the output is buffered and thus the lock is acquired only when writing a block (8kb). As per the benchmark below there is a slight performance penalty. Reference: https://rustmagazine.org/issue-3/javascript-compiler/#printing-is-slow ## Benchmarks _Output is truncated to only contain useful information:_ Command: `check --isolated --no-cache --select=ALL --show-source ./test-repos/cpython"` Latest HEAD (361d45f2b2f7e69d37d4e6d8270e448f72cae9a7) with and without the manual lock on stdout: ```console Benchmark 1: With lock Time (mean ± σ): 5.687 s ± 0.075 s [User: 17.110 s, System: 0.486 s] Range (min … max): 5.615 s … 5.860 s 10 runs Benchmark 2: Without lock Time (mean ± σ): 5.719 s ± 0.064 s [User: 17.095 s, System: 0.491 s] Range (min … max): 5.640 s … 5.865 s 10 runs Summary (1) ran 1.01 ± 0.02 times faster than (2) ``` This PR: ```console Benchmark 1: This PR Time (mean ± σ): 5.855 s ± 0.058 s [User: 17.197 s, System: 0.491 s] Range (min … max): 5.786 s … 5.987 s 10 runs Benchmark 2: Latest HEAD with lock Time (mean ± σ): 5.645 s ± 0.033 s [User: 16.922 s, System: 0.495 s] Range (min … max): 5.600 s … 5.712 s 10 runs Summary (2) ran 1.04 ± 0.01 times faster than (1) ``` ## Test Plan Run all of the commands which gives output with and without the `--output-file=ruff.out` option: * `--show-settings` * `--show-files` * `--show-fixes` * `--diff` * `--select=ALL` * `--select=All --show-source` * `--watch` (only stdout allowed) resolves: #4754 --- crates/ruff_cli/src/args.rs | 5 ++ crates/ruff_cli/src/commands/show_files.rs | 6 +-- crates/ruff_cli/src/commands/show_settings.rs | 10 ++-- crates/ruff_cli/src/lib.rs | 28 +++++++--- crates/ruff_cli/src/printer.rs | 51 ++++++++++--------- docs/configuration.md | 2 + 6 files changed, 64 insertions(+), 38 deletions(-) diff --git a/crates/ruff_cli/src/args.rs b/crates/ruff_cli/src/args.rs index eb3efc09a5..0199d2f552 100644 --- a/crates/ruff_cli/src/args.rs +++ b/crates/ruff_cli/src/args.rs @@ -107,6 +107,9 @@ pub struct CheckArgs { /// Output serialization format for violations. #[arg(long, value_enum, env = "RUFF_FORMAT")] pub format: Option, + /// Specify file to write the linter output to (default: stdout). + #[arg(short, long)] + pub output_file: Option, /// The minimum Python version that should be supported. #[arg(long, value_enum)] pub target_version: Option, @@ -399,6 +402,7 @@ impl CheckArgs { ignore_noqa: self.ignore_noqa, isolated: self.isolated, no_cache: self.no_cache, + output_file: self.output_file, show_files: self.show_files, show_settings: self.show_settings, statistics: self.statistics, @@ -465,6 +469,7 @@ pub struct Arguments { pub ignore_noqa: bool, pub isolated: bool, pub no_cache: bool, + pub output_file: Option, pub show_files: bool, pub show_settings: bool, pub statistics: bool, diff --git a/crates/ruff_cli/src/commands/show_files.rs b/crates/ruff_cli/src/commands/show_files.rs index 802501b578..7b13474e93 100644 --- a/crates/ruff_cli/src/commands/show_files.rs +++ b/crates/ruff_cli/src/commands/show_files.rs @@ -1,4 +1,4 @@ -use std::io::{self, BufWriter, Write}; +use std::io::Write; use std::path::PathBuf; use anyhow::Result; @@ -14,6 +14,7 @@ pub(crate) fn show_files( files: &[PathBuf], pyproject_config: &PyprojectConfig, overrides: &Overrides, + writer: &mut impl Write, ) -> Result<()> { // Collect all files in the hierarchy. let (paths, _resolver) = resolver::python_files_in_path(files, pyproject_config, overrides)?; @@ -24,13 +25,12 @@ pub(crate) fn show_files( } // Print the list of files. - let mut stdout = BufWriter::new(io::stdout().lock()); for entry in paths .iter() .flatten() .sorted_by(|a, b| a.path().cmp(b.path())) { - writeln!(stdout, "{}", entry.path().to_string_lossy())?; + writeln!(writer, "{}", entry.path().to_string_lossy())?; } Ok(()) diff --git a/crates/ruff_cli/src/commands/show_settings.rs b/crates/ruff_cli/src/commands/show_settings.rs index 48a88b811d..8f91668be0 100644 --- a/crates/ruff_cli/src/commands/show_settings.rs +++ b/crates/ruff_cli/src/commands/show_settings.rs @@ -1,4 +1,4 @@ -use std::io::{self, BufWriter, Write}; +use std::io::Write; use std::path::PathBuf; use anyhow::{bail, Result}; @@ -14,6 +14,7 @@ pub(crate) fn show_settings( files: &[PathBuf], pyproject_config: &PyprojectConfig, overrides: &Overrides, + writer: &mut impl Write, ) -> Result<()> { // Collect all files in the hierarchy. let (paths, resolver) = resolver::python_files_in_path(files, pyproject_config, overrides)?; @@ -28,12 +29,11 @@ pub(crate) fn show_settings( let path = entry.path(); let settings = resolver.resolve(path, pyproject_config); - let mut stdout = BufWriter::new(io::stdout().lock()); - writeln!(stdout, "Resolved settings for: {path:?}")?; + writeln!(writer, "Resolved settings for: {path:?}")?; if let Some(settings_path) = pyproject_config.path.as_ref() { - writeln!(stdout, "Settings path: {settings_path:?}")?; + writeln!(writer, "Settings path: {settings_path:?}")?; } - writeln!(stdout, "{settings:#?}")?; + writeln!(writer, "{settings:#?}")?; Ok(()) } diff --git a/crates/ruff_cli/src/lib.rs b/crates/ruff_cli/src/lib.rs index 01b3e229e1..4b63ad4a07 100644 --- a/crates/ruff_cli/src/lib.rs +++ b/crates/ruff_cli/src/lib.rs @@ -1,3 +1,4 @@ +use std::fs::File; use std::io::{self, stdout, BufWriter, Write}; use std::path::{Path, PathBuf}; use std::process::ExitCode; @@ -171,12 +172,26 @@ pub fn check(args: CheckArgs, log_level: LogLevel) -> Result { cli.stdin_filename.as_deref(), )?; + let mut writer: Box = match cli.output_file { + Some(path) if !cli.watch => { + colored::control::set_override(false); + let file = File::create(path)?; + Box::new(BufWriter::new(file)) + } + _ => Box::new(BufWriter::new(io::stdout())), + }; + if cli.show_settings { - commands::show_settings::show_settings(&cli.files, &pyproject_config, &overrides)?; + commands::show_settings::show_settings( + &cli.files, + &pyproject_config, + &overrides, + &mut writer, + )?; return Ok(ExitStatus::Success); } if cli.show_files { - commands::show_files::show_files(&cli.files, &pyproject_config, &overrides)?; + commands::show_files::show_files(&cli.files, &pyproject_config, &overrides, &mut writer)?; return Ok(ExitStatus::Success); } @@ -276,7 +291,7 @@ pub fn check(args: CheckArgs, log_level: LogLevel) -> Result { noqa.into(), autofix, )?; - printer.write_continuously(&messages)?; + printer.write_continuously(&mut writer, &messages)?; // In watch mode, we may need to re-resolve the configuration. // TODO(charlie): Re-compute other derivative values, like the `printer`. @@ -308,7 +323,7 @@ pub fn check(args: CheckArgs, log_level: LogLevel) -> Result { noqa.into(), autofix, )?; - printer.write_continuously(&messages)?; + printer.write_continuously(&mut writer, &messages)?; } Err(err) => return Err(err.into()), } @@ -341,10 +356,9 @@ pub fn check(args: CheckArgs, log_level: LogLevel) -> Result { // source code goes to stdout). if !(is_stdin && matches!(autofix, flags::FixMode::Apply | flags::FixMode::Diff)) { if cli.statistics { - printer.write_statistics(&diagnostics)?; + printer.write_statistics(&diagnostics, &mut writer)?; } else { - let mut stdout = BufWriter::new(io::stdout().lock()); - printer.write_once(&diagnostics, &mut stdout)?; + printer.write_once(&diagnostics, &mut writer)?; } } diff --git a/crates/ruff_cli/src/printer.rs b/crates/ruff_cli/src/printer.rs index 2062b9e823..2774706703 100644 --- a/crates/ruff_cli/src/printer.rs +++ b/crates/ruff_cli/src/printer.rs @@ -1,8 +1,7 @@ use std::cmp::Reverse; use std::fmt::Display; use std::hash::Hash; -use std::io; -use std::io::{BufWriter, Write}; +use std::io::Write; use anyhow::Result; use bitflags::bitflags; @@ -98,7 +97,7 @@ impl Printer { } } - fn write_summary_text(&self, stdout: &mut dyn Write, diagnostics: &Diagnostics) -> Result<()> { + fn write_summary_text(&self, writer: &mut dyn Write, diagnostics: &Diagnostics) -> Result<()> { if self.log_level >= LogLevel::Default { if self.flags.contains(Flags::SHOW_VIOLATIONS) { let fixed = diagnostics @@ -111,12 +110,12 @@ impl Printer { if fixed > 0 { let s = if total == 1 { "" } else { "s" }; writeln!( - stdout, + writer, "Found {total} error{s} ({fixed} fixed, {remaining} remaining)." )?; } else if remaining > 0 { let s = if remaining == 1 { "" } else { "s" }; - writeln!(stdout, "Found {remaining} error{s}.")?; + writeln!(writer, "Found {remaining} error{s}.")?; } if show_fix_status(self.autofix_level) { @@ -127,7 +126,7 @@ impl Printer { .count(); if num_fixable > 0 { writeln!( - stdout, + writer, "[{}] {num_fixable} potentially fixable with the --fix option.", "*".cyan(), )?; @@ -142,9 +141,9 @@ impl Printer { if fixed > 0 { let s = if fixed == 1 { "" } else { "s" }; if self.autofix_level.is_apply() { - writeln!(stdout, "Fixed {fixed} error{s}.")?; + writeln!(writer, "Fixed {fixed} error{s}.")?; } else { - writeln!(stdout, "Would fix {fixed} error{s}.")?; + writeln!(writer, "Would fix {fixed} error{s}.")?; } } } @@ -155,7 +154,7 @@ impl Printer { pub(crate) fn write_once( &self, diagnostics: &Diagnostics, - writer: &mut impl Write, + writer: &mut dyn Write, ) -> Result<()> { if matches!(self.log_level, LogLevel::Silent) { return Ok(()); @@ -241,7 +240,11 @@ impl Printer { Ok(()) } - pub(crate) fn write_statistics(&self, diagnostics: &Diagnostics) -> Result<()> { + pub(crate) fn write_statistics( + &self, + diagnostics: &Diagnostics, + writer: &mut dyn Write, + ) -> Result<()> { let statistics: Vec = diagnostics .messages .iter() @@ -277,7 +280,6 @@ impl Printer { return Ok(()); } - let mut stdout = BufWriter::new(io::stdout().lock()); match self.format { SerializationFormat::Text => { // Compute the maximum number of digits in the count and code, for all messages, @@ -302,7 +304,7 @@ impl Printer { // By default, we mimic Flake8's `--statistics` format. for statistic in statistics { writeln!( - stdout, + writer, "{:>count_width$}\t{: { - writeln!(stdout, "{}", serde_json::to_string_pretty(&statistics)?)?; + writeln!(writer, "{}", serde_json::to_string_pretty(&statistics)?)?; } _ => { anyhow::bail!( @@ -331,12 +333,16 @@ impl Printer { } } - stdout.flush()?; + writer.flush()?; Ok(()) } - pub(crate) fn write_continuously(&self, diagnostics: &Diagnostics) -> Result<()> { + pub(crate) fn write_continuously( + &self, + writer: &mut dyn Write, + diagnostics: &Diagnostics, + ) -> Result<()> { if matches!(self.log_level, LogLevel::Silent) { return Ok(()); } @@ -353,19 +359,18 @@ impl Printer { ); } - let mut stdout = BufWriter::new(io::stdout().lock()); if !diagnostics.messages.is_empty() { if self.log_level >= LogLevel::Default { - writeln!(stdout)?; + writeln!(writer)?; } let context = EmitterContext::new(&diagnostics.source_kind); TextEmitter::default() .with_show_fix_status(show_fix_status(self.autofix_level)) .with_show_source(self.flags.contains(Flags::SHOW_SOURCE)) - .emit(&mut stdout, &diagnostics.messages, &context)?; + .emit(writer, &diagnostics.messages, &context)?; } - stdout.flush()?; + writer.flush()?; Ok(()) } @@ -394,7 +399,7 @@ const fn show_fix_status(autofix_level: flags::FixMode) -> bool { !autofix_level.is_apply() } -fn print_fix_summary(stdout: &mut T, fixed: &FxHashMap) -> Result<()> { +fn print_fix_summary(writer: &mut dyn Write, fixed: &FxHashMap) -> Result<()> { let total = fixed .values() .map(|table| table.values().sum::()) @@ -410,14 +415,14 @@ fn print_fix_summary(stdout: &mut T, fixed: &FxHashMap(stdout: &mut T, fixed: &FxHashMapnum_digits$} × {} ({})", rule.noqa_code().to_string().red().bold(), rule.as_ref(), diff --git a/docs/configuration.md b/docs/configuration.md index 91897bac80..538e19ce8f 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -212,6 +212,8 @@ Options: Ignore any `# noqa` comments --format Output serialization format for violations [env: RUFF_FORMAT=] [possible values: text, json, json-lines, junit, grouped, github, gitlab, pylint, azure] + -o, --output-file + Specify file to write the linter output to (default: stdout) --target-version The minimum Python version that should be supported [possible values: py37, py38, py39, py310, py311] --config