From f71c80af6837a3e8cdc2d5b1a305c82e7d583054 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Tue, 3 Oct 2023 14:50:06 -0400 Subject: [PATCH] Show changed files when running under `--check` (#7788) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary We now list each changed file when running with `--check`. Closes https://github.com/astral-sh/ruff/issues/7782. ## Test Plan ``` ❯ cargo run -p ruff_cli -- format foo.py --check Compiling ruff_cli v0.0.292 (/Users/crmarsh/workspace/ruff/crates/ruff_cli) rgo + Finished dev [unoptimized + debuginfo] target(s) in 1.41s Running `target/debug/ruff format foo.py --check` warning: `ruff format` is a work-in-progress, subject to change at any time, and intended only for experimentation. Would reformat: foo.py 1 file would be reformatted ``` --- crates/ruff_cli/src/commands/format.rs | 120 +++++++++++-------- crates/ruff_cli/src/commands/format_stdin.rs | 12 +- crates/ruff_python_ast/src/lib.rs | 14 +-- 3 files changed, 84 insertions(+), 62 deletions(-) diff --git a/crates/ruff_cli/src/commands/format.rs b/crates/ruff_cli/src/commands/format.rs index 42776e8773043..bd6ff4327ba8c 100644 --- a/crates/ruff_cli/src/commands/format.rs +++ b/crates/ruff_cli/src/commands/format.rs @@ -66,23 +66,21 @@ pub(crate) fn format( .filter_map(|entry| { match entry { Ok(entry) => { - let path = entry.path(); + let path = entry.into_path(); - let SourceType::Python(source_type) = SourceType::from(path) else { + let SourceType::Python(source_type) = SourceType::from(&path) else { // Ignore any non-Python files. return None; }; - let resolved_settings = resolver.resolve(path, &pyproject_config); + let resolved_settings = resolver.resolve(&path, &pyproject_config); Some( match catch_unwind(|| { - format_path(path, &resolved_settings.formatter, source_type, mode) + format_path(&path, &resolved_settings.formatter, source_type, mode) }) { - Ok(inner) => inner, - Err(error) => { - Err(FormatCommandError::Panic(Some(path.to_path_buf()), error)) - } + Ok(inner) => inner.map(|result| FormatPathResult { path, result }), + Err(error) => Err(FormatCommandError::Panic(Some(path), error)), }, ) } @@ -106,7 +104,7 @@ pub(crate) fn format( error!("{error}"); } - let summary = FormatResultSummary::new(results, mode); + let summary = FormatSummary::new(results.as_slice(), mode); // Report on the formatting changes. if log_level >= LogLevel::Default { @@ -126,7 +124,7 @@ pub(crate) fn format( } FormatMode::Check => { if errors.is_empty() { - if summary.formatted > 0 { + if summary.any_formatted() { Ok(ExitStatus::Failure) } else { Ok(ExitStatus::Success) @@ -145,11 +143,11 @@ fn format_path( settings: &FormatterSettings, source_type: PySourceType, mode: FormatMode, -) -> Result { +) -> Result { // Extract the sources from the file. let source_kind = match SourceKind::from_path(path, source_type) { Ok(Some(source_kind)) => source_kind, - Ok(None) => return Ok(FormatCommandResult::Unchanged), + Ok(None) => return Ok(FormatResult::Unchanged), Err(err) => { return Err(FormatCommandError::Read(Some(path.to_path_buf()), err)); } @@ -166,9 +164,9 @@ fn format_path( .write(&mut writer) .map_err(|err| FormatCommandError::Write(Some(path.to_path_buf()), err))?; } - Ok(FormatCommandResult::Formatted) + Ok(FormatResult::Formatted) } - FormattedSource::Unchanged(_) => Ok(FormatCommandResult::Unchanged), + FormattedSource::Unchanged(_) => Ok(FormatResult::Unchanged), } } @@ -180,11 +178,11 @@ pub(crate) enum FormattedSource { Unchanged(SourceKind), } -impl From for FormatCommandResult { +impl From for FormatResult { fn from(value: FormattedSource) -> Self { match value { - FormattedSource::Formatted(_) => FormatCommandResult::Formatted, - FormattedSource::Unchanged(_) => FormatCommandResult::Unchanged, + FormattedSource::Formatted(_) => FormatResult::Formatted, + FormattedSource::Unchanged(_) => FormatResult::Unchanged, } } } @@ -292,73 +290,97 @@ pub(crate) fn format_source( } } +/// The result of an individual formatting operation. #[derive(Debug, Clone, Copy, is_macro::Is)] -pub(crate) enum FormatCommandResult { +pub(crate) enum FormatResult { /// The file was formatted. Formatted, /// The file was unchanged, as the formatted contents matched the existing contents. Unchanged, } +/// The coupling of a [`FormatResult`] with the path of the file that was analyzed. #[derive(Debug)] -struct FormatResultSummary { +struct FormatPathResult { + path: PathBuf, + result: FormatResult, +} + +/// A summary of the formatting results. +#[derive(Debug)] +struct FormatSummary<'a> { + /// The individual formatting results. + results: &'a [FormatPathResult], /// The format mode that was used. mode: FormatMode, - /// The number of files that were formatted. - formatted: usize, - /// The number of files that were unchanged. - unchanged: usize, } -impl FormatResultSummary { - fn new(diagnostics: Vec, mode: FormatMode) -> Self { - let mut summary = Self { - mode, - formatted: 0, - unchanged: 0, - }; - for diagnostic in diagnostics { - match diagnostic { - FormatCommandResult::Formatted => summary.formatted += 1, - FormatCommandResult::Unchanged => summary.unchanged += 1, - } - } - summary +impl<'a> FormatSummary<'a> { + fn new(results: &'a [FormatPathResult], mode: FormatMode) -> Self { + Self { results, mode } + } + + /// Returns `true` if any of the files require formatting. + fn any_formatted(&self) -> bool { + self.results + .iter() + .any(|result| result.result.is_formatted()) } } -impl Display for FormatResultSummary { +impl Display for FormatSummary<'_> { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - if self.formatted > 0 && self.unchanged > 0 { + // Compute the number of changed and unchanged files. + let mut formatted = 0u32; + let mut unchanged = 0u32; + for result in self.results { + match result.result { + FormatResult::Formatted => { + // If we're running in check mode, report on any files that would be formatted. + if self.mode.is_check() { + writeln!( + f, + "Would reformat: {}", + fs::relativize_path(&result.path).bold() + )?; + } + formatted += 1; + } + FormatResult::Unchanged => unchanged += 1, + } + } + + // Write out a summary of the formatting results. + if formatted > 0 && unchanged > 0 { write!( f, "{} file{} {}, {} file{} left unchanged", - self.formatted, - if self.formatted == 1 { "" } else { "s" }, + formatted, + if formatted == 1 { "" } else { "s" }, match self.mode { FormatMode::Write => "reformatted", FormatMode::Check => "would be reformatted", }, - self.unchanged, - if self.unchanged == 1 { "" } else { "s" }, + unchanged, + if unchanged == 1 { "" } else { "s" }, ) - } else if self.formatted > 0 { + } else if formatted > 0 { write!( f, "{} file{} {}", - self.formatted, - if self.formatted == 1 { "" } else { "s" }, + formatted, + if formatted == 1 { "" } else { "s" }, match self.mode { FormatMode::Write => "reformatted", FormatMode::Check => "would be reformatted", } ) - } else if self.unchanged > 0 { + } else if unchanged > 0 { write!( f, "{} file{} left unchanged", - self.unchanged, - if self.unchanged == 1 { "" } else { "s" }, + unchanged, + if unchanged == 1 { "" } else { "s" }, ) } else { Ok(()) diff --git a/crates/ruff_cli/src/commands/format_stdin.rs b/crates/ruff_cli/src/commands/format_stdin.rs index a790754d2003b..060e92491f3f6 100644 --- a/crates/ruff_cli/src/commands/format_stdin.rs +++ b/crates/ruff_cli/src/commands/format_stdin.rs @@ -2,7 +2,7 @@ use std::io::stdout; use std::path::Path; use anyhow::Result; -use log::warn; +use log::error; use ruff_linter::source_kind::SourceKind; use ruff_python_ast::{PySourceType, SourceType}; @@ -10,7 +10,7 @@ use ruff_workspace::resolver::python_file_at_path; use ruff_workspace::FormatterSettings; use crate::args::{CliOverrides, FormatArguments}; -use crate::commands::format::{format_source, FormatCommandError, FormatCommandResult, FormatMode}; +use crate::commands::format::{format_source, FormatCommandError, FormatMode, FormatResult}; use crate::resolve::resolve; use crate::stdin::read_from_stdin; use crate::ExitStatus; @@ -59,7 +59,7 @@ pub(crate) fn format_stdin(cli: &FormatArguments, overrides: &CliOverrides) -> R } }, Err(err) => { - warn!("{err}"); + error!("{err}"); Ok(ExitStatus::Error) } } @@ -71,14 +71,14 @@ fn format_source_code( settings: &FormatterSettings, source_type: PySourceType, mode: FormatMode, -) -> Result { +) -> Result { // Read the source from stdin. let source_code = read_from_stdin() .map_err(|err| FormatCommandError::Read(path.map(Path::to_path_buf), err.into()))?; let source_kind = match SourceKind::from_source_code(source_code, source_type) { Ok(Some(source_kind)) => source_kind, - Ok(None) => return Ok(FormatCommandResult::Unchanged), + Ok(None) => return Ok(FormatResult::Unchanged), Err(err) => { return Err(FormatCommandError::Read(path.map(Path::to_path_buf), err)); } @@ -96,5 +96,5 @@ fn format_source_code( .map_err(|err| FormatCommandError::Write(path.map(Path::to_path_buf), err))?; } - Ok(FormatCommandResult::from(formatted)) + Ok(FormatResult::from(formatted)) } diff --git a/crates/ruff_python_ast/src/lib.rs b/crates/ruff_python_ast/src/lib.rs index 2060ac960230c..1c3f182921976 100644 --- a/crates/ruff_python_ast/src/lib.rs +++ b/crates/ruff_python_ast/src/lib.rs @@ -41,13 +41,13 @@ impl Default for SourceType { } } -impl From<&Path> for SourceType { - fn from(path: &Path) -> Self { - match path.file_name() { +impl> From

for SourceType { + fn from(path: P) -> Self { + match path.as_ref().file_name() { Some(filename) if filename == "pyproject.toml" => Self::Toml(TomlSourceType::Pyproject), Some(filename) if filename == "Pipfile" => Self::Toml(TomlSourceType::Pipfile), Some(filename) if filename == "poetry.lock" => Self::Toml(TomlSourceType::Poetry), - _ => match path.extension() { + _ => match path.as_ref().extension() { Some(ext) if ext == "toml" => Self::Toml(TomlSourceType::Unrecognized), _ => Self::Python(PySourceType::from(path)), }, @@ -79,9 +79,9 @@ pub enum PySourceType { Ipynb, } -impl From<&Path> for PySourceType { - fn from(path: &Path) -> Self { - match path.extension() { +impl> From

for PySourceType { + fn from(path: P) -> Self { + match path.as_ref().extension() { Some(ext) if ext == "py" => PySourceType::Python, Some(ext) if ext == "pyi" => PySourceType::Stub, Some(ext) if ext == "ipynb" => PySourceType::Ipynb,