Implement SIM220 and SIM221 (#1630)

README.md

@@ -960,6 +960,8 @@ For more, see [flake8-simplify](https://pypi.org/project/flake8-simplify/0.19.3/
 
 | Code | Name | Message | Fix |
 | ---- | ---- | ------- | --- |
+| SIM220 | AAndNotA | Use `False` instead of `... and not ...` | 🛠 |
+| SIM221 | AOrNotA | Use `True` instead of `... or not ...` | 🛠 |
 | SIM118 | KeyInDict | Use `key in dict` instead of `key in dict.keys()` | 🛠 |
 | SIM222 | OrTrue | Use `True` instead of `... or True` | 🛠 |
 | SIM223 | AndFalse | Use `False` instead of `... and False` | 🛠 |
@@ -1352,9 +1354,9 @@ natively, including:
 - [`flake8-print`](https://pypi.org/project/flake8-print/)
 - [`flake8-quotes`](https://pypi.org/project/flake8-quotes/)
 - [`flake8-return`](https://pypi.org/project/flake8-return/)
-- [`flake8-simplify`](https://pypi.org/project/flake8-simplify/) (4/37)
+- [`flake8-simplify`](https://pypi.org/project/flake8-simplify/) (6/30)
 - [`flake8-super`](https://pypi.org/project/flake8-super/)
-- [`flake8-tidy-imports`](https://pypi.org/project/flake8-tidy-imports/) (1/3)
+- [`flake8-tidy-imports`](https://pypi.org/project/flake8-tidy-imports/)
 - [`isort`](https://pypi.org/project/isort/)
 - [`mccabe`](https://pypi.org/project/mccabe/)
 - [`pep8-naming`](https://pypi.org/project/pep8-naming/)
@@ -1410,9 +1412,9 @@ Today, Ruff can be used to replace Flake8 when used with any of the following pl
 - [`flake8-print`](https://pypi.org/project/flake8-print/)
 - [`flake8-quotes`](https://pypi.org/project/flake8-quotes/)
 - [`flake8-return`](https://pypi.org/project/flake8-return/)
-- [`flake8-simplify`](https://pypi.org/project/flake8-simplify/) (4/37)
+- [`flake8-simplify`](https://pypi.org/project/flake8-simplify/) (6/30)
 - [`flake8-super`](https://pypi.org/project/flake8-super/)
-- [`flake8-tidy-imports`](https://pypi.org/project/flake8-tidy-imports/) (1/3)
+- [`flake8-tidy-imports`](https://pypi.org/project/flake8-tidy-imports/)
 - [`mccabe`](https://pypi.org/project/mccabe/)
 - [`pep8-naming`](https://pypi.org/project/pep8-naming/)
 - [`pydocstyle`](https://pypi.org/project/pydocstyle/)

resources/test/fixtures/flake8_simplify/SIM220.py

@@ -0,0 +1,17 @@
+if a and not a:
+    pass
+
+if (a and not a) and b:
+    pass
+
+if (a and not a) or b:
+    pass
+
+if a:
+    pass
+
+if not a:
+    pass
+
+if a and not b:
+    pass

resources/test/fixtures/flake8_simplify/SIM221.py

@@ -0,0 +1,17 @@
+if a or not a:
+    pass
+
+if (a or not a) or b:
+    pass
+
+if (a or not a) and b:
+    pass
+
+if a:
+    pass
+
+if not a:
+    pass
+
+if a or not b:
+    pass

ruff.schema.json

@@ -899,6 +899,8 @@
         "SIM118",
         "SIM2",
         "SIM22",
+        "SIM220",
+        "SIM221",
         "SIM222",
         "SIM223",
         "SIM3",

src/checkers/ast.rs

@@ -2578,6 +2578,12 @@ where
                 if self.settings.enabled.contains(&CheckCode::PLR1701) {
                     pylint::plugins::merge_isinstance(self, expr, op, values);
                 }
+                if self.settings.enabled.contains(&CheckCode::SIM220) {
+                    flake8_simplify::plugins::a_and_not_a(self, expr);
+                }
+                if self.settings.enabled.contains(&CheckCode::SIM221) {
+                    flake8_simplify::plugins::a_or_not_a(self, expr);
+                }
                 if self.settings.enabled.contains(&CheckCode::SIM222) {
                     flake8_simplify::plugins::or_true(self, expr);
                 }

src/flake8_simplify/mod.rs

@@ -16,6 +16,8 @@ mod tests {
     #[test_case(CheckCode::SIM222, Path::new("SIM222.py"); "SIM222")]
     #[test_case(CheckCode::SIM223, Path::new("SIM223.py"); "SIM223")]
     #[test_case(CheckCode::SIM300, Path::new("SIM300.py"); "SIM300")]
+    #[test_case(CheckCode::SIM221, Path::new("SIM221.py"); "SIM221")]
+    #[test_case(CheckCode::SIM220, Path::new("SIM220.py"); "SIM220")]
     fn checks(check_code: CheckCode, path: &Path) -> Result<()> {
         let snapshot = format!("{}_{}", check_code.as_ref(), path.to_string_lossy());
         let checks = test_path(

src/flake8_simplify/plugins/bool_ops.rs

@@ -1,10 +1,115 @@
-use rustpython_ast::{Boolop, Constant, Expr, ExprKind};
+use rustpython_ast::{Boolop, Constant, Expr, ExprKind, Unaryop};
 
 use crate::ast::types::Range;
 use crate::autofix::Fix;
 use crate::checkers::ast::Checker;
 use crate::registry::{Check, CheckCode, CheckKind};
 
+fn is_same_expr<'a>(a: &'a Expr, b: &'a Expr) -> Option<&'a str> {
+    if let (ExprKind::Name { id: a, .. }, ExprKind::Name { id: b, .. }) = (&a.node, &b.node) {
+        if a == b {
+            return Some(a);
+        }
+    }
+    None
+}
+
+/// SIM220
+pub fn a_and_not_a(checker: &mut Checker, expr: &Expr) {
+    let ExprKind::BoolOp { op: Boolop::And, values, } = &expr.node else {
+        return;
+    };
+    if values.len() < 2 {
+        return;
+    }
+
+    // Collect all negated and non-negated expressions.
+    let mut negated_expr = vec![];
+    let mut non_negated_expr = vec![];
+    for expr in values {
+        if let ExprKind::UnaryOp {
+            op: Unaryop::Not,
+            operand,
+        } = &expr.node
+        {
+            negated_expr.push(operand);
+        } else {
+            non_negated_expr.push(expr);
+        }
+    }
+
+    if negated_expr.is_empty() {
+        return;
+    }
+
+    for negate_expr in negated_expr {
+        for non_negate_expr in &non_negated_expr {
+            if let Some(id) = is_same_expr(negate_expr, non_negate_expr) {
+                let mut check = Check::new(
+                    CheckKind::AAndNotA(id.to_string()),
+                    Range::from_located(expr),
+                );
+                if checker.patch(&CheckCode::SIM220) {
+                    check.amend(Fix::replacement(
+                        "False".to_string(),
+                        expr.location,
+                        expr.end_location.unwrap(),
+                    ));
+                }
+                checker.add_check(check);
+            }
+        }
+    }
+}
+
+/// SIM221
+pub fn a_or_not_a(checker: &mut Checker, expr: &Expr) {
+    let ExprKind::BoolOp { op: Boolop::Or, values, } = &expr.node else {
+        return;
+    };
+    if values.len() < 2 {
+        return;
+    }
+
+    // Collect all negated and non-negated expressions.
+    let mut negated_expr = vec![];
+    let mut non_negated_expr = vec![];
+    for expr in values {
+        if let ExprKind::UnaryOp {
+            op: Unaryop::Not,
+            operand,
+        } = &expr.node
+        {
+            negated_expr.push(operand);
+        } else {
+            non_negated_expr.push(expr);
+        }
+    }
+
+    if negated_expr.is_empty() {
+        return;
+    }
+
+    for negate_expr in negated_expr {
+        for non_negate_expr in &non_negated_expr {
+            if let Some(id) = is_same_expr(negate_expr, non_negate_expr) {
+                let mut check = Check::new(
+                    CheckKind::AOrNotA(id.to_string()),
+                    Range::from_located(expr),
+                );
+                if checker.patch(&CheckCode::SIM220) {
+                    check.amend(Fix::replacement(
+                        "True".to_string(),
+                        expr.location,
+                        expr.end_location.unwrap(),
+                    ));
+                }
+                checker.add_check(check);
+            }
+        }
+    }
+}
+
 /// SIM222
 pub fn or_true(checker: &mut Checker, expr: &Expr) {
     let ExprKind::BoolOp { op: Boolop::Or, values, } = &expr.node else {

src/flake8_simplify/plugins/mod.rs

@@ -1,4 +1,4 @@
-pub use bool_ops::{and_false, or_true};
+pub use bool_ops::{a_and_not_a, a_or_not_a, and_false, or_true};
 pub use key_in_dict::{key_in_dict_compare, key_in_dict_for};
 pub use yoda_conditions::yoda_conditions;
 

src/flake8_simplify/snapshots/ruff__flake8_simplify__tests__SIM220_SIM220.py.snap

@@ -0,0 +1,56 @@
+---
+source: src/flake8_simplify/mod.rs
+expression: checks
+---
+- kind:
+    AAndNotA: a
+  location:
+    row: 1
+    column: 3
+  end_location:
+    row: 1
+    column: 14
+  fix:
+    content: "False"
+    location:
+      row: 1
+      column: 3
+    end_location:
+      row: 1
+      column: 14
+  parent: ~
+- kind:
+    AAndNotA: a
+  location:
+    row: 4
+    column: 4
+  end_location:
+    row: 4
+    column: 15
+  fix:
+    content: "False"
+    location:
+      row: 4
+      column: 4
+    end_location:
+      row: 4
+      column: 15
+  parent: ~
+- kind:
+    AAndNotA: a
+  location:
+    row: 7
+    column: 4
+  end_location:
+    row: 7
+    column: 15
+  fix:
+    content: "False"
+    location:
+      row: 7
+      column: 4
+    end_location:
+      row: 7
+      column: 15
+  parent: ~
+

src/flake8_simplify/snapshots/ruff__flake8_simplify__tests__SIM221_SIM221.py.snap

@@ -0,0 +1,35 @@
+---
+source: src/flake8_simplify/mod.rs
+expression: checks
+---
+- kind:
+    AOrNotA: a
+  location:
+    row: 1
+    column: 3
+  end_location:
+    row: 1
+    column: 13
+  fix: ~
+  parent: ~
+- kind:
+    AOrNotA: a
+  location:
+    row: 4
+    column: 4
+  end_location:
+    row: 4
+    column: 14
+  fix: ~
+  parent: ~
+- kind:
+    AOrNotA: a
+  location:
+    row: 7
+    column: 4
+  end_location:
+    row: 7
+    column: 14
+  fix: ~
+  parent: ~
+

src/registry.rs

@@ -217,6 +217,8 @@ pub enum CheckCode {
     YTT302,
     YTT303,
     // flake8-simplify
+    SIM220,
+    SIM221,
     SIM118,
     SIM222,
     SIM223,
@@ -945,6 +947,8 @@ pub enum CheckKind {
     SysVersionCmpStr10,
     SysVersionSlice1Referenced,
     // flake8-simplify
+    AAndNotA(String),
+    AOrNotA(String),
     KeyInDict(String, String),
     AndFalse,
     OrTrue,
@@ -1374,6 +1378,8 @@ impl CheckCode {
             CheckCode::BLE001 => CheckKind::BlindExcept("Exception".to_string()),
             // flake8-simplify
             CheckCode::SIM118 => CheckKind::KeyInDict("key".to_string(), "dict".to_string()),
+            CheckCode::SIM220 => CheckKind::AAndNotA("...".to_string()),
+            CheckCode::SIM221 => CheckKind::AOrNotA("...".to_string()),
             CheckCode::SIM222 => CheckKind::OrTrue,
             CheckCode::SIM223 => CheckKind::AndFalse,
             CheckCode::SIM300 => CheckKind::YodaConditions("left".to_string(), "right".to_string()),
@@ -1898,6 +1904,8 @@ impl CheckCode {
             CheckCode::S107 => CheckCategory::Flake8Bandit,
             // flake8-simplify
             CheckCode::SIM118 => CheckCategory::Flake8Simplify,
+            CheckCode::SIM220 => CheckCategory::Flake8Simplify,
+            CheckCode::SIM221 => CheckCategory::Flake8Simplify,
             CheckCode::SIM222 => CheckCategory::Flake8Simplify,
             CheckCode::SIM223 => CheckCategory::Flake8Simplify,
             CheckCode::SIM300 => CheckCategory::Flake8Simplify,
@@ -2147,6 +2155,8 @@ impl CheckKind {
             CheckKind::SysVersionSlice1Referenced => &CheckCode::YTT303,
             // flake8-simplify
             CheckKind::KeyInDict(..) => &CheckCode::SIM118,
+            CheckKind::AAndNotA(..) => &CheckCode::SIM220,
+            CheckKind::AOrNotA(..) => &CheckCode::SIM221,
             CheckKind::OrTrue => &CheckCode::SIM222,
             CheckKind::AndFalse => &CheckCode::SIM223,
             CheckKind::YodaConditions(..) => &CheckCode::SIM300,
@@ -2904,6 +2914,8 @@ impl CheckKind {
             CheckKind::KeyInDict(key, dict) => {
                 format!("Use `{key} in {dict}` instead of `{key} in {dict}.keys()`")
             }
+            CheckKind::AAndNotA(name) => format!("Use `False` instead of `{name} and not {name}`"),
+            CheckKind::AOrNotA(name) => format!("Use `True` instead of `{name} or not {name}`"),
             CheckKind::OrTrue => "Use `True` instead of `... or True`".to_string(),
             CheckKind::AndFalse => "Use `False` instead of `... and False`".to_string(),
             CheckKind::YodaConditions(left, right) => {
@@ -3495,9 +3507,11 @@ impl CheckKind {
     pub fn fixable(&self) -> bool {
         matches!(
             self,
-            CheckKind::AmbiguousUnicodeCharacterString(..)
+            CheckKind::AAndNotA(..)
+                | CheckKind::AOrNotA(..)
                 | CheckKind::AmbiguousUnicodeCharacterComment(..)
                 | CheckKind::AmbiguousUnicodeCharacterDocstring(..)
+                | CheckKind::AmbiguousUnicodeCharacterString(..)
                 | CheckKind::AndFalse
                 | CheckKind::BlankLineAfterLastSection(..)
                 | CheckKind::BlankLineAfterSection(..)
@@ -3611,6 +3625,8 @@ impl CheckKind {
     /// The message used to describe the fix action for a given `CheckKind`.
     pub fn commit(&self) -> Option<String> {
         match self {
+            CheckKind::AAndNotA(..) => Some("Replace with `False`".to_string()),
+            CheckKind::AOrNotA(..) => Some("Replace with `True`".to_string()),
             CheckKind::AmbiguousUnicodeCharacterString(confusable, representant)
             | CheckKind::AmbiguousUnicodeCharacterDocstring(confusable, representant)
             | CheckKind::AmbiguousUnicodeCharacterComment(confusable, representant) => {