diff --git a/crates/ruff_linter/resources/test/fixtures/pylint/useless_exception_statement.py b/crates/ruff_linter/resources/test/fixtures/pylint/useless_exception_statement.py new file mode 100644 index 0000000000000..c3ca0b90d2513 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/pylint/useless_exception_statement.py @@ -0,0 +1,116 @@ +# Test case 1: Useless exception statement +from abc import ABC, abstractmethod +from contextlib import suppress + + +def func(): + AssertionError("This is an assertion error") # PLW0133 + + +# Test case 2: Useless exception statement in try-except block +def func(): + try: + Exception("This is an exception") # PLW0133 + except Exception as err: + pass + + +# Test case 3: Useless exception statement in if statement +def func(): + if True: + RuntimeError("This is an exception") # PLW0133 + + +# Test case 4: Useless exception statement in class +def func(): + class Class: + def __init__(self): + TypeError("This is an exception") # PLW0133 + + +# Test case 5: Useless exception statement in function +def func(): + def inner(): + IndexError("This is an exception") # PLW0133 + + inner() + + +# Test case 6: Useless exception statement in while loop +def func(): + while True: + KeyError("This is an exception") # PLW0133 + + +# Test case 7: Useless exception statement in abstract class +def func(): + class Class(ABC): + @abstractmethod + def method(self): + NotImplementedError("This is an exception") # PLW0133 + + +# Test case 8: Useless exception statement inside context manager +def func(): + with suppress(AttributeError): + AttributeError("This is an exception") # PLW0133 + + +# Test case 9: Useless exception statement in parentheses +def func(): + (RuntimeError("This is an exception")) # PLW0133 + + +# Test case 10: Useless exception statement in continuation +def func(): + x = 1; (RuntimeError("This is an exception")); y = 2 # PLW0133 + + +# Non-violation test cases: PLW0133 + + +# Test case 1: Used exception statement in try-except block +def func(): + raise Exception("This is an exception") # OK + + +# Test case 2: Used exception statement in if statement +def func(): + if True: + raise ValueError("This is an exception") # OK + + +# Test case 3: Used exception statement in class +def func(): + class Class: + def __init__(self): + raise TypeError("This is an exception") # OK + + +# Test case 4: Exception statement used in list comprehension +def func(): + [ValueError("This is an exception") for i in range(10)] # OK + + +# Test case 5: Exception statement used when initializing a dictionary +def func(): + {i: TypeError("This is an exception") for i in range(10)} # OK + + +# Test case 6: Exception statement used in function +def func(): + def inner(): + raise IndexError("This is an exception") # OK + + inner() + + +# Test case 7: Exception statement used in variable assignment +def func(): + err = KeyError("This is an exception") # OK + + +# Test case 8: Exception statement inside context manager +def func(): + with suppress(AttributeError): + raise AttributeError("This is an exception") # OK diff --git a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs index d8147fe2db1f1..f8867e68cf5c3 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs @@ -1609,7 +1609,7 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) { refurb::rules::delete_full_slice(checker, delete); } } - Stmt::Expr(ast::StmtExpr { value, range: _ }) => { + Stmt::Expr(expr @ ast::StmtExpr { value, range: _ }) => { if checker.enabled(Rule::UselessComparison) { flake8_bugbear::rules::useless_comparison(checker, value); } @@ -1632,6 +1632,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) { if checker.enabled(Rule::RepeatedAppend) { refurb::rules::repeated_append(checker, stmt); } + if checker.enabled(Rule::UselessExceptionStatement) { + pylint::rules::useless_exception_statement(checker, expr); + } } _ => {} } diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index ae9a1b29ccd47..4f545d1afae88 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -293,6 +293,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Pylint, "W0127") => (RuleGroup::Stable, rules::pylint::rules::SelfAssigningVariable), (Pylint, "W0129") => (RuleGroup::Stable, rules::pylint::rules::AssertOnStringLiteral), (Pylint, "W0131") => (RuleGroup::Stable, rules::pylint::rules::NamedExprWithoutContext), + (Pylint, "W0133") => (RuleGroup::Preview, rules::pylint::rules::UselessExceptionStatement), (Pylint, "W0245") => (RuleGroup::Preview, rules::pylint::rules::SuperWithoutBrackets), (Pylint, "W0406") => (RuleGroup::Stable, rules::pylint::rules::ImportSelf), (Pylint, "W0602") => (RuleGroup::Stable, rules::pylint::rules::GlobalVariableNotAssigned), diff --git a/crates/ruff_linter/src/rules/pylint/mod.rs b/crates/ruff_linter/src/rules/pylint/mod.rs index 8b746c8018db4..e03e9303b3f26 100644 --- a/crates/ruff_linter/src/rules/pylint/mod.rs +++ b/crates/ruff_linter/src/rules/pylint/mod.rs @@ -177,6 +177,10 @@ mod tests { Rule::UnnecessaryDictIndexLookup, Path::new("unnecessary_dict_index_lookup.py") )] + #[test_case( + Rule::UselessExceptionStatement, + Path::new("useless_exception_statement.py") + )] fn rules(rule_code: Rule, path: &Path) -> Result<()> { let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy()); let diagnostics = test_path( diff --git a/crates/ruff_linter/src/rules/pylint/rules/mod.rs b/crates/ruff_linter/src/rules/pylint/rules/mod.rs index 4049b2dca65fd..34289564eed02 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/mod.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/mod.rs @@ -79,6 +79,7 @@ pub(crate) use unnecessary_lambda::*; pub(crate) use unnecessary_list_index_lookup::*; pub(crate) use unspecified_encoding::*; pub(crate) use useless_else_on_loop::*; +pub(crate) use useless_exception_statement::*; pub(crate) use useless_import_alias::*; pub(crate) use useless_return::*; pub(crate) use useless_with_lock::*; @@ -166,6 +167,7 @@ mod unnecessary_lambda; mod unnecessary_list_index_lookup; mod unspecified_encoding; mod useless_else_on_loop; +mod useless_exception_statement; mod useless_import_alias; mod useless_return; mod useless_with_lock; diff --git a/crates/ruff_linter/src/rules/pylint/rules/useless_exception_statement.rs b/crates/ruff_linter/src/rules/pylint/rules/useless_exception_statement.rs new file mode 100644 index 0000000000000..02066cd04b0d8 --- /dev/null +++ b/crates/ruff_linter/src/rules/pylint/rules/useless_exception_statement.rs @@ -0,0 +1,95 @@ +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_text_size::Ranged; + +use crate::checkers::ast::Checker; + +/// ## What it does +/// Checks for an exception that is not raised. +/// +/// ## Why is this bad? +/// It's unnecessary to create an exception without raising it. For example, +/// `ValueError("...")` on its own will have no effect (unlike +/// `raise ValueError("...")`) and is likely a mistake. +/// +/// ## Example +/// ```python +/// ValueError("...") +/// ``` +/// +/// Use instead: +/// ```python +/// raise ValueError("...") +/// ``` +/// +/// ## Fix safety +/// This rule's fix is marked as unsafe, as converting a useless exception +/// statement to a `raise` statement will change the program's behavior. +#[violation] +pub struct UselessExceptionStatement; + +impl Violation for UselessExceptionStatement { + const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes; + + #[derive_message_formats] + fn message(&self) -> String { + format!("Missing `raise` statement on exception") + } + + fn fix_title(&self) -> Option { + Some(format!("Add `raise` keyword")) + } +} + +/// PLW0133 +pub(crate) fn useless_exception_statement(checker: &mut Checker, expr: &ast::StmtExpr) { + let Expr::Call(ast::ExprCall { func, .. }) = expr.value.as_ref() else { + return; + }; + + if is_builtin_exception(func, checker.semantic()) { + let mut diagnostic = Diagnostic::new(UselessExceptionStatement, expr.range()); + diagnostic.set_fix(Fix::unsafe_edit(Edit::insertion( + "raise ".to_string(), + expr.start(), + ))); + checker.diagnostics.push(diagnostic); + } +} + +/// Returns `true` if the given expression is a builtin exception. +/// +/// See: +fn is_builtin_exception(expr: &Expr, semantic: &SemanticModel) -> bool { + return semantic.resolve_call_path(expr).is_some_and(|call_path| { + matches!( + call_path.as_slice(), + [ + "", + "SystemExit" + | "Exception" + | "ArithmeticError" + | "AssertionError" + | "AttributeError" + | "BufferError" + | "EOFError" + | "ImportError" + | "LookupError" + | "IndexError" + | "KeyError" + | "MemoryError" + | "NameError" + | "ReferenceError" + | "RuntimeError" + | "NotImplementedError" + | "StopIteration" + | "SyntaxError" + | "SystemError" + | "TypeError" + | "ValueError" + ] + ) + }); +} diff --git a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLW0133_useless_exception_statement.py.snap b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLW0133_useless_exception_statement.py.snap new file mode 100644 index 0000000000000..91c485cfb4e8c --- /dev/null +++ b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLW0133_useless_exception_statement.py.snap @@ -0,0 +1,195 @@ +--- +source: crates/ruff_linter/src/rules/pylint/mod.rs +--- +useless_exception_statement.py:7:5: PLW0133 [*] Missing `raise` statement on exception + | +6 | def func(): +7 | AssertionError("This is an assertion error") # PLW0133 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PLW0133 + | + = help: Add `raise` keyword + +ℹ Unsafe fix +4 4 | +5 5 | +6 6 | def func(): +7 |- AssertionError("This is an assertion error") # PLW0133 + 7 |+ raise AssertionError("This is an assertion error") # PLW0133 +8 8 | +9 9 | +10 10 | # Test case 2: Useless exception statement in try-except block + +useless_exception_statement.py:13:9: PLW0133 [*] Missing `raise` statement on exception + | +11 | def func(): +12 | try: +13 | Exception("This is an exception") # PLW0133 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PLW0133 +14 | except Exception as err: +15 | pass + | + = help: Add `raise` keyword + +ℹ Unsafe fix +10 10 | # Test case 2: Useless exception statement in try-except block +11 11 | def func(): +12 12 | try: +13 |- Exception("This is an exception") # PLW0133 + 13 |+ raise Exception("This is an exception") # PLW0133 +14 14 | except Exception as err: +15 15 | pass +16 16 | + +useless_exception_statement.py:21:9: PLW0133 [*] Missing `raise` statement on exception + | +19 | def func(): +20 | if True: +21 | RuntimeError("This is an exception") # PLW0133 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PLW0133 + | + = help: Add `raise` keyword + +ℹ Unsafe fix +18 18 | # Test case 3: Useless exception statement in if statement +19 19 | def func(): +20 20 | if True: +21 |- RuntimeError("This is an exception") # PLW0133 + 21 |+ raise RuntimeError("This is an exception") # PLW0133 +22 22 | +23 23 | +24 24 | # Test case 4: Useless exception statement in class + +useless_exception_statement.py:28:13: PLW0133 [*] Missing `raise` statement on exception + | +26 | class Class: +27 | def __init__(self): +28 | TypeError("This is an exception") # PLW0133 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PLW0133 + | + = help: Add `raise` keyword + +ℹ Unsafe fix +25 25 | def func(): +26 26 | class Class: +27 27 | def __init__(self): +28 |- TypeError("This is an exception") # PLW0133 + 28 |+ raise TypeError("This is an exception") # PLW0133 +29 29 | +30 30 | +31 31 | # Test case 5: Useless exception statement in function + +useless_exception_statement.py:34:9: PLW0133 [*] Missing `raise` statement on exception + | +32 | def func(): +33 | def inner(): +34 | IndexError("This is an exception") # PLW0133 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PLW0133 +35 | +36 | inner() + | + = help: Add `raise` keyword + +ℹ Unsafe fix +31 31 | # Test case 5: Useless exception statement in function +32 32 | def func(): +33 33 | def inner(): +34 |- IndexError("This is an exception") # PLW0133 + 34 |+ raise IndexError("This is an exception") # PLW0133 +35 35 | +36 36 | inner() +37 37 | + +useless_exception_statement.py:42:9: PLW0133 [*] Missing `raise` statement on exception + | +40 | def func(): +41 | while True: +42 | KeyError("This is an exception") # PLW0133 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PLW0133 + | + = help: Add `raise` keyword + +ℹ Unsafe fix +39 39 | # Test case 6: Useless exception statement in while loop +40 40 | def func(): +41 41 | while True: +42 |- KeyError("This is an exception") # PLW0133 + 42 |+ raise KeyError("This is an exception") # PLW0133 +43 43 | +44 44 | +45 45 | # Test case 7: Useless exception statement in abstract class + +useless_exception_statement.py:50:13: PLW0133 [*] Missing `raise` statement on exception + | +48 | @abstractmethod +49 | def method(self): +50 | NotImplementedError("This is an exception") # PLW0133 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PLW0133 + | + = help: Add `raise` keyword + +ℹ Unsafe fix +47 47 | class Class(ABC): +48 48 | @abstractmethod +49 49 | def method(self): +50 |- NotImplementedError("This is an exception") # PLW0133 + 50 |+ raise NotImplementedError("This is an exception") # PLW0133 +51 51 | +52 52 | +53 53 | # Test case 8: Useless exception statement inside context manager + +useless_exception_statement.py:56:9: PLW0133 [*] Missing `raise` statement on exception + | +54 | def func(): +55 | with suppress(AttributeError): +56 | AttributeError("This is an exception") # PLW0133 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PLW0133 + | + = help: Add `raise` keyword + +ℹ Unsafe fix +53 53 | # Test case 8: Useless exception statement inside context manager +54 54 | def func(): +55 55 | with suppress(AttributeError): +56 |- AttributeError("This is an exception") # PLW0133 + 56 |+ raise AttributeError("This is an exception") # PLW0133 +57 57 | +58 58 | +59 59 | # Test case 9: Useless exception statement in parentheses + +useless_exception_statement.py:61:5: PLW0133 [*] Missing `raise` statement on exception + | +59 | # Test case 9: Useless exception statement in parentheses +60 | def func(): +61 | (RuntimeError("This is an exception")) # PLW0133 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PLW0133 + | + = help: Add `raise` keyword + +ℹ Unsafe fix +58 58 | +59 59 | # Test case 9: Useless exception statement in parentheses +60 60 | def func(): +61 |- (RuntimeError("This is an exception")) # PLW0133 + 61 |+ raise (RuntimeError("This is an exception")) # PLW0133 +62 62 | +63 63 | +64 64 | # Test case 10: Useless exception statement in continuation + +useless_exception_statement.py:66:12: PLW0133 [*] Missing `raise` statement on exception + | +64 | # Test case 10: Useless exception statement in continuation +65 | def func(): +66 | x = 1; (RuntimeError("This is an exception")); y = 2 # PLW0133 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PLW0133 + | + = help: Add `raise` keyword + +ℹ Unsafe fix +63 63 | +64 64 | # Test case 10: Useless exception statement in continuation +65 65 | def func(): +66 |- x = 1; (RuntimeError("This is an exception")); y = 2 # PLW0133 + 66 |+ x = 1; raise (RuntimeError("This is an exception")); y = 2 # PLW0133 +67 67 | +68 68 | +69 69 | # Non-violation test cases: PLW0133 diff --git a/ruff.schema.json b/ruff.schema.json index d6f734fe60851..5f756e68e6011 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -3314,6 +3314,7 @@ "PLW0129", "PLW013", "PLW0131", + "PLW0133", "PLW02", "PLW024", "PLW0245",