From fe38597279b0747a62059552e85520d0b64a88cf Mon Sep 17 00:00:00 2001
From: Jonathan Plasse <13716151+JonathanPlasse@users.noreply.github.com>
Date: Fri, 31 Mar 2023 20:50:35 +0200
Subject: [PATCH] Fix `SIM222` and `SIM223` false positive (#3832)

---
 .../test/fixtures/flake8_simplify/SIM222.py   | 13 +++++++++++++
 .../test/fixtures/flake8_simplify/SIM223.py   | 19 ++++++++++++++++---
 .../flake8_simplify/rules/ast_bool_op.rs      | 13 ++++++++++---
 3 files changed, 39 insertions(+), 6 deletions(-)

diff --git a/crates/ruff/resources/test/fixtures/flake8_simplify/SIM222.py b/crates/ruff/resources/test/fixtures/flake8_simplify/SIM222.py
index 64faef9126..826b3f01b8 100644
--- a/crates/ruff/resources/test/fixtures/flake8_simplify/SIM222.py
+++ b/crates/ruff/resources/test/fixtures/flake8_simplify/SIM222.py
@@ -29,3 +29,16 @@ if True or f() or a or g() or b:  # SIM222
 
 if a or True or f() or b or g():  # SIM222
     pass
+
+
+if a and f() and b and g() and False:  # OK
+    pass
+
+if a and f() and False and g() and b:  # OK
+    pass
+
+if False and f() and a and g() and b:  # OK
+    pass
+
+if a and False and f() and b and g():  # OK
+    pass
diff --git a/crates/ruff/resources/test/fixtures/flake8_simplify/SIM223.py b/crates/ruff/resources/test/fixtures/flake8_simplify/SIM223.py
index 2ea8d71dfc..80ab9d84a6 100644
--- a/crates/ruff/resources/test/fixtures/flake8_simplify/SIM223.py
+++ b/crates/ruff/resources/test/fixtures/flake8_simplify/SIM223.py
@@ -16,11 +16,24 @@ if False:
 if a and f() and b and g() and False:  # OK
     pass
 
-if a and f() and False and g() and b:  # SIM222
+if a and f() and False and g() and b:  # SIM223
     pass
 
-if False and f() and a and g() and b:  # SIM222
+if False and f() and a and g() and b:  # SIM223
     pass
 
-if a and False and f() and b and g():  # SIM222
+if a and False and f() and b and g():  # SIM223
+    pass
+
+
+if a or f() or b or g() or True:  # OK
+    pass
+
+if a or f() or True or g() or b:  # OK
+    pass
+
+if True or f() or a or g() or b:  # OK
+    pass
+
+if a or True or f() or b or g():  # OK
     pass
diff --git a/crates/ruff/src/rules/flake8_simplify/rules/ast_bool_op.rs b/crates/ruff/src/rules/flake8_simplify/rules/ast_bool_op.rs
index 8b6c8da097..4341d6508b 100644
--- a/crates/ruff/src/rules/flake8_simplify/rules/ast_bool_op.rs
+++ b/crates/ruff/src/rules/flake8_simplify/rules/ast_bool_op.rs
@@ -503,10 +503,17 @@ pub fn expr_or_not_expr(checker: &mut Checker, expr: &Expr) {
     }
 }
 
-pub fn is_short_circuit(ctx: &Context, expr: &Expr) -> Option<(Location, Location)> {
+pub fn is_short_circuit(
+    ctx: &Context,
+    expr: &Expr,
+    expected_op: &Boolop,
+) -> Option<(Location, Location)> {
     let ExprKind::BoolOp { op, values, } = &expr.node else {
         return None;
     };
+    if op != expected_op {
+        return None;
+    }
     let short_circuit_value = match op {
         Boolop::And => false,
         Boolop::Or => true,
@@ -551,7 +558,7 @@ pub fn is_short_circuit(ctx: &Context, expr: &Expr) -> Option<(Location, Locatio
 
 /// SIM222
 pub fn expr_or_true(checker: &mut Checker, expr: &Expr) {
-    let Some((location, end_location)) = is_short_circuit(&checker.ctx, expr) else {
+    let Some((location, end_location)) = is_short_circuit(&checker.ctx, expr, &Boolop::Or) else {
         return;
     };
     let mut diagnostic = Diagnostic::new(
@@ -573,7 +580,7 @@ pub fn expr_or_true(checker: &mut Checker, expr: &Expr) {
 
 /// SIM223
 pub fn expr_and_false(checker: &mut Checker, expr: &Expr) {
-    let Some((location, end_location)) = is_short_circuit(&checker.ctx, expr) else {
+    let Some((location, end_location)) = is_short_circuit(&checker.ctx, expr, &Boolop::And) else {
         return;
     };
     let mut diagnostic = Diagnostic::new(