Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[pylint] Implement misplaced-bare-raise (E0704) #7961

Merged
merged 3 commits into from
Oct 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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;
}
charliermarsh marked this conversation as resolved.
Show resolved Hide resolved

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.

Loading