mirrors/ruff
1
0
Fork 0
mirror of https://github.com/astral-sh/ruff.git synced 2025-08-03 10:22:24 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

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

Browse source
This commit is contained in:
Charlie Marsh 2023-02-02 19:12:17 -05:00 • committed by GitHub
parent fa56fabed9
commit cb0f226962
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 178 additions and 69 deletions
Download patch file Download diff file Expand all files Collapse all files

17
ruff_cli/src/cache.rs
View file

@ -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),
));
}

79
ruff_cli/src/diagnostics.rs
View file

@ -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,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 })
}

4
src/lib_native.rs
View file

@ -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)
}

6
src/lib_wasm.rs
View file

@ -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,

114
src/linter.rs
View file

@ -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<T> {
pub data: T,
pub error: Option<ParseError>,
}
impl<T> LinterResult<T> {
fn new(data: T, error: Option<ParseError>) -> Self {
LinterResult { data, error }
}
fn map<U, F: FnOnce(T) -> U>(self, f: F) -> LinterResult<U> {
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<Diagnostic> {
) -> LinterResult<Vec<Diagnostic>> {
// Aggregate all diagnostics.
let mut diagnostics: Vec<Diagnostic> = 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, "<filename>") {
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<usize> {
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<usize> {
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<usize> {
)
}
/// 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<Message> {
) -> LinterResult<Vec<Message>> {
// Tokenize once.
let tokens: Vec<LexResult> = 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<Message>) {
) -> Result<(LinterResult<Vec<Message>>, 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,
));
}
}

6
src/rules/pandas_vet/mod.rs
View file

@ -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("<filename>"),
None,
&contents,

7
src/rules/pyflakes/mod.rs
View file

@ -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("<filename>"),
None,
&contents,

14
src/test.rs
View file

@ -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::Path>) -> std::path::PathBuf {
std::path::Path::new("./resources/test/").join(path)
pub fn test_resource_path(path: impl AsRef<Path>) -> 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<Vec<Diagnostic>> {
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<Vec<Diagnostic>> {
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,