From f5a25717d22afe7f7a6ad3dec13a10c69029174b Mon Sep 17 00:00:00 2001 From: Martin Thoma Date: Sun, 23 Jan 2022 14:17:48 +0100 Subject: [PATCH] SIM901: Add rule for unnecessary bool function calls (#90) Closes #88 --- README.md | 31 ++++++++++++++++++++++++++++++- flake8_simplify.py | 40 ++++++++++++++++++++++++++++++++++++++++ tests/test_simplify.py | 5 +++++ 3 files changed, 75 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index ffb73ce..d670117 100644 --- a/README.md +++ b/README.md @@ -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: @@ -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 ``` @@ -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 +``` diff --git a/flake8_simplify.py b/flake8_simplify.py index 4b5482d..f3bcba4 100644 --- a/flake8_simplify.py +++ b/flake8_simplify.py @@ -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) @@ -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: diff --git a/tests/test_simplify.py b/tests/test_simplify.py index b61bc7c..bc23a17 100644 --- a/tests/test_simplify.py +++ b/tests/test_simplify.py @@ -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)'"}