Skip to content

Commit

Permalink
ENH: Add SIM114 to detect duplicate blocks
Browse files Browse the repository at this point in the history
See issue #10
  • Loading branch information
MartinThoma committed Nov 29, 2020
1 parent fb0fdb1 commit e0ecf22
Show file tree
Hide file tree
Showing 5 changed files with 175 additions and 37 deletions.
8 changes: 4 additions & 4 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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]
Expand Down
91 changes: 91 additions & 0 deletions flake8_simplify.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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}'"
Expand Down Expand Up @@ -693,6 +695,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)
Expand Down Expand Up @@ -1107,6 +1197,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:
Expand Down
1 change: 0 additions & 1 deletion requirements-dev.in
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
junit2html
mutmut
pip-tools
pre-commit
pytest
Expand Down
56 changes: 25 additions & 31 deletions requirements-dev.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
56 changes: 55 additions & 1 deletion tests/test_simplify.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]:
Expand Down Expand Up @@ -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'"}
Expand Down

0 comments on commit e0ecf22

Please sign in to comment.