Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SIM901: Add rule for unnecessary bool applications #90

Merged
merged 2 commits into from
Jan 23, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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)'"}