Skip to content

Commit 9925910

Browse files
authored
Add a ViolationMetadata::rule method (#18234)
Summary -- This PR adds a macro-generated method to retrieve the `Rule` associated with a given `Violation` struct, which makes it substantially cheaper than parsing from the rule name. The rule is then converted to a `NoqaCode` for storage on the `Message` (and eventually on the new diagnostic type). The `ViolationMetadata::rule_name` method was now unused, so the `rule` method replaces it. Several types had to be moved from the `ruff_diagnostics` crate to the `ruff_linter` crate to make this work, namely the `Violation` traits and the old `Diagnostic` type, which had a constructor generic over a `Violation`. It's actually a fairly small PR, minus the hundreds of import changes. The main changes are in these files: - [crates/ruff_linter/src/message/mod.rs](https://github.com/astral-sh/ruff/pull/18234/files#diff-139754ea310d75f28307008d21c771a190038bd106efe3b9267cc2d6c0fa0921) - [crates/ruff_diagnostics/src/lib.rs](https://github.com/astral-sh/ruff/pull/18234/files#diff-8e8ea5c586935bf21ea439f24253fcfd5955d2cb130f5377c2fa7bfee3ea3a81) - [crates/ruff_linter/src/diagnostic.rs](https://github.com/astral-sh/ruff/pull/18234/files#diff-1d0c9aad90d8f9446079c5be5f284150d97797158715bd9729e6f1f70246297a) - [crates/ruff_linter/src/lib.rs](https://github.com/astral-sh/ruff/pull/18234/files#diff-eb93ef7e78a612f5fa9145412c75cf6b1a5cefba1c2233e4a11a880a1ce1fbcc) Test Plan -- Existing tests
1 parent a3ee6bb commit 9925910

File tree

709 files changed

+931
-888
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

709 files changed

+931
-888
lines changed

Cargo.lock

Lines changed: 0 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/ruff/src/cache.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -349,14 +349,14 @@ impl FileCache {
349349
.iter()
350350
.map(|msg| {
351351
Message::diagnostic(
352-
msg.rule.into(),
353352
msg.body.clone(),
354353
msg.suggestion.clone(),
355354
msg.range,
356355
msg.fix.clone(),
357356
msg.parent,
358357
file.clone(),
359358
msg.noqa_offset,
359+
msg.rule,
360360
)
361361
})
362362
.collect()

crates/ruff/src/commands/check.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ use rayon::prelude::*;
1212
use rustc_hash::FxHashMap;
1313

1414
use ruff_db::panic::catch_unwind;
15-
use ruff_diagnostics::Diagnostic;
15+
use ruff_linter::Diagnostic;
1616
use ruff_linter::message::Message;
1717
use ruff_linter::package::PackageRoot;
1818
use ruff_linter::registry::Rule;

crates/ruff/src/commands/rule.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use serde::ser::SerializeSeq;
66
use serde::{Serialize, Serializer};
77
use strum::IntoEnumIterator;
88

9-
use ruff_diagnostics::FixAvailability;
9+
use ruff_linter::FixAvailability;
1010
use ruff_linter::registry::{Linter, Rule, RuleNamespace};
1111

1212
use crate::args::HelpFormat;

crates/ruff/src/diagnostics.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ use colored::Colorize;
1212
use log::{debug, warn};
1313
use rustc_hash::FxHashMap;
1414

15-
use ruff_diagnostics::Diagnostic;
15+
use ruff_linter::Diagnostic;
1616
use ruff_linter::codes::Rule;
1717
use ruff_linter::linter::{FixTable, FixerResult, LinterResult, ParseSource, lint_fix, lint_only};
1818
use ruff_linter::message::Message;

crates/ruff_dev/Cargo.toml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ license = { workspace = true }
1414
ty = { workspace = true }
1515
ty_project = { workspace = true, features = ["schemars"] }
1616
ruff = { workspace = true }
17-
ruff_diagnostics = { workspace = true }
1817
ruff_formatter = { workspace = true }
1918
ruff_linter = { workspace = true, features = ["schemars"] }
2019
ruff_notebook = { workspace = true }

crates/ruff_dev/src/generate_docs.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ use itertools::Itertools;
1010
use regex::{Captures, Regex};
1111
use strum::IntoEnumIterator;
1212

13-
use ruff_diagnostics::FixAvailability;
13+
use ruff_linter::FixAvailability;
1414
use ruff_linter::registry::{Linter, Rule, RuleNamespace};
1515
use ruff_options_metadata::{OptionEntry, OptionsMetadata};
1616
use ruff_workspace::options::Options;

crates/ruff_dev/src/generate_rules_table.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use std::borrow::Cow;
88
use std::fmt::Write;
99
use strum::IntoEnumIterator;
1010

11-
use ruff_diagnostics::FixAvailability;
11+
use ruff_linter::FixAvailability;
1212
use ruff_linter::registry::{Linter, Rule, RuleNamespace};
1313
use ruff_linter::upstream_categories::UpstreamCategoryAndPrefix;
1414
use ruff_options_metadata::OptionsMetadata;

crates/ruff_diagnostics/Cargo.toml

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,5 @@ doctest = false
1616
[dependencies]
1717
ruff_text_size = { workspace = true }
1818

19-
anyhow = { workspace = true }
20-
log = { workspace = true }
2119
is-macro = { workspace = true }
2220
serde = { workspace = true, optional = true, features = [] }

crates/ruff_diagnostics/src/lib.rs

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,7 @@
1-
pub use diagnostic::Diagnostic;
21
pub use edit::Edit;
32
pub use fix::{Applicability, Fix, IsolationLevel};
43
pub use source_map::{SourceMap, SourceMarker};
5-
pub use violation::{AlwaysFixableViolation, FixAvailability, Violation, ViolationMetadata};
64

7-
mod diagnostic;
85
mod edit;
96
mod fix;
107
mod source_map;
11-
mod violation;

crates/ruff_linter/src/checkers/ast/analyze/bindings.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
1-
use ruff_diagnostics::Fix;
21
use ruff_text_size::Ranged;
32

3+
use crate::Fix;
44
use crate::checkers::ast::Checker;
55
use crate::codes::Rule;
66
use crate::rules::{

crates/ruff_linter/src/checkers/ast/analyze/deferred_scopes.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
1-
use ruff_diagnostics::Fix;
21
use ruff_python_semantic::analyze::visibility;
32
use ruff_python_semantic::{Binding, BindingKind, Imported, ResolvedReference, ScopeKind};
43
use ruff_text_size::Ranged;
54
use rustc_hash::FxHashMap;
65

6+
use crate::Fix;
77
use crate::checkers::ast::Checker;
88
use crate::codes::Rule;
99
use crate::fix;

crates/ruff_linter/src/checkers/ast/mod.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,12 +26,9 @@ use std::path::Path;
2626

2727
use itertools::Itertools;
2828
use log::debug;
29-
use ruff_python_parser::semantic_errors::{
30-
SemanticSyntaxChecker, SemanticSyntaxContext, SemanticSyntaxError, SemanticSyntaxErrorKind,
31-
};
3229
use rustc_hash::{FxHashMap, FxHashSet};
3330

34-
use ruff_diagnostics::{Diagnostic, Edit, IsolationLevel, Violation};
31+
use ruff_diagnostics::IsolationLevel;
3532
use ruff_notebook::{CellOffsets, NotebookIndex};
3633
use ruff_python_ast::helpers::{collect_import_from_member, is_docstring_stmt, to_module_path};
3734
use ruff_python_ast::identifier::Identifier;
@@ -46,6 +43,9 @@ use ruff_python_ast::{
4643
use ruff_python_ast::{PySourceType, helpers, str, visitor};
4744
use ruff_python_codegen::{Generator, Stylist};
4845
use ruff_python_index::Indexer;
46+
use ruff_python_parser::semantic_errors::{
47+
SemanticSyntaxChecker, SemanticSyntaxContext, SemanticSyntaxError, SemanticSyntaxErrorKind,
48+
};
4949
use ruff_python_parser::typing::{AnnotationKind, ParsedAnnotation, parse_type_annotation};
5050
use ruff_python_parser::{ParseError, Parsed, Tokens};
5151
use ruff_python_semantic::all::{DunderAllDefinition, DunderAllFlags};
@@ -73,6 +73,7 @@ use crate::rules::pyflakes::rules::{
7373
use crate::rules::pylint::rules::{AwaitOutsideAsync, LoadBeforeGlobalDeclaration};
7474
use crate::rules::{flake8_pyi, flake8_type_checking, pyflakes, pyupgrade};
7575
use crate::settings::{LinterSettings, TargetVersion, flags};
76+
use crate::{Diagnostic, Edit, Violation};
7677
use crate::{Locator, docstrings, noqa};
7778

7879
mod analyze;

crates/ruff_linter/src/checkers/filesystem.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
use std::path::Path;
22

3-
use ruff_diagnostics::Diagnostic;
43
use ruff_python_ast::PythonVersion;
54
use ruff_python_trivia::CommentRanges;
65

6+
use crate::Diagnostic;
77
use crate::Locator;
88
use crate::package::PackageRoot;
99
use crate::preview::is_allow_nested_roots_enabled;

crates/ruff_linter/src/checkers/imports.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
//! Lint rules based on import analysis.
22
3-
use ruff_diagnostics::Diagnostic;
43
use ruff_notebook::CellOffsets;
54
use ruff_python_ast::statement_visitor::StatementVisitor;
65
use ruff_python_ast::{ModModule, PySourceType, PythonVersion};
76
use ruff_python_codegen::Stylist;
87
use ruff_python_index::Indexer;
98
use ruff_python_parser::Parsed;
109

10+
use crate::Diagnostic;
1111
use crate::Locator;
1212
use crate::directives::IsortDirectives;
1313
use crate::package::PackageRoot;

crates/ruff_linter/src/checkers/logical_lines.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
1-
use ruff_diagnostics::Diagnostic;
21
use ruff_python_codegen::Stylist;
32
use ruff_python_index::Indexer;
43
use ruff_python_parser::{TokenKind, Tokens};
54
use ruff_source_file::LineRanges;
65
use ruff_text_size::{Ranged, TextRange};
76

7+
use crate::Diagnostic;
88
use crate::Locator;
99
use crate::line_width::IndentWidth;
1010
use crate::registry::{AsRule, Rule};

crates/ruff_linter/src/checkers/noqa.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ use std::path::Path;
55
use itertools::Itertools;
66
use rustc_hash::FxHashSet;
77

8-
use ruff_diagnostics::{Diagnostic, Edit, Fix};
98
use ruff_python_trivia::CommentRanges;
109
use ruff_text_size::Ranged;
1110

@@ -21,6 +20,7 @@ use crate::rules::pygrep_hooks;
2120
use crate::rules::ruff;
2221
use crate::rules::ruff::rules::{UnusedCodes, UnusedNOQA};
2322
use crate::settings::LinterSettings;
23+
use crate::{Diagnostic, Edit, Fix};
2424

2525
#[expect(clippy::too_many_arguments)]
2626
pub(crate) fn check_noqa(

crates/ruff_linter/src/checkers/physical_lines.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
//! Lint rules based on checking physical lines.
22
3-
use ruff_diagnostics::Diagnostic;
43
use ruff_python_codegen::Stylist;
54
use ruff_python_index::Indexer;
65
use ruff_source_file::UniversalNewlines;
76
use ruff_text_size::TextSize;
87

8+
use crate::Diagnostic;
99
use crate::Locator;
1010
use crate::registry::Rule;
1111
use crate::rules::flake8_copyright::rules::missing_copyright_notice;

crates/ruff_linter/src/checkers/tokens.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,13 @@
22
33
use std::path::Path;
44

5-
use ruff_diagnostics::Diagnostic;
65
use ruff_notebook::CellOffsets;
76
use ruff_python_ast::PySourceType;
87
use ruff_python_codegen::Stylist;
98
use ruff_python_index::Indexer;
109
use ruff_python_parser::Tokens;
1110

11+
use crate::Diagnostic;
1212
use crate::Locator;
1313
use crate::directives::TodoComment;
1414
use crate::registry::{AsRule, Rule};

crates/ruff_linter/src/codes.rs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use std::fmt::Formatter;
66

77
use strum_macros::{AsRefStr, EnumIter};
88

9-
use crate::registry::{AsRule, Linter};
9+
use crate::registry::Linter;
1010
use crate::rule_selector::is_single_rule_selector;
1111
use crate::rules;
1212

@@ -1156,3 +1156,9 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
11561156
_ => return None,
11571157
})
11581158
}
1159+
1160+
impl std::fmt::Display for Rule {
1161+
fn fmt(&self, f: &mut Formatter) -> std::fmt::Result {
1162+
f.write_str(self.into())
1163+
}
1164+
}

crates/ruff_diagnostics/src/diagnostic.rs renamed to crates/ruff_linter/src/diagnostic.rs

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,30 +3,37 @@ use log::debug;
33

44
use ruff_text_size::{Ranged, TextRange, TextSize};
55

6-
use crate::{Fix, Violation};
6+
use crate::registry::AsRule;
7+
use crate::violation::Violation;
8+
use crate::{Fix, codes::Rule};
79

810
#[derive(Debug, PartialEq, Eq, Clone)]
911
pub struct Diagnostic {
10-
/// The identifier of the diagnostic, used to align the diagnostic with a rule.
11-
pub name: &'static str,
1212
/// The message body to display to the user, to explain the diagnostic.
1313
pub body: String,
1414
/// The message to display to the user, to explain the suggested fix.
1515
pub suggestion: Option<String>,
1616
pub range: TextRange,
1717
pub fix: Option<Fix>,
1818
pub parent: Option<TextSize>,
19+
20+
pub(crate) rule: Rule,
1921
}
2022

2123
impl Diagnostic {
24+
// TODO(brent) We temporarily allow this to avoid updating all of the call sites to add
25+
// references. I expect this method to go away or change significantly with the rest of the
26+
// diagnostic refactor, but if it still exists in this form at the end of the refactor, we
27+
// should just update the call sites.
28+
#[expect(clippy::needless_pass_by_value)]
2229
pub fn new<T: Violation>(kind: T, range: TextRange) -> Self {
2330
Self {
24-
name: T::rule_name(),
2531
body: Violation::message(&kind),
2632
suggestion: Violation::fix_title(&kind),
2733
range,
2834
fix: None,
2935
parent: None,
36+
rule: T::rule(),
3037
}
3138
}
3239

@@ -50,7 +57,7 @@ impl Diagnostic {
5057
pub fn try_set_fix(&mut self, func: impl FnOnce() -> Result<Fix>) {
5158
match func() {
5259
Ok(fix) => self.fix = Some(fix),
53-
Err(err) => debug!("Failed to create fix for {}: {}", self.name, err),
60+
Err(err) => debug!("Failed to create fix for {}: {}", self.rule, err),
5461
}
5562
}
5663

@@ -61,7 +68,7 @@ impl Diagnostic {
6168
match func() {
6269
Ok(None) => {}
6370
Ok(Some(fix)) => self.fix = Some(fix),
64-
Err(err) => debug!("Failed to create fix for {}: {}", self.name, err),
71+
Err(err) => debug!("Failed to create fix for {}: {}", self.rule, err),
6572
}
6673
}
6774

@@ -80,6 +87,12 @@ impl Diagnostic {
8087
}
8188
}
8289

90+
impl AsRule for Diagnostic {
91+
fn rule(&self) -> Rule {
92+
self.rule
93+
}
94+
}
95+
8396
impl Ranged for Diagnostic {
8497
fn range(&self) -> TextRange {
8598
self.range

crates/ruff_linter/src/fix/edits.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
33
use anyhow::{Context, Result};
44

5-
use ruff_diagnostics::Edit;
65
use ruff_python_ast::parenthesize::parenthesized_range;
76
use ruff_python_ast::{self as ast, Arguments, ExceptHandler, Expr, ExprList, Parameters, Stmt};
87
use ruff_python_ast::{AnyNodeRef, ArgOrKeyword};
@@ -16,6 +15,7 @@ use ruff_python_trivia::{
1615
use ruff_source_file::{LineRanges, NewlineWithTrailingNewline, UniversalNewlines};
1716
use ruff_text_size::{Ranged, TextLen, TextRange, TextSize};
1817

18+
use crate::Edit;
1919
use crate::Locator;
2020
use crate::cst::matchers::{match_function_def, match_indented_block, match_statement};
2121
use crate::fix::codemods;
@@ -595,18 +595,19 @@ mod tests {
595595
use ruff_source_file::SourceFileBuilder;
596596
use test_case::test_case;
597597

598-
use ruff_diagnostics::{Diagnostic, Edit, Fix};
599598
use ruff_python_ast::Stmt;
600599
use ruff_python_codegen::Stylist;
601600
use ruff_python_parser::{parse_expression, parse_module};
602601
use ruff_text_size::{Ranged, TextRange, TextSize};
603602

604603
use crate::Locator;
604+
use crate::codes::Rule;
605605
use crate::fix::apply_fixes;
606606
use crate::fix::edits::{
607607
add_to_dunder_all, make_redundant_alias, next_stmt_break, trailing_semicolon,
608608
};
609609
use crate::message::Message;
610+
use crate::{Diagnostic, Edit, Fix};
610611

611612
/// Parse the given source using [`Mode::Module`] and return the first statement.
612613
fn parse_first_stmt(source: &str) -> Result<Stmt> {
@@ -746,14 +747,14 @@ x = 1 \
746747
iter,
747748
));
748749
Message::diagnostic(
749-
diag.name,
750750
diag.body,
751751
diag.suggestion,
752752
diag.range,
753753
diag.fix,
754754
diag.parent,
755755
SourceFileBuilder::new("<filename>", "<code>").finish(),
756756
None,
757+
Rule::MissingNewlineAtEndOfFile,
757758
)
758759
};
759760
assert_eq!(apply_fixes([diag].iter(), &locator).code, expect);

0 commit comments

Comments
 (0)