Skip to content

Commit d04d40d

Browse files
committed
avoid converting to Rule to collect rule codes
restore Rule in output
1 parent 0cc4259 commit d04d40d

File tree

4 files changed

+37
-24
lines changed

4 files changed

+37
-24
lines changed

crates/ruff/src/printer.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use anyhow::Result;
66
use bitflags::bitflags;
77
use colored::Colorize;
88
use itertools::{Itertools, iterate};
9-
use ruff_linter::codes::NoqaCode;
9+
use ruff_linter::codes::{NoqaCode, Rule};
1010
use serde::Serialize;
1111

1212
use ruff_linter::fs::relativize_path;
@@ -498,12 +498,13 @@ fn print_fix_summary(writer: &mut dyn Write, fixed: &FixMap) -> Result<()> {
498498
relativize_path(filename).bold(),
499499
":".cyan()
500500
)?;
501-
for (rule, count) in table.iter().sorted_by_key(|(.., count)| Reverse(*count)) {
501+
for (code, count) in table.iter().sorted_by_key(|(.., count)| Reverse(*count)) {
502+
let code = code.to_string();
502503
writeln!(
503504
writer,
504505
" {count:>num_digits$} × {} ({})",
505-
rule.noqa_code().to_string().red().bold(),
506-
rule.as_ref(),
506+
code.red().bold(),
507+
Rule::from_code(&code).map_or(code, |rule| rule.to_string()),
507508
)?;
508509
}
509510
}

crates/ruff_linter/src/codes.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ use crate::registry::Linter;
1010
use crate::rule_selector::is_single_rule_selector;
1111
use crate::rules;
1212

13-
#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]
13+
#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
1414
pub struct NoqaCode(&'static str, &'static str);
1515

1616
impl NoqaCode {

crates/ruff_linter/src/fix/mod.rs

Lines changed: 25 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ use ruff_diagnostics::{IsolationLevel, SourceMap};
77
use ruff_text_size::{Ranged, TextLen, TextRange, TextSize};
88

99
use crate::Locator;
10+
use crate::codes::NoqaCode;
1011
use crate::linter::FixTable;
1112
use crate::message::OldDiagnostic;
1213
use crate::registry::Rule;
@@ -63,7 +64,7 @@ fn apply_fixes<'a>(
6364
let mut source_map = SourceMap::default();
6465

6566
for (rule, fix) in diagnostics
66-
.filter_map(|msg| msg.to_rule().map(|rule| (rule, msg)))
67+
.filter_map(|msg| msg.to_noqa_code().map(|rule| (rule, msg)))
6768
.filter_map(|(rule, diagnostic)| diagnostic.fix().map(|fix| (rule, fix)))
6869
.sorted_by(|(rule1, fix1), (rule2, fix2)| cmp_fix(*rule1, *rule2, fix1, fix2))
6970
{
@@ -125,34 +126,44 @@ fn apply_fixes<'a>(
125126
}
126127

127128
/// Compare two fixes.
128-
fn cmp_fix(rule1: Rule, rule2: Rule, fix1: &Fix, fix2: &Fix) -> std::cmp::Ordering {
129+
fn cmp_fix(rule1: NoqaCode, rule2: NoqaCode, fix1: &Fix, fix2: &Fix) -> std::cmp::Ordering {
129130
// Always apply `RedefinedWhileUnused` before `UnusedImport`, as the latter can end up fixing
130131
// the former. But we can't apply this just for `RedefinedWhileUnused` and `UnusedImport` because it violates
131132
// `< is transitive: a < b and b < c implies a < c. The same must hold for both == and >.`
132133
// See https://github.com/astral-sh/ruff/issues/12469#issuecomment-2244392085
133-
match (rule1, rule2) {
134-
(Rule::RedefinedWhileUnused, Rule::RedefinedWhileUnused) => std::cmp::Ordering::Equal,
135-
(Rule::RedefinedWhileUnused, _) => std::cmp::Ordering::Less,
136-
(_, Rule::RedefinedWhileUnused) => std::cmp::Ordering::Greater,
137-
_ => std::cmp::Ordering::Equal,
134+
let redefined_while_unused = Rule::RedefinedWhileUnused.noqa_code();
135+
if (rule1, rule2) == (redefined_while_unused, redefined_while_unused) {
136+
std::cmp::Ordering::Equal
137+
} else if rule1 == redefined_while_unused {
138+
std::cmp::Ordering::Less
139+
} else if rule2 == redefined_while_unused {
140+
std::cmp::Ordering::Greater
141+
} else {
142+
std::cmp::Ordering::Equal
138143
}
139144
// Apply fixes in order of their start position.
140145
.then_with(|| fix1.min_start().cmp(&fix2.min_start()))
141146
// Break ties in the event of overlapping rules, for some specific combinations.
142-
.then_with(|| match (&rule1, &rule2) {
147+
.then_with(|| {
148+
let rules = (rule1, rule2);
143149
// Apply `MissingTrailingPeriod` fixes before `NewLineAfterLastParagraph` fixes.
144-
(Rule::MissingTrailingPeriod, Rule::NewLineAfterLastParagraph) => std::cmp::Ordering::Less,
145-
(Rule::NewLineAfterLastParagraph, Rule::MissingTrailingPeriod) => {
150+
let missing_trailing_period = Rule::MissingTrailingPeriod.noqa_code();
151+
let newline_after_last_paragraph = Rule::NewLineAfterLastParagraph.noqa_code();
152+
let if_else_instead_of_dict_get = Rule::IfElseBlockInsteadOfDictGet.noqa_code();
153+
let if_else_instead_of_if_exp = Rule::IfElseBlockInsteadOfIfExp.noqa_code();
154+
if rules == (missing_trailing_period, newline_after_last_paragraph) {
155+
std::cmp::Ordering::Less
156+
} else if rules == (newline_after_last_paragraph, missing_trailing_period) {
146157
std::cmp::Ordering::Greater
147158
}
148159
// Apply `IfElseBlockInsteadOfDictGet` fixes before `IfElseBlockInsteadOfIfExp` fixes.
149-
(Rule::IfElseBlockInsteadOfDictGet, Rule::IfElseBlockInsteadOfIfExp) => {
160+
else if rules == (if_else_instead_of_dict_get, if_else_instead_of_if_exp) {
150161
std::cmp::Ordering::Less
151-
}
152-
(Rule::IfElseBlockInsteadOfIfExp, Rule::IfElseBlockInsteadOfDictGet) => {
162+
} else if rules == (if_else_instead_of_if_exp, if_else_instead_of_dict_get) {
153163
std::cmp::Ordering::Greater
164+
} else {
165+
std::cmp::Ordering::Equal
154166
}
155-
_ => std::cmp::Ordering::Equal,
156167
})
157168
}
158169

crates/ruff_linter/src/linter.rs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ use crate::checkers::imports::check_imports;
2222
use crate::checkers::noqa::check_noqa;
2323
use crate::checkers::physical_lines::check_physical_lines;
2424
use crate::checkers::tokens::check_tokens;
25+
use crate::codes::NoqaCode;
2526
use crate::directives::Directives;
2627
use crate::doc_lines::{doc_lines_from_ast, doc_lines_from_tokens};
2728
use crate::fix::{FixResult, fix_file};
@@ -83,7 +84,7 @@ impl LinterResult {
8384
}
8485
}
8586

86-
pub type FixTable = FxHashMap<Rule, usize>;
87+
pub type FixTable = FxHashMap<NoqaCode, usize>;
8788

8889
pub struct FixerResult<'a> {
8990
/// The result returned by the linter, after applying any fixes.
@@ -707,18 +708,18 @@ pub fn lint_fix<'a>(
707708
}
708709
}
709710

710-
fn collect_rule_codes(rules: impl IntoIterator<Item = Rule>) -> String {
711+
fn collect_rule_codes(rules: impl IntoIterator<Item = NoqaCode>) -> String {
711712
rules
712713
.into_iter()
713-
.map(|rule| rule.noqa_code().to_string())
714+
.map(|rule| rule.to_string())
714715
.sorted_unstable()
715716
.dedup()
716717
.join(", ")
717718
}
718719

719720
#[expect(clippy::print_stderr)]
720721
fn report_failed_to_converge_error(path: &Path, transformed: &str, diagnostics: &[OldDiagnostic]) {
721-
let codes = collect_rule_codes(diagnostics.iter().filter_map(OldDiagnostic::to_rule));
722+
let codes = collect_rule_codes(diagnostics.iter().filter_map(OldDiagnostic::to_noqa_code));
722723
if cfg!(debug_assertions) {
723724
eprintln!(
724725
"{}{} Failed to converge after {} iterations in `{}` with rule codes {}:---\n{}\n---",
@@ -754,7 +755,7 @@ fn report_fix_syntax_error(
754755
path: &Path,
755756
transformed: &str,
756757
error: &ParseError,
757-
rules: impl IntoIterator<Item = Rule>,
758+
rules: impl IntoIterator<Item = NoqaCode>,
758759
) {
759760
let codes = collect_rule_codes(rules);
760761
if cfg!(debug_assertions) {

0 commit comments

Comments
 (0)