Skip to content

Commit

Permalink
Avoid SIM105 autofixes that would remove comments
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed May 9, 2023
1 parent 7b91a16 commit 3baff99
Show file tree
Hide file tree
Showing 7 changed files with 190 additions and 147 deletions.
37 changes: 30 additions & 7 deletions crates/ruff/resources/test/fixtures/flake8_simplify/SIM105.py
Original file line number Diff line number Diff line change
@@ -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.
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ def foo():
pass


# SIM105
try:
foo()
except ValueError:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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::{AlwaysAutofixableViolation, 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;
Expand All @@ -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})`")
})
}
}

Expand Down Expand Up @@ -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",
Expand Down
Loading

0 comments on commit 3baff99

Please sign in to comment.