Add rule to enforce parentheses in a or b and c

crates/ruff_linter/resources/test/fixtures/ruff/RUF021.py

@@ -0,0 +1,98 @@
+# See https://docs.python.org/3/reference/expressions.html#operator-precedence
+# for the official docs on operator precedence.
+#
+# Most importantly, `and` *always* takes precedence over `or`.
+#
+# `not` (the third boolean/logical operator) takes precedence over both,
+# but the rule there is easier to remember,
+# so we don't emit a diagnostic if a `not` expression is unparenthesized
+# as part of a chain.
+
+a, b, c = 1, 0, 2
+x = a or b and c  # RUF021: => `a or (b and c)`
+
+a, b, c = 0, 1, 2
+y = a and b or c  # RUF021: => `(a and b) or c`
+
+a, b, c, d = 1, 2, 0, 3
+if a or b or c and d:  # RUF021: => `a or b or (c and d)`
+    pass
+
+a, b, c, d = 0, 0, 2, 3
+
+if bool():
+    pass
+elif a or b and c or d:  # RUF021: => `a or (b and c) or d`
+    pass
+
+a, b, c, d = 0, 1, 0, 2
+while a and b or c and d:  # RUF021: => `(and b) or (c and d)`
+    pass
+
+b, c, d, e = 2, 3, 0, 4
+z = [a for a in range(5) if a or b or c or d and e]  # RUF021: => `a or b or c or (d and e)`
+
+a, b, c, d = 0, 1, 3, 0
+assert not a and b or c or d  # RUF021: => `(not a and b) or c or d`
+
+if (not a) and b or c or d:  # RUF021: => `((not a) and b) or c or d`
+    if (not a and b) or c or d:  # OK
+        pass
+
+#############################################
+# If they're all the same operator, it's fine
+#############################################
+
+x = not a and c  # OK
+
+if a or b or c:  # OK
+    pass
+
+while a and b and c:  # OK
+    pass
+
+###########################################################
+# We don't consider `not` as part of a chain as problematic
+###########################################################
+
+x = not a or not b or not c  # OK
+
+#####################################
+# If they're parenthesized, it's fine
+#####################################
+
+a, b, c = 1, 0, 2
+x = a or (b and c)  # OK
+x2 = (a or b) and c  # OK
+x3 = (a or b) or c  # OK
+x4 = (a and b) and c  # OK
+
+a, b, c = 0, 1, 2
+y = (a and b) or c  # OK
+yy = a and (b or c)  # OK
+
+a, b, c, d = 1, 2, 0, 3
+if a or b or (c and d):  # OK
+    pass
+
+a, b, c, d = 0, 0, 2, 3
+
+if bool():
+    pass
+elif a or (b and c) or d:  # OK
+    pass
+
+a, b, c, d = 0, 1, 0, 2
+while (a and b) or (c and d):  # OK
+    pass
+
+b, c, d, e = 2, 3, 0, 4
+z = [a for a in range(5) if a or b or c or (d and e)]  # OK
+
+a, b = 1, 2
+if (not a) or b:  # OK
+    if (not a) and b:  # OK
+        pass
+
+a, b, c, d = 0, 1, 3, 0
+assert ((not a) and b) or c or d  # OK

crates/ruff_linter/src/checkers/ast/analyze/expression.rs

@@ -1492,6 +1492,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
             if checker.enabled(Rule::UnnecessaryKeyCheck) {
                 ruff::rules::unnecessary_key_check(checker, expr);
             }
+            if checker.enabled(Rule::ParenthesizeChainedOperators) {
+                ruff::rules::parenthesize_chained_logical_operators(checker, bool_op);
+            }
         }
         Expr::NamedExpr(..) => {
             if checker.enabled(Rule::AssignmentInAssert) {

crates/ruff_linter/src/codes.rs

@@ -923,6 +923,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
         (Ruff, "018") => (RuleGroup::Preview, rules::ruff::rules::AssignmentInAssert),
         (Ruff, "019") => (RuleGroup::Preview, rules::ruff::rules::UnnecessaryKeyCheck),
         (Ruff, "020") => (RuleGroup::Preview, rules::ruff::rules::NeverUnion),
+        (Ruff, "021") => (RuleGroup::Preview, rules::ruff::rules::ParenthesizeChainedOperators),
         (Ruff, "100") => (RuleGroup::Stable, rules::ruff::rules::UnusedNOQA),
         (Ruff, "200") => (RuleGroup::Stable, rules::ruff::rules::InvalidPyprojectToml),
 

crates/ruff_linter/src/rules/ruff/mod.rs

@@ -46,6 +46,7 @@ mod tests {
     #[test_case(Rule::AssignmentInAssert, Path::new("RUF018.py"))]
     #[test_case(Rule::UnnecessaryKeyCheck, Path::new("RUF019.py"))]
     #[test_case(Rule::NeverUnion, Path::new("RUF020.py"))]
+    #[test_case(Rule::ParenthesizeChainedOperators, Path::new("RUF021.py"))]
     fn rules(rule_code: Rule, path: &Path) -> Result<()> {
         let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
         let diagnostics = test_path(

crates/ruff_linter/src/rules/ruff/rules/mod.rs

@@ -11,6 +11,7 @@ pub(crate) use mutable_class_default::*;
 pub(crate) use mutable_dataclass_default::*;
 pub(crate) use never_union::*;
 pub(crate) use pairwise_over_zipped::*;
+pub(crate) use parenthesize_logical_operators::*;
 pub(crate) use quadratic_list_summation::*;
 pub(crate) use static_key_dict_comprehension::*;
 pub(crate) use unnecessary_iterable_allocation_for_first_element::*;
@@ -34,6 +35,7 @@ mod mutable_class_default;
 mod mutable_dataclass_default;
 mod never_union;
 mod pairwise_over_zipped;
+mod parenthesize_logical_operators;
 mod static_key_dict_comprehension;
 mod unnecessary_iterable_allocation_for_first_element;
 mod unnecessary_key_check;

crates/ruff_linter/src/rules/ruff/rules/parenthesize_logical_operators.rs

@@ -0,0 +1,95 @@
+use ruff_diagnostics::{Diagnostic, Violation};
+use ruff_macros::{derive_message_formats, violation};
+use ruff_python_ast as ast;
+use ruff_python_ast::parenthesize::parenthesized_range;
+use ruff_text_size::Ranged;
+
+use crate::checkers::ast::Checker;
+
+/// ## What it does
+/// Checks for chained operators where adding parentheses could improve the
+/// clarity of the code.
+///
+/// ## Why is this bad?
+/// `and` always binds more tightly than `or` when chaining the two together,
+/// but this can be hard to remember (and sometimes surprising).
+/// Adding parentheses in these situations can greatly improve code readability,
+/// with no change to semantics or performance.
+///
+/// For example:
+/// ```python
+/// a, b, c = 1, 0, 2
+/// x = a or b and c
+///
+/// d, e, f = 0, 1, 2
+/// y = d and e or f
+/// ```
+///
+/// Use instead:
+/// ```python
+/// a, b, c = 1, 0, 2
+/// x = a or (b and c)
+///
+/// d, e, f = 0, 1, 2
+/// y = (d and e) or f
+/// ````
+#[violation]
+pub struct ParenthesizeChainedOperators;
+
+impl Violation for ParenthesizeChainedOperators {
+    #[derive_message_formats]
+    fn message(&self) -> String {
+        format!(
+            "Parenthesize `a and b` expressions when chaining `and` and `or` together, to make the precedence clear"
+        )
+    }
+}
+
+/// RUF021
+pub(crate) fn parenthesize_chained_logical_operators(
+    checker: &mut Checker,
+    expr: &ast::ExprBoolOp,
+) {
+    // We're only interested in `and` expressions inside `or` expressions:
+    // - `a or b or c` => `BoolOp(values=[Name("a"), Name("b"), Name("c")], op=Or)`
+    // - `a and b and c` => `BoolOp(values=[Name("a"), Name("b"), Name("c")], op=And)`
+    // - `a or b and c` => `BoolOp(value=[Name("a"), BoolOp(values=[Name("b"), Name("c")], op=And), op=Or)`
+    //
+    // While it is *possible* to get an `Or` node inside an `And` node,
+    // you can only achieve it by parenthesizing the `or` subexpression
+    // (since normally, `and` always binds more tightly):
+    // - `a and (b or c)` => `BoolOp(values=[Name("a"), BoolOp(values=[Name("b"), Name("c"), op=Or), op=And)`
+    //
+    // We only care about unparenthesized boolean subexpressions here
+    // (if they're parenthesized already, that's great!),
+    // so we can ignore all cases where an `Or` node
+    // exists inside an `And` node.
+    if expr.op.is_and() {
+        return;
+    }
+    for condition in &expr.values {
+        match condition {
+            ast::Expr::BoolOp(
+                bool_op @ ast::ExprBoolOp {
+                    op: ast::BoolOp::And,
+                    ..
+                },
+            ) => {
+                if parenthesized_range(
+                    bool_op.into(),
+                    expr.into(),
+                    checker.indexer().comment_ranges(),
+                    checker.locator().contents(),
+                )
+                .is_none()
+                {
+                    checker.diagnostics.push(Diagnostic::new(
+                        ParenthesizeChainedOperators,
+                        bool_op.range(),
+                    ));
+                }
+            }
+            _ => continue,
+        };
+    }
+}

crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF021_RUF021.py.snap

@@ -0,0 +1,83 @@
+---
+source: crates/ruff_linter/src/rules/ruff/mod.rs
+---
+RUF021.py:12:10: RUF021 Parenthesize `a and b` expressions when chaining `and` and `or` together, to make the precedence clear
+   |
+11 | a, b, c = 1, 0, 2
+12 | x = a or b and c  # RUF021: => `a or (b and c)`
+   |          ^^^^^^^ RUF021
+13 | 
+14 | a, b, c = 0, 1, 2
+   |
+
+RUF021.py:15:5: RUF021 Parenthesize `a and b` expressions when chaining `and` and `or` together, to make the precedence clear
+   |
+14 | a, b, c = 0, 1, 2
+15 | y = a and b or c  # RUF021: => `(a and b) or c`
+   |     ^^^^^^^ RUF021
+16 | 
+17 | a, b, c, d = 1, 2, 0, 3
+   |
+
+RUF021.py:18:14: RUF021 Parenthesize `a and b` expressions when chaining `and` and `or` together, to make the precedence clear
+   |
+17 | a, b, c, d = 1, 2, 0, 3
+18 | if a or b or c and d:  # RUF021: => `a or b or (c and d)`
+   |              ^^^^^^^ RUF021
+19 |     pass
+   |
+
+RUF021.py:25:11: RUF021 Parenthesize `a and b` expressions when chaining `and` and `or` together, to make the precedence clear
+   |
+23 | if bool():
+24 |     pass
+25 | elif a or b and c or d:  # RUF021: => `a or (b and c) or d`
+   |           ^^^^^^^ RUF021
+26 |     pass
+   |
+
+RUF021.py:29:7: RUF021 Parenthesize `a and b` expressions when chaining `and` and `or` together, to make the precedence clear
+   |
+28 | a, b, c, d = 0, 1, 0, 2
+29 | while a and b or c and d:  # RUF021: => `(and b) or (c and d)`
+   |       ^^^^^^^ RUF021
+30 |     pass
+   |
+
+RUF021.py:29:18: RUF021 Parenthesize `a and b` expressions when chaining `and` and `or` together, to make the precedence clear
+   |
+28 | a, b, c, d = 0, 1, 0, 2
+29 | while a and b or c and d:  # RUF021: => `(and b) or (c and d)`
+   |                  ^^^^^^^ RUF021
+30 |     pass
+   |
+
+RUF021.py:33:44: RUF021 Parenthesize `a and b` expressions when chaining `and` and `or` together, to make the precedence clear
+   |
+32 | b, c, d, e = 2, 3, 0, 4
+33 | z = [a for a in range(5) if a or b or c or d and e]  # RUF021: => `a or b or c or (d and e)`
+   |                                            ^^^^^^^ RUF021
+34 | 
+35 | a, b, c, d = 0, 1, 3, 0
+   |
+
+RUF021.py:36:8: RUF021 Parenthesize `a and b` expressions when chaining `and` and `or` together, to make the precedence clear
+   |
+35 | a, b, c, d = 0, 1, 3, 0
+36 | assert not a and b or c or d  # RUF021: => `(not a and b) or c or d`
+   |        ^^^^^^^^^^^ RUF021
+37 | 
+38 | if (not a) and b or c or d:  # RUF021: => `((not a) and b) or c or d`
+   |
+
+RUF021.py:38:4: RUF021 Parenthesize `a and b` expressions when chaining `and` and `or` together, to make the precedence clear
+   |
+36 | assert not a and b or c or d  # RUF021: => `(not a and b) or c or d`
+37 | 
+38 | if (not a) and b or c or d:  # RUF021: => `((not a) and b) or c or d`
+   |    ^^^^^^^^^^^^^ RUF021
+39 |     if (not a and b) or c or d:  # OK
+40 |         pass
+   |
+
+