diff --git a/crates/ruff/resources/test/fixtures/pylint/binary_op_exception.py b/crates/ruff/resources/test/fixtures/pylint/binary_op_exception.py new file mode 100644 index 0000000000..b0c20dfe68 --- /dev/null +++ b/crates/ruff/resources/test/fixtures/pylint/binary_op_exception.py @@ -0,0 +1,14 @@ +try: + 1 / 0 +except ZeroDivisionError or ValueError as e: # [binary-op-exception] + pass + +try: + raise ValueError +except ZeroDivisionError and ValueError as e: # [binary-op-exception] + print(e) + +try: + pass +except (ValueError, Exception, IOError): + pass diff --git a/crates/ruff/src/checkers/ast/mod.rs b/crates/ruff/src/checkers/ast/mod.rs index 99701ee996..13ed09648b 100644 --- a/crates/ruff/src/checkers/ast/mod.rs +++ b/crates/ruff/src/checkers/ast/mod.rs @@ -3706,6 +3706,10 @@ where if self.settings.rules.enabled(Rule::ReraiseNoCause) { tryceratops::rules::reraise_no_cause(self, body); } + + if self.settings.rules.enabled(Rule::BinaryOpException) { + pylint::rules::binary_op_exception(self, excepthandler); + } match name { Some(name) => { if self.settings.rules.enabled(Rule::AmbiguousVariableName) { diff --git a/crates/ruff/src/codes.rs b/crates/ruff/src/codes.rs index 73f3bceeba..57447fc21a 100644 --- a/crates/ruff/src/codes.rs +++ b/crates/ruff/src/codes.rs @@ -199,6 +199,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option { (Pylint, "W0129") => Rule::AssertOnStringLiteral, (Pylint, "W0602") => Rule::GlobalVariableNotAssigned, (Pylint, "W0603") => Rule::GlobalStatement, + (Pylint, "W0711") => Rule::BinaryOpException, (Pylint, "W1508") => Rule::InvalidEnvvarDefault, (Pylint, "W2901") => Rule::RedefinedLoopName, diff --git a/crates/ruff/src/registry.rs b/crates/ruff/src/registry.rs index 8355d645f9..5969873bfe 100644 --- a/crates/ruff/src/registry.rs +++ b/crates/ruff/src/registry.rs @@ -155,6 +155,7 @@ ruff_macros::register_rules!( rules::pylint::rules::InvalidEnvvarValue, rules::pylint::rules::BadStringFormatType, rules::pylint::rules::BidirectionalUnicode, + rules::pylint::rules::BinaryOpException, rules::pylint::rules::InvalidCharacterBackspace, rules::pylint::rules::InvalidCharacterSub, rules::pylint::rules::InvalidCharacterEsc, diff --git a/crates/ruff/src/rules/pylint/mod.rs b/crates/ruff/src/rules/pylint/mod.rs index d7bb7fec74..ddc9bbb496 100644 --- a/crates/ruff/src/rules/pylint/mod.rs +++ b/crates/ruff/src/rules/pylint/mod.rs @@ -23,6 +23,7 @@ mod tests { #[test_case(Rule::BadStrStripCall, Path::new("bad_str_strip_call.py"); "PLE01310")] #[test_case(Rule::BadStringFormatType, Path::new("bad_string_format_type.py"); "PLE1307")] #[test_case(Rule::BidirectionalUnicode, Path::new("bidirectional_unicode.py"); "PLE2502")] + #[test_case(Rule::BinaryOpException, Path::new("binary_op_exception.py"); "PLW0711")] #[test_case(Rule::CollapsibleElseIf, Path::new("collapsible_else_if.py"); "PLR5501")] #[test_case(Rule::CompareToEmptyString, Path::new("compare_to_empty_string.py"); "PLC1901")] #[test_case(Rule::ComparisonOfConstant, Path::new("comparison_of_constant.py"); "PLR0133")] diff --git a/crates/ruff/src/rules/pylint/rules/binary_op_exception.rs b/crates/ruff/src/rules/pylint/rules/binary_op_exception.rs new file mode 100644 index 0000000000..e6b197ca62 --- /dev/null +++ b/crates/ruff/src/rules/pylint/rules/binary_op_exception.rs @@ -0,0 +1,81 @@ +use rustpython_parser::ast::{Excepthandler, ExcepthandlerKind, ExprKind}; + +use ruff_diagnostics::{Diagnostic, Violation}; +use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::types::Range; + +use crate::checkers::ast::Checker; + +#[derive(Debug, PartialEq, Eq, Copy, Clone)] +enum Boolop { + And, + Or, +} + +impl From<&rustpython_parser::ast::Boolop> for Boolop { + fn from(op: &rustpython_parser::ast::Boolop) -> Self { + match op { + rustpython_parser::ast::Boolop::And => Boolop::And, + rustpython_parser::ast::Boolop::Or => Boolop::Or, + } + } +} + +/// ## What it does +/// Checks for `except` clauses that attempt to catch multiple +/// exceptions with a binary operation (`and` or `or`). +/// +/// ## Why is this bad? +/// A binary operation will not catch multiple exceptions. Instead, the binary +/// operation will be evaluated first, and the result of _that_ operation will +/// be caught (for an `or` operation, this is typically the first exception in +/// the list). This is almost never the desired behavior. +/// +/// ## Example +/// ```python +/// try: +/// pass +/// except A or B: +/// pass +/// ``` +/// +/// Use instead: +/// ```python +/// try: +/// pass +/// except (A ,B): +/// pass +/// ``` +#[violation] +pub struct BinaryOpException { + op: Boolop, +} + +impl Violation for BinaryOpException { + #[derive_message_formats] + fn message(&self) -> String { + let BinaryOpException { op } = self; + match op { + Boolop::And => format!("Exception to catch is the result of a binary `and` operation"), + Boolop::Or => format!("Exception to catch is the result of a binary `or` operation"), + } + } +} + +/// PLW0711 +pub fn binary_op_exception(checker: &mut Checker, excepthandler: &Excepthandler) { + let ExcepthandlerKind::ExceptHandler { type_, .. } = &excepthandler.node; + + let Some(type_) = type_ else { + return; + }; + + let ExprKind::BoolOp { op, .. } = &type_.node else { + return; + }; + + checker.diagnostics.push(Diagnostic::new( + BinaryOpException { op: op.into() }, + Range::from(type_), + )); +} diff --git a/crates/ruff/src/rules/pylint/rules/mod.rs b/crates/ruff/src/rules/pylint/rules/mod.rs index 3140626970..06c8e6a2d2 100644 --- a/crates/ruff/src/rules/pylint/rules/mod.rs +++ b/crates/ruff/src/rules/pylint/rules/mod.rs @@ -3,6 +3,7 @@ pub use await_outside_async::{await_outside_async, AwaitOutsideAsync}; pub use bad_str_strip_call::{bad_str_strip_call, BadStrStripCall}; pub use bad_string_format_type::{bad_string_format_type, BadStringFormatType}; pub use bidirectional_unicode::{bidirectional_unicode, BidirectionalUnicode}; +pub use binary_op_exception::{binary_op_exception, BinaryOpException}; pub use collapsible_else_if::{collapsible_else_if, CollapsibleElseIf}; pub use compare_to_empty_string::{compare_to_empty_string, CompareToEmptyString}; pub use comparison_of_constant::{comparison_of_constant, ComparisonOfConstant}; @@ -46,6 +47,7 @@ mod await_outside_async; mod bad_str_strip_call; mod bad_string_format_type; mod bidirectional_unicode; +mod binary_op_exception; mod collapsible_else_if; mod compare_to_empty_string; mod comparison_of_constant; diff --git a/crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLW0711_binary_op_exception.py.snap b/crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLW0711_binary_op_exception.py.snap new file mode 100644 index 0000000000..21e7211e11 --- /dev/null +++ b/crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLW0711_binary_op_exception.py.snap @@ -0,0 +1,31 @@ +--- +source: crates/ruff/src/rules/pylint/mod.rs +expression: diagnostics +--- +- kind: + name: BinaryOpException + body: "Exception to catch is the result of a binary `or` operation" + suggestion: ~ + fixable: false + location: + row: 3 + column: 7 + end_location: + row: 3 + column: 38 + fix: ~ + parent: ~ +- kind: + name: BinaryOpException + body: "Exception to catch is the result of a binary `and` operation" + suggestion: ~ + fixable: false + location: + row: 8 + column: 7 + end_location: + row: 8 + column: 39 + fix: ~ + parent: ~ + diff --git a/ruff.schema.json b/ruff.schema.json index d5f325a007..9560585595 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -1909,6 +1909,9 @@ "PLW060", "PLW0602", "PLW0603", + "PLW07", + "PLW071", + "PLW0711", "PLW1", "PLW15", "PLW150",