Skip to content

Commit

Permalink
SIM901: Add rule for unnecessary bool function calls (#90)
Browse files Browse the repository at this point in the history
Closes #88
  • Loading branch information
MartinThoma committed Jan 23, 2022
1 parent 136c60d commit f5a2571
Show file tree
Hide file tree
Showing 3 changed files with 75 additions and 1 deletion.
31 changes: 30 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ Simplifying Comparations:
* [`SIM221`](https://github.com/MartinThoma/flake8-simplify/issues/6): Use 'True' instead of 'a or not a' ([example](#SIM221))
* [`SIM222`](https://github.com/MartinThoma/flake8-simplify/issues/6): Use 'True' instead of '... or True' ([example](#SIM222))
* [`SIM223`](https://github.com/MartinThoma/flake8-simplify/issues/6): Use 'False' instead of '... and False' ([example](#SIM223))
* [`SIM224`](https://github.com/MartinThoma/flake8-simplify/issues/88): Reserved for SIM901 once it's stable
* [`SIM300`](https://github.com/MartinThoma/flake8-simplify/issues/16): Use 'age == 42' instead of '42 == age' ([example](#SIM300))

Simplifying usage of dictionaries:
Expand All @@ -80,11 +81,24 @@ General Code Style:
* [`SIM106`](https://github.com/MartinThoma/flake8-simplify/issues/8): Handle error-cases first ([example](#SIM106))
* [`SIM112`](https://github.com/MartinThoma/flake8-simplify/issues/19): Use CAPITAL environment variables ([example](#SIM112))

**Experimental rules:**

Getting rules right for every possible case is hard. I don't want to disturb
peoples workflows. For this reason, flake8-simplify introduces new rules with
the `SIM9` prefix. Every new rule will start with SIM9 and stay there for at
least 6 months. In this time people can give feedback. Once the rule is stable,
the code will change to another number.

Current experimental rules:

* `SIM901`: Use comparisons directly instead of wrapping them in a `bool(...)` call ([example](#SIM901))


You might have good reasons to ignore some rules. To do that, use the standard Flake8 configuration. For example, within the `setup.cfg` file:

```python
[flake8]
ignore = SIM106, SIM113, SIM119
ignore = SIM106, SIM113, SIM119, SIM9
```


Expand Down Expand Up @@ -494,3 +508,18 @@ else:
# Good
thing = a_dict.get(key, "default_value")
```

### SIM901

This rule will be renamed to `SIM224` after its 6-month trial period is over.
Please report any issues you encounter with this rule!

The trial period starts on 23rd of January and will end on 23rd of August 2022.

```python
# Bad
bool(a == b)

# Good
a == b
```
40 changes: 40 additions & 0 deletions flake8_simplify.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ def __init__(self, orig: ast.Call) -> None:
"SIM401 Use '{value} = {dict}.get({key}, {default_value})' "
"instead of an if-block"
)
SIM901 = "SIM901 Use '{better}' instead of '{current}'"

# ast.Constant in Python 3.8, ast.NameConstant in Python 3.6 and 3.7
BOOL_CONST_TYPES = (ast.Constant, ast.NameConstant)
Expand Down Expand Up @@ -1775,12 +1776,51 @@ def _get_sim401(node: ast.If) -> List[Tuple[int, int, str]]:
return errors


def _get_sim901(node: ast.Call) -> List[Tuple[int, int, str]]:
"""
Get a list of all calls of the type "bool(comparison)".
Call(
func=Name(id='bool', ctx=Load()),
args=[
Compare(
left=Name(id='a', ctx=Load()),
ops=[Eq()],
comparators=[Name(id='b', ctx=Load())],
),
],
keywords=[],
)
"""
errors: List[Tuple[int, int, str]] = []
if not (
isinstance(node.func, ast.Name)
and node.func.id == "bool"
and len(node.args) == 1
and isinstance(node.args[0], ast.Compare)
):
return errors

current = to_source(node)
better = to_source(node.args[0])

errors.append(
(
node.lineno,
node.col_offset,
SIM901.format(current=current, better=better),
)
)
return errors


class Visitor(ast.NodeVisitor):
def __init__(self) -> None:
self.errors: List[Tuple[int, int, str]] = []

def visit_Call(self, node: ast.Call) -> Any:
self.errors += _get_sim115(Call(node))
self.errors += _get_sim901(node)
self.generic_visit(node)

def visit_With(self, node: ast.With) -> Any:
Expand Down
5 changes: 5 additions & 0 deletions tests/test_simplify.py
Original file line number Diff line number Diff line change
Expand Up @@ -878,3 +878,8 @@ def test_sim401_positive_msg_check_issue89():
has_sim401 = True
assert msg == expected_proposal
assert has_sim401


def test_sim901():
results = _results("bool(a == b)")
assert results == {"1:0 SIM901 Use 'a == b' instead of 'bool(a == b)'"}

0 comments on commit f5a2571

Please sign in to comment.