Make `TRY201` always autofixable (#6008)

## Summary

Make `TRY201` always autofiable.

## Test Plan

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

ref:
https://github.com/astral-sh/ruff/issues/4333#issuecomment-1646359788
This commit is contained in:
Dhruv Manilawala 2023-07-24 07:53:15 +05:30 committed by GitHub
parent 3b56f6d616
commit 700c816fd5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 55 additions and 19 deletions

View File

@ -1,10 +1,11 @@
use rustpython_parser::ast::{self, ExceptHandler, Expr, Ranged, Stmt}; 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_macros::{derive_message_formats, violation};
use ruff_python_ast::statement_visitor::{walk_stmt, StatementVisitor}; use ruff_python_ast::statement_visitor::{walk_stmt, StatementVisitor};
use crate::checkers::ast::Checker; use crate::checkers::ast::Checker;
use crate::registry::AsRule;
/// ## What it does /// ## What it does
/// Checks for needless exception names in `raise` statements. /// Checks for needless exception names in `raise` statements.
@ -33,16 +34,20 @@ use crate::checkers::ast::Checker;
#[violation] #[violation]
pub struct VerboseRaise; pub struct VerboseRaise;
impl Violation for VerboseRaise { impl AlwaysAutofixableViolation for VerboseRaise {
#[derive_message_formats] #[derive_message_formats]
fn message(&self) -> String { fn message(&self) -> String {
format!("Use `raise` without specifying exception name") format!("Use `raise` without specifying exception name")
} }
fn autofix_title(&self) -> String {
format!("Remove exception name")
}
} }
#[derive(Default)] #[derive(Default)]
struct RaiseStatementVisitor<'a> { struct RaiseStatementVisitor<'a> {
raises: Vec<(Option<&'a Expr>, Option<&'a Expr>)>, raises: Vec<&'a ast::StmtRaise>,
} }
impl<'a, 'b> StatementVisitor<'b> for RaiseStatementVisitor<'a> impl<'a, 'b> StatementVisitor<'b> for RaiseStatementVisitor<'a>
@ -51,12 +56,8 @@ where
{ {
fn visit_stmt(&mut self, stmt: &'b Stmt) { fn visit_stmt(&mut self, stmt: &'b Stmt) {
match stmt { match stmt {
Stmt::Raise(ast::StmtRaise { Stmt::Raise(raise @ ast::StmtRaise { .. }) => {
exc, self.raises.push(raise);
cause,
range: _,
}) => {
self.raises.push((exc.as_deref(), cause.as_deref()));
} }
Stmt::Try(ast::StmtTry { Stmt::Try(ast::StmtTry {
body, finalbody, .. body, finalbody, ..
@ -88,17 +89,22 @@ pub(crate) fn verbose_raise(checker: &mut Checker, handlers: &[ExceptHandler]) {
visitor.visit_body(body); visitor.visit_body(body);
visitor.raises visitor.raises
}; };
for (exc, cause) in raises { for raise in raises {
if cause.is_some() { if raise.cause.is_some() {
continue; continue;
} }
if let Some(exc) = exc { if let Some(exc) = raise.exc.as_ref() {
// ...and the raised object is bound to the same name... // ...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() { if id == exception_name.as_str() {
checker let mut diagnostic = Diagnostic::new(VerboseRaise, exc.range());
.diagnostics if checker.patch(diagnostic.kind.rule()) {
.push(Diagnostic::new(VerboseRaise, exc.range())); diagnostic.set_fix(Fix::suggested(Edit::range_replacement(
"raise".to_string(),
raise.range(),
)));
}
checker.diagnostics.push(diagnostic);
} }
} }
} }

View File

@ -1,27 +1,57 @@
--- ---
source: crates/ruff/src/rules/tryceratops/mod.rs 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: 18 | except MyException as e:
19 | logger.exception("process failed") 19 | logger.exception("process failed")
20 | raise e 20 | raise e
| ^ TRY201 | ^ 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") 61 | logger.exception("process failed")
62 | if True: 62 | if True:
63 | raise e 63 | raise e
| ^ TRY201 | ^ TRY201
| |
= help: Remove exception name
TRY201.py:74:23: TRY201 Use `raise` without specifying 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
| |
73 | def foo(): 73 | def foo():
74 | raise e 74 | raise e
| ^ TRY201 | ^ TRY201
| |
= help: Remove exception name
Suggested fix
71 71 | if True:
72 72 |
73 73 | def foo():
74 |- raise e
74 |+ raise