diff --git a/ruff_cli/src/cache.rs b/ruff_cli/src/cache.rs index 6da56155a14c1..de102aa011e53 100644 --- a/ruff_cli/src/cache.rs +++ b/ruff_cli/src/cache.rs @@ -85,6 +85,10 @@ fn read_sync(cache_dir: &Path, key: u64) -> Result, 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>( path: P, @@ -137,3 +141,16 @@ pub fn set>( error!("Failed to write to cache: {e:?}"); } } + +/// Delete a value from the cache. +pub fn del>( + path: P, + package: Option<&P>, + settings: &AllSettings, + autofix: flags::Autofix, +) { + drop(del_sync( + &settings.cli.cache_dir, + cache_key(path, package, &settings.lib, autofix), + )); +} diff --git a/ruff_cli/src/diagnostics.rs b/ruff_cli/src/diagnostics.rs index a167e0cf9ac30..dae3399d06839 100644 --- a/ruff_cli/src/diagnostics.rs +++ b/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 { // 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,9 +169,9 @@ 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, @@ -151,8 +179,17 @@ pub fn lint_stdin( 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 }) } diff --git a/src/lib_native.rs b/src/lib_native.rs index 0e5b0feb93f9d..128c6f948b86e 100644 --- a/src/lib_native.rs +++ b/src/lib_native.rs @@ -49,7 +49,7 @@ pub fn check(path: &Path, contents: &str, autofix: bool) -> Result Result Result { let directives = directives::extract_directives(&tokens, directives::Flags::empty()); // Generate checks. - let diagnostics = check_path( + let LinterResult { + data: diagnostics, .. + } = check_path( Path::new(""), None, contents, diff --git a/src/linter.rs b/src/linter.rs index ef0f42c59050b..02c1bb4420968 100644 --- a/src/linter.rs +++ b/src/linter.rs @@ -2,6 +2,7 @@ use std::path::Path; use anyhow::Result; use colored::Colorize; +use rustpython_parser::error::ParseError; use rustpython_parser::lexer::LexResult; use crate::autofix::fix_file; @@ -24,6 +25,23 @@ use crate::{directives, fs, rustpython_helpers}; const CARGO_PKG_NAME: &str = env!("CARGO_PKG_NAME"); const CARGO_PKG_REPOSITORY: &str = env!("CARGO_PKG_REPOSITORY"); +/// A [`Result`]-like type that returns both data and an error. Used to return diagnostics even in +/// the face of parse errors, since many diagnostics can be generated without a full AST. +pub struct LinterResult { + pub data: T, + pub error: Option, +} + +impl LinterResult { + fn new(data: T, error: Option) -> Self { + LinterResult { data, error } + } + + fn map U>(self, f: F) -> LinterResult { + LinterResult::new(f(self.data), self.error) + } +} + /// Generate `Diagnostic`s from the source code contents at the /// given `Path`. #[allow(clippy::too_many_arguments)] @@ -39,9 +57,10 @@ pub fn check_path( settings: &Settings, autofix: flags::Autofix, noqa: flags::Noqa, -) -> Vec { +) -> LinterResult> { // Aggregate all diagnostics. - let mut diagnostics: Vec = vec![]; + let mut diagnostics = vec![]; + let mut error = None; // Collect doc lines. This requires a rare mix of tokens (for comments) and AST // (for docstrings), which demands special-casing at this level. @@ -80,7 +99,7 @@ pub fn check_path( .iter_enabled() .any(|rule_code| matches!(rule_code.lint_source(), LintSource::Imports)); if use_ast || use_imports || use_doc_lines { - match rustpython_helpers::parse_program_tokens(tokens, "") { + match rustpython_helpers::parse_program_tokens(tokens, &path.to_string_lossy()) { Ok(python_ast) => { if use_ast { diagnostics.extend(check_ast( @@ -117,6 +136,7 @@ pub fn check_path( if settings.rules.enabled(&Rule::SyntaxError) { pycodestyle::rules::syntax_error(&mut diagnostics, &parse_error); } + error = Some(parse_error); } } } @@ -169,7 +189,7 @@ pub fn check_path( ); } - diagnostics + LinterResult::new(diagnostics, error) } const MAX_ITERATIONS: usize = 100; @@ -196,7 +216,10 @@ pub fn add_noqa_to_path(path: &Path, settings: &Settings) -> Result { directives::extract_directives(&tokens, directives::Flags::from_settings(settings)); // Generate diagnostics, ignoring any existing `noqa` directives. - let diagnostics = check_path( + let LinterResult { + data: diagnostics, + error, + } = check_path( path, None, &contents, @@ -210,6 +233,19 @@ pub fn add_noqa_to_path(path: &Path, settings: &Settings) -> Result { flags::Noqa::Disabled, ); + // Log any parse errors. + if let Some(err) = error { + eprintln!( + "{}{} {}{}{} {err:?}", + "error".red().bold(), + ":".bold(), + "Failed to parse ".bold(), + fs::relativize_path(path).bold(), + ":".bold() + ); + } + + // Add any missing `# noqa` pragmas. add_noqa( path, &diagnostics, @@ -220,15 +256,14 @@ pub fn add_noqa_to_path(path: &Path, settings: &Settings) -> Result { ) } -/// Generate `Diagnostic`s (optionally including any autofix -/// patches) from source code content. +/// Generate a [`Message`] for each [`Diagnostic`] triggered by the given source code. pub fn lint_only( contents: &str, path: &Path, package: Option<&Path>, settings: &Settings, autofix: flags::Autofix, -) -> Vec { +) -> LinterResult> { // Tokenize once. let tokens: Vec = rustpython_helpers::tokenize(contents); @@ -246,7 +281,7 @@ pub fn lint_only( directives::extract_directives(&tokens, directives::Flags::from_settings(settings)); // Generate diagnostics. - let diagnostics = check_path( + let result = check_path( path, package, contents, @@ -262,17 +297,19 @@ pub fn lint_only( // Convert from diagnostics to messages. let path_lossy = path.to_string_lossy(); - diagnostics - .into_iter() - .map(|diagnostic| { - let source = if settings.show_source { - Some(Source::from_diagnostic(&diagnostic, &locator)) - } else { - None - }; - Message::from_diagnostic(diagnostic, path_lossy.to_string(), source) - }) - .collect() + result.map(|diagnostics| { + diagnostics + .into_iter() + .map(|diagnostic| { + let source = if settings.show_source { + Some(Source::from_diagnostic(&diagnostic, &locator)) + } else { + None + }; + Message::from_diagnostic(diagnostic, path_lossy.to_string(), source) + }) + .collect() + }) } /// Generate `Diagnostic`s from source code content, iteratively autofixing @@ -282,7 +319,7 @@ pub fn lint_fix( path: &Path, package: Option<&Path>, settings: &Settings, -) -> (String, usize, Vec) { +) -> Result<(LinterResult>, String, usize)> { let mut contents = contents.to_string(); // Track the number of fixed errors across iterations. @@ -310,7 +347,7 @@ pub fn lint_fix( directives::extract_directives(&tokens, directives::Flags::from_settings(settings)); // Generate diagnostics. - let diagnostics = check_path( + let result = check_path( path, package, &contents, @@ -325,7 +362,7 @@ pub fn lint_fix( ); // Apply autofix. - if let Some((fixed_contents, applied)) = fix_file(&diagnostics, &locator) { + if let Some((fixed_contents, applied)) = fix_file(&result.data, &locator) { if iterations < MAX_ITERATIONS { // Count the number of fixed errors. fixed += applied; @@ -350,7 +387,7 @@ This likely indicates a bug in `{}`. If you could open an issue at: quoting the contents of `{}`, along with the `pyproject.toml` settings and executed command, we'd be very appreciative! "#, - "warning".yellow().bold(), + "error".red().bold(), MAX_ITERATIONS, CARGO_PKG_NAME, CARGO_PKG_REPOSITORY, @@ -360,17 +397,22 @@ quoting the contents of `{}`, along with the `pyproject.toml` settings and execu // Convert to messages. let path_lossy = path.to_string_lossy(); - let messages = diagnostics - .into_iter() - .map(|diagnostic| { - let source = if settings.show_source { - Some(Source::from_diagnostic(&diagnostic, &locator)) - } else { - None - }; - Message::from_diagnostic(diagnostic, path_lossy.to_string(), source) - }) - .collect(); - return (contents, fixed, messages); + return Ok(( + result.map(|diagnostics| { + diagnostics + .into_iter() + .map(|diagnostic| { + let source = if settings.show_source { + Some(Source::from_diagnostic(&diagnostic, &locator)) + } else { + None + }; + Message::from_diagnostic(diagnostic, path_lossy.to_string(), source) + }) + .collect() + }), + contents, + fixed, + )); } } diff --git a/src/rules/pandas_vet/mod.rs b/src/rules/pandas_vet/mod.rs index 515938a4871fd..69dc1fe4c3e0f 100644 --- a/src/rules/pandas_vet/mod.rs +++ b/src/rules/pandas_vet/mod.rs @@ -12,7 +12,7 @@ mod tests { use test_case::test_case; use textwrap::dedent; - use crate::linter::check_path; + use crate::linter::{check_path, LinterResult}; use crate::registry::{Rule, RuleCodePrefix}; use crate::settings::flags; use crate::source_code::{Indexer, Locator, Stylist}; @@ -28,7 +28,9 @@ mod tests { let indexer: Indexer = tokens.as_slice().into(); let directives = directives::extract_directives(&tokens, directives::Flags::from_settings(&settings)); - let diagnostics = check_path( + let LinterResult { + data: diagnostics, .. + } = check_path( Path::new(""), None, &contents, diff --git a/src/rules/pyflakes/mod.rs b/src/rules/pyflakes/mod.rs index 4b2c058fdeebd..01a1d2a969805 100644 --- a/src/rules/pyflakes/mod.rs +++ b/src/rules/pyflakes/mod.rs @@ -14,7 +14,7 @@ mod tests { use test_case::test_case; use textwrap::dedent; - use crate::linter::check_path; + use crate::linter::{check_path, LinterResult}; use crate::registry::{Rule, RuleCodePrefix}; use crate::settings::flags; use crate::source_code::{Indexer, Locator, Stylist}; @@ -217,7 +217,10 @@ mod tests { let indexer: Indexer = tokens.as_slice().into(); let directives = directives::extract_directives(&tokens, directives::Flags::from_settings(&settings)); - let mut diagnostics = check_path( + let LinterResult { + data: mut diagnostics, + .. + } = check_path( Path::new(""), None, &contents, diff --git a/src/test.rs b/src/test.rs index e81796535a6b6..b20e7169cbe96 100644 --- a/src/test.rs +++ b/src/test.rs @@ -4,6 +4,7 @@ use std::path::Path; use anyhow::Result; use rustpython_parser::lexer::LexResult; +use crate::linter::LinterResult; use crate::{ autofix::fix_file, directives, fs, @@ -15,8 +16,8 @@ use crate::{ source_code::{Indexer, Locator, Stylist}, }; -pub fn test_resource_path(path: impl AsRef) -> std::path::PathBuf { - std::path::Path::new("./resources/test/").join(path) +pub fn test_resource_path(path: impl AsRef) -> std::path::PathBuf { + Path::new("./resources/test/").join(path) } /// A convenient wrapper around [`check_path`], that additionally @@ -30,7 +31,10 @@ pub fn test_path(path: &Path, settings: &Settings) -> Result> { let indexer: Indexer = tokens.as_slice().into(); let directives = directives::extract_directives(&tokens, directives::Flags::from_settings(settings)); - let mut diagnostics = check_path( + let LinterResult { + data: mut diagnostics, + .. + } = check_path( &path, path.parent() .and_then(|parent| detect_package_root(parent, &settings.namespace_packages)), @@ -62,7 +66,9 @@ pub fn test_path(path: &Path, settings: &Settings) -> Result> { let indexer: Indexer = tokens.as_slice().into(); let directives = directives::extract_directives(&tokens, directives::Flags::from_settings(settings)); - let diagnostics = check_path( + let LinterResult { + data: diagnostics, .. + } = check_path( &path, None, &contents,