Simplify SIM201, SIM202, SIM208 (#1666)

Flake8 simplify #998 

SIM201, SIM202 and SIM208 is done here with fixes.

Note: SIM203 == E713 

Co-authored-by: Charlie Marsh <[email protected]>

README.md

@@ -978,6 +978,9 @@ For more, see [flake8-simplify](https://pypi.org/project/flake8-simplify/0.19.3/
 | SIM111 | ConvertLoopToAll | Use `return all(x for x in y)` instead of `for` loop | 🛠 |
 | SIM117 | MultipleWithStatements | Use a single `with` statement with multiple contexts instead of nested `with` statements |  |
 | SIM118 | KeyInDict | Use `key in dict` instead of `key in dict.keys()` | 🛠 |
+| SIM201 | NegateEqualOp | Use `left != right` instead of `not left == right` | 🛠 |
+| SIM202 | NegateNotEqualOp | Use `left == right` instead of `not left != right` | 🛠 |
+| SIM208 | DoubleNegation | Use `expr` instead of `not (not expr)` | 🛠 |
 | SIM220 | AAndNotA | Use `False` instead of `... and not ...` | 🛠 |
 | SIM221 | AOrNotA | Use `True` instead of `... or not ...` | 🛠 |
 | SIM222 | OrTrue | Use `True` instead of `... or True` | 🛠 |

resources/test/fixtures/flake8_simplify/SIM201.py

@@ -0,0 +1,17 @@
+if not a == b:  # SIM201
+    pass
+
+if not a == (b + c):  # SIM201
+    pass
+
+if not (a + b) == c:  # SIM201
+    pass
+
+if not a != b:  # OK
+    pass
+
+if a == b:  # OK
+    pass
+
+if not a == b:  # OK
+    raise ValueError()

resources/test/fixtures/flake8_simplify/SIM202.py

@@ -0,0 +1,14 @@
+if not a != b:  # SIM202
+    pass
+
+if not a != (b + c):  # SIM202
+    pass
+
+if not (a + b) != c:  # SIM202
+    pass
+
+if not a == b:  # OK
+    pass
+
+if a != b:  # OK
+    pass

resources/test/fixtures/flake8_simplify/SIM208.py

@@ -0,0 +1,14 @@
+if not (not a):  # SIM208
+    pass
+
+if not (not (a == b)):  # SIM208
+    pass
+
+if not a:  # OK
+    pass
+
+if not a == b:  # OK
+    pass
+
+if not a != b:  # OK
+    pass

ruff.schema.json

@@ -950,6 +950,10 @@
         "SIM117",
         "SIM118",
         "SIM2",
+        "SIM20",
+        "SIM201",
+        "SIM202",
+        "SIM208",
         "SIM22",
         "SIM220",
         "SIM221",

src/checkers/ast.rs

@@ -2548,6 +2548,16 @@ where
                 if self.settings.enabled.contains(&CheckCode::B002) {
                     flake8_bugbear::plugins::unary_prefix_increment(self, expr, op, operand);
                 }
+
+                if self.settings.enabled.contains(&CheckCode::SIM201) {
+                    flake8_simplify::plugins::negation_with_equal_op(self, expr, op, operand);
+                }
+                if self.settings.enabled.contains(&CheckCode::SIM202) {
+                    flake8_simplify::plugins::negation_with_not_equal_op(self, expr, op, operand);
+                }
+                if self.settings.enabled.contains(&CheckCode::SIM208) {
+                    flake8_simplify::plugins::double_negation(self, expr, op, operand);
+                }
             }
             ExprKind::Compare {
                 left,

src/flake8_simplify/mod.rs

@@ -20,6 +20,9 @@ mod tests {
     #[test_case(CheckCode::SIM110, Path::new("SIM110.py"); "SIM110")]
     #[test_case(CheckCode::SIM111, Path::new("SIM111.py"); "SIM111")]
     #[test_case(CheckCode::SIM117, Path::new("SIM117.py"); "SIM117")]
+    #[test_case(CheckCode::SIM201, Path::new("SIM201.py"); "SIM201")]
+    #[test_case(CheckCode::SIM202, Path::new("SIM202.py"); "SIM202")]
+    #[test_case(CheckCode::SIM208, Path::new("SIM208.py"); "SIM208")]
     #[test_case(CheckCode::SIM118, Path::new("SIM118.py"); "SIM118")]
     #[test_case(CheckCode::SIM220, Path::new("SIM220.py"); "SIM220")]
     #[test_case(CheckCode::SIM221, Path::new("SIM221.py"); "SIM221")]

src/flake8_simplify/plugins/mod.rs

@@ -6,6 +6,7 @@ pub use ast_if::{nested_if_statements, use_ternary_operator};
 pub use ast_with::multiple_with_statements;
 pub use key_in_dict::{key_in_dict_compare, key_in_dict_for};
 pub use return_in_try_except_finally::return_in_try_except_finally;
+pub use unary_ops::{double_negation, negation_with_equal_op, negation_with_not_equal_op};
 pub use use_contextlib_suppress::use_contextlib_suppress;
 pub use yoda_conditions::yoda_conditions;
 
@@ -15,5 +16,6 @@ mod ast_if;
 mod ast_with;
 mod key_in_dict;
 mod return_in_try_except_finally;
+mod unary_ops;
 mod use_contextlib_suppress;
 mod yoda_conditions;

src/flake8_simplify/plugins/unary_ops.rs

@@ -0,0 +1,129 @@
+use rustpython_ast::{Cmpop, Expr, ExprKind, Stmt, StmtKind, Unaryop};
+
+use crate::ast::helpers::{create_expr, unparse_expr};
+use crate::ast::types::Range;
+use crate::autofix::Fix;
+use crate::checkers::ast::Checker;
+use crate::registry::{Check, CheckKind};
+
+fn is_exception_check(stmt: &Stmt) -> bool {
+    let StmtKind::If {test: _, body, orelse: _} = &stmt.node else {
+        return false;
+    };
+    if body.len() != 1 {
+        return false;
+    }
+    if matches!(body[0].node, StmtKind::Raise { .. }) {
+        return true;
+    }
+    false
+}
+
+/// SIM201
+pub fn negation_with_equal_op(checker: &mut Checker, expr: &Expr, op: &Unaryop, operand: &Expr) {
+    if !matches!(op, Unaryop::Not) {
+        return;
+    }
+    let ExprKind::Compare{ left, ops, comparators} = &operand.node else {
+        return;
+    };
+    if !matches!(&ops[..], [Cmpop::Eq]) {
+        return;
+    }
+    if is_exception_check(checker.current_stmt()) {
+        return;
+    }
+
+    let mut check = Check::new(
+        CheckKind::NegateEqualOp(
+            unparse_expr(left, checker.style),
+            unparse_expr(&comparators[0], checker.style),
+        ),
+        Range::from_located(operand),
+    );
+    if checker.patch(check.kind.code()) {
+        check.amend(Fix::replacement(
+            unparse_expr(
+                &create_expr(ExprKind::Compare {
+                    left: left.clone(),
+                    ops: vec![Cmpop::NotEq],
+                    comparators: comparators.clone(),
+                }),
+                checker.style,
+            ),
+            expr.location,
+            expr.end_location.unwrap(),
+        ));
+    }
+    checker.add_check(check);
+}
+
+/// SIM202
+pub fn negation_with_not_equal_op(
+    checker: &mut Checker,
+    expr: &Expr,
+    op: &Unaryop,
+    operand: &Expr,
+) {
+    if !matches!(op, Unaryop::Not) {
+        return;
+    }
+    let ExprKind::Compare{ left, ops, comparators} = &operand.node else {
+        return;
+    };
+    if !matches!(&ops[..], [Cmpop::NotEq]) {
+        return;
+    }
+    if is_exception_check(checker.current_stmt()) {
+        return;
+    }
+
+    let mut check = Check::new(
+        CheckKind::NegateNotEqualOp(
+            unparse_expr(left, checker.style),
+            unparse_expr(&comparators[0], checker.style),
+        ),
+        Range::from_located(operand),
+    );
+    if checker.patch(check.kind.code()) {
+        check.amend(Fix::replacement(
+            unparse_expr(
+                &create_expr(ExprKind::Compare {
+                    left: left.clone(),
+                    ops: vec![Cmpop::Eq],
+                    comparators: comparators.clone(),
+                }),
+                checker.style,
+            ),
+            expr.location,
+            expr.end_location.unwrap(),
+        ));
+    }
+    checker.add_check(check);
+}
+
+/// SIM208
+pub fn double_negation(checker: &mut Checker, expr: &Expr, op: &Unaryop, operand: &Expr) {
+    if !matches!(op, Unaryop::Not) {
+        return;
+    }
+    let ExprKind::UnaryOp { op: operand_op, operand } = &operand.node else {
+        return;
+    };
+    if !matches!(operand_op, Unaryop::Not) {
+        return;
+    }
+
+    let mut check = Check::new(
+        CheckKind::DoubleNegation(operand.to_string()),
+        Range::from_located(operand),
+    );
+    if checker.patch(check.kind.code()) {
+        check.amend(Fix::replacement(
+            unparse_expr(operand, checker.style),
+            expr.location,
+            expr.end_location.unwrap(),
+        ));
+    }
+    checker.add_check(check);
+}

src/flake8_simplify/snapshots/ruff__flake8_simplify__tests__SIM201_SIM201.py.snap

@@ -0,0 +1,62 @@
+---
+source: src/flake8_simplify/mod.rs
+expression: checks
+---
+- kind:
+    NegateEqualOp:
+      - a
+      - b
+  location:
+    row: 1
+    column: 7
+  end_location:
+    row: 1
+    column: 13
+  fix:
+    content: a != b
+    location:
+      row: 1
+      column: 3
+    end_location:
+      row: 1
+      column: 13
+  parent: ~
+- kind:
+    NegateEqualOp:
+      - a
+      - b + c
+  location:
+    row: 4
+    column: 7
+  end_location:
+    row: 4
+    column: 19
+  fix:
+    content: a != b + c
+    location:
+      row: 4
+      column: 3
+    end_location:
+      row: 4
+      column: 19
+  parent: ~
+- kind:
+    NegateEqualOp:
+      - a + b
+      - c
+  location:
+    row: 7
+    column: 7
+  end_location:
+    row: 7
+    column: 19
+  fix:
+    content: a + b != c
+    location:
+      row: 7
+      column: 3
+    end_location:
+      row: 7
+      column: 19
+  parent: ~
+

src/flake8_simplify/snapshots/ruff__flake8_simplify__tests__SIM201_SIM201_2.py.snap

@@ -0,0 +1,6 @@
+---
+source: src/flake8_simplify/mod.rs
+expression: checks
+---
+[]
+

src/flake8_simplify/snapshots/ruff__flake8_simplify__tests__SIM202_SIM202.py.snap

@@ -0,0 +1,62 @@
+---
+source: src/flake8_simplify/mod.rs
+expression: checks
+---
+- kind:
+    NegateNotEqualOp:
+      - a
+      - b
+  location:
+    row: 1
+    column: 7
+  end_location:
+    row: 1
+    column: 13
+  fix:
+    content: a == b
+    location:
+      row: 1
+      column: 3
+    end_location:
+      row: 1
+      column: 13
+  parent: ~
+- kind:
+    NegateNotEqualOp:
+      - a
+      - b + c
+  location:
+    row: 4
+    column: 7
+  end_location:
+    row: 4
+    column: 19
+  fix:
+    content: a == b + c
+    location:
+      row: 4
+      column: 3
+    end_location:
+      row: 4
+      column: 19
+  parent: ~
+- kind:
+    NegateNotEqualOp:
+      - a + b
+      - c
+  location:
+    row: 7
+    column: 7
+  end_location:
+    row: 7
+    column: 19
+  fix:
+    content: a + b == c
+    location:
+      row: 7
+      column: 3
+    end_location:
+      row: 7
+      column: 19
+  parent: ~
+