Always report parse errors back to the user (#2505)

ruff_cli/src/cache.rs

@@ -85,6 +85,10 @@ fn read_sync(cache_dir: &Path, key: u64) -> Result<Vec<u8>, std::io::Error> {
  fs::read(cache_dir.join(content_dir()).join(format!("{key:x}")))
 }
 
+fn del_sync(cache_dir: &Path, key: u64) -> Result<(), std::io::Error> {
+ fs::remove_file(cache_dir.join(content_dir()).join(format!("{key:x}")))
+}
+
 /// Get a value from the cache.
 pub fn get<P: AsRef<Path>>(
  path: P,
@@ -137,3 +141,16 @@ pub fn set<P: AsRef<Path>>(
  error!("Failed to write to cache: {e:?}");
  }
 }
+
+/// Delete a value from the cache.
+pub fn del<P: AsRef<Path>>(
+ path: P,
+ package: Option<&P>,
+ settings: &AllSettings,
+ autofix: flags::Autofix,
+) {
+ drop(del_sync(
+ &settings.cli.cache_dir,
+ cache_key(path, package, &settings.lib, autofix),
+ ));
+}

ruff_cli/src/diagnostics.rs

@@ -6,8 +6,9 @@ use std::ops::AddAssign;
 use std::path::Path;
 
 use anyhow::Result;
+use colored::Colorize;
 use log::debug;
-use ruff::linter::{lint_fix, lint_only};
+use ruff::linter::{lint_fix, lint_only, LinterResult};
 use ruff::message::Message;
 use ruff::settings::{flags, AllSettings, Settings};
 use ruff::{fix, fs};
@@ -67,8 +68,14 @@ pub fn lint_path(
  let contents = fs::read_file(path)?;
 
  // Lint the file.
- let (messages, fixed) = if matches!(autofix, fix::FixMode::Apply | fix::FixMode::Diff) {
- let (transformed, fixed, messages) = lint_fix(&contents, path, package, &settings.lib);
+ let (
+ LinterResult {
+ data: messages,
+ error: parse_error,
+ },
+ fixed,
+ ) = if matches!(autofix, fix::FixMode::Apply | fix::FixMode::Diff) {
+ let (result, transformed, fixed) = lint_fix(&contents, path, package, &settings.lib)?;
  if fixed > 0 {
  if matches!(autofix, fix::FixMode::Apply) {
  write(path, transformed)?;
@@ -82,23 +89,38 @@ pub fn lint_path(
  stdout.flush()?;
  }
  }
- (messages, fixed)
+ (result, fixed)
  } else {
- let messages = lint_only(&contents, path, package, &settings.lib, autofix.into());
+ let result = lint_only(&contents, path, package, &settings.lib, autofix.into());
  let fixed = 0;
- (messages, fixed)
+ (result, fixed)
  };
 
- // Re-populate the cache.
- if let Some(metadata) = metadata {
- cache::set(
- path,
- package.as_ref(),
- &metadata,
- settings,
- autofix.into(),
- &messages,
+ if let Some(err) = parse_error {
+  // Notify the user of any parse errors.
+ eprintln!(
+ "{}{} {}{}{} {err}",
+ "error".red().bold(),
+ ":".bold(),
+ "Failed to parse ".bold(),
+ fs::relativize_path(path).bold(),
+ ":".bold()
  );
+
+ // Purge the cache.
+ cache::del(path, package.as_ref(), settings, autofix.into());
+ } else {
+ // Re-populate the cache.
+ if let Some(metadata) = metadata {
+ cache::set(
+ path,
+ package.as_ref(),
+ &metadata,
+ settings,
+ autofix.into(),
+ &messages,
+ );
+ }
  }
 
  Ok(Diagnostics { messages, fixed })
@@ -114,13 +136,19 @@ pub fn lint_stdin(
  autofix: fix::FixMode,
 ) -> Result<Diagnostics> {
  // Lint the inputs.
- let (messages, fixed) = if matches!(autofix, fix::FixMode::Apply | fix::FixMode::Diff) {
- let (transformed, fixed, messages) = lint_fix(
+ let (
+ LinterResult {
+ data: messages,
+ error: parse_error,
+ },
+ fixed,
+ ) = if matches!(autofix, fix::FixMode::Apply | fix::FixMode::Diff) {
+ let (result, transformed, fixed) = lint_fix(
  contents,
  path.unwrap_or_else(|| Path::new("-")),
  package,
  settings,
- );
+ )?;
 
  if matches!(autofix, fix::FixMode::Apply) {
  // Write the contents to stdout, regardless of whether any errors were fixed.
@@ -141,18 +169,27 @@ pub fn lint_stdin(
  }
  }
 
- (messages, fixed)
+ (result, fixed)
  } else {
- let messages = lint_only(
+ let result = lint_only(
  contents,
  path.unwrap_or_else(|| Path::new("-")),
  package,
  settings,
  autofix.into(),
  );
  let fixed = 0;
- (messages, fixed)
+ (result, fixed)
  };
 
+ if let Some(err) = parse_error {
+ eprintln!(
+ "{}{} Failed to parse {}: {err}",
+ "error".red().bold(),
+ ":".bold(),
+ path.map_or_else(|| "-".into(), fs::relativize_path).bold()
+ );
+ }
+
  Ok(Diagnostics { messages, fixed })
 }

src/lib_native.rs

@@ -49,7 +49,7 @@ pub fn check(path: &Path, contents: &str, autofix: bool) -> Result<Vec<Diagnosti
  directives::extract_directives(&tokens, directives::Flags::from_settings(&settings));
 
  // Generate diagnostics.
- let diagnostics = check_path(
+ let result = check_path(
  path,
  packaging::detect_package_root(path, &settings.namespace_packages),
  contents,
@@ -63,5 +63,5 @@ pub fn check(path: &Path, contents: &str, autofix: bool) -> Result<Vec<Diagnosti
  flags::Noqa::Enabled,
  );
 
- Ok(diagnostics)
+ Ok(result.data)
 }

src/lib_wasm.rs

@@ -6,7 +6,7 @@ use serde::Serialize;
 use wasm_bindgen::prelude::*;
 
 use crate::directives;
-use crate::linter::check_path;
+use crate::linter::{check_path, LinterResult};
 use crate::registry::Rule;
 use crate::rules::{
  flake8_annotations, flake8_bandit, flake8_bugbear, flake8_builtins, flake8_errmsg,
@@ -187,7 +187,9 @@ pub fn check(contents: &str, options: JsValue) -> Result<JsValue, JsValue> {
  let directives = directives::extract_directives(&tokens, directives::Flags::empty());
 
  // Generate checks.
- let diagnostics = check_path(
+ let LinterResult {
+ data: diagnostics, ..
+ } = check_path(
  Path::new("<filename>"),
  None,
  contents,