Avoid SIM105 autofixes that would remove comments

crates/ruff/resources/test/fixtures/flake8_simplify/SIM105.py → crates/ruff/resources/test/fixtures/flake8_simplify/SIM105_0.py

@@ -1,73 +1,96 @@
 def foo():
     pass
 
+
+# SIM105
 try:
     foo()
-except ValueError:  # SIM105
+except ValueError:
     pass
 
+# SIM105
 try:
     foo()
-except (ValueError, OSError):  # SIM105
+except (ValueError, OSError):
     pass
 
+# SIM105
 try:
     foo()
-except:  # SIM105
+except:
     pass
 
+# SIM105
 try:
     foo()
-except (a.Error, b.Error):  # SIM105
+except (a.Error, b.Error):
     pass
 
+# OK
 try:
     foo()
 except ValueError:
-    print('foo')
+    print("foo")
 except OSError:
     pass
 
+# OK
 try:
     foo()
 except ValueError:
     pass
 else:
-    print('bar')
+    print("bar")
 
+# OK
 try:
     foo()
 except ValueError:
     pass
 finally:
-    print('bar')
+    print("bar")
 
+# OK
 try:
     foo()
     foo()
 except ValueError:
     pass
 
+# OK
 try:
     for i in range(3):
         foo()
 except ValueError:
     pass
 
+
 def bar():
+    # OK
     try:
         return foo()
     except ValueError:
         pass
 
+
 def with_ellipsis():
+    # OK
     try:
         foo()
     except ValueError:
         ...
 
+
 def with_ellipsis_and_return():
+    # OK
     try:
         return foo()
     except ValueError:
         ...
+
+
+def with_comment():
+    try:
+        foo()
+    except (ValueError, OSError):
+        pass  # Trailing comment.

crates/ruff/resources/test/fixtures/flake8_simplify/SIM105_1.py

@@ -1,7 +1,8 @@
 """Case: There's a random import, so it should add `contextlib` after it."""
 import math
 
+# SIM105
 try:
     math.sqrt(-1)
-except ValueError:  # SIM105
+except ValueError:
     pass

crates/ruff/resources/test/fixtures/flake8_simplify/SIM105_2.py

@@ -6,6 +6,7 @@ def foo():
     pass
 
 
+# SIM105
 try:
     foo()
 except ValueError:

crates/ruff/src/rules/flake8_simplify/mod.rs

@@ -16,7 +16,7 @@ mod tests {
     #[test_case(Rule::DuplicateIsinstanceCall, Path::new("SIM101.py"); "SIM101")]
     #[test_case(Rule::CollapsibleIf, Path::new("SIM102.py"); "SIM102")]
     #[test_case(Rule::NeedlessBool, Path::new("SIM103.py"); "SIM103")]
-    #[test_case(Rule::SuppressibleException, Path::new("SIM105.py"); "SIM105")]
+    #[test_case(Rule::SuppressibleException, Path::new("SIM105_0.py"); "SIM105_0")]
     #[test_case(Rule::SuppressibleException, Path::new("SIM105_1.py"); "SIM105_1")]
     #[test_case(Rule::SuppressibleException, Path::new("SIM105_2.py"); "SIM105_2")]
     #[test_case(Rule::ReturnInTryExceptFinally, Path::new("SIM107.py"); "SIM107")]

crates/ruff/src/rules/flake8_simplify/rules/suppressible_exception.rs

@@ -3,10 +3,11 @@ use rustpython_parser::ast::{
     Constant, Excepthandler, ExcepthandlerKind, ExprKind, Located, Stmt, StmtKind,
 };
 
-use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix};
+use ruff_diagnostics::{AutofixKind, Diagnostic, Edit, Fix, Violation};
 use ruff_macros::{derive_message_formats, violation};
 use ruff_python_ast::call_path::compose_call_path;
 use ruff_python_ast::helpers;
+use ruff_python_ast::helpers::has_comments;
 
 use crate::autofix::actions::get_or_import_symbol;
 use crate::checkers::ast::Checker;
@@ -15,18 +16,23 @@ use crate::registry::AsRule;
 #[violation]
 pub struct SuppressibleException {
     pub exception: String,
+    pub fixable: bool,
 }
 
-impl AlwaysAutofixableViolation for SuppressibleException {
+impl Violation for SuppressibleException {
+    const AUTOFIX: AutofixKind = AutofixKind::Sometimes;
+
     #[derive_message_formats]
     fn message(&self) -> String {
-        let SuppressibleException { exception } = self;
+        let SuppressibleException { exception, .. } = self;
         format!("Use `contextlib.suppress({exception})` instead of `try`-`except`-`pass`")
     }
 
-    fn autofix_title(&self) -> String {
-        let SuppressibleException { exception } = self;
-        format!("Replace with `contextlib.suppress({exception})`")
+    fn autofix_title_formatter(&self) -> Option<fn(&Self) -> String> {
+        self.fixable
+            .then_some(|SuppressibleException { exception, .. }| {
+                format!("Replace with `contextlib.suppress({exception})`")
+            })
     }
 }
 
@@ -82,14 +88,16 @@ pub fn suppressible_exception(
             } else {
                 handler_names.join(", ")
             };
+            let fixable = !has_comments(stmt, checker.locator);
             let mut diagnostic = Diagnostic::new(
                 SuppressibleException {
                     exception: exception.clone(),
+                    fixable,
                 },
                 stmt.range(),
             );
 
-            if checker.patch(diagnostic.kind.rule()) {
+            if fixable && checker.patch(diagnostic.kind.rule()) {
                 diagnostic.try_set_fix(|| {
                     let (import_edit, binding) = get_or_import_symbol(
                         "contextlib",