Removed False-positives (#118)

Some rules around comparisons were removed. They had false-positives which I can probably never fix:

* SIM204
* SIM205
* SIM206
* SIM207

Closes #116

Those rules were moved to https://github.com/MartinThoma/flake8-scream

Rule SIM113 was adjusted. Closes #39

README.md

@@ -57,10 +57,10 @@ Simplifying Comparations:
 * `SIM201`: Use 'a != b' instead of 'not a == b' ([example](#SIM201))
 * `SIM202`: Use 'a == b' instead of 'not a != b' ([example](#SIM202))
 * `SIM203`: Use 'a not in b' instead of 'not (a in b)' ([example](#SIM203))
-* `SIM204`: Use 'a >= b' instead of 'not (a < b)' ([example](#SIM204))
-* `SIM205`: Use 'a > b' instead of 'not (a <= b)' ([example](#SIM205))
-* `SIM206`: Use 'a <= b' instead of 'not (a > b)' ([example](#SIM206))
-* `SIM207`: Use 'a < b' instead of 'not (a <= b)' ([example](#SIM207))
+* `SIM204`: Moved to [flake8-scream](https://github.com/MartinThoma/flake8-scream) due to [issue 116](https://github.com/MartinThoma/flake8-simplify/issues/116)
+* `SIM205`: Moved to [flake8-scream](https://github.com/MartinThoma/flake8-scream) due to [issue 116](https://github.com/MartinThoma/flake8-simplify/issues/116)
+* `SIM206`: Moved to [flake8-scream](https://github.com/MartinThoma/flake8-scream) due to [issue 116](https://github.com/MartinThoma/flake8-simplify/issues/116)
+* `SIM207`: Moved to [flake8-scream](https://github.com/MartinThoma/flake8-scream) due to [issue 116](https://github.com/MartinThoma/flake8-simplify/issues/116)
 * `SIM208`: Use 'a' instead of 'not (not a)' ([example](#SIM208))
 * `SIM210`: Use 'bool(a)' instead of 'True if a else False' ([example](#SIM210))
 * `SIM211`: Use 'not a' instead of 'False if a else True' ([example](#SIM211))
@@ -394,45 +394,6 @@ not a in b
 a not in b
 ```
 
-### SIM204
-
-```python
-# Bad
-not a < b
-
-# Good
-a >= b
-```
-
-### SIM205
-
-```python
-# Bad
-not a <= b
-
-# Good
-a > b
-```
-
-### SIM206
-
-```python
-# Bad
-not a > b
-
-# Good
-a <= b
-```
-
-### SIM207
-
-```python
-# Bad
-not a >= b
-
-# Good
-a < b
-```
 
 ### SIM208
 

flake8_simplify/__init__.py

@@ -42,14 +42,10 @@
     get_sim201,
     get_sim202,
     get_sim203,
-    get_sim204,
-    get_sim205,
-    get_sim206,
-    get_sim207,
     get_sim208,
 )
 from flake8_simplify.rules.ast_with import get_sim117
-from flake8_simplify.utils import Call, UnaryOp
+from flake8_simplify.utils import Call, For, UnaryOp
 
 logger = logging.getLogger(__name__)
 
@@ -106,7 +102,7 @@ def visit_If(self, node: ast.If) -> None:
     def visit_For(self, node: ast.For) -> None:
         self.errors += get_sim104(node)
         self.errors += get_sim110_sim111(node)
-        self.errors += get_sim113(node)
+        self.errors += get_sim113(For(node))
         self.generic_visit(node)
 
     def visit_Try(self, node: ast.Try) -> None:
@@ -119,10 +115,6 @@ def visit_UnaryOp(self, node_v: ast.UnaryOp) -> None:
         self.errors += get_sim201(node)
         self.errors += get_sim202(node)
         self.errors += get_sim203(node)
-        self.errors += get_sim204(node)
-        self.errors += get_sim205(node)
-        self.errors += get_sim206(node)
-        self.errors += get_sim207(node)
         self.errors += get_sim208(node)
         self.generic_visit(node)
 

flake8_simplify/rules/ast_for.py

@@ -5,6 +5,7 @@
 # First party
 from flake8_simplify.constants import BOOL_CONST_TYPES
 from flake8_simplify.utils import (
+    For,
     body_contains_continue,
     is_constant_increase,
     to_source,
@@ -129,7 +130,7 @@ def get_sim110_sim111(node: ast.For) -> List[Tuple[int, int, str]]:
     return errors
 
 
-def get_sim113(node: ast.For) -> List[Tuple[int, int, str]]:
+def get_sim113(node: For) -> List[Tuple[int, int, str]]:
     """
     Find loops in which "enumerate" should be used.
 
@@ -182,13 +183,17 @@ def get_sim113(node: ast.For) -> List[Tuple[int, int, str]]:
     if len(matches) == 0:
         return errors
 
+    sibling = node.previous_sibling
+    while sibling is not None:
+        sibling = sibling.previous_sibling
+
     for match in matches:
         variable = to_source(match)
         errors.append(
             (
                 match.lineno,
                 match.col_offset,
-                f"SIM113 Use enumerate instead of '{variable}'",
+                f"SIM113 Use enumerate for '{variable}'",
             )
         )
     return errors

flake8_simplify/rules/ast_unary_op.py

@@ -91,102 +91,6 @@ def get_sim203(node: UnaryOp) -> List[Tuple[int, int, str]]:
     return errors
 
 
-def get_sim204(node: UnaryOp) -> List[Tuple[int, int, str]]:
-    """Get a list of all calls of the type "not (a < b)"."""
-    SIM204 = "SIM204 Use '{a} >= {b}' instead of 'not ({a} < {b})'"
-    errors: List[Tuple[int, int, str]] = []
-    if (
-        (
-            not isinstance(node.op, ast.Not)
-            or not isinstance(node.operand, ast.Compare)
-            or len(node.operand.ops) != 1
-            or not isinstance(node.operand.ops[0], ast.Lt)
-        )
-        or isinstance(node.parent, ast.If)
-        and is_exception_check(node.parent)
-    ):
-        return errors
-    comparison = node.operand
-    left = to_source(comparison.left)
-    right = to_source(comparison.comparators[0])
-    errors.append(
-        (node.lineno, node.col_offset, SIM204.format(a=left, b=right))
-    )
-    return errors
-
-
-def get_sim205(node: UnaryOp) -> List[Tuple[int, int, str]]:
-    """Get a list of all calls of the type "not (a <= b)"."""
-    SIM205 = "SIM205 Use '{a} > {b}' instead of 'not ({a} <= {b})'"
-    errors: List[Tuple[int, int, str]] = []
-    if (
-        (
-            not isinstance(node.op, ast.Not)
-            or not isinstance(node.operand, ast.Compare)
-            or len(node.operand.ops) != 1
-            or not isinstance(node.operand.ops[0], ast.LtE)
-        )
-        or isinstance(node.parent, ast.If)
-        and is_exception_check(node.parent)
-    ):
-        return errors
-    comparison = node.operand
-    left = to_source(comparison.left)
-    right = to_source(comparison.comparators[0])
-    errors.append(
-        (node.lineno, node.col_offset, SIM205.format(a=left, b=right))
-    )
-    return errors
-
-
-def get_sim206(node: UnaryOp) -> List[Tuple[int, int, str]]:
-    """Get a list of all calls of the type "not (a > b)"."""
-    SIM206 = "SIM206 Use '{a} <= {b}' instead of 'not ({a} > {b})'"
-    errors: List[Tuple[int, int, str]] = []
-    if (
-        (
-            not isinstance(node.op, ast.Not)
-            or not isinstance(node.operand, ast.Compare)
-            or len(node.operand.ops) != 1
-            or not isinstance(node.operand.ops[0], ast.Gt)
-        )
-        or isinstance(node.parent, ast.If)
-        and is_exception_check(node.parent)
-    ):
-        return errors
-    comparison = node.operand
-    left = to_source(comparison.left)
-    right = to_source(comparison.comparators[0])
-    errors.append(
-        (node.lineno, node.col_offset, SIM206.format(a=left, b=right))
-    )
-    return errors
-
-
-def get_sim207(node: UnaryOp) -> List[Tuple[int, int, str]]:
-    """Get a list of all calls of the type "not (a >= b)"."""
-    SIM207 = "SIM207 Use '{a} < {b}' instead of 'not ({a} >= {b})'"
-    errors: List[Tuple[int, int, str]] = []
-    if (
-        (
-            not isinstance(node.op, ast.Not)
-            or not isinstance(node.operand, ast.Compare)
-            or len(node.operand.ops) != 1
-            or not isinstance(node.operand.ops[0], ast.GtE)
-        )
-        or isinstance(node.parent, ast.If)
-        and is_exception_check(node.parent)
-    ):
-        return errors
-    comparison = node.operand
-    left = to_source(comparison.left)
-    right = to_source(comparison.comparators[0])
-    errors.append(
-        (node.lineno, node.col_offset, SIM207.format(a=left, b=right))
-    )
-    return errors
-
-
 def get_sim208(node: ast.UnaryOp) -> List[Tuple[int, int, str]]:
     """Get a list of all calls of the type "not (not a)"."""
     SIM208 = "SIM208 Use '{a}' instead of 'not (not {a})'"

flake8_simplify/utils.py

@@ -21,16 +21,37 @@ def __init__(self, orig: ast.UnaryOp) -> None:
 
 
 class Call(ast.Call):
+    """For mypy so that it knows that added attributes exist."""
+
     def __init__(self, orig: ast.Call) -> None:
         self.func = orig.func
         self.args = orig.args
         self.keywords = orig.keywords
         # For all ast.*:
         self.lineno = orig.lineno
         self.col_offset = orig.col_offset
+
+        # Added attributes
         self.parent: ast.Expr = orig.parent  # type: ignore
 
 
+class For(ast.For):
+    """For mypy so that it knows that added attributes exist."""
+
+    def __init__(self, orig: ast.For) -> None:
+        self.target = orig.target
+        self.iter = orig.iter
+        self.body = orig.body
+        self.orelse = orig.orelse
+        # For all ast.*:
+        self.lineno = orig.lineno
+        self.col_offset = orig.col_offset
+
+        # Added attributes
+        self.parent: ast.AST = orig.parent  # type: ignore
+        self.previous_sibling = orig.previous_sibling  # type: ignore
+
+
 def to_source(
     node: Union[None, ast.expr, ast.Expr, ast.withitem, ast.slice]
 ) -> str:

tests/test_100_rules.py

@@ -288,7 +288,17 @@ def test_sim113():
 for el in iterable:
     idx += 1"""
     )
-    assert ret == {"2:0 SIM113 Use enumerate instead of 'idx'"}
+    assert ret == {"2:0 SIM113 Use enumerate for 'idx'"}
+
+
+def test_sim113_false_positive2():
+    ret = _results(
+        """nb_points = 0
+for line in lines:
+    for point in line:
+        nb_points += 1"""
+    )
+    assert ret == set()
 
 
 @pytest.mark.parametrize(

tests/test_200_rules.py

@@ -25,26 +25,6 @@ def test_sim203_base():
     assert ret == {("1:0 SIM203 Use 'a not in b' instead of 'not a in b'")}
 
 
-def test_sim204_base():
-    ret = _results("not a < b")
-    assert ret == {("1:0 SIM204 Use 'a >= b' instead of 'not (a < b)'")}
-
-
-def test_sim205_base():
-    ret = _results("not a <= b")
-    assert ret == {("1:0 SIM205 Use 'a > b' instead of 'not (a <= b)'")}
-
-
-def test_sim206_base():
-    ret = _results("not a > b")
-    assert ret == {("1:0 SIM206 Use 'a <= b' instead of 'not (a > b)'")}
-
-
-def test_sim207_base():
-    ret = _results("not a >= b")
-    assert ret == {("1:0 SIM207 Use 'a < b' instead of 'not (a >= b)'")}
-
-
 def test_sim208_base():
     ret = _results("not (not a)")
     assert ret == {("1:0 SIM208 Use 'a' instead of 'not (not a)'")}