From 34eaac11d1381f84ae1a9da81a3be2cf0d6381f1 Mon Sep 17 00:00:00 2001 From: ScDor <18174994+ScDor@users.noreply.github.com> Date: Fri, 19 May 2023 14:27:04 +0300 Subject: [PATCH 1/3] SIM911 --- README.md | 2 + flake8_simplify/__init__.py | 2 + flake8_simplify/rules/ast_call.py | 64 +++++++++++++++++++++++++++++++ tests/test_900_rules.py | 34 ++++++++++++++++ 4 files changed, 102 insertions(+) diff --git a/README.md b/README.md index c1ef70d..78682e8 100644 --- a/README.md +++ b/README.md @@ -77,6 +77,7 @@ Simplifying usage of dictionaries: * [`SIM401`](https://github.com/MartinThoma/flake8-simplify/issues/72): Use 'a_dict.get(key, "default_value")' instead of an if-block ([example](#SIM401)) * [`SIM118`](https://github.com/MartinThoma/flake8-simplify/issues/40): Use 'key in dict' instead of 'key in dict.keys()' ([example](#SIM118)) +* `SIM119` Reserved for [SIM911](#sim911) once it's stable General Code Style: @@ -106,6 +107,7 @@ Current experimental rules: * [`SIM908`](https://github.com/MartinThoma/flake8-simplify/issues/50): Use dict.get(key) ([example](#SIM908)) * [`SIM909`](https://github.com/MartinThoma/flake8-simplify/issues/114): Avoid reflexive assignments ([example](#SIM909)) * [`SIM910`](https://github.com/MartinThoma/flake8-simplify/issues/171): Avoid to use `dict.get(key, None)` ([example](#SIM910)) +* [`SIM911`](https://github.com/MartinThoma/flake8-simplify/issues/161): Avoid using `zip(dict.keys(), dict.values())` ([example](#SIM911)) ## Disabling Rules diff --git a/flake8_simplify/__init__.py b/flake8_simplify/__init__.py index 3fd1f1e..0f728d6 100644 --- a/flake8_simplify/__init__.py +++ b/flake8_simplify/__init__.py @@ -20,6 +20,7 @@ get_sim905, get_sim906, get_sim910, + get_sim911, ) from flake8_simplify.rules.ast_classdef import get_sim120 from flake8_simplify.rules.ast_compare import get_sim118, get_sim300 @@ -76,6 +77,7 @@ def visit_Call(self, node: ast.Call) -> Any: self.errors += get_sim905(node) self.errors += get_sim906(node) self.errors += get_sim910(Call(node)) + self.errors += get_sim911(node) self.generic_visit(node) def visit_With(self, node: ast.With) -> Any: diff --git a/flake8_simplify/rules/ast_call.py b/flake8_simplify/rules/ast_call.py index 60947f6..0e0d7f7 100644 --- a/flake8_simplify/rules/ast_call.py +++ b/flake8_simplify/rules/ast_call.py @@ -233,3 +233,67 @@ def get_sim910(node: Call) -> List[Tuple[int, int, str]]: ) ) return errors + + +def get_sim911(node: ast.AST) -> List[Tuple[int, int, str]]: + """ + Find nodes representing the expression "zip(_.keys(), _.values())". + + Returns a list of tuples containing the line number and column offset + of each identified node. + + Expr( + value=Call( + func=Name(id='zip', ctx=Load()), + args=[ + Call( + func=Attribute( + value=Name(id='_', ctx=Load()), + attr='keys', + ctx=Load()), + args=[], + keywords=[]), + Call( + func=Attribute( + value=Name(id='_', ctx=Load()), + attr='values', + ctx=Load()), + args=[], + keywords=[])], + keywords=[ + keyword( + arg='strict', + value=Constant(value=False)) + ] + ) + ) + """ + RULE = "SIM911 Use '{name}.items()' instead of 'zip({name}.keys(),{name}.values())'" + errors: List[Tuple[int, int, str]] = [] + + if isinstance(node, ast.Call): + if ( + isinstance(node.func, ast.Name) + and node.func.id == "zip" + and len(node.args) == 2 + ): + first_arg, second_arg = node.args + if ( + isinstance(first_arg, ast.Call) + and isinstance(first_arg.func, ast.Attribute) + and isinstance(first_arg.func.value, ast.Name) + and first_arg.func.attr == "keys" + and isinstance(second_arg, ast.Call) + and isinstance(second_arg.func, ast.Attribute) + and isinstance(second_arg.func.value, ast.Name) + and second_arg.func.attr == "values" + and first_arg.func.value.id == second_arg.func.value.id + ): + errors.append( + ( + node.lineno, + node.col_offset, + RULE.format(name=first_arg.func.value.id), + ) + ) + return errors diff --git a/tests/test_900_rules.py b/tests/test_900_rules.py index 3f1d17b..a504d95 100644 --- a/tests/test_900_rules.py +++ b/tests/test_900_rules.py @@ -1,3 +1,6 @@ +# Core Library +from typing import Optional, Set + # Third party import pytest @@ -204,3 +207,34 @@ def test_sim910(s, msg): expected = {msg} if msg is not None else set() results = _results(s) assert results == expected + + +@pytest.mark.parametrize( + ("s", "expected"), + ( + ( + "zip(d.keys(), d.values())", + { + "1:0 SIM911 Use 'd.items()' instead of 'zip(d.keys(), d.values())'" + }, + ), + ( + "zip(d.keys(), d.keys())", + None, + ), + ( + "zip(d1.keys(), d2.values())", + None, + ), + ( + "zip(d1.keys(), values)", + None, + ), + ( + "zip(keys, values)", + None, + ), + ), +) +def test_sim911(s: str, expected: Optional[Set]): + assert _results(s) == (expected or set()) From 465c0a94d193736772f255ec49036d60eabfb0e3 Mon Sep 17 00:00:00 2001 From: ScDor <18174994+ScDor@users.noreply.github.com> Date: Fri, 19 May 2023 14:28:35 +0300 Subject: [PATCH 2/3] remove types from UT --- tests/test_900_rules.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_900_rules.py b/tests/test_900_rules.py index a504d95..3e4efff 100644 --- a/tests/test_900_rules.py +++ b/tests/test_900_rules.py @@ -236,5 +236,5 @@ def test_sim910(s, msg): ), ), ) -def test_sim911(s: str, expected: Optional[Set]): +def test_sim911(s, expected): assert _results(s) == (expected or set()) From 9bcd0388234b9f9e4a8a82d2a482f1dfbb5b2511 Mon Sep 17 00:00:00 2001 From: ScDor <18174994+ScDor@users.noreply.github.com> Date: Fri, 19 May 2023 14:36:24 +0300 Subject: [PATCH 3/3] remove unused imports --- tests/test_900_rules.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/test_900_rules.py b/tests/test_900_rules.py index 3e4efff..34b3368 100644 --- a/tests/test_900_rules.py +++ b/tests/test_900_rules.py @@ -1,6 +1,3 @@ -# Core Library -from typing import Optional, Set - # Third party import pytest