diff --git a/crates/ruff_linter/resources/test/fixtures/pylint/misplaced_bare_raise.py b/crates/ruff_linter/resources/test/fixtures/pylint/misplaced_bare_raise.py new file mode 100644 index 00000000000000..aee627366aee29 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/pylint/misplaced_bare_raise.py @@ -0,0 +1,52 @@ +# bare raise is an except block +try: + pass +except Exception: + raise + +try: + pass +except Exception: + if True: + raise + +# bare raise is in a finally block which is in an except block +try: + raise Exception +except Exception: + try: + raise Exception + finally: + raise + +# bare raise is in an __exit__ method +class ContextManager: + def __enter__(self): + return self + def __exit__(self, *args): + raise + +try: + raise # [misplaced-bare-raise] +except Exception: + pass + +def f(): + try: + raise # [misplaced-bare-raise] + except Exception: + pass + +def g(): + raise # [misplaced-bare-raise] + +def h(): + try: + if True: + def i(): + raise # [misplaced-bare-raise] + except Exception: + pass + raise # [misplaced-bare-raise] + +raise # [misplaced-bare-raise] diff --git a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs index a39fd0734355c8..8d68b92f5f59a4 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs @@ -1004,6 +1004,11 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) { flake8_raise::rules::unnecessary_paren_on_raise_exception(checker, expr); } } + if checker.enabled(Rule::MisplacedBareRaise) { + if exc.is_none() { + pylint::rules::misplaced_bare_raise(checker, stmt); + } + } } Stmt::AugAssign(ast::StmtAugAssign { target, .. }) => { if checker.enabled(Rule::GlobalStatement) { diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index a0722c0741b9b0..892e2db724275e 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -224,6 +224,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Pylint, "E0307") => (RuleGroup::Unspecified, rules::pylint::rules::InvalidStrReturnType), (Pylint, "E0604") => (RuleGroup::Unspecified, rules::pylint::rules::InvalidAllObject), (Pylint, "E0605") => (RuleGroup::Unspecified, rules::pylint::rules::InvalidAllFormat), + (Pylint, "E0704") => (RuleGroup::Preview, rules::pylint::rules::MisplacedBareRaise), (Pylint, "E1142") => (RuleGroup::Unspecified, rules::pylint::rules::AwaitOutsideAsync), (Pylint, "E1205") => (RuleGroup::Unspecified, rules::pylint::rules::LoggingTooManyArgs), (Pylint, "E1206") => (RuleGroup::Unspecified, rules::pylint::rules::LoggingTooFewArgs), diff --git a/crates/ruff_linter/src/rules/pylint/mod.rs b/crates/ruff_linter/src/rules/pylint/mod.rs index 21b0c0250f911f..bb88ed91191938 100644 --- a/crates/ruff_linter/src/rules/pylint/mod.rs +++ b/crates/ruff_linter/src/rules/pylint/mod.rs @@ -135,6 +135,7 @@ mod tests { )] #[test_case(Rule::BadDunderMethodName, Path::new("bad_dunder_method_name.py"))] #[test_case(Rule::NoSelfUse, Path::new("no_self_use.py"))] + #[test_case(Rule::MisplacedBareRaise, Path::new("misplaced_bare_raise.py"))] fn rules(rule_code: Rule, path: &Path) -> Result<()> { let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy()); let diagnostics = test_path( diff --git a/crates/ruff_linter/src/rules/pylint/rules/misplaced_bare_raise.rs b/crates/ruff_linter/src/rules/pylint/rules/misplaced_bare_raise.rs new file mode 100644 index 00000000000000..9ecf52add9f440 --- /dev/null +++ b/crates/ruff_linter/src/rules/pylint/rules/misplaced_bare_raise.rs @@ -0,0 +1,100 @@ +use ruff_diagnostics::{Diagnostic, Violation}; +use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::statement_visitor::StatementVisitor; +use ruff_python_ast::{statement_visitor, Stmt}; +use ruff_python_semantic::NodeRef; +use ruff_text_size::Ranged; + +use crate::checkers::ast::Checker; + +/// ## What it does +/// +/// This rule triggers an error when a bare raise statement is not in an except or finally block. +/// +/// ## Why is this bad? +/// +/// If raise statement is not in an except or finally block, there is no active exception to +/// re-raise, so it will fail with a `RuntimeError` exception. +/// +/// ## Example +/// ```python +/// def validate_positive(x): +/// if x <= 0: +/// raise +/// ``` +/// +/// Use instead: +/// ```python +/// def validate_positive(x): +/// if x <= 0: +/// raise ValueError(f"{x} is not positive") +/// ``` +#[violation] +pub struct MisplacedBareRaise; + +impl Violation for MisplacedBareRaise { + #[derive_message_formats] + fn message(&self) -> String { + format!("The raise statement is not inside an except clause") + } +} + +#[derive(Default)] +struct RaiseFinder<'a> { + raises: Vec<&'a Stmt>, +} + +impl<'a, 'b> StatementVisitor<'b> for RaiseFinder<'a> +where + 'b: 'a, +{ + fn visit_stmt(&mut self, stmt: &'b Stmt) { + match stmt { + Stmt::Raise(_) => self.raises.push(stmt), + _ => statement_visitor::walk_stmt(self, stmt), + } + } +} + +/// PLE0704 +pub(crate) fn misplaced_bare_raise(checker: &mut Checker, raise_stmt: &Stmt) { + for id in checker.semantic().current_statement_ids() { + let node = checker.semantic().node(id); + if let NodeRef::Stmt(stmt) = node { + match stmt { + Stmt::Try(try_stmt) => { + // check whether the raise is in the except handlers + let mut raise_finder = RaiseFinder::default(); + for handler in &try_stmt.handlers { + raise_finder.visit_except_handler(handler); + } + if raise_finder.raises.contains(&raise_stmt) { + return; + } + + // if the raise is in the finally block, it might be ok if that finally is in + // an except block + let mut raise_finder = RaiseFinder::default(); + raise_finder.visit_body(try_stmt.finalbody.as_slice()); + if raise_finder.raises.contains(&raise_stmt) { + continue; + } + break; + } + Stmt::FunctionDef(fd) => { + // allow bare raise in __exit__ methods + if let Some(Stmt::ClassDef(_)) = checker.semantic().parent_statement(id) { + if fd.name.as_str() == "__exit__" { + return; + } + } + break; + } + _ => {} + } + } + } + checker + .diagnostics + .push(Diagnostic::new(MisplacedBareRaise, raise_stmt.range())); +} diff --git a/crates/ruff_linter/src/rules/pylint/rules/mod.rs b/crates/ruff_linter/src/rules/pylint/rules/mod.rs index 317657266794bb..2f20fa0f622838 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/mod.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/mod.rs @@ -28,6 +28,7 @@ pub(crate) use load_before_global_declaration::*; pub(crate) use logging::*; pub(crate) use magic_value_comparison::*; pub(crate) use manual_import_from::*; +pub(crate) use misplaced_bare_raise::*; pub(crate) use named_expr_without_context::*; pub(crate) use nested_min_max::*; pub(crate) use no_self_use::*; @@ -88,6 +89,7 @@ mod load_before_global_declaration; mod logging; mod magic_value_comparison; mod manual_import_from; +mod misplaced_bare_raise; mod named_expr_without_context; mod nested_min_max; mod no_self_use; diff --git a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE0704_misplaced_bare_raise.py.snap b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE0704_misplaced_bare_raise.py.snap new file mode 100644 index 00000000000000..fb981707ccb89a --- /dev/null +++ b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE0704_misplaced_bare_raise.py.snap @@ -0,0 +1,60 @@ +--- +source: crates/ruff_linter/src/rules/pylint/mod.rs +--- +misplaced_bare_raise.py:30:5: PLE0704 The raise statement is not inside an except clause + | +29 | try: +30 | raise # [misplaced-bare-raise] + | ^^^^^ PLE0704 +31 | except Exception: +32 | pass + | + +misplaced_bare_raise.py:36:9: PLE0704 The raise statement is not inside an except clause + | +34 | def f(): +35 | try: +36 | raise # [misplaced-bare-raise] + | ^^^^^ PLE0704 +37 | except Exception: +38 | pass + | + +misplaced_bare_raise.py:41:5: PLE0704 The raise statement is not inside an except clause + | +40 | def g(): +41 | raise # [misplaced-bare-raise] + | ^^^^^ PLE0704 +42 | +43 | def h(): + | + +misplaced_bare_raise.py:47:17: PLE0704 The raise statement is not inside an except clause + | +45 | if True: +46 | def i(): +47 | raise # [misplaced-bare-raise] + | ^^^^^ PLE0704 +48 | except Exception: +49 | pass + | + +misplaced_bare_raise.py:50:5: PLE0704 The raise statement is not inside an except clause + | +48 | except Exception: +49 | pass +50 | raise # [misplaced-bare-raise] + | ^^^^^ PLE0704 +51 | +52 | raise # [misplaced-bare-raise] + | + +misplaced_bare_raise.py:52:1: PLE0704 The raise statement is not inside an except clause + | +50 | raise # [misplaced-bare-raise] +51 | +52 | raise # [misplaced-bare-raise] + | ^^^^^ PLE0704 + | + + diff --git a/ruff.schema.json b/ruff.schema.json index 7be275a4100e0f..b7f515b2f5ecc2 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -2905,6 +2905,9 @@ "PLE060", "PLE0604", "PLE0605", + "PLE07", + "PLE070", + "PLE0704", "PLE1", "PLE11", "PLE114",