From 58d5ac08a8389819d57a89976263d829e2adee4a Mon Sep 17 00:00:00 2001 From: Denis Gavrilyuk Date: Tue, 24 Jan 2023 15:25:26 +0300 Subject: [PATCH] feat: implement TRY301 (#2113) --- README.md | 1 + resources/test/fixtures/tryceratops/TRY301.py | 29 ++++++++++ ruff.schema.json | 1 + src/checkers/ast.rs | 3 ++ src/registry.rs | 1 + src/rules/tryceratops/mod.rs | 1 + src/rules/tryceratops/rules/mod.rs | 2 + .../tryceratops/rules/raise_within_try.rs | 54 +++++++++++++++++++ ...ps__tests__raise-within-try_TRY301.py.snap | 35 ++++++++++++ 9 files changed, 127 insertions(+) create mode 100644 resources/test/fixtures/tryceratops/TRY301.py create mode 100644 src/rules/tryceratops/rules/raise_within_try.rs create mode 100644 src/rules/tryceratops/snapshots/ruff__rules__tryceratops__tests__raise-within-try_TRY301.py.snap diff --git a/README.md b/README.md index b254a48feb4b2..594c393478c95 100644 --- a/README.md +++ b/README.md @@ -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) diff --git a/resources/test/fixtures/tryceratops/TRY301.py b/resources/test/fixtures/tryceratops/TRY301.py new file mode 100644 index 0000000000000..e585c92f84e8c --- /dev/null +++ b/resources/test/fixtures/tryceratops/TRY301.py @@ -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") diff --git a/ruff.schema.json b/ruff.schema.json index 543380e05c082..48c1be3b19f5b 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -1818,6 +1818,7 @@ "TRY3", "TRY30", "TRY300", + "TRY301", "TYP", "TYP0", "TYP00", diff --git a/src/checkers/ast.rs b/src/checkers/ast.rs index 2187687b6ca7b..9d60685d3e5b8 100644 --- a/src/checkers/ast.rs +++ b/src/checkers/ast.rs @@ -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) { diff --git a/src/registry.rs b/src/registry.rs index 5e49b30a8ecc7..f472b15a40c3f 100644 --- a/src/registry.rs +++ b/src/registry.rs @@ -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, diff --git a/src/rules/tryceratops/mod.rs b/src/rules/tryceratops/mod.rs index 59529ecc11f92..fb975c8d16938 100644 --- a/src/rules/tryceratops/mod.rs +++ b/src/rules/tryceratops/mod.rs @@ -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( diff --git a/src/rules/tryceratops/rules/mod.rs b/src/rules/tryceratops/rules/mod.rs index 47c43a5f7ef60..8ec550da4afc3 100644 --- a/src/rules/tryceratops/rules/mod.rs +++ b/src/rules/tryceratops/rules/mod.rs @@ -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; diff --git a/src/rules/tryceratops/rules/raise_within_try.rs b/src/rules/tryceratops/rules/raise_within_try.rs new file mode 100644 index 0000000000000..a7581d0a77120 --- /dev/null +++ b/src/rules/tryceratops/rules/raise_within_try.rs @@ -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))); + } +} diff --git a/src/rules/tryceratops/snapshots/ruff__rules__tryceratops__tests__raise-within-try_TRY301.py.snap b/src/rules/tryceratops/snapshots/ruff__rules__tryceratops__tests__raise-within-try_TRY301.py.snap new file mode 100644 index 0000000000000..fe297a291e5db --- /dev/null +++ b/src/rules/tryceratops/snapshots/ruff__rules__tryceratops__tests__raise-within-try_TRY301.py.snap @@ -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: ~ +