Skip to content

Commit

Permalink
Do not consider E999 (syntax error) as a rule
Browse files Browse the repository at this point in the history
  • Loading branch information
@dhruvmanila
dhruvmanila committed Jun 26, 2024
1 parent 3e33b0e commit 119f7a5
Show file tree
Hide file tree
Showing 54 changed files with 1,204 additions and 413 deletions.
30 changes: 15 additions & 15 deletions crates/ruff/src/cache.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use tempfile::NamedTempFile;

use ruff_cache::{CacheKey, CacheKeyHasher};
use ruff_diagnostics::{DiagnosticKind, Fix};
use ruff_linter::message::Message;
use ruff_linter::message::{DiagnosticMessage, Message};
use ruff_linter::{warn_user, VERSION};
use ruff_macros::CacheKey;
use ruff_notebook::NotebookIndex;
Expand Down Expand Up @@ -333,12 +333,14 @@ impl FileCache {
let file = SourceFileBuilder::new(path.to_string_lossy(), &*lint.source).finish();
lint.messages
.iter()
.map(|msg| Message {
kind: msg.kind.clone(),
range: msg.range,
fix: msg.fix.clone(),
file: file.clone(),
noqa_offset: msg.noqa_offset,
.map(|msg| {
Message::Diagnostic(DiagnosticMessage {
kind: msg.kind.clone(),
range: msg.range,
fix: msg.fix.clone(),
file: file.clone(),
noqa_offset: msg.noqa_offset,
})
})
.collect()
};
Expand Down Expand Up @@ -412,18 +414,19 @@ impl LintCacheData {
notebook_index: Option<NotebookIndex>,
) -> Self {
let source = if let Some(msg) = messages.first() {
msg.file.source_text().to_owned()
msg.source_file().source_text().to_owned()
} else {
String::new() // No messages, no need to keep the source!
};

let messages = messages
.iter()
.filter_map(|message| message.as_diagnostic_message())
.map(|msg| {
// Make sure that all message use the same source file.
assert_eq!(
msg.file,
messages.first().unwrap().file,
&msg.file,
messages.first().unwrap().source_file(),
"message uses a different source file"
);
CacheMessage {
Expand Down Expand Up @@ -571,6 +574,7 @@ mod tests {
use test_case::test_case;

use ruff_cache::CACHE_DIR_NAME;
use ruff_linter::message::Message;
use ruff_linter::settings::flags;
use ruff_linter::settings::types::UnsafeFixes;
use ruff_python_ast::PySourceType;
Expand Down Expand Up @@ -633,11 +637,7 @@ mod tests {
UnsafeFixes::Enabled,
)
.unwrap();
if diagnostics
.messages
.iter()
.any(|m| m.kind.name == "SyntaxError")
{
if diagnostics.messages.iter().any(Message::is_syntax_error) {
parse_errors.push(path.clone());
}
paths.push(path);
Expand Down
92 changes: 48 additions & 44 deletions crates/ruff/src/diagnostics.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,18 +10,18 @@ use std::path::Path;
use anyhow::{Context, Result};
use colored::Colorize;
use log::{debug, error, warn};
use ruff_linter::codes::Rule;
use rustc_hash::FxHashMap;

use ruff_diagnostics::Diagnostic;
use ruff_linter::linter::{lint_fix, lint_only, FixTable, FixerResult, LinterResult, ParseSource};
use ruff_linter::logging::DisplayParseError;
use ruff_linter::message::Message;
use ruff_linter::message::{Message, SyntaxErrorMessage};
use ruff_linter::pyproject_toml::lint_pyproject_toml;
use ruff_linter::registry::AsRule;
use ruff_linter::settings::types::UnsafeFixes;
use ruff_linter::settings::{flags, LinterSettings};
use ruff_linter::source_kind::{SourceError, SourceKind};
use ruff_linter::{fs, IOError, SyntaxError};
use ruff_linter::{fs, IOError};
use ruff_notebook::{Notebook, NotebookError, NotebookIndex};
use ruff_python_ast::{PySourceType, SourceType, TomlSourceType};
use ruff_source_file::SourceFileBuilder;
Expand Down Expand Up @@ -55,57 +55,61 @@ impl Diagnostics {
path: Option<&Path>,
settings: &LinterSettings,
) -> Self {
let diagnostic = match err {
match err {
// IO errors.
SourceError::Io(_)
| SourceError::Notebook(NotebookError::Io(_) | NotebookError::Json(_)) => {
Diagnostic::new(
IOError {
message: err.to_string(),
},
TextRange::default(),
)
if settings.rules.enabled(Rule::IOError) {
let name = path.map_or_else(|| "-".into(), Path::to_string_lossy);
let source_file = SourceFileBuilder::new(name, "").finish();
Self::new(
vec![Message::from_diagnostic(
Diagnostic::new(
IOError {
message: err.to_string(),
},
TextRange::default(),
),
source_file,
TextSize::default(),
)],
FxHashMap::default(),
)
} else {
match path {
Some(path) => {
warn!(
"{}{}{} {err}",
"Failed to lint ".bold(),
fs::relativize_path(path).bold(),
":".bold()
);
}
None => {
warn!("{}{} {err}", "Failed to lint".bold(), ":".bold());
}
}

Self::default()
}
}
// Syntax errors.
SourceError::Notebook(
NotebookError::InvalidJson(_)
| NotebookError::InvalidSchema(_)
| NotebookError::InvalidFormat(_),
) => Diagnostic::new(
SyntaxError {
message: err.to_string(),
},
TextRange::default(),
),
};

if settings.rules.enabled(diagnostic.kind.rule()) {
let name = path.map_or_else(|| "-".into(), Path::to_string_lossy);
let dummy = SourceFileBuilder::new(name, "").finish();
Self::new(
vec![Message::from_diagnostic(
diagnostic,
dummy,
TextSize::default(),
)],
FxHashMap::default(),
)
} else {
match path {
Some(path) => {
warn!(
"{}{}{} {err}",
"Failed to lint ".bold(),
fs::relativize_path(path).bold(),
":".bold()
);
}
None => {
warn!("{}{} {err}", "Failed to lint".bold(), ":".bold());
}
) => {
let name = path.map_or_else(|| "-".into(), Path::to_string_lossy);
let dummy = SourceFileBuilder::new(name, "").finish();
Self::new(
vec![Message::SyntaxError(SyntaxErrorMessage {
message: err.to_string(),
range: TextRange::default(),
file: dummy,
})],
FxHashMap::default(),
)
}

Self::default()
}
}
}
Expand Down
63 changes: 36 additions & 27 deletions crates/ruff/src/printer.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,11 @@ use ruff_linter::fs::relativize_path;
use ruff_linter::logging::LogLevel;
use ruff_linter::message::{
AzureEmitter, Emitter, EmitterContext, GithubEmitter, GitlabEmitter, GroupedEmitter,
JsonEmitter, JsonLinesEmitter, JunitEmitter, PylintEmitter, RdjsonEmitter, SarifEmitter,
TextEmitter,
JsonEmitter, JsonLinesEmitter, JunitEmitter, Message, MessageKind, PylintEmitter,
RdjsonEmitter, SarifEmitter, TextEmitter,
};
use ruff_linter::notify_user;
use ruff_linter::registry::{AsRule, Rule};
use ruff_linter::registry::Rule;
use ruff_linter::settings::flags::{self};
use ruff_linter::settings::types::{OutputFormat, UnsafeFixes};

Expand All @@ -37,12 +37,13 @@ bitflags! {

#[derive(Serialize)]
struct ExpandedStatistics {
code: SerializeRuleAsCode,
name: SerializeRuleAsTitle,
code: Option<SerializeRuleAsCode>,
name: SerializeMessageKindAsTitle,
count: usize,
fixable: bool,
}

#[derive(Copy, Clone)]
struct SerializeRuleAsCode(Rule);

impl Serialize for SerializeRuleAsCode {
Expand All @@ -66,26 +67,26 @@ impl From<Rule> for SerializeRuleAsCode {
}
}

struct SerializeRuleAsTitle(Rule);
struct SerializeMessageKindAsTitle(MessageKind);

impl Serialize for SerializeRuleAsTitle {
impl Serialize for SerializeMessageKindAsTitle {
fn serialize<S>(&self, serializer: S) -> std::result::Result<S::Ok, S::Error>
where
S: serde::Serializer,
{
serializer.serialize_str(self.0.as_ref())
serializer.serialize_str(self.0.as_str())
}
}

impl Display for SerializeRuleAsTitle {
impl Display for SerializeMessageKindAsTitle {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(f, "{}", self.0.as_ref())
write!(f, "{}", self.0.as_str())
}
}

impl From<Rule> for SerializeRuleAsTitle {
fn from(rule: Rule) -> Self {
Self(rule)
impl From<MessageKind> for SerializeMessageKindAsTitle {
fn from(kind: MessageKind) -> Self {
Self(kind)
}
}

Expand Down Expand Up @@ -341,24 +342,23 @@ impl Printer {
let statistics: Vec<ExpandedStatistics> = diagnostics
.messages
.iter()
.map(|message| (message.kind.rule(), message.fix.is_some()))
.sorted()
.fold(vec![], |mut acc, (rule, fixable)| {
if let Some((prev_rule, _, count)) = acc.last_mut() {
if *prev_rule == rule {
.sorted_by_key(|message| (message.rule(), message.fixable()))
.fold(vec![], |mut acc: Vec<(&Message, usize)>, message| {
if let Some((prev_message, count)) = acc.last_mut() {
if prev_message.rule() == message.rule() {
*count += 1;
return acc;
}
}
acc.push((rule, fixable, 1));
acc.push((message, 1));
acc
})
.iter()
.map(|(rule, fixable, count)| ExpandedStatistics {
code: (*rule).into(),
name: (*rule).into(),
count: *count,
fixable: *fixable,
.map(|&(message, count)| ExpandedStatistics {
code: message.rule().map(std::convert::Into::into),
name: message.kind().into(),
count,
fixable: message.fixable(),
})
.sorted_by_key(|statistic| Reverse(statistic.count))
.collect();
Expand All @@ -381,7 +381,12 @@ impl Printer {
);
let code_width = statistics
.iter()
.map(|statistic| statistic.code.to_string().len())
.map(|statistic| {
statistic
.code
.map_or_else(String::new, |rule| rule.to_string())
.len()
})
.max()
.unwrap();
let any_fixable = statistics.iter().any(|statistic| statistic.fixable);
Expand All @@ -395,7 +400,11 @@ impl Printer {
writer,
"{:>count_width$}\t{:<code_width$}\t{}{}",
statistic.count.to_string().bold(),
statistic.code.to_string().red().bold(),
statistic
.code
.map_or_else(String::new, |rule| rule.to_string())
.red()
.bold(),
if any_fixable {
if statistic.fixable {
&fixable
Expand Down Expand Up @@ -545,7 +554,7 @@ impl FixableStatistics {
let mut unapplicable_unsafe = 0;

for message in &diagnostics.messages {
if let Some(fix) = &message.fix {
if let Some(fix) = message.fix() {
if fix.applies(unsafe_fixes.required_applicability()) {
applicable += 1;
} else {
Expand Down
Loading

0 comments on commit 119f7a5

Please sign in to comment.