From 980ec9160eac90a152d67144bafb12ff7c2afdc9 Mon Sep 17 00:00:00 2001 From: Martin Thoma Date: Tue, 29 Mar 2022 11:14:22 +0200 Subject: [PATCH] SIM903: Removed due to false-positives Closes #130 --- README.md | 20 +-------- flake8_simplify/__init__.py | 2 - flake8_simplify/rules/ast_call.py | 68 ------------------------------- tests/test_900_rules.py | 45 -------------------- 4 files changed, 2 insertions(+), 133 deletions(-) diff --git a/README.md b/README.md index 4f47211..c592b89 100644 --- a/README.md +++ b/README.md @@ -86,7 +86,7 @@ General Code Style: * [`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` / 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 +* `SIM123` / SIM902: ![](https://img.shields.io/badge/-removed-lightgrey) Moved to [flake8-scream](https://github.com/MartinThoma/flake8-scream) due to [issue 130](https://github.com/MartinThoma/flake8-simplify/issues/130) * `SIM124`: Reserved for SIM909 once it's stable **Experimental rules:** @@ -100,7 +100,7 @@ the code will change to another number. Current experimental rules: * `SIM901`: Use comparisons directly instead of wrapping them in a `bool(...)` call ([example](#SIM901)) -* `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)) * [`SIM906`](https://github.com/MartinThoma/flake8-simplify/issues/101): Merge nested os.path.join calls ([example](#SIM906)) @@ -508,22 +508,6 @@ a == b ``` - -### SIM903 - -This rule will be renamed to `SIM123` after its 6-month trial period is over. -Please report any issues you encounter with this rule! - -The trial period starts on 12th of February and will end on 12th of September 2022. - -```python -# Bad -foo(42, 1.234) - -# Good -foo(the_answer=42, flux_compensation=1.234) -``` - ### SIM904 This rule will be renamed to `SIM224` after its 6-month trial period is over. diff --git a/flake8_simplify/__init__.py b/flake8_simplify/__init__.py index 3ee2012..f7f37ce 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_sim903, get_sim905, get_sim906, ) @@ -73,7 +72,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_sim903(Call(node)) self.errors += get_sim905(node) self.errors += get_sim906(node) self.generic_visit(node) diff --git a/flake8_simplify/rules/ast_call.py b/flake8_simplify/rules/ast_call.py index 00606fe..b78c460 100644 --- a/flake8_simplify/rules/ast_call.py +++ b/flake8_simplify/rules/ast_call.py @@ -92,74 +92,6 @@ def get_sim901(node: ast.Call) -> List[Tuple[int, int, str]]: 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}'" - acceptable_magic_numbers = (0, 1, 2) - 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 nb_args <= 1 or call_name.startswith("_"): - return errors - - functions_any_arg = ["partial", "min", "max", "minimum", "maximum"] - functions_1_arg = ["sqrt", "sleep", "hideColumn"] - functions_2_args = [ - "arange", - "uniform", - "zeros", - "percentile", - "setColumnWidth", - "float_power", - "power", - "pow", - "float_power", - "binomial", - ] - if any( - ( - call_name in functions_any_arg, - call_name in functions_1_arg and nb_args == 1, - call_name in functions_2_args and nb_args == 2, - call_name in ["linspace"] and nb_args == 3, - "color" in call_name.lower() and nb_args in [3, 4], - "point" in call_name.lower() and nb_args in [2, 3], - ) - ): - return errors - - has_bare_int = any( - isinstance(call_arg, ast.Num) - and call_arg.n not in acceptable_magic_numbers - for call_arg in node.args - ) - - is_setter = call_name.lower().startswith("set") and nb_args <= 2 - is_exception = isinstance(node.func, ast.Name) and node.func.id == "range" - is_exception = is_exception or ( - isinstance(node.func, ast.Attribute) - and node.func.attr - in [ - "get", - "insert", - ] - ) - if has_bare_int and not (is_exception or is_setter): - source = to_source(node) - errors.append( - (node.lineno, node.col_offset, SIM903.format(func=source)) - ) - return errors - - def get_sim905(node: ast.Call) -> List[Tuple[int, int, str]]: RULE = "SIM905 Use '{expected}' instead of '{actual}'" errors: List[Tuple[int, int, str]] = [] diff --git a/tests/test_900_rules.py b/tests/test_900_rules.py index 31226aa..9f49abf 100644 --- a/tests/test_900_rules.py +++ b/tests/test_900_rules.py @@ -10,51 +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, 123123)", "foo(a, b, 123.123)"), - ids=["int", "float"], -) -def test_sim903_true_positive_check(s): - error_messages = _results(s) - assert any("SIM903" in error_message for error_message in error_messages) - - -@pytest.mark.parametrize( - "s", - ( - "dict.get('foo', 123)", - "set_foo(1.23)", - "line.set_foo(1.23)", - "partial(foo, 1, 2, 3)", - "min(0.5, g_norm)", - "QColor(53, 53, 53, 128)", - ), - ids=[ - "get_exception", - "set_function", - "set_method", - "partial", - "min", - "color", - ], -) -def test_sim903_false_positive_check(s): - error_messages = _results(s) - for error_message in error_messages: - assert "SIM903" not in error_message - - -def test_sim903_insert_exception(): - ret = _results("sys.path.insert(0, 'foo')") - assert ret == set() - - -def test_sim903_range_exception(): - ret = _results("range(42)") - assert ret == set() - - @pytest.mark.parametrize( "s", (