Skip to content

Commit

Permalink
[pylint] Implement misplaced-bare-raise (E0704)
Browse files Browse the repository at this point in the history
  • Loading branch information
clemux committed Oct 14, 2023
1 parent bd06cbe commit 4b1ece8
Show file tree
Hide file tree
Showing 8 changed files with 224 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -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]
5 changes: 5 additions & 0 deletions crates/ruff_linter/src/checkers/ast/analyze/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
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 @@ -214,6 +214,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
#[allow(deprecated)]
(Pylint, "C1901") => (RuleGroup::Nursery, rules::pylint::rules::CompareToEmptyString),
(Pylint, "C3002") => (RuleGroup::Unspecified, rules::pylint::rules::UnnecessaryDirectLambdaCall),
(Pylint, "E0704") => (RuleGroup::Unspecified, rules::pylint::rules::MisplacedBareRaise),
(Pylint, "E0100") => (RuleGroup::Unspecified, rules::pylint::rules::YieldInInit),
(Pylint, "E0101") => (RuleGroup::Unspecified, rules::pylint::rules::ReturnInInit),
(Pylint, "E0116") => (RuleGroup::Unspecified, rules::pylint::rules::ContinueInFinally),
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
100 changes: 100 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,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()));
}
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
Original file line number Diff line number Diff line change
@@ -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
|


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 4b1ece8

Please sign in to comment.