Remove per-diagnostic check for fixability (#7919)

## Summary

Throughout the codebase, we have this pattern:

```rust
let mut diagnostic = ...
if checker.patch(Rule::UnusedVariable) {
    // Do the fix.
}
diagnostics.push(diagnostic)
```

This was helpful when we computed fixes lazily; however, we now compute
fixes eagerly, and this is _only_ used to ensure that we don't generate
fixes for rules marked as unfixable.

We often forget to add this, and it leads to bugs in enforcing
`--unfixable`.

This PR instead removes all of these checks, moving the responsibility
of enforcing `--unfixable` up to `check_path`. This is similar to how
@zanieb handled the `--extend-unsafe` logic: we post-process the
diagnostics to remove any fixes that should be ignored.

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

@@ -32,15 +32,10 @@ pub(crate) fn bindings(checker: &mut Checker) {
                     },
                     binding.range(),
                 );
-                if checker.patch(Rule::UnusedVariable) {
-                    diagnostic.try_set_fix(|| {
-                        pyflakes::fixes::remove_exception_handler_assignment(
-                            binding,
-                            checker.locator,
-                        )
+                diagnostic.try_set_fix(|| {
+                    pyflakes::fixes::remove_exception_handler_assignment(binding, checker.locator)
                         .map(Fix::safe_edit)
-                    });
-                }
+                });
                 checker.diagnostics.push(diagnostic);
             }
         }

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

@@ -143,11 +143,6 @@ impl<'a> Checker<'a> {
 }
 
 impl<'a> Checker<'a> {
-    /// Return `true` if a patch should be generated for a given [`Rule`].
-    pub(crate) fn patch(&self, code: Rule) -> bool {
-        self.settings.rules.should_fix(code)
-    }
-
     /// Return `true` if a [`Rule`] is disabled by a `noqa` directive.
     pub(crate) fn rule_is_ignored(&self, code: Rule, offset: TextSize) -> bool {
         // TODO(charlie): `noqa` directives are mostly enforced in `check_lines.rs`.

crates/ruff_linter/src/checkers/logical_lines.rs

@@ -5,7 +5,7 @@ use ruff_python_parser::TokenKind;
 use ruff_source_file::Locator;
 use ruff_text_size::{Ranged, TextRange};
 
-use crate::registry::{AsRule, Rule};
+use crate::registry::AsRule;
 use crate::rules::pycodestyle::rules::logical_lines::{
     extraneous_whitespace, indentation, missing_whitespace, missing_whitespace_after_keyword,
     missing_whitespace_around_operator, space_after_comma, space_around_operator,
@@ -38,17 +38,6 @@ pub(crate) fn check_logical_lines(
 ) -> Vec<Diagnostic> {
     let mut context = LogicalLinesContext::new(settings);
 
-    let should_fix_missing_whitespace = settings.rules.should_fix(Rule::MissingWhitespace);
-    let should_fix_whitespace_before_parameters =
-        settings.rules.should_fix(Rule::WhitespaceBeforeParameters);
-    let should_fix_whitespace_after_open_bracket =
-        settings.rules.should_fix(Rule::WhitespaceAfterOpenBracket);
-    let should_fix_whitespace_before_close_bracket = settings
-        .rules
-        .should_fix(Rule::WhitespaceBeforeCloseBracket);
-    let should_fix_whitespace_before_punctuation =
-        settings.rules.should_fix(Rule::WhitespaceBeforePunctuation);
-
     let mut prev_line = None;
     let mut prev_indent_level = None;
     let indent_char = stylist.indentation().as_char();
@@ -58,7 +47,7 @@ pub(crate) fn check_logical_lines(
             space_around_operator(&line, &mut context);
             whitespace_around_named_parameter_equals(&line, &mut context);
             missing_whitespace_around_operator(&line, &mut context);
-            missing_whitespace(&line, should_fix_missing_whitespace, &mut context);
+            missing_whitespace(&line, &mut context);
         }
         if line.flags().contains(TokenFlags::PUNCTUATION) {
             space_after_comma(&line, &mut context);
@@ -68,13 +57,7 @@ pub(crate) fn check_logical_lines(
             .flags()
             .intersects(TokenFlags::OPERATOR | TokenFlags::BRACKET | TokenFlags::PUNCTUATION)
         {
-            extraneous_whitespace(
-                &line,
-                &mut context,
-                should_fix_whitespace_after_open_bracket,
-                should_fix_whitespace_before_close_bracket,
-                should_fix_whitespace_before_punctuation,
-            );
+            extraneous_whitespace(&line, &mut context);
         }
 
         if line.flags().contains(TokenFlags::KEYWORD) {
@@ -87,11 +70,7 @@ pub(crate) fn check_logical_lines(
         }
 
         if line.flags().contains(TokenFlags::BRACKET) {
-            whitespace_before_parameters(
-                &line,
-                should_fix_whitespace_before_parameters,
-                &mut context,
-            );
+            whitespace_before_parameters(&line, &mut context);
         }
 
         // Extract the indentation level.

crates/ruff_linter/src/checkers/noqa.rs

@@ -109,10 +109,8 @@ pub(crate) fn check_noqa(
                     if line.matches.is_empty() {
                         let mut diagnostic =
                             Diagnostic::new(UnusedNOQA { codes: None }, directive.range());
-                        if settings.rules.should_fix(diagnostic.kind.rule()) {
-                            diagnostic
-                                .set_fix(Fix::safe_edit(delete_noqa(directive.range(), locator)));
-                        }
+                        diagnostic.set_fix(Fix::safe_edit(delete_noqa(directive.range(), locator)));
+
                         diagnostics.push(diagnostic);
                     }
                 }
@@ -173,18 +171,14 @@ pub(crate) fn check_noqa(
                             },
                             directive.range(),
                         );
-                        if settings.rules.should_fix(diagnostic.kind.rule()) {
-                            if valid_codes.is_empty() {
-                                diagnostic.set_fix(Fix::safe_edit(delete_noqa(
-                                    directive.range(),
-                                    locator,
-                                )));
-                            } else {
-                                diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(
-                                    format!("# noqa: {}", valid_codes.join(", ")),
-                                    directive.range(),
-                                )));
-                            }
+                        if valid_codes.is_empty() {
+                            diagnostic
+                                .set_fix(Fix::safe_edit(delete_noqa(directive.range(), locator)));
+                        } else {
+                            diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(
+                                format!("# noqa: {}", valid_codes.join(", ")),
+                                directive.range(),
+                            )));
                         }
                         diagnostics.push(diagnostic);
                     }

crates/ruff_linter/src/checkers/physical_lines.rs

@@ -71,11 +71,7 @@ pub(crate) fn check_physical_lines(
     }
 
     if enforce_no_newline_at_end_of_file {
-        if let Some(diagnostic) = no_newline_at_end_of_file(
-            locator,
-            stylist,
-            settings.rules.should_fix(Rule::MissingNewlineAtEndOfFile),
-        ) {
+        if let Some(diagnostic) = no_newline_at_end_of_file(locator, stylist) {
             diagnostics.push(diagnostic);
         }
     }

crates/ruff_linter/src/checkers/tokens.rs

@@ -72,7 +72,7 @@ pub(crate) fn check_tokens(
     }
 
     if settings.rules.enabled(Rule::UTF8EncodingDeclaration) {
-        pyupgrade::rules::unnecessary_coding_comment(&mut diagnostics, locator, indexer, settings);
+        pyupgrade::rules::unnecessary_coding_comment(&mut diagnostics, locator, indexer);
     }
 
     if settings.rules.enabled(Rule::InvalidEscapeSequence) {
@@ -83,7 +83,6 @@ pub(crate) fn check_tokens(
                 indexer,
                 tok,
                 *range,
-                settings.rules.should_fix(Rule::InvalidEscapeSequence),
             );
         }
     }
@@ -109,13 +108,7 @@ pub(crate) fn check_tokens(
         Rule::MultipleStatementsOnOneLineSemicolon,
         Rule::UselessSemicolon,
     ]) {
-        pycodestyle::rules::compound_statements(
-            &mut diagnostics,
-            tokens,
-            locator,
-            indexer,
-            settings,
-        );
+        pycodestyle::rules::compound_statements(&mut diagnostics, tokens, locator, indexer);
     }
 
     if settings.rules.enabled(Rule::AvoidableEscapedQuote) && settings.flake8_quotes.avoid_escape {
@@ -148,11 +141,11 @@ pub(crate) fn check_tokens(
         Rule::TrailingCommaOnBareTuple,
         Rule::ProhibitedTrailingComma,
     ]) {
-        flake8_commas::rules::trailing_commas(&mut diagnostics, tokens, locator, settings);
+        flake8_commas::rules::trailing_commas(&mut diagnostics, tokens, locator);
     }
 
     if settings.rules.enabled(Rule::ExtraneousParentheses) {
-        pyupgrade::rules::extraneous_parentheses(&mut diagnostics, tokens, locator, settings);
+        pyupgrade::rules::extraneous_parentheses(&mut diagnostics, tokens, locator);
     }
 
     if is_stub && settings.rules.enabled(Rule::TypeCommentInStub) {
@@ -166,7 +159,7 @@ pub(crate) fn check_tokens(
         Rule::ShebangNotFirstLine,
         Rule::ShebangMissingPython,
     ]) {
-        flake8_executable::rules::from_tokens(tokens, path, locator, settings, &mut diagnostics);
+        flake8_executable::rules::from_tokens(tokens, path, locator, &mut diagnostics);
     }
 
     if settings.rules.any_enabled(&[
@@ -191,7 +184,7 @@ pub(crate) fn check_tokens(
                 TodoComment::from_comment(comment, *comment_range, i)
             })
             .collect();
-        flake8_todos::rules::todos(&mut diagnostics, &todo_comments, locator, indexer, settings);
+        flake8_todos::rules::todos(&mut diagnostics, &todo_comments, locator, indexer);
         flake8_fixme::rules::todos(&mut diagnostics, &todo_comments);
     }
 

crates/ruff_linter/src/linter.rs

@@ -260,6 +260,13 @@ pub fn check_path(
         }
     }
 
+    // Remove fixes for any rules marked as unfixable.
+    for diagnostic in &mut diagnostics {
+        if !settings.rules.should_fix(diagnostic.kind.rule()) {
+            diagnostic.fix = None;
+        }
+    }
+
     // Update fix applicability to account for overrides
     if !settings.extend_safe_fixes.is_empty() || !settings.extend_unsafe_fixes.is_empty() {
         for diagnostic in &mut diagnostics {

crates/ruff_linter/src/rules/eradicate/rules/commented_out_code.rs

@@ -3,7 +3,6 @@ use ruff_macros::{derive_message_formats, violation};
 use ruff_python_index::Indexer;
 use ruff_source_file::Locator;
 
-use crate::registry::Rule;
 use crate::settings::LinterSettings;
 
 use super::super::detection::comment_contains_code;
@@ -67,11 +66,9 @@ pub(crate) fn commented_out_code(
         if is_standalone_comment(line) && comment_contains_code(line, &settings.task_tags[..]) {
             let mut diagnostic = Diagnostic::new(CommentedOutCode, *range);
 
-            if settings.rules.should_fix(Rule::CommentedOutCode) {
-                diagnostic.set_fix(Fix::display_edit(Edit::range_deletion(
-                    locator.full_lines_range(*range),
-                )));
-            }
+            diagnostic.set_fix(Fix::display_edit(Edit::range_deletion(
+                locator.full_lines_range(*range),
+            )));
             diagnostics.push(diagnostic);
         }
     }

crates/ruff_linter/src/rules/flake8_annotations/rules/definition.rs

@@ -11,7 +11,7 @@ use ruff_python_stdlib::typing::simple_magic_return_type;
 use ruff_text_size::Ranged;
 
 use crate::checkers::ast::Checker;
-use crate::registry::{AsRule, Rule};
+use crate::registry::Rule;
 use crate::rules::ruff::typing::type_hint_resolves_to_any;
 
 /// ## What it does
@@ -702,12 +702,10 @@ pub(crate) fn definition(
                         },
                         function.identifier(),
                     );
-                    if checker.patch(diagnostic.kind.rule()) {
-                        diagnostic.set_fix(Fix::unsafe_edit(Edit::insertion(
-                            " -> None".to_string(),
-                            function.parameters.range().end(),
-                        )));
-                    }
+                    diagnostic.set_fix(Fix::unsafe_edit(Edit::insertion(
+                        " -> None".to_string(),
+                        function.parameters.range().end(),
+                    )));
                     diagnostics.push(diagnostic);
                 }
             }
@@ -719,13 +717,11 @@ pub(crate) fn definition(
                     },
                     function.identifier(),
                 );
-                if checker.patch(diagnostic.kind.rule()) {
-                    if let Some(return_type) = simple_magic_return_type(name) {
-                        diagnostic.set_fix(Fix::unsafe_edit(Edit::insertion(
-                            format!(" -> {return_type}"),
-                            function.parameters.range().end(),
-                        )));
-                    }
+                if let Some(return_type) = simple_magic_return_type(name) {
+                    diagnostic.set_fix(Fix::unsafe_edit(Edit::insertion(
+                        format!(" -> {return_type}"),
+                        function.parameters.range().end(),
+                    )));
                 }
                 diagnostics.push(diagnostic);
             }

crates/ruff_linter/src/rules/flake8_bugbear/rules/assert_false.rs

@@ -6,7 +6,6 @@ use ruff_macros::{derive_message_formats, violation};
 use ruff_python_ast::helpers::is_const_false;
 
 use crate::checkers::ast::Checker;
-use crate::registry::AsRule;
 
 /// ## What it does
 /// Checks for uses of `assert False`.
@@ -75,11 +74,9 @@ pub(crate) fn assert_false(checker: &mut Checker, stmt: &Stmt, test: &Expr, msg:
     }
 
     let mut diagnostic = Diagnostic::new(AssertFalse, test.range());
-    if checker.patch(diagnostic.kind.rule()) {
-        diagnostic.set_fix(Fix::unsafe_edit(Edit::range_replacement(
-            checker.generator().stmt(&assertion_error(msg)),
-            stmt.range(),
-        )));
-    }
+    diagnostic.set_fix(Fix::unsafe_edit(Edit::range_replacement(
+        checker.generator().stmt(&assertion_error(msg)),
+        stmt.range(),
+    )));
     checker.diagnostics.push(diagnostic);
 }

crates/ruff_linter/src/rules/flake8_bugbear/rules/duplicate_exceptions.rs

@@ -11,7 +11,7 @@ use ruff_python_ast::call_path::CallPath;
 
 use crate::checkers::ast::Checker;
 use crate::fix::edits::pad;
-use crate::registry::{AsRule, Rule};
+use crate::registry::Rule;
 
 /// ## What it does
 /// Checks for `try-except` blocks with duplicate exception handlers.
@@ -146,24 +146,22 @@ fn duplicate_handler_exceptions<'a>(
                 },
                 expr.range(),
             );
-            if checker.patch(diagnostic.kind.rule()) {
-                diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(
-                    // Single exceptions don't require parentheses, but since we're _removing_
-                    // parentheses, insert whitespace as needed.
-                    if let [elt] = unique_elts.as_slice() {
-                        pad(
-                            checker.generator().expr(elt),
-                            expr.range(),
-                            checker.locator(),
-                        )
-                    } else {
-                        // Multiple exceptions must always be parenthesized. This is done
-                        // manually as the generator never parenthesizes lone tuples.
-                        format!("({})", checker.generator().expr(&type_pattern(unique_elts)))
-                    },
-                    expr.range(),
-                )));
-            }
+            diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(
+                // Single exceptions don't require parentheses, but since we're _removing_
+                // parentheses, insert whitespace as needed.
+                if let [elt] = unique_elts.as_slice() {
+                    pad(
+                        checker.generator().expr(elt),
+                        expr.range(),
+                        checker.locator(),
+                    )
+                } else {
+                    // Multiple exceptions must always be parenthesized. This is done
+                    // manually as the generator never parenthesizes lone tuples.
+                    format!("({})", checker.generator().expr(&type_pattern(unique_elts)))
+                },
+                expr.range(),
+            )));
             checker.diagnostics.push(diagnostic);
         }
     }