From 7f1044f2ef893b6ed8adbd6780334b3e0b821deb Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Fri, 17 Mar 2023 15:48:55 -0400 Subject: [PATCH 1/5] Expand C416 to dict comprehensions --- README.rst | 5 +++-- src/flake8_comprehensions/__init__.py | 23 +++++++++++++++++++---- tests/test_flake8_comprehensions.py | 14 ++++++++++++++ 3 files changed, 36 insertions(+), 6 deletions(-) diff --git a/README.rst b/README.rst index b4391d4..2e73c3b 100644 --- a/README.rst +++ b/README.rst @@ -150,13 +150,14 @@ For example: * Rewrite ``sorted(iterable)[::-1]`` as ``sorted(iterable, reverse=True)`` * Rewrite ``reversed(iterable[::-1])`` as ``iterable`` -C416: Unnecessary ```` comprehension - rewrite using ````\(). +C416: Unnecessary ```` comprehension - rewrite using ````\(). --------------------------------------------------------------------------------- It's unnecessary to use a list comprehension if the elements are unchanged. -The iterable should be wrapped in ``list()`` or ``set()`` instead. +The iterable should be wrapped in ``dict()``, ``list()``, or ``set()`` instead. For example: +* Rewrite ``{a:b for a, b in iterable}`` as ``dict(iterable)`` * Rewrite ``[x for x in iterable]`` as ``list(iterable)`` * Rewrite ``{x for x in iterable}`` as ``set(iterable)`` diff --git a/src/flake8_comprehensions/__init__.py b/src/flake8_comprehensions/__init__.py index 11e5563..d6203e6 100644 --- a/src/flake8_comprehensions/__init__.py +++ b/src/flake8_comprehensions/__init__.py @@ -295,15 +295,30 @@ def run(self) -> Generator[tuple[int, int, str, type[Any]], None, None]: type(self), ) - elif isinstance(node, (ast.ListComp, ast.SetComp)): + elif isinstance(node, (ast.DictComp, ast.ListComp, ast.SetComp)): if ( len(node.generators) == 1 and not node.generators[0].ifs and not node.generators[0].is_async and ( - isinstance(node.elt, ast.Name) - and isinstance(node.generators[0].target, ast.Name) - and node.elt.id == node.generators[0].target.id + ( + isinstance(node, (ast.ListComp, ast.SetComp)) + and isinstance(node.elt, ast.Name) + and isinstance(node.generators[0].target, ast.Name) + and node.elt.id == node.generators[0].target.id + ) + or ( + isinstance(node, ast.DictComp) + and isinstance(node.key, ast.Name) + and isinstance(node.value, ast.Name) + and isinstance(node.generators[0].target, ast.Tuple) + and tuple( + x.id + for x in node.generators[0].target.elts + if isinstance(x, ast.Name) + ) + == (node.key.id, node.value.id) + ) ) ): yield ( diff --git a/tests/test_flake8_comprehensions.py b/tests/test_flake8_comprehensions.py index 5e9626a..0924614 100644 --- a/tests/test_flake8_comprehensions.py +++ b/tests/test_flake8_comprehensions.py @@ -810,6 +810,20 @@ def test_C416_pass(code, flake8_path): + "rewrite using set().", ], ), + ( + "{x: y for x, y in zip(range(5), range(5))}", + [ + "./example.py:1:1: C416 Unnecessary dict comprehension - " + + "rewrite using dict().", + ], + ), + ( + "{x: y for (x, y) in zip(range(5), range(5))}", + [ + "./example.py:1:1: C416 Unnecessary dict comprehension - " + + "rewrite using dict().", + ], + ), ], ) def test_C416_fail(code, failures, flake8_path): From 112b0632d54f95f5a97a9a689c3a4fb7577032ef Mon Sep 17 00:00:00 2001 From: Adam Johnson Date: Sat, 18 Mar 2023 09:47:15 +0000 Subject: [PATCH 2/5] Add changelog note --- CHANGELOG.rst | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index f9ad2b0..eddb07d 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -2,6 +2,10 @@ Changelog ========= +* Expand C416 to ``dict`` comprehensions. + + Thanks to Aaron Gokaslan in `PR #490 `__. + 3.10.1 (2022-10-29) ------------------- From 20382d50c49b14f9c273bde6c61224c9caad20f4 Mon Sep 17 00:00:00 2001 From: Adam Johnson Date: Sat, 18 Mar 2023 09:47:25 +0000 Subject: [PATCH 3/5] Improve README --- README.rst | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/README.rst b/README.rst index 2e73c3b..277b333 100644 --- a/README.rst +++ b/README.rst @@ -151,13 +151,13 @@ For example: * Rewrite ``reversed(iterable[::-1])`` as ``iterable`` C416: Unnecessary ```` comprehension - rewrite using ````\(). ---------------------------------------------------------------------------------- +------------------------------------------------------------------------------------------- -It's unnecessary to use a list comprehension if the elements are unchanged. -The iterable should be wrapped in ``dict()``, ``list()``, or ``set()`` instead. +It's unnecessary to use a dict/list/set comprehension to build a data structure if the elements are unchanged. +Wrap the iterable with ``dict()``, ``list()``, or ``set()`` instead. For example: -* Rewrite ``{a:b for a, b in iterable}`` as ``dict(iterable)`` +* Rewrite ``{a: b for a, b in iterable}`` as ``dict(iterable)`` * Rewrite ``[x for x in iterable]`` as ``list(iterable)`` * Rewrite ``{x for x in iterable}`` as ``set(iterable)`` From 226707209eafa748ca2c72a65fc22001a7306dee Mon Sep 17 00:00:00 2001 From: Adam Johnson Date: Sat, 18 Mar 2023 09:47:35 +0000 Subject: [PATCH 4/5] Add some adversarial test cases and fix for them --- src/flake8_comprehensions/__init__.py | 11 +++++------ tests/test_flake8_comprehensions.py | 4 ++++ 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/flake8_comprehensions/__init__.py b/src/flake8_comprehensions/__init__.py index d6203e6..9f25550 100644 --- a/src/flake8_comprehensions/__init__.py +++ b/src/flake8_comprehensions/__init__.py @@ -312,12 +312,11 @@ def run(self) -> Generator[tuple[int, int, str, type[Any]], None, None]: and isinstance(node.key, ast.Name) and isinstance(node.value, ast.Name) and isinstance(node.generators[0].target, ast.Tuple) - and tuple( - x.id - for x in node.generators[0].target.elts - if isinstance(x, ast.Name) - ) - == (node.key.id, node.value.id) + and len(node.generators[0].target.elts) == 2 + and isinstance(node.generators[0].target.elts[0], ast.Name) + and node.generators[0].target.elts[0].id == node.key.id + and isinstance(node.generators[0].target.elts[1], ast.Name) + and node.generators[0].target.elts[1].id == node.value.id ) ) ): diff --git a/tests/test_flake8_comprehensions.py b/tests/test_flake8_comprehensions.py index 0924614..2d67eab 100644 --- a/tests/test_flake8_comprehensions.py +++ b/tests/test_flake8_comprehensions.py @@ -757,6 +757,10 @@ def test_C415_fail(code, failures, flake8_path): @pytest.mark.parametrize( "code", [ + "{x, y for x, y, z in zip('abc', '123', 'def')}", + "{y: x for x, y in zip('abc', '123')}", + "{x: y for x, (y,) in zip('a', ('1',))}", + "{x: z for x, (y,), z in zip('a', ('1',), 'b')}", "[str(x) for x in range(5)]", "[x + 1 for x in range(5)]", "[x for x in range(5) if x % 2]", From 695923ce95297057c3b9390a5d4fe61da311c090 Mon Sep 17 00:00:00 2001 From: Adam Johnson Date: Sat, 18 Mar 2023 09:47:50 +0000 Subject: [PATCH 5/5] Alphabetize test cases --- tests/test_flake8_comprehensions.py | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/tests/test_flake8_comprehensions.py b/tests/test_flake8_comprehensions.py index 2d67eab..72e5114 100644 --- a/tests/test_flake8_comprehensions.py +++ b/tests/test_flake8_comprehensions.py @@ -801,31 +801,31 @@ def test_C416_pass(code, flake8_path): "code,failures", [ ( - "[x for x in range(5)]", + "{x: y for x, y in zip(range(5), range(5))}", [ - f"./example.py:1:{1 + list_comp_col_offset}: C416 Unnecessary " - + "list comprehension - rewrite using list()." + "./example.py:1:1: C416 Unnecessary dict comprehension - " + + "rewrite using dict().", ], ), ( - "{x for x in range(5)}", + "{x: y for (x, y) in zip(range(5), range(5))}", [ - "./example.py:1:1: C416 Unnecessary set comprehension - " - + "rewrite using set().", + "./example.py:1:1: C416 Unnecessary dict comprehension - " + + "rewrite using dict().", ], ), ( - "{x: y for x, y in zip(range(5), range(5))}", + "[x for x in range(5)]", [ - "./example.py:1:1: C416 Unnecessary dict comprehension - " - + "rewrite using dict().", + f"./example.py:1:{1 + list_comp_col_offset}: C416 Unnecessary " + + "list comprehension - rewrite using list()." ], ), ( - "{x: y for (x, y) in zip(range(5), range(5))}", + "{x for x in range(5)}", [ - "./example.py:1:1: C416 Unnecessary dict comprehension - " - + "rewrite using dict().", + "./example.py:1:1: C416 Unnecessary set comprehension - " + + "rewrite using set().", ], ), ],