mirror of https://github.com/astral-sh/ruff
[pylint] Implement misplaced-bare-raise (E0704) (#7961)
## 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)
This commit is contained in:
parent
4113d65836
commit
bf0e5788ef
|
|
@ -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]
|
||||
|
|
@ -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) {
|
||||
|
|
|
|||
|
|
@ -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) {
|
||||
|
|
|
|||
|
|
@ -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),
|
||||
|
|
|
|||
|
|
@ -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 {
|
||||
|
|
|
|||
|
|
@ -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(
|
||||
|
|
|
|||
|
|
@ -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()));
|
||||
}
|
||||
|
|
@ -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;
|
||||
|
|
|
|||
|
|
@ -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()));
|
||||
|
|
|
|||
|
|
@ -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()));
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
||||
|
||||
|
||||
|
|
@ -2895,6 +2895,9 @@
|
|||
"PLE060",
|
||||
"PLE0604",
|
||||
"PLE0605",
|
||||
"PLE07",
|
||||
"PLE070",
|
||||
"PLE0704",
|
||||
"PLE1",
|
||||
"PLE11",
|
||||
"PLE114",
|
||||
|
|
|
|||
Loading…
Reference in New Issue