diff --git a/crates/ruff/src/cache.rs b/crates/ruff/src/cache.rs index 68df73cfdd..fdaf6b4a74 100644 --- a/crates/ruff/src/cache.rs +++ b/crates/ruff/src/cache.rs @@ -442,7 +442,7 @@ impl LintCacheData { // Parse the kebab-case rule name into a `Rule`. This will fail for syntax errors, so // this also serves to filter them out, but we shouldn't be caching files with syntax // errors anyway. - .filter_map(|msg| Some((msg.noqa_code().and_then(|code| code.rule())?, msg))) + .filter_map(|msg| Some((msg.name().parse().ok()?, msg))) .map(|(rule, msg)| { // Make sure that all message use the same source file. assert_eq!( diff --git a/crates/ruff_linter/src/checkers/ast/mod.rs b/crates/ruff_linter/src/checkers/ast/mod.rs index aee72035eb..9b0a306d34 100644 --- a/crates/ruff_linter/src/checkers/ast/mod.rs +++ b/crates/ruff_linter/src/checkers/ast/mod.rs @@ -28,7 +28,7 @@ use itertools::Itertools; use log::debug; use rustc_hash::{FxHashMap, FxHashSet}; -use ruff_diagnostics::IsolationLevel; +use ruff_diagnostics::{Applicability, Fix, IsolationLevel}; use ruff_notebook::{CellOffsets, NotebookIndex}; use ruff_python_ast::helpers::{collect_import_from_member, is_docstring_stmt, to_module_path}; use ruff_python_ast::identifier::Identifier; @@ -3154,6 +3154,7 @@ impl<'a> LintContext<'a> { DiagnosticGuard { context: self, diagnostic: Some(OldDiagnostic::new(kind, range, &self.source_file)), + rule: T::rule(), } } @@ -3167,10 +3168,12 @@ impl<'a> LintContext<'a> { kind: T, range: TextRange, ) -> Option> { - if self.is_rule_enabled(T::rule()) { + let rule = T::rule(); + if self.is_rule_enabled(rule) { Some(DiagnosticGuard { context: self, diagnostic: Some(OldDiagnostic::new(kind, range, &self.source_file)), + rule, }) } else { None @@ -3222,6 +3225,7 @@ pub(crate) struct DiagnosticGuard<'a, 'b> { /// /// This is always `Some` until the `Drop` (or `defuse`) call. diagnostic: Option, + rule: Rule, } impl DiagnosticGuard<'_, '_> { @@ -3234,6 +3238,50 @@ impl DiagnosticGuard<'_, '_> { } } +impl DiagnosticGuard<'_, '_> { + fn resolve_applicability(&self, fix: &Fix) -> Applicability { + self.context + .settings + .fix_safety + .resolve_applicability(self.rule, fix.applicability()) + } + + /// Set the [`Fix`] used to fix the diagnostic. + #[inline] + pub(crate) fn set_fix(&mut self, fix: Fix) { + if !self.context.rules.should_fix(self.rule) { + self.fix = None; + return; + } + let applicability = self.resolve_applicability(&fix); + self.fix = Some(fix.with_applicability(applicability)); + } + + /// Set the [`Fix`] used to fix the diagnostic, if the provided function returns `Ok`. + /// Otherwise, log the error. + #[inline] + pub(crate) fn try_set_fix(&mut self, func: impl FnOnce() -> anyhow::Result) { + match func() { + Ok(fix) => self.set_fix(fix), + Err(err) => log::debug!("Failed to create fix for {}: {}", self.name(), err), + } + } + + /// Set the [`Fix`] used to fix the diagnostic, if the provided function returns `Ok`. + /// Otherwise, log the error. + #[inline] + pub(crate) fn try_set_optional_fix( + &mut self, + func: impl FnOnce() -> anyhow::Result>, + ) { + match func() { + Ok(None) => {} + Ok(Some(fix)) => self.set_fix(fix), + Err(err) => log::debug!("Failed to create fix for {}: {}", self.name(), err), + } + } +} + impl std::ops::Deref for DiagnosticGuard<'_, '_> { type Target = OldDiagnostic; diff --git a/crates/ruff_linter/src/fix/edits.rs b/crates/ruff_linter/src/fix/edits.rs index b59d477fa5..dc2b7b68ac 100644 --- a/crates/ruff_linter/src/fix/edits.rs +++ b/crates/ruff_linter/src/fix/edits.rs @@ -749,15 +749,16 @@ x = 1 \ let diag = { use crate::rules::pycodestyle::rules::MissingNewlineAtEndOfFile; let mut iter = edits.into_iter(); - OldDiagnostic::new( + let mut diagnostic = OldDiagnostic::new( MissingNewlineAtEndOfFile, // The choice of rule here is arbitrary. TextRange::default(), &SourceFileBuilder::new("", "").finish(), - ) - .with_fix(Fix::safe_edits( + ); + diagnostic.fix = Some(Fix::safe_edits( iter.next().ok_or(anyhow!("expected edits nonempty"))?, iter, - )) + )); + diagnostic }; assert_eq!(apply_fixes([diag].iter(), &locator).code, expect); Ok(()) diff --git a/crates/ruff_linter/src/fix/mod.rs b/crates/ruff_linter/src/fix/mod.rs index 7c269f4438..39cb60c70b 100644 --- a/crates/ruff_linter/src/fix/mod.rs +++ b/crates/ruff_linter/src/fix/mod.rs @@ -186,12 +186,13 @@ mod tests { edit.into_iter() .map(|edit| { // The choice of rule here is arbitrary. - let diagnostic = OldDiagnostic::new( + let mut diagnostic = OldDiagnostic::new( MissingNewlineAtEndOfFile, edit.range(), &SourceFileBuilder::new(filename, source).finish(), ); - diagnostic.with_fix(Fix::safe_edit(edit)) + diagnostic.fix = Some(Fix::safe_edit(edit)); + diagnostic }) .collect() } diff --git a/crates/ruff_linter/src/linter.rs b/crates/ruff_linter/src/linter.rs index cf1852b032..a21b06ca8e 100644 --- a/crates/ruff_linter/src/linter.rs +++ b/crates/ruff_linter/src/linter.rs @@ -378,32 +378,7 @@ pub fn check_path( let (mut diagnostics, source_file) = context.into_parts(); - if parsed.has_valid_syntax() { - // Remove fixes for any rules marked as unfixable. - for diagnostic in &mut diagnostics { - if diagnostic - .noqa_code() - .and_then(|code| code.rule()) - .is_none_or(|rule| !settings.rules.should_fix(rule)) - { - diagnostic.fix = None; - } - } - - // Update fix applicability to account for overrides - if !settings.fix_safety.is_empty() { - for diagnostic in &mut diagnostics { - if let Some(fix) = diagnostic.fix.take() { - if let Some(rule) = diagnostic.noqa_code().and_then(|code| code.rule()) { - let fixed_applicability = settings - .fix_safety - .resolve_applicability(rule, fix.applicability()); - diagnostic.set_fix(fix.with_applicability(fixed_applicability)); - } - } - } - } - } else { + if !parsed.has_valid_syntax() { // Avoid fixing in case the source code contains syntax errors. for diagnostic in &mut diagnostics { diagnostic.fix = None; diff --git a/crates/ruff_linter/src/message/mod.rs b/crates/ruff_linter/src/message/mod.rs index c2b918accd..0c9afe2862 100644 --- a/crates/ruff_linter/src/message/mod.rs +++ b/crates/ruff_linter/src/message/mod.rs @@ -186,41 +186,6 @@ impl OldDiagnostic { ) } - /// Consumes `self` and returns a new `Diagnostic` with the given `fix`. - #[inline] - #[must_use] - pub fn with_fix(mut self, fix: Fix) -> Self { - self.set_fix(fix); - self - } - - /// Set the [`Fix`] used to fix the diagnostic. - #[inline] - pub fn set_fix(&mut self, fix: Fix) { - self.fix = Some(fix); - } - - /// Set the [`Fix`] used to fix the diagnostic, if the provided function returns `Ok`. - /// Otherwise, log the error. - #[inline] - pub fn try_set_fix(&mut self, func: impl FnOnce() -> anyhow::Result) { - match func() { - Ok(fix) => self.fix = Some(fix), - Err(err) => log::debug!("Failed to create fix for {}: {}", self.name(), err), - } - } - - /// Set the [`Fix`] used to fix the diagnostic, if the provided function returns `Ok`. - /// Otherwise, log the error. - #[inline] - pub fn try_set_optional_fix(&mut self, func: impl FnOnce() -> anyhow::Result>) { - match func() { - Ok(None) => {} - Ok(Some(fix)) => self.fix = Some(fix), - Err(err) => log::debug!("Failed to create fix for {}: {}", self.name(), err), - } - } - /// Consumes `self` and returns a new `Diagnostic` with the given parent node. #[inline] #[must_use] diff --git a/crates/ruff_linter/src/registry.rs b/crates/ruff_linter/src/registry.rs index ed2218b3b7..9351cb5de2 100644 --- a/crates/ruff_linter/src/registry.rs +++ b/crates/ruff_linter/src/registry.rs @@ -215,6 +215,12 @@ pub enum Linter { } pub trait RuleNamespace: Sized { + /// Returns the prefix that every single code that ruff uses to identify + /// rules from this linter starts with. In the case that multiple + /// `#[prefix]`es are configured for the variant in the `Linter` enum + /// definition this is the empty string. + fn common_prefix(&self) -> &'static str; + /// Attempts to parse the given rule code. If the prefix is recognized /// returns the respective variant along with the code with the common /// prefix stripped. diff --git a/crates/ruff_linter/src/rule_selector.rs b/crates/ruff_linter/src/rule_selector.rs index 62a016ffd2..74f069b976 100644 --- a/crates/ruff_linter/src/rule_selector.rs +++ b/crates/ruff_linter/src/rule_selector.rs @@ -265,6 +265,7 @@ mod schema { use strum::IntoEnumIterator; use crate::RuleSelector; + use crate::registry::RuleNamespace; use crate::rule_selector::{Linter, RuleCodePrefix}; impl JsonSchema for RuleSelector { diff --git a/crates/ruff_macros/src/map_codes.rs b/crates/ruff_macros/src/map_codes.rs index 6873df40a9..39993af6af 100644 --- a/crates/ruff_macros/src/map_codes.rs +++ b/crates/ruff_macros/src/map_codes.rs @@ -254,11 +254,9 @@ fn generate_rule_to_code(linter_to_rules: &BTreeMap NoqaCode(crate::registry::Linter::#linter.common_prefix(), #code), }); - let const_ident = quote::format_ident!("NOQA_PREFIX_{}", i); - noqa_code_consts.extend(quote! { - const #const_ident: &str = crate::registry::Linter::#linter.common_prefix(); - }); - noqa_code_rule_match_arms.extend(quote! { - #(#attrs)* NoqaCode(#const_ident, #code) => Some(Rule::#rule_name), - }); - rule_group_match_arms.extend(quote! { #(#attrs)* Rule::#rule_name => #group, }); @@ -350,16 +340,6 @@ See also https://github.com/astral-sh/ruff/issues/2186. } } } - - impl NoqaCode { - pub fn rule(&self) -> Option { - #noqa_code_consts - match self { - #noqa_code_rule_match_arms - _ => None - } - } - } }; rule_to_code } diff --git a/crates/ruff_macros/src/rule_namespace.rs b/crates/ruff_macros/src/rule_namespace.rs index 464bb47809..34ef3252da 100644 --- a/crates/ruff_macros/src/rule_namespace.rs +++ b/crates/ruff_macros/src/rule_namespace.rs @@ -118,6 +118,10 @@ pub(crate) fn derive_impl(input: DeriveInput) -> syn::Result &'static str { + match self { #common_prefix_match_arms } + } + fn name(&self) -> &'static str { match self { #name_match_arms } } @@ -126,16 +130,6 @@ pub(crate) fn derive_impl(input: DeriveInput) -> syn::Result &'static str { - match self { #common_prefix_match_arms } - } - } }) } diff --git a/crates/ruff_workspace/src/configuration.rs b/crates/ruff_workspace/src/configuration.rs index 802f73ae23..a844d6403a 100644 --- a/crates/ruff_workspace/src/configuration.rs +++ b/crates/ruff_workspace/src/configuration.rs @@ -22,7 +22,7 @@ use ruff_cache::cache_dir; use ruff_formatter::IndentStyle; use ruff_graph::{AnalyzeSettings, Direction}; use ruff_linter::line_width::{IndentWidth, LineLength}; -use ruff_linter::registry::{INCOMPATIBLE_CODES, Rule, RuleSet}; +use ruff_linter::registry::{INCOMPATIBLE_CODES, Rule, RuleNamespace, RuleSet}; use ruff_linter::rule_selector::{PreviewOptions, Specificity}; use ruff_linter::rules::{flake8_import_conventions, isort, pycodestyle}; use ruff_linter::settings::fix_safety_table::FixSafetyTable;