Skip to content

Commit

Permalink
feat: Implement TRY201 (#2073)
Browse files Browse the repository at this point in the history
  • Loading branch information
alonme committed Jan 22, 2023
1 parent ebfdefd commit 4fb0c6e
Show file tree
Hide file tree
Showing 9 changed files with 168 additions and 0 deletions.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -1196,6 +1196,7 @@ For more, see [tryceratops](https://pypi.org/project/tryceratops/1.1.0/) on PyPI
| Code | Name | Message | Fix |
| ---- | ---- | ------- | --- |
| TRY004 | prefer-type-error | Prefer `TypeError` exception for invalid type | 🛠 |
| TRY201 | verbose-raise | Use `raise` without specifying exception name | |
| TRY300 | try-consider-else | Consider `else` block | |

### flake8-use-pathlib (PTH)
Expand Down
54 changes: 54 additions & 0 deletions resources/test/fixtures/tryceratops/TRY201.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
"""
Violation:
Raising an exception using its assigned name is verbose and unrequired
"""
import logging

logger = logging.getLogger(__name__)


class MyException(Exception):
pass


def bad():
try:
process()
except MyException as e:
logger.exception("process failed")
raise e


def good():
try:
process()
except MyException:
logger.exception("process failed")
raise

def still_good():
try:
process()
except MyException as e:
print(e)
raise


def bad_that_needs_recursion():
try:
process()
except MyException as e:
logger.exception("process failed")
if True:
raise e


def bad_that_needs_recursion_2():
try:
process()
except MyException as e:
logger.exception("process failed")
if True:
def foo():
raise e
3 changes: 3 additions & 0 deletions ruff.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -1782,6 +1782,9 @@
"TRY0",
"TRY00",
"TRY004",
"TRY2",
"TRY20",
"TRY201",
"TRY3",
"TRY30",
"TRY300",
Expand Down
3 changes: 3 additions & 0 deletions src/checkers/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1578,6 +1578,9 @@ where
if self.settings.rules.enabled(&Rule::TryConsiderElse) {
tryceratops::rules::try_consider_else(self, body, orelse);
}
if self.settings.rules.enabled(&Rule::VerboseRaise) {
tryceratops::rules::verbose_raise(self, handlers);
}
}
StmtKind::Assign { targets, value, .. } => {
if self.settings.rules.enabled(&Rule::DoNotAssignLambda) {
Expand Down
1 change: 1 addition & 0 deletions src/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -428,6 +428,7 @@ ruff_macros::define_rule_mapping!(
TYP005 => rules::flake8_type_checking::rules::EmptyTypeCheckingBlock,
// tryceratops
TRY004 => rules::tryceratops::rules::PreferTypeError,
TRY201 => rules::tryceratops::rules::VerboseRaise,
TRY300 => rules::tryceratops::rules::TryConsiderElse,
// flake8-use-pathlib
PTH100 => rules::flake8_use_pathlib::violations::PathlibAbspath,
Expand Down
1 change: 1 addition & 0 deletions src/rules/tryceratops/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ mod tests {
use crate::settings;

#[test_case(Rule::PreferTypeError, Path::new("TRY004.py"); "TRY004")]
#[test_case(Rule::VerboseRaise, Path::new("TRY201.py"); "TRY201")]
#[test_case(Rule::TryConsiderElse, Path::new("TRY300.py"); "TRY300")]
fn rules(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!("{}_{}", rule_code.as_ref(), path.to_string_lossy());
Expand Down
2 changes: 2 additions & 0 deletions src/rules/tryceratops/rules/mod.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
pub use prefer_type_error::{prefer_type_error, PreferTypeError};
pub use try_consider_else::{try_consider_else, TryConsiderElse};
pub use verbose_raise::{verbose_raise, VerboseRaise};

mod prefer_type_error;
mod try_consider_else;
mod verbose_raise;
68 changes: 68 additions & 0 deletions src/rules/tryceratops/rules/verbose_raise.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
use ruff_macros::derive_message_formats;
use rustpython_ast::{Excepthandler, ExcepthandlerKind, Expr, ExprKind, Stmt, StmtKind};

use crate::ast::types::Range;
use crate::ast::visitor;
use crate::ast::visitor::Visitor;
use crate::checkers::ast::Checker;
use crate::define_violation;
use crate::registry::Diagnostic;
use crate::violation::Violation;

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

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

impl<'a, 'b> Visitor<'b> for RaiseStatementVisitor<'a>
where
'b: 'a,
{
fn visit_stmt(&mut self, stmt: &'b Stmt) {
match &stmt.node {
StmtKind::Raise { exc, .. } => self.raises.push(exc.as_ref().map(|expr| &**expr)),
_ => visitor::walk_stmt(self, stmt),
}
}
}

/// TRY201
pub fn verbose_raise(checker: &mut Checker, handlers: &[Excepthandler]) {
for handler in handlers {
// If the handler assigned a name to the exception...
if let ExcepthandlerKind::ExceptHandler {
name: Some(exception_name),
body,
..
} = &handler.node
{
let raises = {
let mut visitor = RaiseStatementVisitor::default();
for stmt in body {
visitor.visit_stmt(stmt);
}
visitor.raises
};
for expr in raises.into_iter().flatten() {
// ...and the raised object is bound to the same name...
if let ExprKind::Name { id, .. } = &expr.node {
if id == exception_name {
checker
.diagnostics
.push(Diagnostic::new(VerboseRaise, Range::from_located(expr)));
}
}
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
---
source: src/rules/tryceratops/mod.rs
expression: diagnostics
---
- kind:
VerboseRaise: ~
location:
row: 20
column: 14
end_location:
row: 20
column: 15
fix: ~
parent: ~
- kind:
VerboseRaise: ~
location:
row: 44
column: 18
end_location:
row: 44
column: 19
fix: ~
parent: ~
- kind:
VerboseRaise: ~
location:
row: 54
column: 22
end_location:
row: 54
column: 23
fix: ~
parent: ~

0 comments on commit 4fb0c6e

Please sign in to comment.