Skip to content

Commit 3243906

Browse files
[pylint] Include builtin warnings in useless-exception-statement (PLW0133) (#10394)
## Summary Closes #10392.
1 parent 4db5c29 commit 3243906

File tree

5 files changed

+140
-76
lines changed

5 files changed

+140
-76
lines changed

crates/ruff_linter/resources/test/fixtures/pylint/useless_exception_statement.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
1-
# Test case 1: Useless exception statement
21
from abc import ABC, abstractmethod
32
from contextlib import suppress
43

54

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

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

6868

69+
# Test case 11: Useless warning statement
70+
def func():
71+
UserWarning("This is an assertion error") # PLW0133
72+
73+
6974
# Non-violation test cases: PLW0133
7075

7176

crates/ruff_linter/src/rules/pylint/rules/useless_exception_statement.rs

Lines changed: 7 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation};
22
use ruff_macros::{derive_message_formats, violation};
33
use ruff_python_ast::{self as ast, Expr};
44
use ruff_python_semantic::SemanticModel;
5+
use ruff_python_stdlib::builtins;
56
use ruff_text_size::Ranged;
67

78
use crate::checkers::ast::Checker;
@@ -14,6 +15,10 @@ use crate::checkers::ast::Checker;
1415
/// `ValueError("...")` on its own will have no effect (unlike
1516
/// `raise ValueError("...")`) and is likely a mistake.
1617
///
18+
/// ## Known problems
19+
/// This rule only detects built-in exceptions, like `ValueError`, and does
20+
/// not catch user-defined exceptions.
21+
///
1722
/// ## Example
1823
/// ```python
1924
/// ValueError("...")
@@ -60,38 +65,8 @@ pub(crate) fn useless_exception_statement(checker: &mut Checker, expr: &ast::Stm
6065
}
6166

6267
/// Returns `true` if the given expression is a builtin exception.
63-
///
64-
/// See: <https://docs.python.org/3/library/exceptions.html#exception-hierarchy>
6568
fn is_builtin_exception(expr: &Expr, semantic: &SemanticModel) -> bool {
66-
return semantic
69+
semantic
6770
.resolve_qualified_name(expr)
68-
.is_some_and(|qualified_name| {
69-
matches!(
70-
qualified_name.segments(),
71-
[
72-
"",
73-
"SystemExit"
74-
| "Exception"
75-
| "ArithmeticError"
76-
| "AssertionError"
77-
| "AttributeError"
78-
| "BufferError"
79-
| "EOFError"
80-
| "ImportError"
81-
| "LookupError"
82-
| "IndexError"
83-
| "KeyError"
84-
| "MemoryError"
85-
| "NameError"
86-
| "ReferenceError"
87-
| "RuntimeError"
88-
| "NotImplementedError"
89-
| "StopIteration"
90-
| "SyntaxError"
91-
| "SystemError"
92-
| "TypeError"
93-
| "ValueError"
94-
]
95-
)
96-
});
71+
.is_some_and(|qualified_name| matches!(qualified_name.segments(), ["", name] if builtins::is_exception(name)))
9772
}

crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLW0133_useless_exception_statement.py.snap

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ source: crates/ruff_linter/src/rules/pylint/mod.rs
33
---
44
useless_exception_statement.py:7:5: PLW0133 [*] Missing `raise` statement on exception
55
|
6+
5 | # Test case 1: Useless exception statement
67
6 | def func():
78
7 | AssertionError("This is an assertion error") # PLW0133
89
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PLW0133
@@ -11,7 +12,7 @@ useless_exception_statement.py:7:5: PLW0133 [*] Missing `raise` statement on exc
1112

1213
Unsafe fix
1314
4 4 |
14-
5 5 |
15+
5 5 | # Test case 1: Useless exception statement
1516
6 6 | def func():
1617
7 |- AssertionError("This is an assertion error") # PLW0133
1718
7 |+ raise AssertionError("This is an assertion error") # PLW0133
@@ -192,4 +193,23 @@ useless_exception_statement.py:66:12: PLW0133 [*] Missing `raise` statement on e
192193
66 |+ x = 1; raise (RuntimeError("This is an exception")); y = 2 # PLW0133
193194
67 67 |
194195
68 68 |
195-
69 69 | # Non-violation test cases: PLW0133
196+
69 69 | # Test case 11: Useless warning statement
197+
198+
useless_exception_statement.py:71:5: PLW0133 [*] Missing `raise` statement on exception
199+
|
200+
69 | # Test case 11: Useless warning statement
201+
70 | def func():
202+
71 | UserWarning("This is an assertion error") # PLW0133
203+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PLW0133
204+
|
205+
= help: Add `raise` keyword
206+
207+
Unsafe fix
208+
68 68 |
209+
69 69 | # Test case 11: Useless warning statement
210+
70 70 | def func():
211+
71 |- UserWarning("This is an assertion error") # PLW0133
212+
71 |+ raise UserWarning("This is an assertion error") # PLW0133
213+
72 72 |
214+
73 73 |
215+
74 74 | # Non-violation test cases: PLW0133

crates/ruff_linter/src/rules/tryceratops/rules/type_check_without_type_error.rs

Lines changed: 29 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
use ruff_diagnostics::{Diagnostic, Violation};
22
use ruff_macros::{derive_message_formats, violation};
3+
use ruff_python_ast::helpers::map_callable;
34
use ruff_python_ast::statement_visitor::{walk_stmt, StatementVisitor};
45
use ruff_python_ast::{self as ast, Expr, Stmt, StmtIf};
6+
use ruff_python_semantic::SemanticModel;
57
use ruff_text_size::Ranged;
68

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

99-
/// Returns `true` if `exc` is a reference to a builtin exception.
100-
fn is_builtin_exception(checker: &mut Checker, exc: &Expr) -> bool {
101-
return checker
102-
.semantic()
103-
.resolve_qualified_name(exc)
101+
fn check_raise(checker: &mut Checker, exc: &Expr, item: &Stmt) {
102+
if is_builtin_exception(exc, checker.semantic()) {
103+
checker
104+
.diagnostics
105+
.push(Diagnostic::new(TypeCheckWithoutTypeError, item.range()));
106+
}
107+
}
108+
109+
/// Search the body of an if-condition for raises.
110+
fn check_body(checker: &mut Checker, body: &[Stmt]) {
111+
for item in body {
112+
if has_control_flow(item) {
113+
return;
114+
}
115+
if let Stmt::Raise(ast::StmtRaise { exc: Some(exc), .. }) = &item {
116+
check_raise(checker, exc, item);
117+
}
118+
}
119+
}
120+
121+
/// Returns `true` if the given expression is a builtin exception.
122+
///
123+
/// This function only matches to a subset of the builtin exceptions, and omits `TypeError`.
124+
fn is_builtin_exception(expr: &Expr, semantic: &SemanticModel) -> bool {
125+
semantic
126+
.resolve_qualified_name(map_callable(expr))
104127
.is_some_and(|qualified_name| {
105128
matches!(
106129
qualified_name.segments(),
@@ -123,42 +146,7 @@ fn is_builtin_exception(checker: &mut Checker, exc: &Expr) -> bool {
123146
| "ValueError"
124147
]
125148
)
126-
});
127-
}
128-
129-
/// Returns `true` if an [`Expr`] is a reference to a builtin exception.
130-
fn check_raise_type(checker: &mut Checker, exc: &Expr) -> bool {
131-
match exc {
132-
Expr::Name(_) => is_builtin_exception(checker, exc),
133-
Expr::Call(ast::ExprCall { func, .. }) => {
134-
if let Expr::Name(_) = func.as_ref() {
135-
is_builtin_exception(checker, func)
136-
} else {
137-
false
138-
}
139-
}
140-
_ => false,
141-
}
142-
}
143-
144-
fn check_raise(checker: &mut Checker, exc: &Expr, item: &Stmt) {
145-
if check_raise_type(checker, exc) {
146-
checker
147-
.diagnostics
148-
.push(Diagnostic::new(TypeCheckWithoutTypeError, item.range()));
149-
}
150-
}
151-
152-
/// Search the body of an if-condition for raises.
153-
fn check_body(checker: &mut Checker, body: &[Stmt]) {
154-
for item in body {
155-
if has_control_flow(item) {
156-
return;
157-
}
158-
if let Stmt::Raise(ast::StmtRaise { exc: Some(exc), .. }) = &item {
159-
check_raise(checker, exc, item);
160-
}
161-
}
149+
})
162150
}
163151

164152
/// TRY004

crates/ruff_python_stdlib/src/builtins.rs

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -368,3 +368,79 @@ pub fn is_ipython_builtin(name: &str) -> bool {
368368
// Constructed by converting the `IPYTHON_BUILTINS` slice to a `match` expression.
369369
matches!(name, "__IPYTHON__" | "display" | "get_ipython")
370370
}
371+
372+
/// Returns `true` if the given name is that of a builtin exception.
373+
///
374+
/// See: <https://docs.python.org/3/library/exceptions.html#exception-hierarchy>
375+
pub fn is_exception(name: &str) -> bool {
376+
matches!(
377+
name,
378+
"BaseException"
379+
| "BaseExceptionGroup"
380+
| "GeneratorExit"
381+
| "KeyboardInterrupt"
382+
| "SystemExit"
383+
| "Exception"
384+
| "ArithmeticError"
385+
| "FloatingPointError"
386+
| "OverflowError"
387+
| "ZeroDivisionError"
388+
| "AssertionError"
389+
| "AttributeError"
390+
| "BufferError"
391+
| "EOFError"
392+
| "ExceptionGroup"
393+
| "ImportError"
394+
| "ModuleNotFoundError"
395+
| "LookupError"
396+
| "IndexError"
397+
| "KeyError"
398+
| "MemoryError"
399+
| "NameError"
400+
| "UnboundLocalError"
401+
| "OSError"
402+
| "BlockingIOError"
403+
| "ChildProcessError"
404+
| "ConnectionError"
405+
| "BrokenPipeError"
406+
| "ConnectionAbortedError"
407+
| "ConnectionRefusedError"
408+
| "ConnectionResetError"
409+
| "FileExistsError"
410+
| "FileNotFoundError"
411+
| "InterruptedError"
412+
| "IsADirectoryError"
413+
| "NotADirectoryError"
414+
| "PermissionError"
415+
| "ProcessLookupError"
416+
| "TimeoutError"
417+
| "ReferenceError"
418+
| "RuntimeError"
419+
| "NotImplementedError"
420+
| "RecursionError"
421+
| "StopAsyncIteration"
422+
| "StopIteration"
423+
| "SyntaxError"
424+
| "IndentationError"
425+
| "TabError"
426+
| "SystemError"
427+
| "TypeError"
428+
| "ValueError"
429+
| "UnicodeError"
430+
| "UnicodeDecodeError"
431+
| "UnicodeEncodeError"
432+
| "UnicodeTranslateError"
433+
| "Warning"
434+
| "BytesWarning"
435+
| "DeprecationWarning"
436+
| "EncodingWarning"
437+
| "FutureWarning"
438+
| "ImportWarning"
439+
| "PendingDeprecationWarning"
440+
| "ResourceWarning"
441+
| "RuntimeWarning"
442+
| "SyntaxWarning"
443+
| "UnicodeWarning"
444+
| "UserWarning"
445+
)
446+
}

0 commit comments

Comments
 (0)