ENH: Add SIM114 to detect duplicate blocks

See issue #10

.pre-commit-config.yaml

@@ -1,7 +1,7 @@
 # pre-commit run --all-files
 repos:
 -   repo: https://github.com/pre-commit/pre-commit-hooks
-    rev: v3.2.0
+    rev: v3.3.0
     hooks:
     -   id: check-ast
     -   id: check-byte-order-marker
@@ -18,23 +18,23 @@ repos:
 #     hooks:
 #     -   id: flake8
 -   repo: https://github.com/pre-commit/mirrors-mypy
-    rev: v0.782
+    rev: v0.790
     hooks:
     -   id: mypy
 -   repo: https://github.com/asottile/seed-isort-config
     rev: v2.2.0
     hooks:
     -   id: seed-isort-config
 -   repo: https://github.com/pre-commit/mirrors-isort
-    rev: v5.5.2
+    rev: v5.6.4
     hooks:
     -   id: isort
 -   repo: https://github.com/psf/black
     rev: 20.8b1
     hooks:
     -   id: black
 -   repo: https://github.com/asottile/pyupgrade
-    rev: v2.7.2
+    rev: v2.7.4
     hooks:
     -   id: pyupgrade
         args: [--py36-plus]

flake8_simplify.py

@@ -1,5 +1,6 @@
 # Core Library
 import ast
+import itertools
 import sys
 from collections import defaultdict
 from typing import Any, DefaultDict, Dict, Generator, List, Tuple, Type, Union
@@ -34,6 +35,7 @@
 SIM111 = "SIM111 Use 'return all({check} for {target} in {iterable})'"
 SIM112 = "SIM112 Use '{expected}' instead of '{original}'"
 SIM113 = "SIM113 Use enumerate instead of '{variable}'"
+SIM114 = "SIM114 Use logical or (({cond1}) or ({cond2})) and a single body"
 SIM201 = "SIM201 Use '{left} != {right}' instead of 'not {left} == {right}'"
 SIM202 = "SIM202 Use '{left} == {right}' instead of 'not {left} != {right}'"
 SIM203 = "SIM203 Use '{a} not in {b}' instead of 'not {a} in {b}'"
@@ -585,20 +587,30 @@ def _get_sim112(node: ast.Expr) -> List[Tuple[int, int, str]]:
         and isinstance(node.value.value.value, ast.Name)
         and node.value.value.value.id == "os"
         and node.value.value.attr == "environ"
-        and isinstance(node.value.slice, ast.Index)
-        and isinstance(node.value.slice.value, STR_TYPES)
+        and (
+            (
+                isinstance(node.value.slice, ast.Index)
+                and isinstance(node.value.slice.value, STR_TYPES)
+            )
+            or isinstance(node.value.slice, ast.Constant)
+        )
     )
     if is_index_call:
         subscript = node.value
         assert isinstance(subscript, ast.Subscript)
         slice_ = subscript.slice
-        assert isinstance(slice_, ast.Index)
-        string_part = slice_.value
-        assert isinstance(string_part, STR_TYPES)
-        if isinstance(string_part, ast.Str):
-            env_name = string_part.s  # Python 3.6 / 3.7 fallback
-        else:
-            env_name = string_part.value
+        if isinstance(slice_, ast.Index):
+            # TODO: Python < 3.9
+            string_part = slice_.value
+            assert isinstance(string_part, STR_TYPES)
+            if isinstance(string_part, ast.Str):
+                env_name = string_part.s  # Python 3.6 / 3.7 fallback
+            else:
+                env_name = string_part.value
+        elif isinstance(slice_, ast.Constant):
+            # Python 3.9
+            env_name = slice_.value
+
         # Check if this has a change
         has_change = env_name != env_name.upper()
 
@@ -693,6 +705,94 @@ def _get_sim113(node: ast.For) -> List[Tuple[int, int, str]]:
     return errors
 
 
+def _get_sim114(node: ast.If) -> List[Tuple[int, int, str]]:
+    """
+    Find same bodys.
+
+    Examples
+    --------
+        If(
+            test=Name(id='a', ctx=Load()),
+            body=[
+                Expr(
+                    value=Name(id='b', ctx=Load()),
+                ),
+            ],
+            orelse=[
+                If(
+                    test=Name(id='c', ctx=Load()),
+                    body=[
+                        Expr(
+                            value=Name(id='b', ctx=Load()),
+                        ),
+                    ],
+                    orelse=[],
+                ),
+            ],
+        ),
+    """
+    errors: List[Tuple[int, int, str]] = []
+    if_body_pairs = get_if_body_pairs(node)
+    error_pairs = []
+    for ifbody1, ifbody2 in itertools.combinations(if_body_pairs, 2):
+        if is_body_same(ifbody1[1], ifbody2[1]):
+            error_pairs.append((ifbody1, ifbody2))
+    for ifbody1, ifbody2 in error_pairs:
+        errors.append(
+            (
+                ifbody1[0].lineno,
+                ifbody1[0].col_offset,
+                SIM114.format(
+                    cond1=to_source(ifbody1[0]), cond2=to_source(ifbody2[0])
+                ),
+            )
+        )
+    return errors
+
+
+def get_if_body_pairs(node: ast.If) -> List[Tuple[ast.expr, List[ast.stmt]]]:
+    pairs = [(node.test, node.body)]
+    orelse = node.orelse
+    while (
+        isinstance(orelse, list)
+        and len(orelse) == 1
+        and isinstance(orelse[0], ast.If)
+    ):
+        pairs.append((orelse[0].test, orelse[0].body))
+        orelse = orelse[0].orelse
+    return pairs
+
+
+def is_body_same(body1: List[ast.stmt], body2: List[ast.stmt]) -> bool:
+    """Check if two lists of expressions are equivalent."""
+    if len(body1) != len(body2):
+        return False
+    for a, b in zip(body1, body2):
+        try:
+            stmt_equal = is_stmt_equal(a, b)
+        except RecursionError:  # maximum recursion depth
+            stmt_equal = False
+        if not stmt_equal:
+            return False
+    return True
+
+
+def is_stmt_equal(a: ast.stmt, b: ast.stmt) -> bool:
+    if type(a) is not type(b):
+        return False
+    if isinstance(a, ast.AST):
+        for k, v in vars(a).items():
+            if k in ("lineno", "col_offset", "ctx", "end_lineno"):
+                continue
+            if not is_stmt_equal(v, getattr(b, k)):
+                return False
+        return True
+    elif isinstance(a, list):
+        return all(itertools.starmap(is_stmt_equal, zip(a, b)))
+    else:
+        return a == b
+
+
 def is_constant_increase(expr: ast.AugAssign) -> bool:
     return isinstance(expr.op, ast.Add) and isinstance(
         expr.value, (ast.Constant, ast.Num)
@@ -1107,6 +1207,7 @@ def visit_If(self, node: ast.If) -> None:
         self.errors += _get_sim103(node)
         self.errors += _get_sim106(node)
         self.errors += _get_sim108(node)
+        self.errors += _get_sim114(node)
         self.generic_visit(node)
 
     def visit_For(self, node: ast.For) -> None:

requirements-dev.in

@@ -1,5 +1,4 @@
 junit2html
-mutmut
 pip-tools
 pre-commit
 pytest

requirements-dev.txt

@@ -5,56 +5,50 @@
 #    pip-compile requirements-dev.in
 #
 appdirs==1.4.4            # via virtualenv
-attrs==20.2.0             # via pytest
+attrs==20.3.0             # via pytest
 bleach==3.2.1             # via readme-renderer
-certifi==2020.6.20        # via requests
-cffi==1.14.2              # via cryptography
+certifi==2020.11.8        # via requests
+cffi==1.14.4              # via cryptography
 cfgv==3.2.0               # via pre-commit
 chardet==3.0.4            # via requests
-click==7.1.2              # via mutmut, pip-tools
-colorama==0.4.3           # via twine
+click==7.1.2              # via pip-tools
+colorama==0.4.4           # via twine
 coverage==5.3             # via pytest-cov
-cryptography==3.1         # via secretstorage
+cryptography==3.2.1       # via secretstorage
 distlib==0.3.1            # via virtualenv
 docutils==0.16            # via readme-renderer
 filelock==3.0.12          # via virtualenv
-glob2==0.7                # via mutmut
-identify==1.5.3           # via pre-commit
+identify==1.5.10          # via pre-commit
 idna==2.10                # via requests
-iniconfig==1.0.1          # via pytest
-jeepney==0.4.3            # via keyring, secretstorage
-junit-xml==1.8            # via mutmut
-junit2html==25            # via -r requirements-dev.in
-keyring==21.4.0           # via twine
-more-itertools==8.5.0     # via pytest
-mutmut==2.1.0             # via -r requirements-dev.in
+iniconfig==1.1.1          # via pytest
+jeepney==0.6.0            # via keyring, secretstorage
+junit2html==26            # via -r requirements-dev.in
+keyring==21.5.0           # via twine
 nodeenv==1.5.0            # via pre-commit
-packaging==20.4           # via bleach, pytest
-parso==0.8.0              # via mutmut
-pip-tools==5.3.1          # via -r requirements-dev.in
-pkginfo==1.5.0.1          # via twine
+packaging==20.7           # via bleach, pytest
+pip-tools==5.4.0          # via -r requirements-dev.in
+pkginfo==1.6.1            # via twine
 pluggy==0.13.1            # via pytest
-pony==0.7.13              # via mutmut
-pre-commit==2.7.1         # via -r requirements-dev.in
+pre-commit==2.9.2         # via -r requirements-dev.in
 py==1.9.0                 # via pytest
 pycparser==2.20           # via cffi
-pygments==2.7.1           # via readme-renderer
+pygments==2.7.2           # via readme-renderer
 pyparsing==2.4.7          # via packaging
+pytest==6.1.2             # via -r requirements-dev.in, pytest-cov, pytest-timeout
 pytest-cov==2.10.1        # via -r requirements-dev.in
 pytest-timeout==1.4.2     # via -r requirements-dev.in
-pytest==6.0.2             # via -r requirements-dev.in, pytest-cov, pytest-timeout
 pyyaml==5.3.1             # via pre-commit
-readme-renderer==26.0     # via twine
+readme-renderer==28.0     # via twine
+requests==2.25.0          # via requests-toolbelt, twine
 requests-toolbelt==0.9.1  # via twine
-requests==2.24.0          # via requests-toolbelt, twine
 rfc3986==1.4.0            # via twine
-secretstorage==3.1.2      # via keyring
-six==1.15.0               # via bleach, cryptography, junit-xml, packaging, pip-tools, readme-renderer, virtualenv
-toml==0.10.1              # via pre-commit, pytest
-tqdm==4.49.0              # via twine
+secretstorage==3.3.0      # via keyring
+six==1.15.0               # via bleach, cryptography, pip-tools, readme-renderer, virtualenv
+toml==0.10.2              # via pre-commit, pytest
+tqdm==4.54.0              # via twine
 twine==3.2.0              # via -r requirements-dev.in
-urllib3==1.25.10          # via requests
-virtualenv==20.0.31       # via pre-commit
+urllib3==1.26.2           # via requests
+virtualenv==20.2.1        # via pre-commit
 webencodings==0.5.1       # via bleach
 wheel==0.35.1             # via -r requirements-dev.in
 

tests/test_simplify.py

@@ -3,7 +3,7 @@
 from typing import Set
 
 # First party
-from flake8_simplify import Plugin
+from flake8_simplify import Plugin, get_if_body_pairs
 
 
 def _results(code: str) -> Set[str]:
@@ -296,6 +296,60 @@ def test_sim_113_false_positive():
     assert ret == set()
 
 
+def test_sim_114():
+    ret = _results(
+        """if a:
+    b
+elif c:
+    b"""
+    )
+    assert ret == {"1:3 SIM114 Use logical or ((a) or (c)) and a single body"}
+
+
+def test_get_if_body_pairs():
+    ret = ast.parse(
+        """if a == b:
+    foo(a)
+    foo(b)"""
+    ).body[0]
+    assert isinstance(ret, ast.If)
+    result = get_if_body_pairs(ret)
+    assert len(result) == 1
+    comp = result[0][0]
+    assert isinstance(comp, ast.Compare)
+    assert isinstance(result[0][1], list)
+    assert isinstance(comp.left, ast.Name)
+    assert len(comp.ops) == 1
+    assert isinstance(comp.ops[0], ast.Eq)
+    assert len(comp.comparators) == 1
+    assert isinstance(comp.comparators[0], ast.Name)
+    assert comp.left.id == "a"
+    assert comp.comparators[0].id == "b"
+
+
+def test_get_if_body_pairs_2():
+    ret = ast.parse(
+        """if a == b:
+    foo(a)
+    foo(b)
+elif a == b:
+    foo(c)"""
+    ).body[0]
+    assert isinstance(ret, ast.If)
+    result = get_if_body_pairs(ret)
+    assert len(result) == 2
+    comp = result[0][0]
+    assert isinstance(comp, ast.Compare)
+    assert isinstance(result[0][1], list)
+    assert isinstance(comp.left, ast.Name)
+    assert len(comp.ops) == 1
+    assert isinstance(comp.ops[0], ast.Eq)
+    assert len(comp.comparators) == 1
+    assert isinstance(comp.comparators[0], ast.Name)
+    assert comp.left.id == "a"
+    assert comp.comparators[0].id == "b"
+
+
 def test_sim_201():
     ret = _results("not a == b")
     assert ret == {"1:0 SIM201 Use 'a != b' instead of 'not a == b'"}