Repository: MartinThoma/flake8-simplify (python)

Issue:
[Adjust Rule] SIM113 False Positive in Nested Loops
## Desired change
SIM113 False Positive in nested loops

In this case, we could using enumerate is rather tricky. How do you suggest we best do it in nested loops like so?
* **Rule(s)**: SIM113
* **Adjustment**: In which way should it/they be adjusted?
The result should not be triggered in nested loops

## Explanation

Why is the adjustment necessary / better?
Because the enumerate logic actually becomes MORE complicated in this nested loop context. You need to have some complex multiplication to get the proper number and that may not even be possible if ```bars``` is a generator that has variable length. In which case, counting manually here is actually for the best.


## Example

This is an example where the mentioned rule(s) would currently be suboptimal:

```python
count = 0
for foo in foos:
    for bar in bars:
        count +=1
```
In this second one, I could se the argument for epochs (assuming there is not complex increment logic, but it complains about number of iterations)
```python
epoch, i = np.load(resume_file)
while epochs < TOTAL_EPOCHS:
    for b in batches:
        i+=1
```

Pull request:
SIM113: Remove false-positive in double-loop
Thank you, Skylion007!

Closes #25

Patch:
diff --git a/flake8_simplify/__init__.py b/flake8_simplify/__init__.py
index 3686a57..db11afc 100644
--- a/flake8_simplify/__init__.py
+++ b/flake8_simplify/__init__.py
@@ -165,7 +165,7 @@ def add_meta(root: ast.AST, level: int = 0) -> None:
     previous_sibling = None
     for node in ast.iter_child_nodes(root):
         if level == 0:
-            node.parent = None  # type: ignore
+            node.parent = root  # type: ignore
         node.previous_sibling = previous_sibling  # type: ignore
         node.next_sibling = None  # type: ignore
         if previous_sibling:
diff --git a/flake8_simplify/rules/ast_for.py b/flake8_simplify/rules/ast_for.py
index 36ea38b..a8151f7 100644
--- a/flake8_simplify/rules/ast_for.py
+++ b/flake8_simplify/rules/ast_for.py
@@ -140,7 +140,7 @@ def get_sim113(node: ast.For) -> List[Tuple[int, int, str]]:
                 Expr(
                     value=Constant(value=Ellipsis, kind=None),
                 ),
-                AugAssign(
+                AugAssign( -- agument and assign, aka "+= 1"
                     target=Name(id='idx', ctx=Store()),
                     op=Add(),
                     value=Constant(value=1, kind=None),
@@ -150,11 +150,12 @@ def get_sim113(node: ast.For) -> List[Tuple[int, int, str]]:
             type_comment=None,
         ),
     """
-    RULE = "SIM113 Use enumerate instead of '{variable}'"
     errors: List[Tuple[int, int, str]] = []
     variable_candidates = []
     if body_contains_continue(node.body):
         return errors
+
+    # Find variables that might just count the iteration of the current loop
     for expression in node.body:
         if (
             isinstance(expression, ast.AugAssign)
@@ -162,13 +163,32 @@ def get_sim113(node: ast.For) -> List[Tuple[int, int, str]]:
             and isinstance(expression.target, ast.Name)
         ):
             variable_candidates.append(expression.target)
+    str_candidates = [to_source(x) for x in variable_candidates]
+
+    older_siblings = []
+    for older_sibling in node.parent.body:  # type: ignore
+        if older_sibling is node:
+            break
+        older_siblings.append(older_sibling)
+
+    matches = [
+        n.targets[0]
+        for n in older_siblings
+        if isinstance(n, ast.Assign)
+        and len(n.targets) == 1
+        and isinstance(n.targets[0], ast.Name)
+        and to_source(n.targets[0]) in str_candidates
+    ]
+    if len(matches) == 0:
+        return errors
 
-    for candidate in variable_candidates:
+    for match in matches:
+        variable = to_source(match)
         errors.append(
             (
-                candidate.lineno,
-                candidate.col_offset,
-                RULE.format(variable=to_source(candidate)),
+                match.lineno,
+                match.col_offset,
+                f"SIM113 Use enumerate instead of '{variable}'",
             )
         )
     return errors


Test patch:
diff --git a/tests/test_100_rules.py b/tests/test_100_rules.py
index 30b2448..44ed391 100644
--- a/tests/test_100_rules.py
+++ b/tests/test_100_rules.py
@@ -276,36 +276,48 @@ def test_sim112_get_with_default():
 
 def test_sim113():
     ret = _results(
-        """for el in iterable:
+        """
+idx = 0
+for el in iterable:
     idx += 1"""
     )
-    assert ret == {"2:4 SIM113 Use enumerate instead of 'idx'"}
+    assert ret == {"2:0 SIM113 Use enumerate instead of 'idx'"}
 
 
-def test_sim113_false_positive():
-    ret = _results(
+@pytest.mark.parametrize(
+    "s",
+    (
         """for x in xs:
-    cm[x] += 1"""
-    )
-    assert ret == set()
-
-
-def test_sim113_false_positive_add_string():
-    ret = _results(
+    cm[x] += 1""",
         r"""for line in read_list(redis_conn, storage_key):
-    line += '\n'"""
-    )
-    assert ret == set()
-
-
-def test_sim113_false_positive_continue():
-    ret = _results(
+    line += '\n'""",
         """even_numbers = 0
 for el in range(100):
     if el % 2 == 1:
         continue
-    even_numbers += 1"""
-    )
+    even_numbers += 1""",
+        # The following two examples (double-loop and while-loop) were
+        # contributed by Skylion007 via
+        # https://github.com/MartinThoma/flake8-simplify/issues/25
+        """count = 0
+for foo in foos:
+    for bar in bars:
+        count +=1""",
+        """epoch, i = np.load(resume_file)
+while epochs < TOTAL_EPOCHS:
+    for b in batches:
+        i+=1""",
+    ),
+    ids=(
+        "augment-dict",
+        "add-string",
+        "continue",
+        "double-loop",
+        "while-loop",
+    ),
+)
+def test_sim113_false_positive(s):
+    ret = _results(s)
     assert ret == set()