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] Include builtin warnings in useless-exception-statement (PLW0133) #10394

Merged
merged 1 commit into from Mar 13, 2024
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -1,8 +1,8 @@
# Test case 1: Useless exception statement
from abc import ABC, abstractmethod
from contextlib import suppress


# Test case 1: Useless exception statement
def func():
AssertionError("This is an assertion error") # PLW0133

Expand Down Expand Up @@ -66,6 +66,11 @@ def func():
x = 1; (RuntimeError("This is an exception")); y = 2 # PLW0133


# Test case 11: Useless warning statement
def func():
UserWarning("This is an assertion error") # PLW0133


# Non-violation test cases: PLW0133


Expand Down
Expand Up @@ -2,6 +2,7 @@ use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::{self as ast, Expr};
use ruff_python_semantic::SemanticModel;
use ruff_python_stdlib::builtins;
use ruff_text_size::Ranged;

use crate::checkers::ast::Checker;
Expand All @@ -14,6 +15,10 @@ use crate::checkers::ast::Checker;
/// `ValueError("...")` on its own will have no effect (unlike
/// `raise ValueError("...")`) and is likely a mistake.
///
/// ## Known problems
/// This rule only detects built-in exceptions, like `ValueError`, and does
/// not catch user-defined exceptions.
///
/// ## Example
/// ```python
/// ValueError("...")
Expand Down Expand Up @@ -60,38 +65,8 @@ pub(crate) fn useless_exception_statement(checker: &mut Checker, expr: &ast::Stm
}

/// Returns `true` if the given expression is a builtin exception.
///
/// See: <https://docs.python.org/3/library/exceptions.html#exception-hierarchy>
fn is_builtin_exception(expr: &Expr, semantic: &SemanticModel) -> bool {
return semantic
semantic
.resolve_qualified_name(expr)
.is_some_and(|qualified_name| {
matches!(
qualified_name.segments(),
[
"",
"SystemExit"
| "Exception"
| "ArithmeticError"
| "AssertionError"
| "AttributeError"
| "BufferError"
| "EOFError"
| "ImportError"
| "LookupError"
| "IndexError"
| "KeyError"
| "MemoryError"
| "NameError"
| "ReferenceError"
| "RuntimeError"
| "NotImplementedError"
| "StopIteration"
| "SyntaxError"
| "SystemError"
| "TypeError"
| "ValueError"
]
)
});
.is_some_and(|qualified_name| matches!(qualified_name.segments(), ["", name] if builtins::is_exception(name)))
}
Expand Up @@ -3,6 +3,7 @@ source: crates/ruff_linter/src/rules/pylint/mod.rs
---
useless_exception_statement.py:7:5: PLW0133 [*] Missing `raise` statement on exception
|
5 | # Test case 1: Useless exception statement
6 | def func():
7 | AssertionError("This is an assertion error") # PLW0133
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PLW0133
Expand All @@ -11,7 +12,7 @@ useless_exception_statement.py:7:5: PLW0133 [*] Missing `raise` statement on exc

ℹ Unsafe fix
4 4 |
5 5 |
5 5 | # Test case 1: Useless exception statement
6 6 | def func():
7 |- AssertionError("This is an assertion error") # PLW0133
7 |+ raise AssertionError("This is an assertion error") # PLW0133
Expand Down Expand Up @@ -192,4 +193,23 @@ useless_exception_statement.py:66:12: PLW0133 [*] Missing `raise` statement on e
66 |+ x = 1; raise (RuntimeError("This is an exception")); y = 2 # PLW0133
67 67 |
68 68 |
69 69 | # Non-violation test cases: PLW0133
69 69 | # Test case 11: Useless warning statement

useless_exception_statement.py:71:5: PLW0133 [*] Missing `raise` statement on exception
|
69 | # Test case 11: Useless warning statement
70 | def func():
71 | UserWarning("This is an assertion error") # PLW0133
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PLW0133
|
= help: Add `raise` keyword

ℹ Unsafe fix
68 68 |
69 69 | # Test case 11: Useless warning statement
70 70 | def func():
71 |- UserWarning("This is an assertion error") # PLW0133
71 |+ raise UserWarning("This is an assertion error") # PLW0133
72 72 |
73 73 |
74 74 | # Non-violation test cases: PLW0133
@@ -1,7 +1,9 @@
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::helpers::map_callable;
use ruff_python_ast::statement_visitor::{walk_stmt, StatementVisitor};
use ruff_python_ast::{self as ast, Expr, Stmt, StmtIf};
use ruff_python_semantic::SemanticModel;
use ruff_text_size::Ranged;

use crate::checkers::ast::Checker;
Expand Down Expand Up @@ -96,11 +98,32 @@ fn check_type_check_test(checker: &mut Checker, test: &Expr) -> bool {
}
}

/// Returns `true` if `exc` is a reference to a builtin exception.
fn is_builtin_exception(checker: &mut Checker, exc: &Expr) -> bool {
return checker
.semantic()
.resolve_qualified_name(exc)
fn check_raise(checker: &mut Checker, exc: &Expr, item: &Stmt) {
if is_builtin_exception(exc, checker.semantic()) {
checker
.diagnostics
.push(Diagnostic::new(TypeCheckWithoutTypeError, item.range()));
}
}

/// Search the body of an if-condition for raises.
fn check_body(checker: &mut Checker, body: &[Stmt]) {
for item in body {
if has_control_flow(item) {
return;
}
if let Stmt::Raise(ast::StmtRaise { exc: Some(exc), .. }) = &item {
check_raise(checker, exc, item);
}
}
}

/// Returns `true` if the given expression is a builtin exception.
///
/// This function only matches to a subset of the builtin exceptions, and omits `TypeError`.
fn is_builtin_exception(expr: &Expr, semantic: &SemanticModel) -> bool {
semantic
.resolve_qualified_name(map_callable(expr))
.is_some_and(|qualified_name| {
matches!(
qualified_name.segments(),
Expand All @@ -123,42 +146,7 @@ fn is_builtin_exception(checker: &mut Checker, exc: &Expr) -> bool {
| "ValueError"
]
)
});
}

/// Returns `true` if an [`Expr`] is a reference to a builtin exception.
fn check_raise_type(checker: &mut Checker, exc: &Expr) -> bool {
match exc {
Expr::Name(_) => is_builtin_exception(checker, exc),
Expr::Call(ast::ExprCall { func, .. }) => {
if let Expr::Name(_) = func.as_ref() {
is_builtin_exception(checker, func)
} else {
false
}
}
_ => false,
}
}

fn check_raise(checker: &mut Checker, exc: &Expr, item: &Stmt) {
if check_raise_type(checker, exc) {
checker
.diagnostics
.push(Diagnostic::new(TypeCheckWithoutTypeError, item.range()));
}
}

/// Search the body of an if-condition for raises.
fn check_body(checker: &mut Checker, body: &[Stmt]) {
for item in body {
if has_control_flow(item) {
return;
}
if let Stmt::Raise(ast::StmtRaise { exc: Some(exc), .. }) = &item {
check_raise(checker, exc, item);
}
}
})
}

/// TRY004
Expand Down
76 changes: 76 additions & 0 deletions crates/ruff_python_stdlib/src/builtins.rs
Expand Up @@ -368,3 +368,79 @@ pub fn is_ipython_builtin(name: &str) -> bool {
// Constructed by converting the `IPYTHON_BUILTINS` slice to a `match` expression.
matches!(name, "__IPYTHON__" | "display" | "get_ipython")
}

/// Returns `true` if the given name is that of a builtin exception.
///
/// See: <https://docs.python.org/3/library/exceptions.html#exception-hierarchy>
pub fn is_exception(name: &str) -> bool {
matches!(
name,
"BaseException"
| "BaseExceptionGroup"
| "GeneratorExit"
| "KeyboardInterrupt"
| "SystemExit"
| "Exception"
| "ArithmeticError"
| "FloatingPointError"
| "OverflowError"
| "ZeroDivisionError"
| "AssertionError"
| "AttributeError"
| "BufferError"
| "EOFError"
| "ExceptionGroup"
| "ImportError"
| "ModuleNotFoundError"
| "LookupError"
| "IndexError"
| "KeyError"
| "MemoryError"
| "NameError"
| "UnboundLocalError"
| "OSError"
| "BlockingIOError"
| "ChildProcessError"
| "ConnectionError"
| "BrokenPipeError"
| "ConnectionAbortedError"
| "ConnectionRefusedError"
| "ConnectionResetError"
| "FileExistsError"
| "FileNotFoundError"
| "InterruptedError"
| "IsADirectoryError"
| "NotADirectoryError"
| "PermissionError"
| "ProcessLookupError"
| "TimeoutError"
| "ReferenceError"
| "RuntimeError"
| "NotImplementedError"
| "RecursionError"
| "StopAsyncIteration"
| "StopIteration"
| "SyntaxError"
| "IndentationError"
| "TabError"
| "SystemError"
| "TypeError"
| "ValueError"
| "UnicodeError"
| "UnicodeDecodeError"
| "UnicodeEncodeError"
| "UnicodeTranslateError"
| "Warning"
| "BytesWarning"
| "DeprecationWarning"
| "EncodingWarning"
| "FutureWarning"
| "ImportWarning"
| "PendingDeprecationWarning"
| "ResourceWarning"
| "RuntimeWarning"
| "SyntaxWarning"
| "UnicodeWarning"
| "UserWarning"
)
}