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

Allow selective caching for --fix and --diff #8316

Merged
merged 1 commit into from
Oct 29, 2023
Merged
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
47 changes: 31 additions & 16 deletions crates/ruff_cli/src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -188,13 +188,8 @@ pub(crate) fn lint_path(
unsafe_fixes: UnsafeFixes,
) -> Result<Diagnostics> {
// Check the cache.
// TODO(charlie): `fixer::Mode::Apply` and `fixer::Mode::Diff` both have
// side-effects that aren't captured in the cache. (In practice, it's fine
// to cache `fixer::Mode::Apply`, since a file either has no fixes, or we'll
// write the fixes to disk, thus invalidating the cache. But it's a bit hard
// to reason about. We need to come up with a better solution here.)
let caching = match cache {
Some(cache) if noqa.into() && fix_mode.is_generate() => {
Some(cache) if noqa.into() => {
let relative_path = cache
.relative_path(path)
.expect("wrong package cache for file");
Expand All @@ -204,7 +199,17 @@ pub(crate) fn lint_path(
.get(relative_path, &cache_key)
.and_then(|entry| entry.to_diagnostics(path));
if let Some(diagnostics) = cached_diagnostics {
return Ok(diagnostics);
// `FixMode::Generate` and `FixMode::Diff` rely on side-effects (writing to disk,
// and writing the diff to stdout, respectively). If a file has diagnostics, we
// need to avoid reading from and writing to the cache in these modes.
if match fix_mode {
flags::FixMode::Generate => true,
flags::FixMode::Apply | flags::FixMode::Diff => {
diagnostics.messages.is_empty() && diagnostics.fixed.is_empty()
}
} {
return Ok(diagnostics);
}
}

// Stash the file metadata for later so when we update the cache it reflects the prerun
Expand Down Expand Up @@ -304,15 +309,25 @@ pub(crate) fn lint_path(
if let Some((cache, relative_path, key)) = caching {
// We don't cache parsing errors.
if parse_error.is_none() {
cache.update_lint(
relative_path.to_owned(),
&key,
LintCacheData::from_messages(
&messages,
imports.clone(),
source_kind.as_ipy_notebook().map(Notebook::index).cloned(),
),
);
// `FixMode::Generate` and `FixMode::Diff` rely on side-effects (writing to disk,
// and writing the diff to stdout, respectively). If a file has diagnostics, we
// need to avoid reading from and writing to the cache in these modes.
if match fix_mode {
flags::FixMode::Generate => true,
flags::FixMode::Apply | flags::FixMode::Diff => {
messages.is_empty() && fixed.is_empty()
}
} {
cache.update_lint(
relative_path.to_owned(),
&key,
LintCacheData::from_messages(
&messages,
imports.clone(),
source_kind.as_ipy_notebook().map(Notebook::index).cloned(),
),
);
}
}
}

Expand Down
Loading