Skip to content

Commit

Permalink
[pylint] Implement misplaced-bare-raise (E0704) (#7961)
Browse files Browse the repository at this point in the history
## Summary

### 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")
```

## Test Plan

Added unit test and snapshot.
Manually compared ruff and pylint outputs on pylint's tests.

## References

- [pylint
documentation](https://pylint.pycqa.org/en/stable/user_guide/messages/error/misplaced-bare-raise.html)
- [pylint
implementation](https://github.com/pylint-dev/pylint/blob/main/pylint/checkers/exceptions.py#L339)
  • Loading branch information
clemux committed Oct 17, 2023
1 parent 4113d65 commit bf0e578
Show file tree
Hide file tree
Showing 12 changed files with 254 additions and 7 deletions.
Original file line number Diff line number Diff line change
@@ -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]
5 changes: 4 additions & 1 deletion crates/ruff_linter/src/checkers/ast/analyze/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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) {
Expand Down
2 changes: 2 additions & 0 deletions crates/ruff_linter/src/checkers/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());

Expand Down Expand Up @@ -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) {
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
8 changes: 6 additions & 2 deletions crates/ruff_linter/src/rules/pylint/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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 {
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/rules/pylint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
70 changes: 70 additions & 0 deletions crates/ruff_linter/src/rules/pylint/rules/misplaced_bare_raise.rs
Original file line number Diff line number Diff line change
@@ -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()));
}
2 changes: 2 additions & 0 deletions crates/ruff_linter/src/rules/pylint/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::*;
Expand Down Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions crates/ruff_linter/src/rules/pylint/rules/return_in_init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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()));
Expand Down
4 changes: 2 additions & 2 deletions crates/ruff_linter/src/rules/pylint/rules/yield_in_init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()));
Expand Down
Original file line number Diff line number Diff line change
@@ -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
|


3 changes: 3 additions & 0 deletions ruff.schema.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit bf0e578

Please sign in to comment.