Skip to content

Commit

Permalink
SIM106: Remove due to too many false-positives
Browse files Browse the repository at this point in the history
Closes #8
Closes #14
  • Loading branch information
MartinThoma committed Feb 27, 2022
1 parent 42d91e0 commit 3e85770
Show file tree
Hide file tree
Showing 4 changed files with 1 addition and 108 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ General Code Style:

* `SIM102`: Use a single if-statement instead of nested if-statements ([example](#SIM102))
* [`SIM103`](https://github.com/MartinThoma/flake8-simplify/issues/3): Return the boolean condition directly ([example](#SIM103))
* [`SIM106`](https://github.com/MartinThoma/flake8-simplify/issues/8): Handle error-cases first ([example](#SIM106))
* [`SIM106`](https://github.com/MartinThoma/flake8-simplify/issues/8): Handle error-cases first ([example](#SIM106)). This rule was removed due to too many false-positives.
* [`SIM112`](https://github.com/MartinThoma/flake8-simplify/issues/19): Use CAPITAL environment variables ([example](#SIM112))

**Experimental rules:**
Expand Down
2 changes: 0 additions & 2 deletions flake8_simplify/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
from flake8_simplify.rules.ast_if import (
get_sim102,
get_sim103,
get_sim106,
get_sim108,
get_sim114,
get_sim116,
Expand Down Expand Up @@ -98,7 +97,6 @@ def visit_BoolOp(self, node: ast.BoolOp) -> None:
def visit_If(self, node: ast.If) -> None:
self.errors += get_sim102(node)
self.errors += get_sim103(node)
self.errors += get_sim106(node)
self.errors += get_sim108(node)
self.errors += get_sim114(node)
self.errors += get_sim116(node)
Expand Down
64 changes: 0 additions & 64 deletions flake8_simplify/rules/ast_if.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,70 +85,6 @@ def get_sim103(node: ast.If) -> List[Tuple[int, int, str]]:
return errors


def get_sim106(node: ast.If) -> List[Tuple[int, int, str]]:
"""
Get a list of all calls where an exception is raised in else.
if cond:
return True
else:
raise Exception
which is
If(
test=Name(id='cond', ctx=Load()),
body=[
Expr(
value=Name(id='a', ctx=Load()),
),
Expr(
value=Name(id='b', ctx=Load()),
),
Expr(
value=Name(id='c', ctx=Load()),
),
],
orelse=[
Raise(
exc=Name(id='Exception', ctx=Load()),
cause=None,
),
],
)
"""
SIM106 = "SIM106 Handle error-cases first"
errors: List[Tuple[int, int, str]] = []
just_one = (
len(node.orelse) == 1
and len(node.orelse) >= 1
and isinstance(node.orelse[-1], ast.Raise)
and not isinstance(node.body[-1], ast.Raise)
)
many = (
len(node.body) > 2 * len(node.orelse)
and len(node.orelse) >= 1
and isinstance(node.orelse[-1], ast.Raise)
and not isinstance(node.body[-1], ast.Raise)
)
if not (just_one or many):
return errors
ast_raise = node.orelse[-1]
if not isinstance(ast_raise, ast.Raise):
return errors
ast_raised = ast_raise.exc
if (
isinstance(ast_raised, ast.Call)
and ast_raised.func
and isinstance(ast_raised.func, ast.Name)
and ast_raised.func.id in ["ValueError", "NotImplementedError"]
):
return errors

errors.append((node.lineno, node.col_offset, SIM106))
return errors


def get_sim108(node: ast.If) -> List[Tuple[int, int, str]]:
"""
Get a list of all if-elses which could be a ternary operator assignment.
Expand Down
41 changes: 0 additions & 41 deletions tests/test_100_rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -112,47 +112,6 @@ def test_sim105_pokemon():
assert ret == {"1:0 SIM105 Use 'contextlib.suppress(Exception)'"}


def test_sim106():
ret = _results(
"""if cond:
a
b
c
else:
raise Exception"""
)
assert ret == {"1:0 SIM106 Handle error-cases first"}


@pytest.mark.parametrize(
"s",
(
"""if image_extension in ['.jpg', '.jpeg']:
return 'JPEG'
elif image_extension in ['.png']:
return 'PNG'
else:
raise ValueError("Unknwon image extension {image extension}")""",
"""if image_extension in ['.jpg', '.jpeg']:
return 'JPEG'
elif image_extension in ['.png']:
return 'PNG'
else:
logger.error("Unknwon image extension {image extension}")
raise ValueError("Unknwon image extension {image extension}")""",
"""if cond:
raise Exception
else:
raise Exception""",
),
ids=["ValueError", "ValueError-with-logging", "TwoExceptions"],
)
def test_sim106_false_positive(s):
ret = _results(s)
for el in ret:
assert "SIM106" not in el


def test_sim107():
ret = _results(
"""def foo():
Expand Down

0 comments on commit 3e85770

Please sign in to comment.