Skip to content
46 changes: 23 additions & 23 deletions crates/ruff/src/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,19 +13,19 @@ use itertools::Itertools;
use log::{debug, error};
use rayon::iter::ParallelIterator;
use rayon::iter::{IntoParallelIterator, ParallelBridge};
use ruff_linter::{codes::Rule, registry::AsRule};
use ruff_linter::codes::Rule;
use rustc_hash::FxHashMap;
use tempfile::NamedTempFile;

use ruff_cache::{CacheKey, CacheKeyHasher};
use ruff_diagnostics::Fix;
use ruff_linter::message::{DiagnosticMessage, Message};
use ruff_linter::message::Message;
use ruff_linter::package::PackageRoot;
use ruff_linter::{VERSION, warn_user};
use ruff_macros::CacheKey;
use ruff_notebook::NotebookIndex;
use ruff_source_file::SourceFileBuilder;
use ruff_text_size::{TextRange, TextSize};
use ruff_text_size::{Ranged, TextRange, TextSize};
use ruff_workspace::Settings;
use ruff_workspace::resolver::Resolver;

Expand Down Expand Up @@ -348,16 +348,16 @@ impl FileCache {
lint.messages
.iter()
.map(|msg| {
Message::Diagnostic(DiagnosticMessage {
name: msg.rule.into(),
body: msg.body.clone(),
suggestion: msg.suggestion.clone(),
range: msg.range,
fix: msg.fix.clone(),
file: file.clone(),
noqa_offset: msg.noqa_offset,
parent: msg.parent,
})
Message::diagnostic(
msg.rule.into(),
msg.body.clone(),
msg.suggestion.clone(),
msg.range,
msg.fix.clone(),
msg.parent,
file.clone(),
msg.noqa_offset,
)
})
.collect()
};
Expand Down Expand Up @@ -439,22 +439,22 @@ impl LintCacheData {

let messages = messages
.iter()
.filter_map(|message| message.as_diagnostic_message())
.map(|msg| {
.filter_map(|msg| msg.to_rule().map(|rule| (rule, msg)))
.map(|(rule, msg)| {
// Make sure that all message use the same source file.
assert_eq!(
msg.file,
msg.source_file(),
messages.first().unwrap().source_file(),
"message uses a different source file"
);
CacheMessage {
rule: msg.rule(),
body: msg.body.clone(),
suggestion: msg.suggestion.clone(),
range: msg.range,
rule,
body: msg.body().to_string(),
suggestion: msg.suggestion().map(ToString::to_string),
range: msg.range(),
parent: msg.parent,
fix: msg.fix.clone(),
noqa_offset: msg.noqa_offset,
fix: msg.fix().cloned(),
noqa_offset: msg.noqa_offset(),
}
})
.collect();
Expand Down Expand Up @@ -485,7 +485,7 @@ pub(super) struct CacheMessage {
#[bincode(with_serde)]
fix: Option<Fix>,
#[bincode(with_serde)]
noqa_offset: TextSize,
noqa_offset: Option<TextSize>,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change didn't actually end up helping at all (besides None being slightly shorter than TextSize::default()), so I'm happy to revert it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It probably even increases the size of the struct but I think it's more correct overall.

}

pub(crate) trait PackageCaches {
Expand Down
4 changes: 2 additions & 2 deletions crates/ruff/src/commands/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use ruff_linter::settings::types::UnsafeFixes;
use ruff_linter::settings::{LinterSettings, flags};
use ruff_linter::{IOError, fs, warn_user_once};
use ruff_source_file::SourceFileBuilder;
use ruff_text_size::{TextRange, TextSize};
use ruff_text_size::TextRange;
use ruff_workspace::resolver::{
PyprojectConfig, ResolvedFile, match_exclusion, python_files_in_path,
};
Expand Down Expand Up @@ -133,7 +133,7 @@ pub(crate) fn check(
vec![Message::from_diagnostic(
Diagnostic::new(IOError { message }, TextRange::default()),
dummy,
TextSize::default(),
None,
)],
FxHashMap::default(),
)
Expand Down
4 changes: 2 additions & 2 deletions crates/ruff/src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ use ruff_linter::{IOError, fs};
use ruff_notebook::{Notebook, NotebookError, NotebookIndex};
use ruff_python_ast::{PySourceType, SourceType, TomlSourceType};
use ruff_source_file::SourceFileBuilder;
use ruff_text_size::{TextRange, TextSize};
use ruff_text_size::TextRange;
use ruff_workspace::Settings;

use crate::cache::{Cache, FileCacheKey, LintCacheData};
Expand Down Expand Up @@ -71,7 +71,7 @@ impl Diagnostics {
TextRange::default(),
),
source_file,
TextSize::default(),
None,
)],
FxHashMap::default(),
)
Expand Down
88 changes: 22 additions & 66 deletions crates/ruff/src/printer.rs
Original file line number Diff line number Diff line change
@@ -1,23 +1,22 @@
use std::cmp::Reverse;
use std::fmt::Display;
use std::hash::Hash;
use std::io::Write;

use anyhow::Result;
use bitflags::bitflags;
use colored::Colorize;
use itertools::{Itertools, iterate};
use ruff_linter::codes::NoqaCode;
use serde::Serialize;

use ruff_linter::fs::relativize_path;
use ruff_linter::logging::LogLevel;
use ruff_linter::message::{
AzureEmitter, Emitter, EmitterContext, GithubEmitter, GitlabEmitter, GroupedEmitter,
JsonEmitter, JsonLinesEmitter, JunitEmitter, Message, MessageKind, PylintEmitter,
RdjsonEmitter, SarifEmitter, TextEmitter,
JsonEmitter, JsonLinesEmitter, JunitEmitter, Message, PylintEmitter, RdjsonEmitter,
SarifEmitter, TextEmitter,
};
use ruff_linter::notify_user;
use ruff_linter::registry::Rule;
use ruff_linter::settings::flags::{self};
use ruff_linter::settings::types::{OutputFormat, UnsafeFixes};

Expand All @@ -37,59 +36,12 @@ bitflags! {

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

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

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

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

impl From<Rule> for SerializeRuleAsCode {
fn from(rule: Rule) -> Self {
Self(rule)
}
}

struct SerializeMessageKindAsTitle(MessageKind);

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_str())
}
}

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

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

pub(crate) struct Printer {
format: OutputFormat,
log_level: LogLevel,
Expand Down Expand Up @@ -350,21 +302,25 @@ impl Printer {
let statistics: Vec<ExpandedStatistics> = diagnostics
.messages
.iter()
.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;
.map(|message| (message.to_noqa_code(), message))
.sorted_by_key(|(code, message)| (*code, message.fixable()))
.fold(
vec![],
|mut acc: Vec<((Option<NoqaCode>, &Message), usize)>, (code, message)| {
if let Some(((prev_code, _prev_message), count)) = acc.last_mut() {
if *prev_code == code {
*count += 1;
return acc;
}
}
}
acc.push((message, 1));
acc
})
acc.push(((code, message), 1));
acc
},
)
.iter()
.map(|&(message, count)| ExpandedStatistics {
code: message.rule().map(std::convert::Into::into),
name: message.kind().into(),
.map(|&((code, message), count)| ExpandedStatistics {
code,
name: message.name(),
count,
fixable: if let Some(fix) = message.fix() {
fix.applies(self.unsafe_fixes.required_applicability())
Expand Down
7 changes: 6 additions & 1 deletion crates/ruff_db/src/diagnostic/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -493,6 +493,11 @@ impl Annotation {
&self.span
}

/// Sets the span on this annotation.
pub fn set_span(&mut self, span: Span) {
self.span = span;
}

/// Returns the tags associated with this annotation.
pub fn get_tags(&self) -> &[DiagnosticTag] {
&self.tags
Expand Down Expand Up @@ -616,7 +621,7 @@ impl DiagnosticId {
///
/// Note that this doesn't include the lint's category. It
/// only includes the lint's name.
pub fn as_str(&self) -> &str {
pub fn as_str(&self) -> &'static str {
match self {
DiagnosticId::Panic => "panic",
DiagnosticId::Io => "io",
Expand Down
14 changes: 9 additions & 5 deletions crates/ruff_linter/src/checkers/noqa.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,14 @@ pub(crate) fn check_noqa(

// Remove any ignored diagnostics.
'outer: for (index, diagnostic) in diagnostics.iter().enumerate() {
if matches!(diagnostic.rule(), Rule::BlanketNOQA) {
let rule = diagnostic.rule();

if matches!(rule, Rule::BlanketNOQA) {
continue;
}

let code = rule.noqa_code();

match &exemption {
FileExemption::All(_) => {
// If the file is exempted, ignore all diagnostics.
Expand All @@ -59,7 +63,7 @@ pub(crate) fn check_noqa(
}
FileExemption::Codes(codes) => {
// If the diagnostic is ignored by a global exemption, ignore it.
if codes.contains(&&diagnostic.rule().noqa_code()) {
if codes.contains(&&code) {
ignored_diagnostics.push(index);
continue;
}
Expand All @@ -78,13 +82,13 @@ pub(crate) fn check_noqa(
{
let suppressed = match &directive_line.directive {
Directive::All(_) => {
directive_line.matches.push(diagnostic.rule().noqa_code());
directive_line.matches.push(code);
ignored_diagnostics.push(index);
true
}
Directive::Codes(directive) => {
if directive.includes(diagnostic.rule()) {
directive_line.matches.push(diagnostic.rule().noqa_code());
if directive.includes(code) {
directive_line.matches.push(code);
ignored_diagnostics.push(index);
true
} else {
Expand Down
11 changes: 10 additions & 1 deletion crates/ruff_linter/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use crate::registry::{AsRule, Linter};
use crate::rule_selector::is_single_rule_selector;
use crate::rules;

#[derive(PartialEq, Eq, PartialOrd, Ord)]
#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]
pub struct NoqaCode(&'static str, &'static str);
Comment on lines +13 to 14
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More an idea for a separate PR if you're interested in it. I wonder if we should store the code as a single &str, together with the byte-offset where the prefix ends and the suffix starts. This would have the advantage that it isn't necessary to call to_string to get the full noqa code (and we can still return a &str for both prefix and suffix in O(1))

Thinking about this more. It's probably not even necessary to store the offset and we can simply compute the split point on demand.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added these three (NoqaCode instead of rule, &Span, and this) to my todo list as follow-ups!


impl NoqaCode {
Expand Down Expand Up @@ -46,6 +46,15 @@ impl PartialEq<&str> for NoqaCode {
}
}

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

#[derive(Debug, Copy, Clone)]
pub enum RuleGroup {
/// The rule is stable.
Expand Down
22 changes: 11 additions & 11 deletions crates/ruff_linter/src/fix/edits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -606,7 +606,7 @@ mod tests {
use crate::fix::edits::{
add_to_dunder_all, make_redundant_alias, next_stmt_break, trailing_semicolon,
};
use crate::message::DiagnosticMessage;
use crate::message::Message;

/// Parse the given source using [`Mode::Module`] and return the first statement.
fn parse_first_stmt(source: &str) -> Result<Stmt> {
Expand Down Expand Up @@ -745,16 +745,16 @@ x = 1 \
iter.next().ok_or(anyhow!("expected edits nonempty"))?,
iter,
));
DiagnosticMessage {
name: diag.name,
body: diag.body,
suggestion: diag.suggestion,
range: diag.range,
fix: diag.fix,
parent: diag.parent,
file: SourceFileBuilder::new("<filename>", "<code>").finish(),
noqa_offset: TextSize::default(),
}
Message::diagnostic(
diag.name,
diag.body,
diag.suggestion,
diag.range,
diag.fix,
diag.parent,
SourceFileBuilder::new("<filename>", "<code>").finish(),
None,
)
};
assert_eq!(apply_fixes([diag].iter(), &locator).code, expect);
Ok(())
Expand Down
Loading
Loading