Skip to content

Commit

Permalink
feat: implement TRY301 (#2113)
Browse files Browse the repository at this point in the history
  • Loading branch information
karpa4o4 committed Jan 24, 2023
1 parent cc63a4b commit 58d5ac0
Show file tree
Hide file tree
Showing 9 changed files with 127 additions and 0 deletions.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -1203,6 +1203,7 @@ For more, see [tryceratops](https://pypi.org/project/tryceratops/1.1.0/) on PyPI
| TRY200 | reraise-no-cause | Use `raise from` to specify exception cause | |
| TRY201 | verbose-raise | Use `raise` without specifying exception name | |
| TRY300 | try-consider-else | Consider `else` block | |
| TRY301 | raise-within-try | Abstract `raise` to an inner function | |

### flake8-use-pathlib (PTH)

Expand Down
29 changes: 29 additions & 0 deletions resources/test/fixtures/tryceratops/TRY301.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
class MyException(Exception):
pass


def bad():
try:
a = process()
if not a:
raise MyException(a)

raise MyException(a)

try:
b = process()
if not b:
raise MyException(b)
except Exception:
logger.exception("something failed")
except Exception:
logger.exception("something failed")


def good():
try:
a = process() # This throws the exception now
except MyException:
logger.exception("a failed")
except Exception:
logger.exception("something failed")
1 change: 1 addition & 0 deletions ruff.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -1818,6 +1818,7 @@
"TRY3",
"TRY30",
"TRY300",
"TRY301",
"TYP",
"TYP0",
"TYP00",
Expand Down
3 changes: 3 additions & 0 deletions src/checkers/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1582,6 +1582,9 @@ where
if self.settings.rules.enabled(&Rule::VerboseRaise) {
tryceratops::rules::verbose_raise(self, handlers);
}
if self.settings.rules.enabled(&Rule::RaiseWithinTry) {
tryceratops::rules::raise_within_try(self, body);
}
}
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 @@ -434,6 +434,7 @@ ruff_macros::define_rule_mapping!(
TRY200 => rules::tryceratops::rules::ReraiseNoCause,
TRY201 => rules::tryceratops::rules::VerboseRaise,
TRY300 => rules::tryceratops::rules::TryConsiderElse,
TRY301 => rules::tryceratops::rules::RaiseWithinTry,
// flake8-use-pathlib
PTH100 => rules::flake8_use_pathlib::violations::PathlibAbspath,
PTH101 => rules::flake8_use_pathlib::violations::PathlibChmod,
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 @@ -17,6 +17,7 @@ mod tests {
#[test_case(Rule::ReraiseNoCause, Path::new("TRY200.py"); "TRY200")]
#[test_case(Rule::VerboseRaise, Path::new("TRY201.py"); "TRY201")]
#[test_case(Rule::TryConsiderElse, Path::new("TRY300.py"); "TRY300")]
#[test_case(Rule::RaiseWithinTry , Path::new("TRY301.py"); "TRY301")]
fn rules(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!("{}_{}", rule_code.as_ref(), path.to_string_lossy());
let diagnostics = test_path(
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,9 +1,11 @@
pub use prefer_type_error::{prefer_type_error, PreferTypeError};
pub use raise_within_try::{raise_within_try, RaiseWithinTry};
pub use reraise_no_cause::{reraise_no_cause, ReraiseNoCause};
pub use try_consider_else::{try_consider_else, TryConsiderElse};
pub use verbose_raise::{verbose_raise, VerboseRaise};

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

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

define_violation!(
pub struct RaiseWithinTry;
);
impl Violation for RaiseWithinTry {
#[derive_message_formats]
fn message(&self) -> String {
format!("Abstract `raise` to an inner function")
}
}

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

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

/// TRY301
pub fn raise_within_try(checker: &mut Checker, body: &[Stmt]) {
let raises = {
let mut visitor = RaiseStatementVisitor::default();
for stmt in body {
visitor.visit_stmt(stmt);
}
visitor.raises
};

for stmt in raises {
checker
.diagnostics
.push(Diagnostic::new(RaiseWithinTry, Range::from_located(stmt)));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
---
source: src/rules/tryceratops/mod.rs
expression: diagnostics
---
- kind:
RaiseWithinTry: ~
location:
row: 9
column: 12
end_location:
row: 9
column: 32
fix: ~
parent: ~
- kind:
RaiseWithinTry: ~
location:
row: 11
column: 8
end_location:
row: 11
column: 28
fix: ~
parent: ~
- kind:
RaiseWithinTry: ~
location:
row: 16
column: 16
end_location:
row: 16
column: 36
fix: ~
parent: ~

0 comments on commit 58d5ac0

Please sign in to comment.