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..9bd28a45a29abd --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/pylint/misplaced_bare_raise.py @@ -0,0 +1,71 @@ +# 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] + +try: + pass +except: + def i(): + raise # [misplaced-bare-raise] + +try: + pass +except: + class C: + raise # [misplaced-bare-raise] + +try: + pass +except: + pass +finally: + 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..393dc0237f50ae 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs @@ -964,7 +964,7 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) { } } } - Stmt::Raise(ast::StmtRaise { exc, .. }) => { + Stmt::Raise(raise @ ast::StmtRaise { exc, .. }) => { if checker.enabled(Rule::RaiseNotImplemented) { if let Some(expr) = exc { pyflakes::rules::raise_not_implemented(checker, expr); @@ -1004,6 +1004,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) { flake8_raise::rules::unnecessary_paren_on_raise_exception(checker, expr); } } + if checker.enabled(Rule::MisplacedBareRaise) { + pylint::rules::misplaced_bare_raise(checker, raise); + } } Stmt::AugAssign(ast::StmtAugAssign { target, .. }) => { if checker.enabled(Rule::GlobalStatement) { diff --git a/crates/ruff_linter/src/checkers/ast/mod.rs b/crates/ruff_linter/src/checkers/ast/mod.rs index a111c5cc49cadc..bc3a3c3f9c88c6 100644 --- a/crates/ruff_linter/src/checkers/ast/mod.rs +++ b/crates/ruff_linter/src/checkers/ast/mod.rs @@ -524,6 +524,7 @@ where ); self.semantic.push_definition(definition); self.semantic.push_scope(ScopeKind::Function(function_def)); + self.semantic.flags -= SemanticModelFlags::EXCEPTION_HANDLER; self.deferred.functions.push(self.semantic.snapshot()); @@ -562,6 +563,7 @@ where ); self.semantic.push_definition(definition); self.semantic.push_scope(ScopeKind::Class(class_def)); + self.semantic.flags -= SemanticModelFlags::EXCEPTION_HANDLER; // Extract any global bindings from the class body. if let Some(globals) = Globals::from_body(body) { diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index bd03e13b5a8a3c..46e7bcda2c4829 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::Stable, rules::pylint::rules::InvalidStrReturnType), (Pylint, "E0604") => (RuleGroup::Stable, rules::pylint::rules::InvalidAllObject), (Pylint, "E0605") => (RuleGroup::Stable, rules::pylint::rules::InvalidAllFormat), + (Pylint, "E0704") => (RuleGroup::Preview, rules::pylint::rules::MisplacedBareRaise), (Pylint, "E1142") => (RuleGroup::Stable, rules::pylint::rules::AwaitOutsideAsync), (Pylint, "E1205") => (RuleGroup::Stable, rules::pylint::rules::LoggingTooManyArgs), (Pylint, "E1206") => (RuleGroup::Stable, rules::pylint::rules::LoggingTooFewArgs), diff --git a/crates/ruff_linter/src/rules/pylint/helpers.rs b/crates/ruff_linter/src/rules/pylint/helpers.rs index effffd71fb8b57..7d99ea90422da8 100644 --- a/crates/ruff_linter/src/rules/pylint/helpers.rs +++ b/crates/ruff_linter/src/rules/pylint/helpers.rs @@ -22,7 +22,11 @@ pub(super) fn type_param_name(arguments: &Arguments) -> Option<&str> { } } -pub(super) fn in_dunder_init(semantic: &SemanticModel, settings: &LinterSettings) -> bool { +pub(super) fn in_dunder_method( + dunder_name: &str, + semantic: &SemanticModel, + settings: &LinterSettings, +) -> bool { let scope = semantic.current_scope(); let ScopeKind::Function(ast::StmtFunctionDef { name, @@ -32,7 +36,7 @@ pub(super) fn in_dunder_init(semantic: &SemanticModel, settings: &LinterSettings else { return false; }; - if name != "__init__" { + if name != dunder_name { return false; } let Some(parent) = semantic.first_non_type_parent_scope(scope) else { 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..4f2787e57b52ba --- /dev/null +++ b/crates/ruff_linter/src/rules/pylint/rules/misplaced_bare_raise.rs @@ -0,0 +1,70 @@ +use ruff_diagnostics::{Diagnostic, Violation}; +use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast as ast; +use ruff_text_size::Ranged; + +use crate::checkers::ast::Checker; +use crate::rules::pylint::helpers::in_dunder_method; + +/// ## What it does +/// Checks for bare `raise` statements outside of exception handlers. +/// +/// ## Why is this bad? +/// A bare `raise` statement without an exception object will re-raise the last +/// exception that was active in the current scope, and is typically used +/// within an exception handler to re-raise the caught exception. +/// +/// If a bare `raise` is used outside of an exception handler, it will generate +/// an error due to the lack of an active exception. +/// +/// Note that a bare `raise` within a `finally` block will work in some cases +/// (namely, when the exception is raised within the `try` block), but should +/// be avoided as it can lead to confusing behavior. +/// +/// ## Example +/// ```python +/// from typing import Any +/// +/// +/// def is_some(obj: Any) -> bool: +/// if obj is None: +/// raise +/// ``` +/// +/// Use instead: +/// ```python +/// from typing import Any +/// +/// +/// def is_some(obj: Any) -> bool: +/// if obj is None: +/// raise ValueError("`obj` cannot be `None`") +/// ``` +#[violation] +pub struct MisplacedBareRaise; + +impl Violation for MisplacedBareRaise { + #[derive_message_formats] + fn message(&self) -> String { + format!("Bare `raise` statement is not inside an exception handler") + } +} + +/// PLE0704 +pub(crate) fn misplaced_bare_raise(checker: &mut Checker, raise: &ast::StmtRaise) { + if raise.exc.is_some() { + return; + } + + if checker.semantic().in_exception_handler() { + return; + } + + if in_dunder_method("__exit__", checker.semantic(), checker.settings) { + return; + } + + checker + .diagnostics + .push(Diagnostic::new(MisplacedBareRaise, raise.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/rules/return_in_init.rs b/crates/ruff_linter/src/rules/pylint/rules/return_in_init.rs index 7e18303678f7a2..235752725400be 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/return_in_init.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/return_in_init.rs @@ -6,7 +6,7 @@ use ruff_python_ast::helpers::is_const_none; use ruff_text_size::Ranged; use crate::checkers::ast::Checker; -use crate::rules::pylint::helpers::in_dunder_init; +use crate::rules::pylint::helpers::in_dunder_method; /// ## What it does /// Checks for `__init__` methods that return values. @@ -58,7 +58,7 @@ pub(crate) fn return_in_init(checker: &mut Checker, stmt: &Stmt) { } } - if in_dunder_init(checker.semantic(), checker.settings) { + if in_dunder_method("__init__", checker.semantic(), checker.settings) { checker .diagnostics .push(Diagnostic::new(ReturnInInit, stmt.range())); diff --git a/crates/ruff_linter/src/rules/pylint/rules/yield_in_init.rs b/crates/ruff_linter/src/rules/pylint/rules/yield_in_init.rs index fb7febaf2de3ee..1e98e5d81d7eb1 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/yield_in_init.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/yield_in_init.rs @@ -5,7 +5,7 @@ use ruff_macros::{derive_message_formats, violation}; use ruff_text_size::Ranged; use crate::checkers::ast::Checker; -use crate::rules::pylint::helpers::in_dunder_init; +use crate::rules::pylint::helpers::in_dunder_method; /// ## What it does /// Checks for `__init__` methods that are turned into generators by the @@ -40,7 +40,7 @@ impl Violation for YieldInInit { /// PLE0100 pub(crate) fn yield_in_init(checker: &mut Checker, expr: &Expr) { - if in_dunder_init(checker.semantic(), checker.settings) { + if in_dunder_method("__init__", checker.semantic(), checker.settings) { checker .diagnostics .push(Diagnostic::new(YieldInInit, expr.range())); 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..ce61e96b9c39ef --- /dev/null +++ b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE0704_misplaced_bare_raise.py.snap @@ -0,0 +1,90 @@ +--- +source: crates/ruff_linter/src/rules/pylint/mod.rs +--- +misplaced_bare_raise.py:30:5: PLE0704 Bare `raise` statement is not inside an exception handler + | +29 | try: +30 | raise # [misplaced-bare-raise] + | ^^^^^ PLE0704 +31 | except Exception: +32 | pass + | + +misplaced_bare_raise.py:36:9: PLE0704 Bare `raise` statement is not inside an exception handler + | +34 | def f(): +35 | try: +36 | raise # [misplaced-bare-raise] + | ^^^^^ PLE0704 +37 | except Exception: +38 | pass + | + +misplaced_bare_raise.py:41:5: PLE0704 Bare `raise` statement is not inside an exception handler + | +40 | def g(): +41 | raise # [misplaced-bare-raise] + | ^^^^^ PLE0704 +42 | +43 | def h(): + | + +misplaced_bare_raise.py:47:17: PLE0704 Bare `raise` statement is not inside an exception handler + | +45 | if True: +46 | def i(): +47 | raise # [misplaced-bare-raise] + | ^^^^^ PLE0704 +48 | except Exception: +49 | pass + | + +misplaced_bare_raise.py:50:5: PLE0704 Bare `raise` statement is not inside an exception handler + | +48 | except Exception: +49 | pass +50 | raise # [misplaced-bare-raise] + | ^^^^^ PLE0704 +51 | +52 | raise # [misplaced-bare-raise] + | + +misplaced_bare_raise.py:52:1: PLE0704 Bare `raise` statement is not inside an exception handler + | +50 | raise # [misplaced-bare-raise] +51 | +52 | raise # [misplaced-bare-raise] + | ^^^^^ PLE0704 +53 | +54 | try: + | + +misplaced_bare_raise.py:58:9: PLE0704 Bare `raise` statement is not inside an exception handler + | +56 | except: +57 | def i(): +58 | raise # [misplaced-bare-raise] + | ^^^^^ PLE0704 +59 | +60 | try: + | + +misplaced_bare_raise.py:64:9: PLE0704 Bare `raise` statement is not inside an exception handler + | +62 | except: +63 | class C: +64 | raise # [misplaced-bare-raise] + | ^^^^^ PLE0704 +65 | +66 | try: + | + +misplaced_bare_raise.py:71:5: PLE0704 Bare `raise` statement is not inside an exception handler + | +69 | pass +70 | finally: +71 | raise # [misplaced-bare-raise] + | ^^^^^ PLE0704 + | + + diff --git a/ruff.schema.json b/ruff.schema.json index e6f9230a6cc1a4..81248ece3bf9a7 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -2895,6 +2895,9 @@ "PLE060", "PLE0604", "PLE0605", + "PLE07", + "PLE070", + "PLE0704", "PLE1", "PLE11", "PLE114",