From e2c5b83fe1ff53bc144b60ee97e3eca21ea72fd1 Mon Sep 17 00:00:00 2001 From: Brent Westbrook <36778786+ntBre@users.noreply.github.com> Date: Thu, 15 May 2025 10:27:21 -0400 Subject: [PATCH] Inline `DiagnosticKind` into other diagnostic types (#18074) ## Summary This PR deletes the `DiagnosticKind` type by inlining its three fields (`name`, `body`, and `suggestion`) into three other diagnostic types: `Diagnostic`, `DiagnosticMessage`, and `CacheMessage`. Instead of deferring to an internal `DiagnosticKind`, both `Diagnostic` and `DiagnosticMessage` now have their own macro-generated `AsRule` implementations. This should make both https://github.com/astral-sh/ruff/pull/18051 and another follow-up PR changing the type of `name` on `CacheMessage` easier since its type will be able to change separately from `Diagnostic` and `DiagnosticMessage`. ## Test Plan Existing tests --- Cargo.lock | 1 + Cargo.toml | 1 + crates/ruff/src/cache.rs | 18 +- crates/ruff_diagnostics/src/diagnostic.rs | 24 +- crates/ruff_diagnostics/src/lib.rs | 2 +- crates/ruff_diagnostics/src/violation.rs | 14 -- .../ruff_linter/src/checkers/logical_lines.rs | 9 +- crates/ruff_linter/src/checkers/noqa.rs | 16 +- crates/ruff_linter/src/checkers/tokens.rs | 2 +- crates/ruff_linter/src/fix/edits.rs | 4 +- crates/ruff_linter/src/fix/mod.rs | 31 +-- crates/ruff_linter/src/linter.rs | 6 +- crates/ruff_linter/src/message/mod.rs | 212 +++++++++--------- crates/ruff_linter/src/noqa.rs | 6 +- .../rules/blocking_process_invocation.rs | 56 ++--- .../rules/suspicious_function_call.rs | 65 +++--- .../flake8_bandit/rules/suspicious_imports.rs | 196 +++++++--------- .../function_call_in_argument_default.rs | 15 +- .../rules/flake8_print/rules/print_call.rs | 2 +- .../src/rules/flake8_return/rules/function.rs | 8 +- .../rules/typing_only_runtime_import.rs | 30 +-- .../rules/unused_arguments.rs | 20 +- .../rules/replaceable_by_pathlib.rs | 125 ++++++----- .../src/rules/pandas_vet/rules/call.rs | 16 +- .../src/rules/pandas_vet/rules/subscript.rs | 18 +- .../rules/invalid_first_argument_name.rs | 26 ++- .../rules/logical_lines/indentation.rs | 40 ++-- .../missing_whitespace_around_operator.rs | 53 +++-- crates/ruff_linter/src/rules/pyflakes/mod.rs | 2 +- .../pylint/rules/invalid_string_characters.rs | 24 +- .../rules/pylint/rules/no_method_decorator.rs | 17 +- .../pyupgrade/rules/use_pep604_annotation.rs | 12 +- .../ruff/rules/ambiguous_unicode_character.rs | 35 +-- crates/ruff_linter/src/test.rs | 4 +- crates/ruff_macros/Cargo.toml | 1 + crates/ruff_macros/src/kebab_case.rs | 15 +- crates/ruff_macros/src/lib.rs | 2 +- crates/ruff_macros/src/map_codes.rs | 27 ++- crates/ruff_macros/src/violation_metadata.rs | 2 +- crates/ruff_server/src/lint.rs | 20 +- crates/ruff_wasm/src/lib.rs | 48 ++-- 41 files changed, 604 insertions(+), 621 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 649c18be80..3dd545560f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2866,6 +2866,7 @@ dependencies = [ name = "ruff_macros" version = "0.0.0" dependencies = [ + "heck", "itertools 0.14.0", "proc-macro2", "quote", diff --git a/Cargo.toml b/Cargo.toml index 0114b80027..27c02132d4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -85,6 +85,7 @@ hashbrown = { version = "0.15.0", default-features = false, features = [ "equivalent", "inline-more", ] } +heck = "0.5.0" ignore = { version = "0.4.22" } imara-diff = { version = "0.1.5" } imperative = { version = "1.0.4" } diff --git a/crates/ruff/src/cache.rs b/crates/ruff/src/cache.rs index 127f6292b2..99eed7cbaf 100644 --- a/crates/ruff/src/cache.rs +++ b/crates/ruff/src/cache.rs @@ -13,12 +13,13 @@ use itertools::Itertools; use log::{debug, error}; use rayon::iter::ParallelIterator; use rayon::iter::{IntoParallelIterator, ParallelBridge}; +use ruff_linter::{codes::Rule, registry::AsRule}; use rustc_hash::FxHashMap; use serde::{Deserialize, Serialize}; use tempfile::NamedTempFile; use ruff_cache::{CacheKey, CacheKeyHasher}; -use ruff_diagnostics::{DiagnosticKind, Fix}; +use ruff_diagnostics::Fix; use ruff_linter::message::{DiagnosticMessage, Message}; use ruff_linter::package::PackageRoot; use ruff_linter::{warn_user, VERSION}; @@ -348,7 +349,9 @@ impl FileCache { .iter() .map(|msg| { Message::Diagnostic(DiagnosticMessage { - kind: msg.kind.clone(), + name: msg.rule.into(), + body: msg.body.clone(), + suggestion: msg.suggestion.clone(), range: msg.range, fix: msg.fix.clone(), file: file.clone(), @@ -444,7 +447,9 @@ impl LintCacheData { "message uses a different source file" ); CacheMessage { - kind: msg.kind.clone(), + rule: msg.rule(), + body: msg.body.clone(), + suggestion: msg.suggestion.clone(), range: msg.range, parent: msg.parent, fix: msg.fix.clone(), @@ -464,7 +469,12 @@ impl LintCacheData { /// On disk representation of a diagnostic message. #[derive(Deserialize, Debug, Serialize, PartialEq)] pub(super) struct CacheMessage { - kind: DiagnosticKind, + /// The rule for the cached diagnostic. + rule: Rule, + /// The message body to display to the user, to explain the diagnostic. + body: String, + /// The message to display to the user, to explain the suggested fix. + suggestion: Option, /// Range into the message's [`FileCache::source`]. range: TextRange, parent: Option, diff --git a/crates/ruff_diagnostics/src/diagnostic.rs b/crates/ruff_diagnostics/src/diagnostic.rs index be54a8de0e..f9a93c78f1 100644 --- a/crates/ruff_diagnostics/src/diagnostic.rs +++ b/crates/ruff_diagnostics/src/diagnostic.rs @@ -1,35 +1,29 @@ use anyhow::Result; use log::debug; -#[cfg(feature = "serde")] -use serde::{Deserialize, Serialize}; use ruff_text_size::{Ranged, TextRange, TextSize}; -use crate::Fix; +use crate::{Fix, Violation}; #[derive(Debug, PartialEq, Eq, Clone)] -#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] -pub struct DiagnosticKind { +pub struct Diagnostic { /// The identifier of the diagnostic, used to align the diagnostic with a rule. - pub name: String, + pub name: &'static str, /// The message body to display to the user, to explain the diagnostic. pub body: String, /// The message to display to the user, to explain the suggested fix. pub suggestion: Option, -} - -#[derive(Debug, PartialEq, Eq, Clone)] -pub struct Diagnostic { - pub kind: DiagnosticKind, pub range: TextRange, pub fix: Option, pub parent: Option, } impl Diagnostic { - pub fn new>(kind: T, range: TextRange) -> Self { + pub fn new(kind: T, range: TextRange) -> Self { Self { - kind: kind.into(), + name: T::rule_name(), + body: Violation::message(&kind), + suggestion: Violation::fix_title(&kind), range, fix: None, parent: None, @@ -56,7 +50,7 @@ impl Diagnostic { pub fn try_set_fix(&mut self, func: impl FnOnce() -> Result) { match func() { Ok(fix) => self.fix = Some(fix), - Err(err) => debug!("Failed to create fix for {}: {}", self.kind.name, err), + Err(err) => debug!("Failed to create fix for {}: {}", self.name, err), } } @@ -67,7 +61,7 @@ impl Diagnostic { match func() { Ok(None) => {} Ok(Some(fix)) => self.fix = Some(fix), - Err(err) => debug!("Failed to create fix for {}: {}", self.kind.name, err), + Err(err) => debug!("Failed to create fix for {}: {}", self.name, err), } } diff --git a/crates/ruff_diagnostics/src/lib.rs b/crates/ruff_diagnostics/src/lib.rs index 75a9bec240..fd97fc1408 100644 --- a/crates/ruff_diagnostics/src/lib.rs +++ b/crates/ruff_diagnostics/src/lib.rs @@ -1,4 +1,4 @@ -pub use diagnostic::{Diagnostic, DiagnosticKind}; +pub use diagnostic::Diagnostic; pub use edit::Edit; pub use fix::{Applicability, Fix, IsolationLevel}; pub use source_map::{SourceMap, SourceMarker}; diff --git a/crates/ruff_diagnostics/src/violation.rs b/crates/ruff_diagnostics/src/violation.rs index bf6690c5b2..d0c7b9c763 100644 --- a/crates/ruff_diagnostics/src/violation.rs +++ b/crates/ruff_diagnostics/src/violation.rs @@ -1,4 +1,3 @@ -use crate::DiagnosticKind; use std::fmt::{Debug, Display}; #[derive(Debug, Copy, Clone)] @@ -79,16 +78,3 @@ impl Violation for V { ::message_formats() } } - -impl From for DiagnosticKind -where - T: Violation, -{ - fn from(value: T) -> Self { - Self { - body: Violation::message(&value), - suggestion: Violation::fix_title(&value), - name: T::rule_name().to_string(), - } - } -} diff --git a/crates/ruff_linter/src/checkers/logical_lines.rs b/crates/ruff_linter/src/checkers/logical_lines.rs index 1933889387..42ed63a404 100644 --- a/crates/ruff_linter/src/checkers/logical_lines.rs +++ b/crates/ruff_linter/src/checkers/logical_lines.rs @@ -169,16 +169,17 @@ pub(crate) fn check_logical_lines( let indent_size = 4; if enforce_indentation { - for kind in indentation( + for diagnostic in indentation( &line, prev_line.as_ref(), indent_char, indent_level, prev_indent_level, indent_size, + range, ) { - if settings.rules.enabled(kind.rule()) { - context.push_diagnostic(Diagnostic::new(kind, range)); + if settings.rules.enabled(diagnostic.rule()) { + context.push_diagnostic(diagnostic); } } } @@ -206,7 +207,7 @@ impl<'a> LogicalLinesContext<'a> { } pub(crate) fn push_diagnostic(&mut self, diagnostic: Diagnostic) { - if self.settings.rules.enabled(diagnostic.kind.rule()) { + if self.settings.rules.enabled(diagnostic.rule()) { self.diagnostics.push(diagnostic); } } diff --git a/crates/ruff_linter/src/checkers/noqa.rs b/crates/ruff_linter/src/checkers/noqa.rs index f765da9c91..0a0a8da90f 100644 --- a/crates/ruff_linter/src/checkers/noqa.rs +++ b/crates/ruff_linter/src/checkers/noqa.rs @@ -47,7 +47,7 @@ pub(crate) fn check_noqa( // Remove any ignored diagnostics. 'outer: for (index, diagnostic) in diagnostics.iter().enumerate() { - if matches!(diagnostic.kind.rule(), Rule::BlanketNOQA) { + if matches!(diagnostic.rule(), Rule::BlanketNOQA) { continue; } @@ -59,7 +59,7 @@ pub(crate) fn check_noqa( } FileExemption::Codes(codes) => { // If the diagnostic is ignored by a global exemption, ignore it. - if codes.contains(&&diagnostic.kind.rule().noqa_code()) { + if codes.contains(&&diagnostic.rule().noqa_code()) { ignored_diagnostics.push(index); continue; } @@ -78,17 +78,13 @@ pub(crate) fn check_noqa( { let suppressed = match &directive_line.directive { Directive::All(_) => { - directive_line - .matches - .push(diagnostic.kind.rule().noqa_code()); + directive_line.matches.push(diagnostic.rule().noqa_code()); ignored_diagnostics.push(index); true } Directive::Codes(directive) => { - if directive.includes(diagnostic.kind.rule()) { - directive_line - .matches - .push(diagnostic.kind.rule().noqa_code()); + if directive.includes(diagnostic.rule()) { + directive_line.matches.push(diagnostic.rule().noqa_code()); ignored_diagnostics.push(index); true } else { @@ -161,7 +157,7 @@ pub(crate) fn check_noqa( let is_code_used = if is_file_level { diagnostics .iter() - .any(|diag| diag.kind.rule().noqa_code() == code) + .any(|diag| diag.rule().noqa_code() == code) } else { matches.iter().any(|match_| *match_ == code) } || settings diff --git a/crates/ruff_linter/src/checkers/tokens.rs b/crates/ruff_linter/src/checkers/tokens.rs index 7564d498e2..7fa2f8b8ee 100644 --- a/crates/ruff_linter/src/checkers/tokens.rs +++ b/crates/ruff_linter/src/checkers/tokens.rs @@ -184,7 +184,7 @@ pub(crate) fn check_tokens( ); } - diagnostics.retain(|diagnostic| settings.rules.enabled(diagnostic.kind.rule())); + diagnostics.retain(|diagnostic| settings.rules.enabled(diagnostic.rule())); diagnostics } diff --git a/crates/ruff_linter/src/fix/edits.rs b/crates/ruff_linter/src/fix/edits.rs index 3d9f08d35b..ae226d6a94 100644 --- a/crates/ruff_linter/src/fix/edits.rs +++ b/crates/ruff_linter/src/fix/edits.rs @@ -746,7 +746,9 @@ x = 1 \ iter, )); DiagnosticMessage { - kind: diag.kind, + name: diag.name, + body: diag.body, + suggestion: diag.suggestion, range: diag.range, fix: diag.fix, parent: diag.parent, diff --git a/crates/ruff_linter/src/fix/mod.rs b/crates/ruff_linter/src/fix/mod.rs index c0441c58e4..6a29c5cb5c 100644 --- a/crates/ruff_linter/src/fix/mod.rs +++ b/crates/ruff_linter/src/fix/mod.rs @@ -64,12 +64,7 @@ fn apply_fixes<'a>( let mut source_map = SourceMap::default(); for (rule, fix) in diagnostics - .filter_map(|diagnostic| { - diagnostic - .fix - .as_ref() - .map(|fix| (diagnostic.kind.rule(), fix)) - }) + .filter_map(|diagnostic| diagnostic.fix.as_ref().map(|fix| (diagnostic.rule(), fix))) .sorted_by(|(rule1, fix1), (rule2, fix2)| cmp_fix(*rule1, *rule2, fix1, fix2)) { let mut edits = fix @@ -163,7 +158,7 @@ fn cmp_fix(rule1: Rule, rule2: Rule, fix1: &Fix, fix2: &Fix) -> std::cmp::Orderi #[cfg(test)] mod tests { - use ruff_diagnostics::{Edit, Fix, SourceMarker}; + use ruff_diagnostics::{Diagnostic, Edit, Fix, SourceMarker}; use ruff_source_file::SourceFileBuilder; use ruff_text_size::{Ranged, TextSize}; @@ -177,15 +172,21 @@ mod tests { source: &str, edit: impl IntoIterator, ) -> Vec { + // The choice of rule here is arbitrary. edit.into_iter() - .map(|edit| DiagnosticMessage { - // The choice of rule here is arbitrary. - kind: MissingNewlineAtEndOfFile.into(), - range: edit.range(), - fix: Some(Fix::safe_edit(edit)), - parent: None, - file: SourceFileBuilder::new(filename, source).finish(), - noqa_offset: TextSize::default(), + .map(|edit| { + let range = edit.range(); + let diagnostic = Diagnostic::new(MissingNewlineAtEndOfFile, range); + DiagnosticMessage { + name: diagnostic.name, + body: diagnostic.body, + suggestion: diagnostic.suggestion, + range, + fix: Some(Fix::safe_edit(edit)), + parent: None, + file: SourceFileBuilder::new(filename, source).finish(), + noqa_offset: TextSize::default(), + } }) .collect() } diff --git a/crates/ruff_linter/src/linter.rs b/crates/ruff_linter/src/linter.rs index 7427b75d9c..58c0a3b2ee 100644 --- a/crates/ruff_linter/src/linter.rs +++ b/crates/ruff_linter/src/linter.rs @@ -308,7 +308,7 @@ pub fn check_path( RuleSet::empty() }; if !per_file_ignores.is_empty() { - diagnostics.retain(|diagnostic| !per_file_ignores.contains(diagnostic.kind.rule())); + diagnostics.retain(|diagnostic| !per_file_ignores.contains(diagnostic.rule())); } // Enforce `noqa` directives. @@ -338,7 +338,7 @@ pub fn check_path( if parsed.has_valid_syntax() { // Remove fixes for any rules marked as unfixable. for diagnostic in &mut diagnostics { - if !settings.rules.should_fix(diagnostic.kind.rule()) { + if !settings.rules.should_fix(diagnostic.rule()) { diagnostic.fix = None; } } @@ -349,7 +349,7 @@ pub fn check_path( if let Some(fix) = diagnostic.fix.take() { let fixed_applicability = settings .fix_safety - .resolve_applicability(diagnostic.kind.rule(), fix.applicability()); + .resolve_applicability(diagnostic.rule(), fix.applicability()); diagnostic.set_fix(fix.with_applicability(fixed_applicability)); } } diff --git a/crates/ruff_linter/src/message/mod.rs b/crates/ruff_linter/src/message/mod.rs index 9ca16bc2be..8e1961d025 100644 --- a/crates/ruff_linter/src/message/mod.rs +++ b/crates/ruff_linter/src/message/mod.rs @@ -17,7 +17,7 @@ pub use json_lines::JsonLinesEmitter; pub use junit::JunitEmitter; pub use pylint::PylintEmitter; pub use rdjson::RdjsonEmitter; -use ruff_diagnostics::{Diagnostic, DiagnosticKind, Fix}; +use ruff_diagnostics::{Diagnostic, Fix}; use ruff_notebook::NotebookIndex; use ruff_python_parser::{ParseError, UnsupportedSyntaxError}; use ruff_source_file::{LineColumn, SourceFile}; @@ -53,7 +53,9 @@ pub enum Message { /// A diagnostic message corresponding to a rule violation. #[derive(Clone, Debug, PartialEq, Eq)] pub struct DiagnosticMessage { - pub kind: DiagnosticKind, + pub name: &'static str, + pub body: String, + pub suggestion: Option, pub range: TextRange, pub fix: Option, pub parent: Option, @@ -96,7 +98,9 @@ impl Message { ) -> Message { Message::Diagnostic(DiagnosticMessage { range: diagnostic.range(), - kind: diagnostic.kind, + name: diagnostic.name, + body: diagnostic.body, + suggestion: diagnostic.suggestion, fix: diagnostic.fix, parent: diagnostic.parent, file, @@ -183,7 +187,7 @@ impl Message { /// Returns a message kind. pub fn kind(&self) -> MessageKind { match self { - Message::Diagnostic(m) => MessageKind::Diagnostic(m.kind.rule()), + Message::Diagnostic(m) => MessageKind::Diagnostic(m.rule()), Message::SyntaxError(_) => MessageKind::SyntaxError, } } @@ -191,7 +195,7 @@ impl Message { /// Returns the name used to represent the diagnostic. pub fn name(&self) -> &str { match self { - Message::Diagnostic(m) => &m.kind.name, + Message::Diagnostic(m) => m.name, Message::SyntaxError(_) => "SyntaxError", } } @@ -199,7 +203,7 @@ impl Message { /// Returns the message body to display to the user. pub fn body(&self) -> &str { match self { - Message::Diagnostic(m) => &m.kind.body, + Message::Diagnostic(m) => &m.body, Message::SyntaxError(m) => m .primary_annotation() .expect("Expected a primary annotation for a ruff diagnostic") @@ -211,7 +215,7 @@ impl Message { /// Returns the fix suggestion for the violation. pub fn suggestion(&self) -> Option<&str> { match self { - Message::Diagnostic(m) => m.kind.suggestion.as_deref(), + Message::Diagnostic(m) => m.suggestion.as_deref(), Message::SyntaxError(_) => None, } } @@ -240,7 +244,7 @@ impl Message { /// Returns the [`Rule`] corresponding to the diagnostic message. pub fn rule(&self) -> Option { match self { - Message::Diagnostic(m) => Some(m.kind.rule()), + Message::Diagnostic(m) => Some(m.rule()), Message::SyntaxError(_) => None, } } @@ -379,13 +383,13 @@ impl<'a> EmitterContext<'a> { mod tests { use rustc_hash::FxHashMap; - use ruff_diagnostics::{Diagnostic, DiagnosticKind, Edit, Fix}; + use ruff_diagnostics::{Edit, Fix}; use ruff_notebook::NotebookIndex; use ruff_python_parser::{parse_unchecked, Mode, ParseOptions}; use ruff_source_file::{OneIndexed, SourceFileBuilder}; - use ruff_text_size::{Ranged, TextRange, TextSize}; + use ruff_text_size::{TextRange, TextSize}; - use crate::message::{Emitter, EmitterContext, Message}; + use crate::message::{DiagnosticMessage, Emitter, EmitterContext, Message}; use crate::Locator; pub(super) fn create_syntax_error_messages() -> Vec { @@ -421,54 +425,56 @@ def fibonacci(n): return fibonacci(n - 1) + fibonacci(n - 2) "#; - let unused_import = Diagnostic::new( - DiagnosticKind { - name: "UnusedImport".to_string(), - body: "`os` imported but unused".to_string(), - suggestion: Some("Remove unused import: `os`".to_string()), - }, - TextRange::new(TextSize::from(7), TextSize::from(9)), - ) - .with_fix(Fix::unsafe_edit(Edit::range_deletion(TextRange::new( - TextSize::from(0), - TextSize::from(10), - )))); - let fib_source = SourceFileBuilder::new("fib.py", fib).finish(); - let unused_variable = Diagnostic::new( - DiagnosticKind { - name: "UnusedVariable".to_string(), - body: "Local variable `x` is assigned to but never used".to_string(), - suggestion: Some("Remove assignment to unused variable `x`".to_string()), - }, - TextRange::new(TextSize::from(94), TextSize::from(95)), - ) - .with_fix(Fix::unsafe_edit(Edit::deletion( - TextSize::from(94), - TextSize::from(99), - ))); + let unused_import_start = TextSize::from(7); + let unused_import = DiagnosticMessage { + name: "unused-import", + body: "`os` imported but unused".to_string(), + suggestion: Some("Remove unused import: `os`".to_string()), + range: TextRange::new(unused_import_start, TextSize::from(9)), + fix: Some(Fix::unsafe_edit(Edit::range_deletion(TextRange::new( + TextSize::from(0), + TextSize::from(10), + )))), + parent: None, + noqa_offset: unused_import_start, + file: fib_source.clone(), + }; + + let unused_variable_start = TextSize::from(94); + let unused_variable = DiagnosticMessage { + name: "unused-variable", + body: "Local variable `x` is assigned to but never used".to_string(), + suggestion: Some("Remove assignment to unused variable `x`".to_string()), + range: TextRange::new(unused_variable_start, TextSize::from(95)), + fix: Some(Fix::unsafe_edit(Edit::deletion( + TextSize::from(94), + TextSize::from(99), + ))), + parent: None, + noqa_offset: unused_variable_start, + file: fib_source, + }; let file_2 = r"if a == 1: pass"; - let undefined_name = Diagnostic::new( - DiagnosticKind { - name: "UndefinedName".to_string(), - body: "Undefined name `a`".to_string(), - suggestion: None, - }, - TextRange::new(TextSize::from(3), TextSize::from(4)), - ); + let undefined_name_start = TextSize::from(3); + let undefined_name = DiagnosticMessage { + name: "undefined-name", + body: "Undefined name `a`".to_string(), + suggestion: None, + range: TextRange::new(undefined_name_start, TextSize::from(4)), + fix: None, + parent: None, + noqa_offset: undefined_name_start, + file: SourceFileBuilder::new("undef.py", file_2).finish(), + }; - let file_2_source = SourceFileBuilder::new("undef.py", file_2).finish(); - - let unused_import_start = unused_import.start(); - let unused_variable_start = unused_variable.start(); - let undefined_name_start = undefined_name.start(); vec![ - Message::from_diagnostic(unused_import, fib_source.clone(), unused_import_start), - Message::from_diagnostic(unused_variable, fib_source, unused_variable_start), - Message::from_diagnostic(undefined_name, file_2_source, undefined_name_start), + Message::Diagnostic(unused_import), + Message::Diagnostic(unused_variable), + Message::Diagnostic(undefined_name), ] } @@ -485,47 +491,53 @@ def foo(): x = 1 "; - let unused_import_os = Diagnostic::new( - DiagnosticKind { - name: "UnusedImport".to_string(), - body: "`os` imported but unused".to_string(), - suggestion: Some("Remove unused import: `os`".to_string()), - }, - TextRange::new(TextSize::from(16), TextSize::from(18)), - ) - .with_fix(Fix::safe_edit(Edit::range_deletion(TextRange::new( - TextSize::from(9), - TextSize::from(19), - )))); - - let unused_import_math = Diagnostic::new( - DiagnosticKind { - name: "UnusedImport".to_string(), - body: "`math` imported but unused".to_string(), - suggestion: Some("Remove unused import: `math`".to_string()), - }, - TextRange::new(TextSize::from(35), TextSize::from(39)), - ) - .with_fix(Fix::safe_edit(Edit::range_deletion(TextRange::new( - TextSize::from(28), - TextSize::from(40), - )))); - - let unused_variable = Diagnostic::new( - DiagnosticKind { - name: "UnusedVariable".to_string(), - body: "Local variable `x` is assigned to but never used".to_string(), - suggestion: Some("Remove assignment to unused variable `x`".to_string()), - }, - TextRange::new(TextSize::from(98), TextSize::from(99)), - ) - .with_fix(Fix::unsafe_edit(Edit::deletion( - TextSize::from(94), - TextSize::from(104), - ))); - let notebook_source = SourceFileBuilder::new("notebook.ipynb", notebook).finish(); + let unused_import_os_start = TextSize::from(16); + let unused_import_os = DiagnosticMessage { + name: "unused-import", + body: "`os` imported but unused".to_string(), + suggestion: Some("Remove unused import: `os`".to_string()), + range: TextRange::new(unused_import_os_start, TextSize::from(18)), + fix: Some(Fix::safe_edit(Edit::range_deletion(TextRange::new( + TextSize::from(9), + TextSize::from(19), + )))), + parent: None, + file: notebook_source.clone(), + noqa_offset: unused_import_os_start, + }; + + let unused_import_math_start = TextSize::from(35); + let unused_import_math = DiagnosticMessage { + name: "unused-import", + body: "`math` imported but unused".to_string(), + suggestion: Some("Remove unused import: `math`".to_string()), + range: TextRange::new(unused_import_math_start, TextSize::from(39)), + fix: Some(Fix::safe_edit(Edit::range_deletion(TextRange::new( + TextSize::from(28), + TextSize::from(40), + )))), + parent: None, + file: notebook_source.clone(), + noqa_offset: unused_import_math_start, + }; + + let unused_variable_start = TextSize::from(98); + let unused_variable = DiagnosticMessage { + name: "unused-variable", + body: "Local variable `x` is assigned to but never used".to_string(), + suggestion: Some("Remove assignment to unused variable `x`".to_string()), + range: TextRange::new(unused_variable_start, TextSize::from(99)), + fix: Some(Fix::unsafe_edit(Edit::deletion( + TextSize::from(94), + TextSize::from(104), + ))), + parent: None, + file: notebook_source, + noqa_offset: unused_variable_start, + }; + let mut notebook_indexes = FxHashMap::default(); notebook_indexes.insert( "notebook.ipynb".to_string(), @@ -557,23 +569,11 @@ def foo(): ), ); - let unused_import_os_start = unused_import_os.start(); - let unused_import_math_start = unused_import_math.start(); - let unused_variable_start = unused_variable.start(); - ( vec![ - Message::from_diagnostic( - unused_import_os, - notebook_source.clone(), - unused_import_os_start, - ), - Message::from_diagnostic( - unused_import_math, - notebook_source.clone(), - unused_import_math_start, - ), - Message::from_diagnostic(unused_variable, notebook_source, unused_variable_start), + Message::Diagnostic(unused_import_os), + Message::Diagnostic(unused_import_math), + Message::Diagnostic(unused_variable), ], notebook_indexes, ) diff --git a/crates/ruff_linter/src/noqa.rs b/crates/ruff_linter/src/noqa.rs index e4875c0f87..bda945e320 100644 --- a/crates/ruff_linter/src/noqa.rs +++ b/crates/ruff_linter/src/noqa.rs @@ -855,7 +855,7 @@ fn find_noqa_comments<'a>( } FileExemption::Codes(codes) => { // If the diagnostic is ignored by a global exemption, don't add a noqa directive. - if codes.contains(&&diagnostic.kind.rule().noqa_code()) { + if codes.contains(&&diagnostic.rule().noqa_code()) { comments_by_line.push(None); continue; } @@ -873,7 +873,7 @@ fn find_noqa_comments<'a>( continue; } Directive::Codes(codes) => { - if codes.includes(diagnostic.kind.rule()) { + if codes.includes(diagnostic.rule()) { comments_by_line.push(None); continue; } @@ -884,7 +884,7 @@ fn find_noqa_comments<'a>( let noqa_offset = noqa_line_for.resolve(diagnostic.range.start()); - let rule = diagnostic.kind.rule(); + let rule = diagnostic.rule(); // Or ignored by the directive itself? if let Some(directive_line) = directives.find_line_with_directive(noqa_offset) { diff --git a/crates/ruff_linter/src/rules/flake8_async/rules/blocking_process_invocation.rs b/crates/ruff_linter/src/rules/flake8_async/rules/blocking_process_invocation.rs index 693a345ba6..e363f702f1 100644 --- a/crates/ruff_linter/src/rules/flake8_async/rules/blocking_process_invocation.rs +++ b/crates/ruff_linter/src/rules/flake8_async/rules/blocking_process_invocation.rs @@ -1,4 +1,4 @@ -use ruff_diagnostics::{Diagnostic, DiagnosticKind, Violation}; +use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, ViolationMetadata}; use ruff_python_ast::{self as ast, Expr}; use ruff_python_semantic::analyze::typing::find_assigned_value; @@ -117,35 +117,37 @@ pub(crate) fn blocking_process_invocation(checker: &Checker, call: &ast::ExprCal return; } - let Some(diagnostic_kind) = - checker - .semantic() - .resolve_qualified_name(call.func.as_ref()) - .and_then(|qualified_name| match qualified_name.segments() { - ["subprocess", "Popen"] | ["os", "popen"] => { - Some(CreateSubprocessInAsyncFunction.into()) - } - ["os", "system" | "posix_spawn" | "posix_spawnp"] - | ["subprocess", "run" | "call" | "check_call" | "check_output" | "getoutput" - | "getstatusoutput"] => Some(RunProcessInAsyncFunction.into()), - ["os", "wait" | "wait3" | "wait4" | "waitid" | "waitpid"] => { - Some(WaitForProcessInAsyncFunction.into()) - } - ["os", "spawnl" | "spawnle" | "spawnlp" | "spawnlpe" | "spawnv" | "spawnve" - | "spawnvp" | "spawnvpe"] => { - if is_p_wait(call, checker.semantic()) { - Some(RunProcessInAsyncFunction.into()) - } else { - Some(CreateSubprocessInAsyncFunction.into()) - } - } - _ => None, - }) + let Some(qualified_name) = checker + .semantic() + .resolve_qualified_name(call.func.as_ref()) else { return; }; - let diagnostic = Diagnostic::new::(diagnostic_kind, call.func.range()); - if checker.enabled(diagnostic.kind.rule()) { + + let range = call.func.range(); + let diagnostic = match qualified_name.segments() { + ["subprocess", "Popen"] | ["os", "popen"] => { + Diagnostic::new(CreateSubprocessInAsyncFunction, range) + } + ["os", "system" | "posix_spawn" | "posix_spawnp"] + | ["subprocess", "run" | "call" | "check_call" | "check_output" | "getoutput" | "getstatusoutput"] => { + Diagnostic::new(RunProcessInAsyncFunction, range) + } + ["os", "wait" | "wait3" | "wait4" | "waitid" | "waitpid"] => { + Diagnostic::new(WaitForProcessInAsyncFunction, range) + } + ["os", "spawnl" | "spawnle" | "spawnlp" | "spawnlpe" | "spawnv" | "spawnve" | "spawnvp" + | "spawnvpe"] => { + if is_p_wait(call, checker.semantic()) { + Diagnostic::new(RunProcessInAsyncFunction, range) + } else { + Diagnostic::new(CreateSubprocessInAsyncFunction, range) + } + } + _ => return, + }; + + if checker.enabled(diagnostic.rule()) { checker.report_diagnostic(diagnostic); } } diff --git a/crates/ruff_linter/src/rules/flake8_bandit/rules/suspicious_function_call.rs b/crates/ruff_linter/src/rules/flake8_bandit/rules/suspicious_function_call.rs index dc1765f56e..d066a39d9f 100644 --- a/crates/ruff_linter/src/rules/flake8_bandit/rules/suspicious_function_call.rs +++ b/crates/ruff_linter/src/rules/flake8_bandit/rules/suspicious_function_call.rs @@ -2,7 +2,7 @@ //! //! See: use itertools::Either; -use ruff_diagnostics::{Diagnostic, DiagnosticKind, Violation}; +use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, ViolationMetadata}; use ruff_python_ast::{self as ast, Arguments, Decorator, Expr, ExprCall, Operator}; use ruff_text_size::{Ranged, TextRange}; @@ -1035,39 +1035,39 @@ fn suspicious_function( return; }; - let diagnostic_kind: DiagnosticKind = match qualified_name.segments() { + let diagnostic = match qualified_name.segments() { // Pickle ["pickle" | "dill", "load" | "loads" | "Unpickler"] | ["shelve", "open" | "DbfilenameShelf"] | ["jsonpickle", "decode"] | ["jsonpickle", "unpickler", "decode"] - | ["pandas", "read_pickle"] => SuspiciousPickleUsage.into(), + | ["pandas", "read_pickle"] => Diagnostic::new(SuspiciousPickleUsage, range), // Marshal - ["marshal", "load" | "loads"] => SuspiciousMarshalUsage.into(), + ["marshal", "load" | "loads"] => Diagnostic::new(SuspiciousMarshalUsage, range), // InsecureHash ["Crypto" | "Cryptodome", "Hash", "SHA" | "MD2" | "MD3" | "MD4" | "MD5", "new"] | ["cryptography", "hazmat", "primitives", "hashes", "SHA1" | "MD5"] => { - SuspiciousInsecureHashUsage.into() + Diagnostic::new(SuspiciousInsecureHashUsage, range) } // InsecureCipher ["Crypto" | "Cryptodome", "Cipher", "ARC2" | "Blowfish" | "DES" | "XOR", "new"] | ["cryptography", "hazmat", "primitives", "ciphers", "algorithms", "ARC4" | "Blowfish" | "IDEA"] => { - SuspiciousInsecureCipherUsage.into() + Diagnostic::new(SuspiciousInsecureCipherUsage, range) } // InsecureCipherMode ["cryptography", "hazmat", "primitives", "ciphers", "modes", "ECB"] => { - SuspiciousInsecureCipherModeUsage.into() + Diagnostic::new(SuspiciousInsecureCipherModeUsage, range) } // Mktemp - ["tempfile", "mktemp"] => SuspiciousMktempUsage.into(), + ["tempfile", "mktemp"] => Diagnostic::new(SuspiciousMktempUsage, range), // Eval - ["" | "builtins", "eval"] => SuspiciousEvalUsage.into(), + ["" | "builtins", "eval"] => Diagnostic::new(SuspiciousEvalUsage, range), // MarkSafe ["django", "utils", "safestring" | "html", "mark_safe"] => { @@ -1078,7 +1078,7 @@ fn suspicious_function( } } } - SuspiciousMarkSafeUsage.into() + Diagnostic::new(SuspiciousMarkSafeUsage, range) } // URLOpen (`Request`) @@ -1100,7 +1100,7 @@ fn suspicious_function( } } } - SuspiciousURLOpenUsage.into() + Diagnostic::new(SuspiciousURLOpenUsage, range) } // URLOpen (`urlopen`, `urlretrieve`) @@ -1146,64 +1146,75 @@ fn suspicious_function( } } } - SuspiciousURLOpenUsage.into() + Diagnostic::new(SuspiciousURLOpenUsage, range) } // URLOpen (`URLopener`, `FancyURLopener`) ["urllib", "request", "URLopener" | "FancyURLopener"] | ["six", "moves", "urllib", "request", "URLopener" | "FancyURLopener"] => { - SuspiciousURLOpenUsage.into() + Diagnostic::new(SuspiciousURLOpenUsage, range) } // NonCryptographicRandom ["random", "Random" | "random" | "randrange" | "randint" | "choice" | "choices" | "uniform" - | "triangular" | "randbytes"] => SuspiciousNonCryptographicRandomUsage.into(), + | "triangular" | "randbytes"] => { + Diagnostic::new(SuspiciousNonCryptographicRandomUsage, range) + } // UnverifiedContext - ["ssl", "_create_unverified_context"] => SuspiciousUnverifiedContextUsage.into(), + ["ssl", "_create_unverified_context"] => { + Diagnostic::new(SuspiciousUnverifiedContextUsage, range) + } // XMLCElementTree ["xml", "etree", "cElementTree", "parse" | "iterparse" | "fromstring" | "XMLParser"] => { - SuspiciousXMLCElementTreeUsage.into() + Diagnostic::new(SuspiciousXMLCElementTreeUsage, range) } // XMLElementTree ["xml", "etree", "ElementTree", "parse" | "iterparse" | "fromstring" | "XMLParser"] => { - SuspiciousXMLElementTreeUsage.into() + Diagnostic::new(SuspiciousXMLElementTreeUsage, range) } // XMLExpatReader - ["xml", "sax", "expatreader", "create_parser"] => SuspiciousXMLExpatReaderUsage.into(), + ["xml", "sax", "expatreader", "create_parser"] => { + Diagnostic::new(SuspiciousXMLExpatReaderUsage, range) + } // XMLExpatBuilder ["xml", "dom", "expatbuilder", "parse" | "parseString"] => { - SuspiciousXMLExpatBuilderUsage.into() + Diagnostic::new(SuspiciousXMLExpatBuilderUsage, range) } // XMLSax - ["xml", "sax", "parse" | "parseString" | "make_parser"] => SuspiciousXMLSaxUsage.into(), + ["xml", "sax", "parse" | "parseString" | "make_parser"] => { + Diagnostic::new(SuspiciousXMLSaxUsage, range) + } // XMLMiniDOM - ["xml", "dom", "minidom", "parse" | "parseString"] => SuspiciousXMLMiniDOMUsage.into(), + ["xml", "dom", "minidom", "parse" | "parseString"] => { + Diagnostic::new(SuspiciousXMLMiniDOMUsage, range) + } // XMLPullDOM - ["xml", "dom", "pulldom", "parse" | "parseString"] => SuspiciousXMLPullDOMUsage.into(), + ["xml", "dom", "pulldom", "parse" | "parseString"] => { + Diagnostic::new(SuspiciousXMLPullDOMUsage, range) + } // XMLETree ["lxml", "etree", "parse" | "fromstring" | "RestrictedElement" | "GlobalParserTLS" | "getDefaultParser" - | "check_docinfo"] => SuspiciousXMLETreeUsage.into(), + | "check_docinfo"] => Diagnostic::new(SuspiciousXMLETreeUsage, range), // Telnet - ["telnetlib", ..] => SuspiciousTelnetUsage.into(), + ["telnetlib", ..] => Diagnostic::new(SuspiciousTelnetUsage, range), // FTPLib - ["ftplib", ..] => SuspiciousFTPLibUsage.into(), + ["ftplib", ..] => Diagnostic::new(SuspiciousFTPLibUsage, range), _ => return, }; - let diagnostic = Diagnostic::new(diagnostic_kind, range); - if checker.enabled(diagnostic.kind.rule()) { + if checker.enabled(diagnostic.rule()) { checker.report_diagnostic(diagnostic); } } diff --git a/crates/ruff_linter/src/rules/flake8_bandit/rules/suspicious_imports.rs b/crates/ruff_linter/src/rules/flake8_bandit/rules/suspicious_imports.rs index deb208863d..f6dd1b6384 100644 --- a/crates/ruff_linter/src/rules/flake8_bandit/rules/suspicious_imports.rs +++ b/crates/ruff_linter/src/rules/flake8_bandit/rules/suspicious_imports.rs @@ -1,7 +1,7 @@ //! Check for imports of or from suspicious modules. //! //! See: -use ruff_diagnostics::{Diagnostic, DiagnosticKind, Violation}; +use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, ViolationMetadata}; use ruff_python_ast::{self as ast, Stmt}; use ruff_text_size::{Ranged, TextRange}; @@ -361,76 +361,44 @@ pub(crate) fn suspicious_imports(checker: &Checker, stmt: &Stmt) { Stmt::Import(ast::StmtImport { names, .. }) => { for name in names { match name.name.as_str() { - "telnetlib" => check_and_push_diagnostic( - checker, - DiagnosticKind::from(SuspiciousTelnetlibImport), - name.range, - ), - "ftplib" => check_and_push_diagnostic( - checker, - DiagnosticKind::from(SuspiciousFtplibImport), - name.range, - ), - "pickle" | "cPickle" | "dill" | "shelve" => check_and_push_diagnostic( - checker, - DiagnosticKind::from(SuspiciousPickleImport), - name.range, - ), - "subprocess" => check_and_push_diagnostic( - checker, - DiagnosticKind::from(SuspiciousSubprocessImport), - name.range, - ), - "xml.etree.cElementTree" | "xml.etree.ElementTree" => { - check_and_push_diagnostic( - checker, - DiagnosticKind::from(SuspiciousXmlEtreeImport), - name.range, - ); + "telnetlib" => { + check_and_push_diagnostic(checker, SuspiciousTelnetlibImport, name.range); + } + "ftplib" => { + check_and_push_diagnostic(checker, SuspiciousFtplibImport, name.range); + } + "pickle" | "cPickle" | "dill" | "shelve" => { + check_and_push_diagnostic(checker, SuspiciousPickleImport, name.range); + } + "subprocess" => { + check_and_push_diagnostic(checker, SuspiciousSubprocessImport, name.range); + } + "xml.etree.cElementTree" | "xml.etree.ElementTree" => { + check_and_push_diagnostic(checker, SuspiciousXmlEtreeImport, name.range); + } + "xml.sax" => { + check_and_push_diagnostic(checker, SuspiciousXmlSaxImport, name.range); + } + "xml.dom.expatbuilder" => { + check_and_push_diagnostic(checker, SuspiciousXmlExpatImport, name.range); + } + "xml.dom.minidom" => { + check_and_push_diagnostic(checker, SuspiciousXmlMinidomImport, name.range); + } + "xml.dom.pulldom" => { + check_and_push_diagnostic(checker, SuspiciousXmlPulldomImport, name.range); + } + "lxml" => check_and_push_diagnostic(checker, SuspiciousLxmlImport, name.range), + "xmlrpc" => { + check_and_push_diagnostic(checker, SuspiciousXmlrpcImport, name.range); } - "xml.sax" => check_and_push_diagnostic( - checker, - DiagnosticKind::from(SuspiciousXmlSaxImport), - name.range, - ), - "xml.dom.expatbuilder" => check_and_push_diagnostic( - checker, - DiagnosticKind::from(SuspiciousXmlExpatImport), - name.range, - ), - "xml.dom.minidom" => check_and_push_diagnostic( - checker, - DiagnosticKind::from(SuspiciousXmlMinidomImport), - name.range, - ), - "xml.dom.pulldom" => check_and_push_diagnostic( - checker, - DiagnosticKind::from(SuspiciousXmlPulldomImport), - name.range, - ), - "lxml" => check_and_push_diagnostic( - checker, - DiagnosticKind::from(SuspiciousLxmlImport), - name.range, - ), - "xmlrpc" => check_and_push_diagnostic( - checker, - DiagnosticKind::from(SuspiciousXmlrpcImport), - name.range, - ), "Crypto.Cipher" | "Crypto.Hash" | "Crypto.IO" | "Crypto.Protocol" | "Crypto.PublicKey" | "Crypto.Random" | "Crypto.Signature" | "Crypto.Util" => { - check_and_push_diagnostic( - checker, - DiagnosticKind::from(SuspiciousPycryptoImport), - name.range, - ); + check_and_push_diagnostic(checker, SuspiciousPycryptoImport, name.range); + } + "pyghmi" => { + check_and_push_diagnostic(checker, SuspiciousPyghmiImport, name.range); } - "pyghmi" => check_and_push_diagnostic( - checker, - DiagnosticKind::from(SuspiciousPyghmiImport), - name.range, - ), _ => {} } } @@ -440,22 +408,18 @@ pub(crate) fn suspicious_imports(checker: &Checker, stmt: &Stmt) { match identifier.as_str() { "telnetlib" => check_and_push_diagnostic( checker, - DiagnosticKind::from(SuspiciousTelnetlibImport), - identifier.range(), - ), - "ftplib" => check_and_push_diagnostic( - checker, - DiagnosticKind::from(SuspiciousFtplibImport), - identifier.range(), - ), - "pickle" | "cPickle" | "dill" | "shelve" => check_and_push_diagnostic( - checker, - DiagnosticKind::from(SuspiciousPickleImport), + SuspiciousTelnetlibImport, identifier.range(), ), + "ftplib" => { + check_and_push_diagnostic(checker, SuspiciousFtplibImport, identifier.range()); + } + "pickle" | "cPickle" | "dill" | "shelve" => { + check_and_push_diagnostic(checker, SuspiciousPickleImport, identifier.range()); + } "subprocess" => check_and_push_diagnostic( checker, - DiagnosticKind::from(SuspiciousSubprocessImport), + SuspiciousSubprocessImport, identifier.range(), ), "xml.etree" => { @@ -463,7 +427,7 @@ pub(crate) fn suspicious_imports(checker: &Checker, stmt: &Stmt) { if matches!(name.name.as_str(), "cElementTree" | "ElementTree") { check_and_push_diagnostic( checker, - DiagnosticKind::from(SuspiciousXmlEtreeImport), + SuspiciousXmlEtreeImport, identifier.range(), ); } @@ -472,7 +436,7 @@ pub(crate) fn suspicious_imports(checker: &Checker, stmt: &Stmt) { "xml.etree.cElementTree" | "xml.etree.ElementTree" => { check_and_push_diagnostic( checker, - DiagnosticKind::from(SuspiciousXmlEtreeImport), + SuspiciousXmlEtreeImport, identifier.range(), ); } @@ -481,70 +445,66 @@ pub(crate) fn suspicious_imports(checker: &Checker, stmt: &Stmt) { if name.name.as_str() == "sax" { check_and_push_diagnostic( checker, - DiagnosticKind::from(SuspiciousXmlSaxImport), + SuspiciousXmlSaxImport, identifier.range(), ); } } } - "xml.sax" => check_and_push_diagnostic( - checker, - DiagnosticKind::from(SuspiciousXmlSaxImport), - identifier.range(), - ), + "xml.sax" => { + check_and_push_diagnostic(checker, SuspiciousXmlSaxImport, identifier.range()); + } "xml.dom" => { for name in names { match name.name.as_str() { "expatbuilder" => check_and_push_diagnostic( checker, - DiagnosticKind::from(SuspiciousXmlExpatImport), + SuspiciousXmlExpatImport, identifier.range(), ), "minidom" => check_and_push_diagnostic( checker, - DiagnosticKind::from(SuspiciousXmlMinidomImport), + SuspiciousXmlMinidomImport, identifier.range(), ), "pulldom" => check_and_push_diagnostic( checker, - DiagnosticKind::from(SuspiciousXmlPulldomImport), + SuspiciousXmlPulldomImport, identifier.range(), ), _ => (), } } } - "xml.dom.expatbuilder" => check_and_push_diagnostic( - checker, - DiagnosticKind::from(SuspiciousXmlExpatImport), - identifier.range(), - ), + "xml.dom.expatbuilder" => { + check_and_push_diagnostic( + checker, + SuspiciousXmlExpatImport, + identifier.range(), + ); + } "xml.dom.minidom" => check_and_push_diagnostic( checker, - DiagnosticKind::from(SuspiciousXmlMinidomImport), + SuspiciousXmlMinidomImport, identifier.range(), ), "xml.dom.pulldom" => check_and_push_diagnostic( checker, - DiagnosticKind::from(SuspiciousXmlPulldomImport), - identifier.range(), - ), - "lxml" => check_and_push_diagnostic( - checker, - DiagnosticKind::from(SuspiciousLxmlImport), - identifier.range(), - ), - "xmlrpc" => check_and_push_diagnostic( - checker, - DiagnosticKind::from(SuspiciousXmlrpcImport), + SuspiciousXmlPulldomImport, identifier.range(), ), + "lxml" => { + check_and_push_diagnostic(checker, SuspiciousLxmlImport, identifier.range()); + } + "xmlrpc" => { + check_and_push_diagnostic(checker, SuspiciousXmlrpcImport, identifier.range()); + } "wsgiref.handlers" => { for name in names { if name.name.as_str() == "CGIHandler" { check_and_push_diagnostic( checker, - DiagnosticKind::from(SuspiciousHttpoxyImport), + SuspiciousHttpoxyImport, identifier.range(), ); } @@ -555,7 +515,7 @@ pub(crate) fn suspicious_imports(checker: &Checker, stmt: &Stmt) { if name.name.as_str() == "CGIScript" { check_and_push_diagnostic( checker, - DiagnosticKind::from(SuspiciousHttpoxyImport), + SuspiciousHttpoxyImport, identifier.range(), ); } @@ -576,7 +536,7 @@ pub(crate) fn suspicious_imports(checker: &Checker, stmt: &Stmt) { ) { check_and_push_diagnostic( checker, - DiagnosticKind::from(SuspiciousPycryptoImport), + SuspiciousPycryptoImport, identifier.range(), ); } @@ -586,15 +546,13 @@ pub(crate) fn suspicious_imports(checker: &Checker, stmt: &Stmt) { | "Crypto.PublicKey" | "Crypto.Random" | "Crypto.Signature" | "Crypto.Util" => { check_and_push_diagnostic( checker, - DiagnosticKind::from(SuspiciousPycryptoImport), + SuspiciousPycryptoImport, identifier.range(), ); } - "pyghmi" => check_and_push_diagnostic( - checker, - DiagnosticKind::from(SuspiciousPyghmiImport), - identifier.range(), - ), + "pyghmi" => { + check_and_push_diagnostic(checker, SuspiciousPyghmiImport, identifier.range()); + } _ => {} } } @@ -602,9 +560,9 @@ pub(crate) fn suspicious_imports(checker: &Checker, stmt: &Stmt) { } } -fn check_and_push_diagnostic(checker: &Checker, diagnostic_kind: DiagnosticKind, range: TextRange) { - let diagnostic = Diagnostic::new::(diagnostic_kind, range); - if checker.enabled(diagnostic.kind.rule()) { +fn check_and_push_diagnostic(checker: &Checker, diagnostic: T, range: TextRange) { + let diagnostic = Diagnostic::new(diagnostic, range); + if checker.enabled(diagnostic.rule()) { checker.report_diagnostic(diagnostic); } } diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/rules/function_call_in_argument_default.rs b/crates/ruff_linter/src/rules/flake8_bugbear/rules/function_call_in_argument_default.rs index 74816eba63..3cd9f00561 100644 --- a/crates/ruff_linter/src/rules/flake8_bugbear/rules/function_call_in_argument_default.rs +++ b/crates/ruff_linter/src/rules/flake8_bugbear/rules/function_call_in_argument_default.rs @@ -1,8 +1,8 @@ use ruff_python_ast::{self as ast, Expr, Parameters}; -use ruff_text_size::{Ranged, TextRange}; +use ruff_text_size::Ranged; +use ruff_diagnostics::Diagnostic; use ruff_diagnostics::Violation; -use ruff_diagnostics::{Diagnostic, DiagnosticKind}; use ruff_macros::{derive_message_formats, ViolationMetadata}; use ruff_python_ast::name::{QualifiedName, UnqualifiedName}; use ruff_python_ast::visitor; @@ -83,7 +83,7 @@ impl Violation for FunctionCallInDefaultArgument { struct ArgumentDefaultVisitor<'a, 'b> { semantic: &'a SemanticModel<'b>, extend_immutable_calls: &'a [QualifiedName<'b>], - diagnostics: Vec<(DiagnosticKind, TextRange)>, + diagnostics: Vec, } impl<'a, 'b> ArgumentDefaultVisitor<'a, 'b> { @@ -109,11 +109,10 @@ impl Visitor<'_> for ArgumentDefaultVisitor<'_, '_> { is_immutable_newtype_call(name, self.semantic, self.extend_immutable_calls) }) { - self.diagnostics.push(( + self.diagnostics.push(Diagnostic::new( FunctionCallInDefaultArgument { name: UnqualifiedName::from_expr(func).map(|name| name.to_string()), - } - .into(), + }, expr.range(), )); } @@ -149,7 +148,5 @@ pub(crate) fn function_call_in_argument_default(checker: &Checker, parameters: & } } - for (check, range) in visitor.diagnostics { - checker.report_diagnostic(Diagnostic::new(check, range)); - } + checker.report_diagnostics(visitor.diagnostics); } diff --git a/crates/ruff_linter/src/rules/flake8_print/rules/print_call.rs b/crates/ruff_linter/src/rules/flake8_print/rules/print_call.rs index ec89576291..b152147538 100644 --- a/crates/ruff_linter/src/rules/flake8_print/rules/print_call.rs +++ b/crates/ruff_linter/src/rules/flake8_print/rules/print_call.rs @@ -124,7 +124,7 @@ pub(crate) fn print_call(checker: &Checker, call: &ast::ExprCall) { _ => return, }; - if !checker.enabled(diagnostic.kind.rule()) { + if !checker.enabled(diagnostic.rule()) { return; } diff --git a/crates/ruff_linter/src/rules/flake8_return/rules/function.rs b/crates/ruff_linter/src/rules/flake8_return/rules/function.rs index aa91da5e74..d8d43d810a 100644 --- a/crates/ruff_linter/src/rules/flake8_return/rules/function.rs +++ b/crates/ruff_linter/src/rules/flake8_return/rules/function.rs @@ -674,7 +674,7 @@ fn superfluous_else_node( elif_else_range(elif_else, checker.locator().contents()) .unwrap_or_else(|| elif_else.range()), ); - if checker.enabled(diagnostic.kind.rule()) { + if checker.enabled(diagnostic.rule()) { diagnostic.try_set_fix(|| { remove_else( elif_else, @@ -692,7 +692,7 @@ fn superfluous_else_node( elif_else_range(elif_else, checker.locator().contents()) .unwrap_or_else(|| elif_else.range()), ); - if checker.enabled(diagnostic.kind.rule()) { + if checker.enabled(diagnostic.rule()) { diagnostic.try_set_fix(|| { remove_else( elif_else, @@ -711,7 +711,7 @@ fn superfluous_else_node( elif_else_range(elif_else, checker.locator().contents()) .unwrap_or_else(|| elif_else.range()), ); - if checker.enabled(diagnostic.kind.rule()) { + if checker.enabled(diagnostic.rule()) { diagnostic.try_set_fix(|| { remove_else( elif_else, @@ -730,7 +730,7 @@ fn superfluous_else_node( elif_else_range(elif_else, checker.locator().contents()) .unwrap_or_else(|| elif_else.range()), ); - if checker.enabled(diagnostic.kind.rule()) { + if checker.enabled(diagnostic.rule()) { diagnostic.try_set_fix(|| { remove_else( elif_else, diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/rules/typing_only_runtime_import.rs b/crates/ruff_linter/src/rules/flake8_type_checking/rules/typing_only_runtime_import.rs index 2ddafcc71e..957cd80fca 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/rules/typing_only_runtime_import.rs +++ b/crates/ruff_linter/src/rules/flake8_type_checking/rules/typing_only_runtime_import.rs @@ -3,10 +3,10 @@ use std::borrow::Cow; use anyhow::Result; use rustc_hash::FxHashMap; -use ruff_diagnostics::{Diagnostic, DiagnosticKind, Fix, FixAvailability, Violation}; +use ruff_diagnostics::{Diagnostic, Fix, FixAvailability, Violation}; use ruff_macros::{derive_message_formats, ViolationMetadata}; use ruff_python_semantic::{Binding, Imported, NodeId, Scope}; -use ruff_text_size::Ranged; +use ruff_text_size::{Ranged, TextRange}; use crate::checkers::ast::Checker; use crate::codes::Rule; @@ -393,10 +393,8 @@ pub(crate) fn typing_only_runtime_import( .. } in imports { - let mut diagnostic = Diagnostic::new( - diagnostic_for(import_type, import.qualified_name().to_string()), - range, - ); + let mut diagnostic = + diagnostic_for(import_type, import.qualified_name().to_string(), range); if let Some(range) = parent_range { diagnostic.set_parent(range.start()); } @@ -417,10 +415,8 @@ pub(crate) fn typing_only_runtime_import( .. } in imports { - let mut diagnostic = Diagnostic::new( - diagnostic_for(import_type, import.qualified_name().to_string()), - range, - ); + let mut diagnostic = + diagnostic_for(import_type, import.qualified_name().to_string(), range); if let Some(range) = parent_range { diagnostic.set_parent(range.start()); } @@ -440,11 +436,17 @@ fn rule_for(import_type: ImportType) -> Rule { } /// Return the [`Diagnostic`] for the given import type. -fn diagnostic_for(import_type: ImportType, qualified_name: String) -> DiagnosticKind { +fn diagnostic_for(import_type: ImportType, qualified_name: String, range: TextRange) -> Diagnostic { match import_type { - ImportType::StandardLibrary => TypingOnlyStandardLibraryImport { qualified_name }.into(), - ImportType::ThirdParty => TypingOnlyThirdPartyImport { qualified_name }.into(), - ImportType::FirstParty => TypingOnlyFirstPartyImport { qualified_name }.into(), + ImportType::StandardLibrary => { + Diagnostic::new(TypingOnlyStandardLibraryImport { qualified_name }, range) + } + ImportType::ThirdParty => { + Diagnostic::new(TypingOnlyThirdPartyImport { qualified_name }, range) + } + ImportType::FirstParty => { + Diagnostic::new(TypingOnlyFirstPartyImport { qualified_name }, range) + } _ => unreachable!("Unexpected import type"), } } diff --git a/crates/ruff_linter/src/rules/flake8_unused_arguments/rules/unused_arguments.rs b/crates/ruff_linter/src/rules/flake8_unused_arguments/rules/unused_arguments.rs index a8ceee48b8..333cd73adf 100644 --- a/crates/ruff_linter/src/rules/flake8_unused_arguments/rules/unused_arguments.rs +++ b/crates/ruff_linter/src/rules/flake8_unused_arguments/rules/unused_arguments.rs @@ -1,12 +1,11 @@ use ruff_python_ast as ast; use ruff_python_ast::{Parameter, Parameters, Stmt, StmtExpr, StmtFunctionDef, StmtRaise}; -use ruff_diagnostics::DiagnosticKind; use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, ViolationMetadata}; use ruff_python_semantic::analyze::{function_type, visibility}; use ruff_python_semantic::{Scope, ScopeKind, SemanticModel}; -use ruff_text_size::Ranged; +use ruff_text_size::{Ranged, TextRange}; use crate::checkers::ast::Checker; use crate::registry::Rule; @@ -223,13 +222,13 @@ enum Argumentable { } impl Argumentable { - fn check_for(self, name: String) -> DiagnosticKind { + fn check_for(self, name: String, range: TextRange) -> Diagnostic { match self { - Self::Function => UnusedFunctionArgument { name }.into(), - Self::Method => UnusedMethodArgument { name }.into(), - Self::ClassMethod => UnusedClassMethodArgument { name }.into(), - Self::StaticMethod => UnusedStaticMethodArgument { name }.into(), - Self::Lambda => UnusedLambdaArgument { name }.into(), + Self::Function => Diagnostic::new(UnusedFunctionArgument { name }, range), + Self::Method => Diagnostic::new(UnusedMethodArgument { name }, range), + Self::ClassMethod => Diagnostic::new(UnusedClassMethodArgument { name }, range), + Self::StaticMethod => Diagnostic::new(UnusedStaticMethodArgument { name }, range), + Self::Lambda => Diagnostic::new(UnusedLambdaArgument { name }, range), } } @@ -313,10 +312,7 @@ fn call<'a>( && binding.is_unused() && !dummy_variable_rgx.is_match(arg.name()) { - Some(Diagnostic::new( - argumentable.check_for(arg.name.to_string()), - binding.range(), - )) + Some(argumentable.check_for(arg.name.to_string(), binding.range())) } else { None } diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/replaceable_by_pathlib.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/replaceable_by_pathlib.rs index bf1b3c0661..1a2e97067e 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/replaceable_by_pathlib.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/replaceable_by_pathlib.rs @@ -1,4 +1,4 @@ -use ruff_diagnostics::{Diagnostic, DiagnosticKind}; +use ruff_diagnostics::Diagnostic; use ruff_python_ast::{self as ast, Expr, ExprBooleanLiteral, ExprCall}; use ruff_python_semantic::analyze::typing; use ruff_python_semantic::SemanticModel; @@ -22,9 +22,10 @@ pub(crate) fn replaceable_by_pathlib(checker: &Checker, call: &ExprCall) { return; }; - let diagnostic_kind: DiagnosticKind = match qualified_name.segments() { + let range = call.func.range(); + let diagnostic = match qualified_name.segments() { // PTH100 - ["os", "path", "abspath"] => OsPathAbspath.into(), + ["os", "path", "abspath"] => Diagnostic::new(OsPathAbspath, range), // PTH101 ["os", "chmod"] => { // `dir_fd` is not supported by pathlib, so check if it's set to non-default values. @@ -41,10 +42,10 @@ pub(crate) fn replaceable_by_pathlib(checker: &Checker, call: &ExprCall) { { return; } - OsChmod.into() + Diagnostic::new(OsChmod, range) } // PTH102 - ["os", "makedirs"] => OsMakedirs.into(), + ["os", "makedirs"] => Diagnostic::new(OsMakedirs, range), // PTH103 ["os", "mkdir"] => { // `dir_fd` is not supported by pathlib, so check if it's set to non-default values. @@ -56,7 +57,7 @@ pub(crate) fn replaceable_by_pathlib(checker: &Checker, call: &ExprCall) { if is_argument_non_default(&call.arguments, "dir_fd", 2) { return; } - OsMkdir.into() + Diagnostic::new(OsMkdir, range) } // PTH104 ["os", "rename"] => { @@ -72,7 +73,7 @@ pub(crate) fn replaceable_by_pathlib(checker: &Checker, call: &ExprCall) { { return; } - OsRename.into() + Diagnostic::new(OsRename, range) } // PTH105 ["os", "replace"] => { @@ -88,7 +89,7 @@ pub(crate) fn replaceable_by_pathlib(checker: &Checker, call: &ExprCall) { { return; } - OsReplace.into() + Diagnostic::new(OsReplace, range) } // PTH106 ["os", "rmdir"] => { @@ -101,7 +102,7 @@ pub(crate) fn replaceable_by_pathlib(checker: &Checker, call: &ExprCall) { if is_argument_non_default(&call.arguments, "dir_fd", 1) { return; } - OsRmdir.into() + Diagnostic::new(OsRmdir, range) } // PTH107 ["os", "remove"] => { @@ -114,7 +115,7 @@ pub(crate) fn replaceable_by_pathlib(checker: &Checker, call: &ExprCall) { if is_argument_non_default(&call.arguments, "dir_fd", 1) { return; } - OsRemove.into() + Diagnostic::new(OsRemove, range) } // PTH108 ["os", "unlink"] => { @@ -127,21 +128,21 @@ pub(crate) fn replaceable_by_pathlib(checker: &Checker, call: &ExprCall) { if is_argument_non_default(&call.arguments, "dir_fd", 1) { return; } - OsUnlink.into() + Diagnostic::new(OsUnlink, range) } // PTH109 - ["os", "getcwd"] => OsGetcwd.into(), - ["os", "getcwdb"] => OsGetcwd.into(), + ["os", "getcwd"] => Diagnostic::new(OsGetcwd, range), + ["os", "getcwdb"] => Diagnostic::new(OsGetcwd, range), // PTH110 - ["os", "path", "exists"] => OsPathExists.into(), + ["os", "path", "exists"] => Diagnostic::new(OsPathExists, range), // PTH111 - ["os", "path", "expanduser"] => OsPathExpanduser.into(), + ["os", "path", "expanduser"] => Diagnostic::new(OsPathExpanduser, range), // PTH112 - ["os", "path", "isdir"] => OsPathIsdir.into(), + ["os", "path", "isdir"] => Diagnostic::new(OsPathIsdir, range), // PTH113 - ["os", "path", "isfile"] => OsPathIsfile.into(), + ["os", "path", "isfile"] => Diagnostic::new(OsPathIsfile, range), // PTH114 - ["os", "path", "islink"] => OsPathIslink.into(), + ["os", "path", "islink"] => Diagnostic::new(OsPathIslink, range), // PTH116 ["os", "stat"] => { // `dir_fd` is not supported by pathlib, so check if it's set to non-default values. @@ -158,45 +159,49 @@ pub(crate) fn replaceable_by_pathlib(checker: &Checker, call: &ExprCall) { { return; } - OsStat.into() + Diagnostic::new(OsStat, range) } // PTH117 - ["os", "path", "isabs"] => OsPathIsabs.into(), + ["os", "path", "isabs"] => Diagnostic::new(OsPathIsabs, range), // PTH118 - ["os", "path", "join"] => OsPathJoin { - module: "path".to_string(), - joiner: if call.arguments.args.iter().any(Expr::is_starred_expr) { - Joiner::Joinpath - } else { - Joiner::Slash + ["os", "path", "join"] => Diagnostic::new( + OsPathJoin { + module: "path".to_string(), + joiner: if call.arguments.args.iter().any(Expr::is_starred_expr) { + Joiner::Joinpath + } else { + Joiner::Slash + }, }, - } - .into(), - ["os", "sep", "join"] => OsPathJoin { - module: "sep".to_string(), - joiner: if call.arguments.args.iter().any(Expr::is_starred_expr) { - Joiner::Joinpath - } else { - Joiner::Slash + range, + ), + ["os", "sep", "join"] => Diagnostic::new( + OsPathJoin { + module: "sep".to_string(), + joiner: if call.arguments.args.iter().any(Expr::is_starred_expr) { + Joiner::Joinpath + } else { + Joiner::Slash + }, }, - } - .into(), + range, + ), // PTH119 - ["os", "path", "basename"] => OsPathBasename.into(), + ["os", "path", "basename"] => Diagnostic::new(OsPathBasename, range), // PTH120 - ["os", "path", "dirname"] => OsPathDirname.into(), + ["os", "path", "dirname"] => Diagnostic::new(OsPathDirname, range), // PTH121 - ["os", "path", "samefile"] => OsPathSamefile.into(), + ["os", "path", "samefile"] => Diagnostic::new(OsPathSamefile, range), // PTH122 - ["os", "path", "splitext"] => OsPathSplitext.into(), + ["os", "path", "splitext"] => Diagnostic::new(OsPathSplitext, range), // PTH202 - ["os", "path", "getsize"] => OsPathGetsize.into(), + ["os", "path", "getsize"] => Diagnostic::new(OsPathGetsize, range), // PTH203 - ["os", "path", "getatime"] => OsPathGetatime.into(), + ["os", "path", "getatime"] => Diagnostic::new(OsPathGetatime, range), // PTH204 - ["os", "path", "getmtime"] => OsPathGetmtime.into(), + ["os", "path", "getmtime"] => Diagnostic::new(OsPathGetmtime, range), // PTH205 - ["os", "path", "getctime"] => OsPathGetctime.into(), + ["os", "path", "getctime"] => Diagnostic::new(OsPathGetctime, range), // PTH123 ["" | "builtins", "open"] => { // `closefd` and `opener` are not supported by pathlib, so check if they are @@ -231,10 +236,10 @@ pub(crate) fn replaceable_by_pathlib(checker: &Checker, call: &ExprCall) { { return; } - BuiltinOpen.into() + Diagnostic::new(BuiltinOpen, range) } // PTH124 - ["py", "path", "local"] => PyPath.into(), + ["py", "path", "local"] => Diagnostic::new(PyPath, range), // PTH207 ["glob", "glob"] => { // `dir_fd` is not supported by pathlib, so check if it's set to non-default values. @@ -247,10 +252,12 @@ pub(crate) fn replaceable_by_pathlib(checker: &Checker, call: &ExprCall) { return; } - Glob { - function: "glob".to_string(), - } - .into() + Diagnostic::new( + Glob { + function: "glob".to_string(), + }, + range, + ) } ["glob", "iglob"] => { @@ -264,10 +271,12 @@ pub(crate) fn replaceable_by_pathlib(checker: &Checker, call: &ExprCall) { return; } - Glob { - function: "iglob".to_string(), - } - .into() + Diagnostic::new( + Glob { + function: "iglob".to_string(), + }, + range, + ) } // PTH115 // Python 3.9+ @@ -281,7 +290,7 @@ pub(crate) fn replaceable_by_pathlib(checker: &Checker, call: &ExprCall) { if is_argument_non_default(&call.arguments, "dir_fd", 1) { return; } - OsReadlink.into() + Diagnostic::new(OsReadlink, range) } // PTH208 ["os", "listdir"] => { @@ -292,13 +301,13 @@ pub(crate) fn replaceable_by_pathlib(checker: &Checker, call: &ExprCall) { { return; } - OsListdir.into() + Diagnostic::new(OsListdir, range) } _ => return, }; - if checker.enabled(diagnostic_kind.rule()) { - checker.report_diagnostic(Diagnostic::new(diagnostic_kind, call.func.range())); + if checker.enabled(diagnostic.rule()) { + checker.report_diagnostic(diagnostic); } } diff --git a/crates/ruff_linter/src/rules/pandas_vet/rules/call.rs b/crates/ruff_linter/src/rules/pandas_vet/rules/call.rs index ce69acc790..7eb242bb10 100644 --- a/crates/ruff_linter/src/rules/pandas_vet/rules/call.rs +++ b/crates/ruff_linter/src/rules/pandas_vet/rules/call.rs @@ -1,7 +1,7 @@ use ruff_python_ast::{self as ast, Expr}; +use ruff_diagnostics::Diagnostic; use ruff_diagnostics::Violation; -use ruff_diagnostics::{Diagnostic, DiagnosticKind}; use ruff_macros::{derive_message_formats, ViolationMetadata}; use ruff_python_semantic::Modules; use ruff_text_size::Ranged; @@ -171,12 +171,14 @@ pub(crate) fn call(checker: &Checker, func: &Expr) { let Expr::Attribute(ast::ExprAttribute { value, attr, .. }) = func else { return; }; - let violation: DiagnosticKind = match attr.as_str() { + + let range = func.range(); + let diagnostic = match attr.as_str() { "isnull" if checker.settings.rules.enabled(Rule::PandasUseOfDotIsNull) => { - PandasUseOfDotIsNull.into() + Diagnostic::new(PandasUseOfDotIsNull, range) } "notnull" if checker.settings.rules.enabled(Rule::PandasUseOfDotNotNull) => { - PandasUseOfDotNotNull.into() + Diagnostic::new(PandasUseOfDotNotNull, range) } "pivot" | "unstack" if checker @@ -184,10 +186,10 @@ pub(crate) fn call(checker: &Checker, func: &Expr) { .rules .enabled(Rule::PandasUseOfDotPivotOrUnstack) => { - PandasUseOfDotPivotOrUnstack.into() + Diagnostic::new(PandasUseOfDotPivotOrUnstack, range) } "stack" if checker.settings.rules.enabled(Rule::PandasUseOfDotStack) => { - PandasUseOfDotStack.into() + Diagnostic::new(PandasUseOfDotStack, range) } _ => return, }; @@ -200,5 +202,5 @@ pub(crate) fn call(checker: &Checker, func: &Expr) { return; } - checker.report_diagnostic(Diagnostic::new(violation, func.range())); + checker.report_diagnostic(diagnostic); } diff --git a/crates/ruff_linter/src/rules/pandas_vet/rules/subscript.rs b/crates/ruff_linter/src/rules/pandas_vet/rules/subscript.rs index 4831569b1f..f6497b3cf9 100644 --- a/crates/ruff_linter/src/rules/pandas_vet/rules/subscript.rs +++ b/crates/ruff_linter/src/rules/pandas_vet/rules/subscript.rs @@ -1,7 +1,7 @@ use ruff_python_ast::{self as ast, Expr}; +use ruff_diagnostics::Diagnostic; use ruff_diagnostics::Violation; -use ruff_diagnostics::{Diagnostic, DiagnosticKind}; use ruff_macros::{derive_message_formats, ViolationMetadata}; use ruff_python_semantic::Modules; use ruff_text_size::Ranged; @@ -152,11 +152,17 @@ pub(crate) fn subscript(checker: &Checker, value: &Expr, expr: &Expr) { return; }; - let violation: DiagnosticKind = match attr.as_str() { - "ix" if checker.settings.rules.enabled(Rule::PandasUseOfDotIx) => PandasUseOfDotIx.into(), - "at" if checker.settings.rules.enabled(Rule::PandasUseOfDotAt) => PandasUseOfDotAt.into(), + let range = expr.range(); + + let diagnostic = match attr.as_str() { + "ix" if checker.settings.rules.enabled(Rule::PandasUseOfDotIx) => { + Diagnostic::new(PandasUseOfDotIx, range) + } + "at" if checker.settings.rules.enabled(Rule::PandasUseOfDotAt) => { + Diagnostic::new(PandasUseOfDotAt, range) + } "iat" if checker.settings.rules.enabled(Rule::PandasUseOfDotIat) => { - PandasUseOfDotIat.into() + Diagnostic::new(PandasUseOfDotIat, range) } _ => return, }; @@ -170,5 +176,5 @@ pub(crate) fn subscript(checker: &Checker, value: &Expr, expr: &Expr) { return; } - checker.report_diagnostic(Diagnostic::new(violation, expr.range())); + checker.report_diagnostic(diagnostic); } diff --git a/crates/ruff_linter/src/rules/pep8_naming/rules/invalid_first_argument_name.rs b/crates/ruff_linter/src/rules/pep8_naming/rules/invalid_first_argument_name.rs index f09e0b476a..7c9ac5c9fe 100644 --- a/crates/ruff_linter/src/rules/pep8_naming/rules/invalid_first_argument_name.rs +++ b/crates/ruff_linter/src/rules/pep8_naming/rules/invalid_first_argument_name.rs @@ -1,6 +1,6 @@ use anyhow::Result; -use ruff_diagnostics::{Diagnostic, DiagnosticKind, Fix, Violation}; +use ruff_diagnostics::{Diagnostic, Fix, Violation}; use ruff_macros::{derive_message_formats, ViolationMetadata}; use ruff_python_ast as ast; use ruff_python_ast::ParameterWithDefault; @@ -8,7 +8,7 @@ use ruff_python_codegen::Stylist; use ruff_python_semantic::analyze::class::{is_metaclass, IsMetaclass}; use ruff_python_semantic::analyze::function_type; use ruff_python_semantic::{Scope, ScopeKind, SemanticModel}; -use ruff_text_size::Ranged; +use ruff_text_size::{Ranged, TextRange}; use crate::checkers::ast::Checker; use crate::registry::Rule; @@ -167,14 +167,18 @@ enum FunctionType { } impl FunctionType { - fn diagnostic_kind(self, argument_name: String) -> DiagnosticKind { + fn diagnostic_kind(self, argument_name: String, range: TextRange) -> Diagnostic { match self { - Self::Method => InvalidFirstArgumentNameForMethod { argument_name }.into(), - Self::ClassMethod => InvalidFirstArgumentNameForClassMethod { - argument_name, - is_new: false, + Self::Method => { + Diagnostic::new(InvalidFirstArgumentNameForMethod { argument_name }, range) } - .into(), + Self::ClassMethod => Diagnostic::new( + InvalidFirstArgumentNameForClassMethod { + argument_name, + is_new: false, + }, + range, + ), } } @@ -262,10 +266,8 @@ pub(crate) fn invalid_first_argument_name(checker: &Checker, scope: &Scope) { return; } - let mut diagnostic = Diagnostic::new( - function_type.diagnostic_kind(self_or_cls.name.to_string()), - self_or_cls.range(), - ); + let mut diagnostic = + function_type.diagnostic_kind(self_or_cls.name.to_string(), self_or_cls.range()); diagnostic.try_set_optional_fix(|| { rename_parameter( scope, diff --git a/crates/ruff_linter/src/rules/pycodestyle/rules/logical_lines/indentation.rs b/crates/ruff_linter/src/rules/pycodestyle/rules/logical_lines/indentation.rs index c5f8925784..dc54a67f3e 100644 --- a/crates/ruff_linter/src/rules/pycodestyle/rules/logical_lines/indentation.rs +++ b/crates/ruff_linter/src/rules/pycodestyle/rules/logical_lines/indentation.rs @@ -1,7 +1,8 @@ -use ruff_diagnostics::DiagnosticKind; +use ruff_diagnostics::Diagnostic; use ruff_diagnostics::Violation; use ruff_macros::{derive_message_formats, ViolationMetadata}; use ruff_python_parser::TokenKind; +use ruff_text_size::TextRange; use super::LogicalLine; @@ -261,18 +262,25 @@ pub(crate) fn indentation( indent_level: usize, prev_indent_level: Option, indent_size: usize, -) -> Vec { + range: TextRange, +) -> Vec { let mut diagnostics = vec![]; if indent_level % indent_size != 0 { diagnostics.push(if logical_line.is_comment_only() { - DiagnosticKind::from(IndentationWithInvalidMultipleComment { - indent_width: indent_size, - }) + Diagnostic::new( + IndentationWithInvalidMultipleComment { + indent_width: indent_size, + }, + range, + ) } else { - DiagnosticKind::from(IndentationWithInvalidMultiple { - indent_width: indent_size, - }) + Diagnostic::new( + IndentationWithInvalidMultiple { + indent_width: indent_size, + }, + range, + ) }); } let indent_expect = prev_logical_line @@ -281,29 +289,29 @@ pub(crate) fn indentation( if indent_expect && indent_level <= prev_indent_level.unwrap_or(0) { diagnostics.push(if logical_line.is_comment_only() { - DiagnosticKind::from(NoIndentedBlockComment) + Diagnostic::new(NoIndentedBlockComment, range) } else { - DiagnosticKind::from(NoIndentedBlock) + Diagnostic::new(NoIndentedBlock, range) }); } else if !indent_expect && prev_indent_level.is_some_and(|prev_indent_level| indent_level > prev_indent_level) { diagnostics.push(if logical_line.is_comment_only() { - DiagnosticKind::from(UnexpectedIndentationComment) + Diagnostic::new(UnexpectedIndentationComment, range) } else { - DiagnosticKind::from(UnexpectedIndentation) + Diagnostic::new(UnexpectedIndentation, range) }); } if indent_expect { let expected_indent_amount = if indent_char == '\t' { 8 } else { 4 }; let expected_indent_level = prev_indent_level.unwrap_or(0) + expected_indent_amount; if indent_level > expected_indent_level { - diagnostics.push( + diagnostics.push(Diagnostic::new( OverIndented { is_comment: logical_line.is_comment_only(), - } - .into(), - ); + }, + range, + )); } } diff --git a/crates/ruff_linter/src/rules/pycodestyle/rules/logical_lines/missing_whitespace_around_operator.rs b/crates/ruff_linter/src/rules/pycodestyle/rules/logical_lines/missing_whitespace_around_operator.rs index 03b8e16c4d..69171396fe 100644 --- a/crates/ruff_linter/src/rules/pycodestyle/rules/logical_lines/missing_whitespace_around_operator.rs +++ b/crates/ruff_linter/src/rules/pycodestyle/rules/logical_lines/missing_whitespace_around_operator.rs @@ -1,7 +1,7 @@ -use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, DiagnosticKind, Edit, Fix}; +use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix}; use ruff_macros::{derive_message_formats, ViolationMetadata}; use ruff_python_parser::TokenKind; -use ruff_text_size::Ranged; +use ruff_text_size::{Ranged, TextRange}; use crate::checkers::logical_lines::LogicalLinesContext; use crate::rules::pycodestyle::helpers::is_non_logical_token; @@ -252,34 +252,31 @@ pub(crate) fn missing_whitespace_around_operator( match (has_leading_trivia, has_trailing_trivia) { // Operator with trailing but no leading space, enforce consistent spacing. (false, true) => { - let mut diagnostic = - Diagnostic::new(diagnostic_kind_for_operator(kind), token.range()); - diagnostic.set_fix(Fix::safe_edit(Edit::insertion( - " ".to_string(), - token.start(), - ))); - context.push_diagnostic(diagnostic); + context.push_diagnostic( + diagnostic_kind_for_operator(kind, token.range()).with_fix(Fix::safe_edit( + Edit::insertion(" ".to_string(), token.start()), + )), + ); } // Operator with leading but no trailing space, enforce consistent spacing. (true, false) => { - let mut diagnostic = - Diagnostic::new(diagnostic_kind_for_operator(kind), token.range()); - diagnostic.set_fix(Fix::safe_edit(Edit::insertion( - " ".to_string(), - token.end(), - ))); - context.push_diagnostic(diagnostic); + context.push_diagnostic( + diagnostic_kind_for_operator(kind, token.range()).with_fix(Fix::safe_edit( + Edit::insertion(" ".to_string(), token.end()), + )), + ); } // Operator with no space, require spaces if it is required by the operator. (false, false) => { if needs_space == NeedsSpace::Yes { - let mut diagnostic = - Diagnostic::new(diagnostic_kind_for_operator(kind), token.range()); - diagnostic.set_fix(Fix::safe_edits( - Edit::insertion(" ".to_string(), token.start()), - [Edit::insertion(" ".to_string(), token.end())], - )); - context.push_diagnostic(diagnostic); + context.push_diagnostic( + diagnostic_kind_for_operator(kind, token.range()).with_fix( + Fix::safe_edits( + Edit::insertion(" ".to_string(), token.start()), + [Edit::insertion(" ".to_string(), token.end())], + ), + ), + ); } } (true, true) => { @@ -317,15 +314,15 @@ impl From for NeedsSpace { } } -fn diagnostic_kind_for_operator(operator: TokenKind) -> DiagnosticKind { +fn diagnostic_kind_for_operator(operator: TokenKind, range: TextRange) -> Diagnostic { if operator == TokenKind::Percent { - DiagnosticKind::from(MissingWhitespaceAroundModuloOperator) + Diagnostic::new(MissingWhitespaceAroundModuloOperator, range) } else if operator.is_bitwise_or_shift() { - DiagnosticKind::from(MissingWhitespaceAroundBitwiseOrShiftOperator) + Diagnostic::new(MissingWhitespaceAroundBitwiseOrShiftOperator, range) } else if operator.is_arithmetic() { - DiagnosticKind::from(MissingWhitespaceAroundArithmeticOperator) + Diagnostic::new(MissingWhitespaceAroundArithmeticOperator, range) } else { - DiagnosticKind::from(MissingWhitespaceAroundOperator) + Diagnostic::new(MissingWhitespaceAroundOperator, range) } } diff --git a/crates/ruff_linter/src/rules/pyflakes/mod.rs b/crates/ruff_linter/src/rules/pyflakes/mod.rs index eb4b4a7e1d..dc20138649 100644 --- a/crates/ruff_linter/src/rules/pyflakes/mod.rs +++ b/crates/ruff_linter/src/rules/pyflakes/mod.rs @@ -777,7 +777,7 @@ mod tests { let actual = messages .iter() .filter_map(Message::as_diagnostic_message) - .map(|diagnostic| diagnostic.kind.rule()) + .map(AsRule::rule) .collect::>(); assert_eq!(actual, expected); } diff --git a/crates/ruff_linter/src/rules/pylint/rules/invalid_string_characters.rs b/crates/ruff_linter/src/rules/pylint/rules/invalid_string_characters.rs index 7a30ee0763..959bb0aa60 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/invalid_string_characters.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/invalid_string_characters.rs @@ -1,4 +1,4 @@ -use ruff_diagnostics::{Diagnostic, DiagnosticKind, Edit, Fix, FixAvailability, Violation}; +use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation}; use ruff_macros::{derive_message_formats, ViolationMetadata}; use ruff_python_parser::{Token, TokenKind}; use ruff_text_size::{Ranged, TextLen, TextRange, TextSize}; @@ -193,23 +193,23 @@ pub(crate) fn invalid_string_characters( }; for (column, match_) in text.match_indices(&['\x08', '\x1A', '\x1B', '\0', '\u{200b}']) { + let location = token.start() + TextSize::try_from(column).unwrap(); let c = match_.chars().next().unwrap(); - let (replacement, rule): (&str, DiagnosticKind) = match c { - '\x08' => ("\\b", InvalidCharacterBackspace.into()), - '\x1A' => ("\\x1A", InvalidCharacterSub.into()), - '\x1B' => ("\\x1B", InvalidCharacterEsc.into()), - '\0' => ("\\0", InvalidCharacterNul.into()), - '\u{200b}' => ("\\u200b", InvalidCharacterZeroWidthSpace.into()), + let range = TextRange::at(location, c.text_len()); + let (replacement, mut diagnostic) = match c { + '\x08' => ("\\b", Diagnostic::new(InvalidCharacterBackspace, range)), + '\x1A' => ("\\x1A", Diagnostic::new(InvalidCharacterSub, range)), + '\x1B' => ("\\x1B", Diagnostic::new(InvalidCharacterEsc, range)), + '\0' => ("\\0", Diagnostic::new(InvalidCharacterNul, range)), + '\u{200b}' => ( + "\\u200b", + Diagnostic::new(InvalidCharacterZeroWidthSpace, range), + ), _ => { continue; } }; - let location = token.start() + TextSize::try_from(column).unwrap(); - let range = TextRange::at(location, c.text_len()); - - let mut diagnostic = Diagnostic::new(rule, range); - if !token.unwrap_string_flags().is_raw_string() { let edit = Edit::range_replacement(replacement.to_string(), range); diagnostic.set_fix(Fix::safe_edit(edit)); diff --git a/crates/ruff_linter/src/rules/pylint/rules/no_method_decorator.rs b/crates/ruff_linter/src/rules/pylint/rules/no_method_decorator.rs index 8f6cd66d20..7bf363c3c8 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/no_method_decorator.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/no_method_decorator.rs @@ -1,6 +1,6 @@ use std::collections::HashMap; -use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, DiagnosticKind, Edit, Fix}; +use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix}; use ruff_macros::{derive_message_formats, ViolationMetadata}; use ruff_python_ast::name::Name; use ruff_python_ast::{self as ast, Expr, Stmt}; @@ -104,9 +104,9 @@ fn get_undecorated_methods(checker: &Checker, class_stmt: &Stmt, method_type: &M let mut explicit_decorator_calls: HashMap = HashMap::default(); - let (method_name, diagnostic_type): (&str, DiagnosticKind) = match method_type { - MethodType::Classmethod => ("classmethod", NoClassmethodDecorator.into()), - MethodType::Staticmethod => ("staticmethod", NoStaticmethodDecorator.into()), + let method_name = match method_type { + MethodType::Classmethod => "classmethod", + MethodType::Staticmethod => "staticmethod", }; // gather all explicit *method calls @@ -170,10 +170,11 @@ fn get_undecorated_methods(checker: &Checker, class_stmt: &Stmt, method_type: &M continue; } - let mut diagnostic = Diagnostic::new( - diagnostic_type.clone(), - TextRange::new(stmt.range().start(), stmt.range().start()), - ); + let range = TextRange::new(stmt.range().start(), stmt.range().start()); + let mut diagnostic = match method_type { + MethodType::Classmethod => Diagnostic::new(NoClassmethodDecorator, range), + MethodType::Staticmethod => Diagnostic::new(NoStaticmethodDecorator, range), + }; let indentation = indentation_at_offset(stmt.range().start(), checker.source()); diff --git a/crates/ruff_linter/src/rules/pyupgrade/rules/use_pep604_annotation.rs b/crates/ruff_linter/src/rules/pyupgrade/rules/use_pep604_annotation.rs index 48b5737fc4..f40fe1f833 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/rules/use_pep604_annotation.rs +++ b/crates/ruff_linter/src/rules/pyupgrade/rules/use_pep604_annotation.rs @@ -1,6 +1,4 @@ -use ruff_diagnostics::{ - Applicability, Diagnostic, DiagnosticKind, Edit, Fix, FixAvailability, Violation, -}; +use ruff_diagnostics::{Applicability, Diagnostic, Edit, Fix, FixAvailability, Violation}; use ruff_macros::{derive_message_formats, ViolationMetadata}; use ruff_python_ast::helpers::{pep_604_optional, pep_604_union}; use ruff_python_ast::PythonVersion; @@ -150,15 +148,15 @@ pub(crate) fn non_pep604_annotation( match operator { Pep604Operator::Optional => { - let (rule, diagnostic_kind) = if is_defer_optional_to_up045_enabled(checker.settings) { + let (rule, mut diagnostic) = if is_defer_optional_to_up045_enabled(checker.settings) { ( Rule::NonPEP604AnnotationOptional, - DiagnosticKind::from(NonPEP604AnnotationOptional), + Diagnostic::new(NonPEP604AnnotationOptional, expr.range()), ) } else { ( Rule::NonPEP604AnnotationUnion, - DiagnosticKind::from(NonPEP604AnnotationUnion), + Diagnostic::new(NonPEP604AnnotationUnion, expr.range()), ) }; @@ -166,8 +164,6 @@ pub(crate) fn non_pep604_annotation( return; } - let mut diagnostic = Diagnostic::new(diagnostic_kind, expr.range()); - if fixable { match slice { Expr::Tuple(_) => { diff --git a/crates/ruff_linter/src/rules/ruff/rules/ambiguous_unicode_character.rs b/crates/ruff_linter/src/rules/ruff/rules/ambiguous_unicode_character.rs index 2b8f8d3e23..3167d61f17 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/ambiguous_unicode_character.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/ambiguous_unicode_character.rs @@ -2,7 +2,7 @@ use std::fmt; use bitflags::bitflags; -use ruff_diagnostics::{Diagnostic, DiagnosticKind, Violation}; +use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, ViolationMetadata}; use ruff_python_ast::{self as ast, StringLike}; use ruff_text_size::{Ranged, TextLen, TextRange, TextSize}; @@ -355,27 +355,30 @@ impl Candidate { fn into_diagnostic(self, context: Context, settings: &LinterSettings) -> Option { if !settings.allowed_confusables.contains(&self.confusable) { let char_range = TextRange::at(self.offset, self.confusable.text_len()); - let diagnostic = Diagnostic::new::( - match context { - Context::String => AmbiguousUnicodeCharacterString { + let diagnostic = match context { + Context::String => Diagnostic::new( + AmbiguousUnicodeCharacterString { confusable: self.confusable, representant: self.representant, - } - .into(), - Context::Docstring => AmbiguousUnicodeCharacterDocstring { + }, + char_range, + ), + Context::Docstring => Diagnostic::new( + AmbiguousUnicodeCharacterDocstring { confusable: self.confusable, representant: self.representant, - } - .into(), - Context::Comment => AmbiguousUnicodeCharacterComment { + }, + char_range, + ), + Context::Comment => Diagnostic::new( + AmbiguousUnicodeCharacterComment { confusable: self.confusable, representant: self.representant, - } - .into(), - }, - char_range, - ); - if settings.rules.enabled(diagnostic.kind.rule()) { + }, + char_range, + ), + }; + if settings.rules.enabled(diagnostic.rule()) { return Some(diagnostic); } } diff --git a/crates/ruff_linter/src/test.rs b/crates/ruff_linter/src/test.rs index 1c8e527408..53d9d568d4 100644 --- a/crates/ruff_linter/src/test.rs +++ b/crates/ruff_linter/src/test.rs @@ -235,7 +235,7 @@ Source with applied fixes: .into_iter() .filter_map(Message::into_diagnostic_message) .map(|mut diagnostic| { - let rule = diagnostic.kind.rule(); + let rule = diagnostic.rule(); let fixable = diagnostic.fix.as_ref().is_some_and(|fix| { matches!( fix.applicability(), @@ -269,7 +269,7 @@ Either ensure you always emit a fix or change `Violation::FIX_AVAILABILITY` to e } assert!( - !(fixable && diagnostic.kind.suggestion.is_none()), + !(fixable && diagnostic.suggestion.is_none()), "Diagnostic emitted by {rule:?} is fixable but \ `Violation::fix_title` returns `None`" ); diff --git a/crates/ruff_macros/Cargo.toml b/crates/ruff_macros/Cargo.toml index c0215053c5..7e72d59855 100644 --- a/crates/ruff_macros/Cargo.toml +++ b/crates/ruff_macros/Cargo.toml @@ -17,6 +17,7 @@ doctest = false [dependencies] ruff_python_trivia = { workspace = true } +heck = { workspace = true } proc-macro2 = { workspace = true } quote = { workspace = true } syn = { workspace = true, features = ["derive", "parsing", "extra-traits", "full"] } diff --git a/crates/ruff_macros/src/kebab_case.rs b/crates/ruff_macros/src/kebab_case.rs index 1f6dcf42fd..3b7ef3f8db 100644 --- a/crates/ruff_macros/src/kebab_case.rs +++ b/crates/ruff_macros/src/kebab_case.rs @@ -1,19 +1,10 @@ +use heck::ToKebabCase; use proc_macro2::TokenStream; pub(crate) fn kebab_case(input: &syn::Ident) -> TokenStream { - let screaming_snake_case = input.to_string(); + let s = input.to_string(); - let mut kebab_case = String::with_capacity(screaming_snake_case.len()); - - for (i, word) in screaming_snake_case.split('_').enumerate() { - if i > 0 { - kebab_case.push('-'); - } - - kebab_case.push_str(&word.to_lowercase()); - } - - let kebab_case_lit = syn::LitStr::new(&kebab_case, input.span()); + let kebab_case_lit = syn::LitStr::new(&s.to_kebab_case(), input.span()); quote::quote!(#kebab_case_lit) } diff --git a/crates/ruff_macros/src/lib.rs b/crates/ruff_macros/src/lib.rs index 99bac807fe..9275cd6adc 100644 --- a/crates/ruff_macros/src/lib.rs +++ b/crates/ruff_macros/src/lib.rs @@ -49,7 +49,7 @@ pub fn derive_combine(input: TokenStream) -> TokenStream { .into() } -/// Converts a screaming snake case identifier to a kebab case string. +/// Converts an identifier to a kebab case string. #[proc_macro] pub fn kebab_case(input: TokenStream) -> TokenStream { let input = parse_macro_input!(input as syn::Ident); diff --git a/crates/ruff_macros/src/map_codes.rs b/crates/ruff_macros/src/map_codes.rs index a38f7cc543..25112809da 100644 --- a/crates/ruff_macros/src/map_codes.rs +++ b/crates/ruff_macros/src/map_codes.rs @@ -404,8 +404,6 @@ fn register_rules<'a>(input: impl Iterator) -> TokenStream { let mut rule_fixable_match_arms = quote!(); let mut rule_explanation_match_arms = quote!(); - let mut from_impls_for_diagnostic_kind = quote!(); - for Rule { name, attrs, path, .. } in input @@ -421,10 +419,6 @@ fn register_rules<'a>(input: impl Iterator) -> TokenStream { quote! {#(#attrs)* Self::#name => <#path as ruff_diagnostics::Violation>::FIX_AVAILABILITY,}, ); rule_explanation_match_arms.extend(quote! {#(#attrs)* Self::#name => #path::explain(),}); - - // Enable conversion from `DiagnosticKind` to `Rule`. - from_impls_for_diagnostic_kind - .extend(quote! {#(#attrs)* stringify!(#name) => Rule::#name,}); } quote! { @@ -443,6 +437,9 @@ fn register_rules<'a>(input: impl Iterator) -> TokenStream { ::ruff_macros::CacheKey, AsRefStr, ::strum_macros::IntoStaticStr, + ::strum_macros::EnumString, + ::serde::Serialize, + ::serde::Deserialize, )] #[repr(u16)] #[strum(serialize_all = "kebab-case")] @@ -466,13 +463,19 @@ fn register_rules<'a>(input: impl Iterator) -> TokenStream { } } - - impl AsRule for ruff_diagnostics::DiagnosticKind { + impl AsRule for ruff_diagnostics::Diagnostic { fn rule(&self) -> Rule { - match self.name.as_str() { - #from_impls_for_diagnostic_kind - _ => unreachable!("invalid rule name: {}", self.name), - } + self.name + .parse() + .unwrap_or_else(|_| unreachable!("invalid rule name: {}", self.name)) + } + } + + impl AsRule for crate::message::DiagnosticMessage { + fn rule(&self) -> Rule { + self.name + .parse() + .unwrap_or_else(|_| unreachable!("invalid rule name: {}", self.name)) } } } diff --git a/crates/ruff_macros/src/violation_metadata.rs b/crates/ruff_macros/src/violation_metadata.rs index 5973c3db3e..3cf60307ab 100644 --- a/crates/ruff_macros/src/violation_metadata.rs +++ b/crates/ruff_macros/src/violation_metadata.rs @@ -12,7 +12,7 @@ pub(crate) fn violation_metadata(input: DeriveInput) -> syn::Result #[expect(deprecated)] impl ruff_diagnostics::ViolationMetadata for #name { fn rule_name() -> &'static str { - stringify!(#name) + ::ruff_macros::kebab_case!(#name) } fn explain() -> Option<&'static str> { diff --git a/crates/ruff_server/src/lint.rs b/crates/ruff_server/src/lint.rs index 670b43cb75..586043a024 100644 --- a/crates/ruff_server/src/lint.rs +++ b/crates/ruff_server/src/lint.rs @@ -9,7 +9,7 @@ use crate::{ session::DocumentQuery, PositionEncoding, DIAGNOSTIC_NAME, }; -use ruff_diagnostics::{Applicability, DiagnosticKind, Edit, Fix}; +use ruff_diagnostics::{Applicability, Edit, Fix}; use ruff_linter::{ directives::{extract_directives, Flags}, generate_noqa_edits, @@ -32,7 +32,7 @@ use ruff_text_size::{Ranged, TextRange}; /// This is serialized on the diagnostic `data` field. #[derive(Serialize, Deserialize, Debug, Clone)] pub(crate) struct AssociatedDiagnosticData { - pub(crate) kind: DiagnosticKind, + pub(crate) title: String, /// Edits to fix the diagnostic. If this is empty, a fix /// does not exist. pub(crate) edits: Vec, @@ -227,10 +227,7 @@ pub(crate) fn fixes_for_diagnostics( Ok(Some(DiagnosticFix { fixed_diagnostic, code: associated_data.code, - title: associated_data - .kind - .suggestion - .unwrap_or(associated_data.kind.name), + title: associated_data.title, noqa_edit: associated_data.noqa_edit, edits: associated_data.edits, })) @@ -248,15 +245,16 @@ fn to_lsp_diagnostic( index: &LineIndex, encoding: PositionEncoding, ) -> (usize, lsp_types::Diagnostic) { + let rule = diagnostic.rule(); let DiagnosticMessage { - kind, range: diagnostic_range, fix, + name, + body, + suggestion, .. } = diagnostic; - let rule = kind.rule(); - let fix = fix.and_then(|fix| fix.applies(Applicability::Unsafe).then_some(fix)); let data = (fix.is_some() || noqa_edit.is_some()) @@ -275,7 +273,7 @@ fn to_lsp_diagnostic( new_text: noqa_edit.into_content().unwrap_or_default().into_string(), }); serde_json::to_value(AssociatedDiagnosticData { - kind: kind.clone(), + title: suggestion.unwrap_or_else(|| name.to_string()), noqa_edit, edits, code: rule.noqa_code().to_string(), @@ -314,7 +312,7 @@ fn to_lsp_diagnostic( }) }), source: Some(DIAGNOSTIC_NAME.into()), - message: kind.body, + message: body, related_information: None, data, }, diff --git a/crates/ruff_wasm/src/lib.rs b/crates/ruff_wasm/src/lib.rs index dc9c20323b..cf2d1857fa 100644 --- a/crates/ruff_wasm/src/lib.rs +++ b/crates/ruff_wasm/src/lib.rs @@ -210,26 +210,34 @@ impl Workspace { let messages: Vec = messages .into_iter() .map(|message| match message { - Message::Diagnostic(DiagnosticMessage { - kind, range, fix, .. - }) => ExpandedMessage { - code: Some(kind.rule().noqa_code().to_string()), - message: kind.body, - start_location: source_code.line_column(range.start()).into(), - end_location: source_code.line_column(range.end()).into(), - fix: fix.map(|fix| ExpandedFix { - message: kind.suggestion, - edits: fix - .edits() - .iter() - .map(|edit| ExpandedEdit { - location: source_code.line_column(edit.start()).into(), - end_location: source_code.line_column(edit.end()).into(), - content: edit.content().map(ToString::to_string), - }) - .collect(), - }), - }, + Message::Diagnostic(m) => { + let rule = m.rule(); + let DiagnosticMessage { + body, + suggestion, + range, + fix, + .. + } = m; + ExpandedMessage { + code: Some(rule.noqa_code().to_string()), + message: body, + start_location: source_code.line_column(range.start()).into(), + end_location: source_code.line_column(range.end()).into(), + fix: fix.map(|fix| ExpandedFix { + message: suggestion, + edits: fix + .edits() + .iter() + .map(|edit| ExpandedEdit { + location: source_code.line_column(edit.start()).into(), + end_location: source_code.line_column(edit.end()).into(), + content: edit.content().map(ToString::to_string), + }) + .collect(), + }), + } + } Message::SyntaxError(_) => ExpandedMessage { code: None, message: message.body().to_string(),