From ad67ae740be94bea5f5e1be947b7c35454115520 Mon Sep 17 00:00:00 2001 From: Martin Thoma Date: Tue, 29 Mar 2022 07:35:55 +0200 Subject: [PATCH] SIM902: Removed due to false-positives (#127) Closes #125 --- README.md | 15 +---------- flake8_simplify/__init__.py | 2 -- flake8_simplify/rules/ast_call.py | 38 ---------------------------- tests/test_900_rules.py | 41 ------------------------------- 4 files changed, 1 insertion(+), 95 deletions(-) diff --git a/README.md b/README.md index 861c827..4f47211 100644 --- a/README.md +++ b/README.md @@ -85,7 +85,7 @@ General Code Style: * [`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)). 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)) -* `SIM122`: Reserved for SIM902 once it's stable +* `SIM122` / SIM902: ![](https://img.shields.io/badge/-removed-lightgrey) Moved to [flake8-scream](https://github.com/MartinThoma/flake8-scream) due to [issue 125](https://github.com/MartinThoma/flake8-simplify/issues/125) * `SIM123`: Reserved for SIM903 once it's stable * `SIM124`: Reserved for SIM909 once it's stable @@ -100,7 +100,6 @@ the code will change to another number. Current experimental rules: * `SIM901`: Use comparisons directly instead of wrapping them in a `bool(...)` call ([example](#SIM901)) -* `SIM902`: Use keyword-argument instead of magic boolean ([example](#SIM902)) * `SIM903`: Use keyword-argument instead of magic number ([example](#SIM903)) * `SIM904`: Assign values to dictionary directly at initialization ([example](#SIM904)) * [`SIM905`](https://github.com/MartinThoma/flake8-simplify/issues/86): Split string directly if only constants are used ([example](#SIM905)) @@ -508,19 +507,7 @@ bool(a == b) a == b ``` -### SIM902 -This rule will be renamed to `SIM122` after its 6-month trial period is over. - -```python -# Bad -foo(False) -bar(True) - -# Good -foo(verbose=False) -bar(enable_magic=True) -``` ### SIM903 diff --git a/flake8_simplify/__init__.py b/flake8_simplify/__init__.py index 7499b1b..3ee2012 100644 --- a/flake8_simplify/__init__.py +++ b/flake8_simplify/__init__.py @@ -17,7 +17,6 @@ from flake8_simplify.rules.ast_call import ( get_sim115, get_sim901, - get_sim902, get_sim903, get_sim905, get_sim906, @@ -74,7 +73,6 @@ def visit_Assign(self, node: ast.Assign) -> Any: def visit_Call(self, node: ast.Call) -> Any: self.errors += get_sim115(Call(node)) self.errors += get_sim901(node) - self.errors += get_sim902(Call(node)) self.errors += get_sim903(Call(node)) self.errors += get_sim905(node) self.errors += get_sim906(node) diff --git a/flake8_simplify/rules/ast_call.py b/flake8_simplify/rules/ast_call.py index af9e8c2..00606fe 100644 --- a/flake8_simplify/rules/ast_call.py +++ b/flake8_simplify/rules/ast_call.py @@ -92,44 +92,6 @@ def get_sim901(node: ast.Call) -> List[Tuple[int, int, str]]: return errors -def get_sim902(node: Call) -> List[Tuple[int, int, str]]: - """Find bare boolean function arguments.""" - SIM902 = ( - "SIM902 Use keyword-argument instead of magic boolean for '{func}'" - ) - errors: List[Tuple[int, int, str]] = [] - - if isinstance(node.func, ast.Attribute): - call_name = node.func.attr - elif isinstance(node.func, ast.Name): - call_name = node.func.id - else: - logger.debug(f"Unknown call type: {type(node.func)}") - return errors - - nb_args = len(node.args) - - if call_name in ["partial", "min", "max"] or call_name.startswith("_"): - return errors - - has_bare_bool = any( - isinstance(call_arg, (ast.Constant, ast.NameConstant)) - and (call_arg.value is True or call_arg.value is False) - for call_arg in node.args - ) - - is_setter = call_name.lower().startswith("set") and nb_args <= 2 - is_exception = isinstance(node.func, ast.Attribute) and node.func.attr in [ - "get" - ] - if has_bare_bool and not (is_exception or is_setter): - source = to_source(node) - errors.append( - (node.lineno, node.col_offset, SIM902.format(func=source)) - ) - return errors - - def get_sim903(node: Call) -> List[Tuple[int, int, str]]: """Find bare numeric function arguments.""" SIM903 = "SIM903 Use keyword-argument instead of magic number for '{func}'" diff --git a/tests/test_900_rules.py b/tests/test_900_rules.py index df174fe..0410811 100644 --- a/tests/test_900_rules.py +++ b/tests/test_900_rules.py @@ -10,47 +10,6 @@ def test_sim901(): assert results == {"1:0 SIM901 Use 'a == b' instead of 'bool(a == b)'"} -@pytest.mark.parametrize( - "s", - ( - "foo(a, b, True)", - "set_foo(a, b, True)", - ), - ids=[ - "basic", - "set_multiple", - ], -) -def test_sim902(s): - error_messages = _results(s) - assert any("SIM902" in error_message for error_message in error_messages) - - -@pytest.mark.parametrize( - "s", - ( - "foo(a, b, foo=True)", - "dict.get('foo', True)", - "set_visible(True)", - "line.set_visible(True)", - "partial(foo, True)", - "partial(foo, bar=True)", - ), - ids=[ - "kw_arg_is_used", - "dict_get", - "boolean_setter_function", - "boolean_setter_method", - "partial_arg", - "partial_kwarg", - ], -) -def test_sim902_false_positive_check(s): - error_messages = _results(s) - for error_message in error_messages: - assert "SIM902" not in error_message - - @pytest.mark.parametrize( "s", ("foo(a, b, 123123)", "foo(a, b, 123.123)"),