Skip to content

Commit

Permalink
Make TRY201 always autofixable (#6008)
Browse files Browse the repository at this point in the history
## Summary

Make `TRY201` always autofiable.

## Test Plan

1. `cargo test`
2. `cargo insta review`

ref:
#4333 (comment)
  • Loading branch information
dhruvmanila authored Jul 24, 2023
1 parent 3b56f6d commit 700c816
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 19 deletions.
38 changes: 22 additions & 16 deletions crates/ruff/src/rules/tryceratops/rules/verbose_raise.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
use rustpython_parser::ast::{self, ExceptHandler, Expr, Ranged, Stmt};

use ruff_diagnostics::{Diagnostic, Violation};
use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::statement_visitor::{walk_stmt, StatementVisitor};

use crate::checkers::ast::Checker;
use crate::registry::AsRule;

/// ## What it does
/// Checks for needless exception names in `raise` statements.
Expand Down Expand Up @@ -33,16 +34,20 @@ use crate::checkers::ast::Checker;
#[violation]
pub struct VerboseRaise;

impl Violation for VerboseRaise {
impl AlwaysAutofixableViolation for VerboseRaise {
#[derive_message_formats]
fn message(&self) -> String {
format!("Use `raise` without specifying exception name")
}

fn autofix_title(&self) -> String {
format!("Remove exception name")
}
}

#[derive(Default)]
struct RaiseStatementVisitor<'a> {
raises: Vec<(Option<&'a Expr>, Option<&'a Expr>)>,
raises: Vec<&'a ast::StmtRaise>,
}

impl<'a, 'b> StatementVisitor<'b> for RaiseStatementVisitor<'a>
Expand All @@ -51,12 +56,8 @@ where
{
fn visit_stmt(&mut self, stmt: &'b Stmt) {
match stmt {
Stmt::Raise(ast::StmtRaise {
exc,
cause,
range: _,
}) => {
self.raises.push((exc.as_deref(), cause.as_deref()));
Stmt::Raise(raise @ ast::StmtRaise { .. }) => {
self.raises.push(raise);
}
Stmt::Try(ast::StmtTry {
body, finalbody, ..
Expand Down Expand Up @@ -88,17 +89,22 @@ pub(crate) fn verbose_raise(checker: &mut Checker, handlers: &[ExceptHandler]) {
visitor.visit_body(body);
visitor.raises
};
for (exc, cause) in raises {
if cause.is_some() {
for raise in raises {
if raise.cause.is_some() {
continue;
}
if let Some(exc) = exc {
if let Some(exc) = raise.exc.as_ref() {
// ...and the raised object is bound to the same name...
if let Expr::Name(ast::ExprName { id, .. }) = exc {
if let Expr::Name(ast::ExprName { id, .. }) = exc.as_ref() {
if id == exception_name.as_str() {
checker
.diagnostics
.push(Diagnostic::new(VerboseRaise, exc.range()));
let mut diagnostic = Diagnostic::new(VerboseRaise, exc.range());
if checker.patch(diagnostic.kind.rule()) {
diagnostic.set_fix(Fix::suggested(Edit::range_replacement(
"raise".to_string(),
raise.range(),
)));
}
checker.diagnostics.push(diagnostic);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,27 +1,57 @@
---
source: crates/ruff/src/rules/tryceratops/mod.rs
---
TRY201.py:20:15: TRY201 Use `raise` without specifying exception name
TRY201.py:20:15: TRY201 [*] Use `raise` without specifying exception name
|
18 | except MyException as e:
19 | logger.exception("process failed")
20 | raise e
| ^ TRY201
|
= help: Remove exception name

TRY201.py:63:19: TRY201 Use `raise` without specifying exception name
Suggested fix
17 17 | process()
18 18 | except MyException as e:
19 19 | logger.exception("process failed")
20 |- raise e
20 |+ raise
21 21 |
22 22 |
23 23 | def good():

TRY201.py:63:19: TRY201 [*] Use `raise` without specifying exception name
|
61 | logger.exception("process failed")
62 | if True:
63 | raise e
| ^ TRY201
|
= help: Remove exception name

Suggested fix
60 60 | except MyException as e:
61 61 | logger.exception("process failed")
62 62 | if True:
63 |- raise e
63 |+ raise
64 64 |
65 65 |
66 66 | def bad_that_needs_recursion_2():

TRY201.py:74:23: TRY201 Use `raise` without specifying exception name
TRY201.py:74:23: TRY201 [*] Use `raise` without specifying exception name
|
73 | def foo():
74 | raise e
| ^ TRY201
|
= help: Remove exception name

Suggested fix
71 71 | if True:
72 72 |
73 73 | def foo():
74 |- raise e
74 |+ raise


0 comments on commit 700c816

Please sign in to comment.