Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Show changed files when running under --check #7788

Merged
merged 1 commit into from
Oct 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
120 changes: 71 additions & 49 deletions crates/ruff_cli/src/commands/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)),
},
)
}
Expand All @@ -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 {
Expand All @@ -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)
Expand All @@ -145,11 +143,11 @@ fn format_path(
settings: &FormatterSettings,
source_type: PySourceType,
mode: FormatMode,
) -> Result<FormatCommandResult, FormatCommandError> {
) -> Result<FormatResult, FormatCommandError> {
// 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));
}
Expand All @@ -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),
}
}

Expand All @@ -180,11 +178,11 @@ pub(crate) enum FormattedSource {
Unchanged(SourceKind),
}

impl From<FormattedSource> for FormatCommandResult {
impl From<FormattedSource> 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,
}
}
}
Expand Down Expand Up @@ -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<FormatCommandResult>, 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(())
Expand Down
12 changes: 6 additions & 6 deletions crates/ruff_cli/src/commands/format_stdin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,15 @@ 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};
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;
Expand Down Expand Up @@ -59,7 +59,7 @@ pub(crate) fn format_stdin(cli: &FormatArguments, overrides: &CliOverrides) -> R
}
},
Err(err) => {
warn!("{err}");
error!("{err}");
Ok(ExitStatus::Error)
}
}
Expand All @@ -71,14 +71,14 @@ fn format_source_code(
settings: &FormatterSettings,
source_type: PySourceType,
mode: FormatMode,
) -> Result<FormatCommandResult, FormatCommandError> {
) -> Result<FormatResult, FormatCommandError> {
// 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));
}
Expand All @@ -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))
}
14 changes: 7 additions & 7 deletions crates/ruff_python_ast/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,13 @@ impl Default for SourceType {
}
}

impl From<&Path> for SourceType {
fn from(path: &Path) -> Self {
match path.file_name() {
impl<P: AsRef<Path>> From<P> 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)),
},
Expand Down Expand Up @@ -79,9 +79,9 @@ pub enum PySourceType {
Ipynb,
}

impl From<&Path> for PySourceType {
fn from(path: &Path) -> Self {
match path.extension() {
impl<P: AsRef<Path>> From<P> 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,
Expand Down
Loading