Skip to content

Commit

Permalink
Avoid loading files for cached format results (#8134)
Browse files Browse the repository at this point in the history
  • Loading branch information
MichaReiser authored Oct 23, 2023
1 parent 08519e2 commit 6199590
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 25 deletions.
2 changes: 0 additions & 2 deletions crates/ruff_cli/src/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -571,7 +571,6 @@ mod tests {
use ruff_cache::CACHE_DIR_NAME;
use ruff_linter::settings::flags;
use ruff_linter::settings::types::UnsafeFixes;
use ruff_linter::source_kind::SourceKind;
use ruff_python_ast::PySourceType;
use ruff_workspace::Settings;

Expand Down Expand Up @@ -1061,7 +1060,6 @@ mod tests {
format_path(
&file_path,
&self.settings.formatter,
&SourceKind::Python(std::fs::read_to_string(&file_path).unwrap()),
PySourceType::Python,
FormatMode::Write,
Some(cache),
Expand Down
54 changes: 31 additions & 23 deletions crates/ruff_cli/src/commands/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,18 +131,6 @@ pub(crate) fn format(
return None;
}

// Extract the sources from the file.
let unformatted = match SourceKind::from_path(path, source_type) {
Ok(Some(source_kind)) => source_kind,
Ok(None) => return None,
Err(err) => {
return Some(Err(FormatCommandError::Read(
Some(path.to_path_buf()),
err,
)));
}
};

let package = path
.parent()
.and_then(|parent| package_roots.get(parent).copied())
Expand All @@ -155,15 +143,13 @@ pub(crate) fn format(
format_path(
path,
&resolved_settings.formatter,
&unformatted,
source_type,
mode,
cache,
)
}) {
Ok(inner) => inner.map(|result| FormatPathResult {
path: resolved_file.path().to_path_buf(),
unformatted,
result,
}),
Err(error) => Err(FormatCommandError::Panic(
Expand Down Expand Up @@ -260,7 +246,6 @@ pub(crate) fn format(
pub(crate) fn format_path(
path: &Path,
settings: &FormatterSettings,
unformatted: &SourceKind,
source_type: PySourceType,
mode: FormatMode,
cache: Option<&Cache>,
Expand All @@ -277,8 +262,18 @@ pub(crate) fn format_path(
}
}

// Extract the sources from the file.
let unformatted = match SourceKind::from_path(path, source_type) {
Ok(Some(source_kind)) => source_kind,
// Non Python Jupyter notebook
Ok(None) => return Ok(FormatResult::Skipped),
Err(err) => {
return Err(FormatCommandError::Read(Some(path.to_path_buf()), err));
}
};

// Format the source.
let format_result = match format_source(unformatted, source_type, Some(path), settings)? {
let format_result = match format_source(&unformatted, source_type, Some(path), settings)? {
FormattedSource::Formatted(formatted) => match mode {
FormatMode::Write => {
let mut writer = File::create(path).map_err(|err| {
Expand All @@ -300,7 +295,10 @@ pub(crate) fn format_path(
FormatResult::Formatted
}
FormatMode::Check => FormatResult::Formatted,
FormatMode::Diff => FormatResult::Diff(formatted),
FormatMode::Diff => FormatResult::Diff {
unformatted,
formatted,
},
},
FormattedSource::Unchanged => {
if let Some(cache) = cache {
Expand Down Expand Up @@ -432,16 +430,21 @@ pub(crate) enum FormatResult {
/// The file was formatted.
Formatted,
/// The file was formatted, [`SourceKind`] contains the formatted code
Diff(SourceKind),
Diff {
unformatted: SourceKind,
formatted: SourceKind,
},
/// The file was unchanged, as the formatted contents matched the existing contents.
Unchanged,

/// Skipped formatting because its an unformatted file format
Skipped,
}

/// The coupling of a [`FormatResult`] with the path of the file that was analyzed.
#[derive(Debug)]
struct FormatPathResult {
path: PathBuf,
unformatted: SourceKind,
result: FormatResult,
}

Expand All @@ -463,15 +466,19 @@ impl<'a> FormatResults<'a> {
fn any_formatted(&self) -> bool {
self.results.iter().any(|result| match result.result {
FormatResult::Formatted | FormatResult::Diff { .. } => true,
FormatResult::Unchanged => false,
FormatResult::Unchanged | FormatResult::Skipped => false,
})
}

fn write_diff(&self, f: &mut impl Write) -> io::Result<()> {
for result in self.results {
if let FormatResult::Diff(formatted) = &result.result {
if let FormatResult::Diff {
unformatted,
formatted,
} = &result.result
{
let text_diff =
TextDiff::from_lines(result.unformatted.source_code(), formatted.source_code());
TextDiff::from_lines(unformatted.source_code(), formatted.source_code());
let mut unified_diff = text_diff.unified_diff();
unified_diff.header(
&fs::relativize_path(&result.path),
Expand Down Expand Up @@ -502,9 +509,10 @@ impl<'a> FormatResults<'a> {
changed += 1;
}
FormatResult::Unchanged => unchanged += 1,
FormatResult::Diff(_) => {
FormatResult::Diff { .. } => {
changed += 1;
}
FormatResult::Skipped => {}
}
}

Expand Down

0 comments on commit 6199590

Please sign in to comment.