pylint: Implement binary-op-exception (PLW0711) (#3639)

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

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) {

crates/ruff/src/codes.rs

@@ -199,6 +199,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<Rule> {
         (Pylint, "W0129") => Rule::AssertOnStringLiteral,
         (Pylint, "W0602") => Rule::GlobalVariableNotAssigned,
         (Pylint, "W0603") => Rule::GlobalStatement,
+        (Pylint, "W0711") => Rule::BinaryOpException,
         (Pylint, "W1508") => Rule::InvalidEnvvarDefault,
         (Pylint, "W2901") => Rule::RedefinedLoopName,
 

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,

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")]

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_),
+    ));
+}

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;

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: ~
+