Drop rules C407 and C412 (#312)

Fixes #247.
adamchainz · Mar 18, 2021 · 4f1b7e6 · 4f1b7e6
1 parent c2b025d
commit 4f1b7e6
Show file tree

Hide file tree

Showing 4 changed files with 11 additions and 404 deletions.
diff --git a/HISTORY.rst b/HISTORY.rst
@@ -2,6 +2,17 @@
 History
 =======
 
+* Remove rules C407 (Unnecessary ``<dict/list>`` comprehension - ``<builtin>``
+  can take a generator) and C412 (Unnecessary ``<dict/list/set>`` comprehension
+  - 'in' can take a generator). Both rules recommended increasing laziness,
+  which is not always desirable and can lead to subtle bugs. Also, a fully
+  exhausted generator is slower than an equivalent comprehension, so the advice
+  did not always improve performance.
+
+  Thanks to David Smith, Dylan Young, and Leonidas Loucas for the report in
+  `Issue #247
+  <https://github.com/adamchainz/flake8-comprehensions/issues/247>`__.
+
 3.3.1 (2020-12-19)
 ------------------
 

diff --git a/README.rst b/README.rst
@@ -88,30 +88,6 @@ For example:
 * Rewrite ``dict(((1, 2),))`` as ``{1: 2}``
 * Rewrite ``dict([])`` as ``{}``
 
-C407: Unnecessary ``<dict/list>`` comprehension - ``<builtin>`` can take a generator
-------------------------------------------------------------------------------------
-
-It's unnecessary to pass a list comprehension to some builtins that can take generators instead.
-For example:
-
-* Rewrite ``sum([x ** 2 for x in range(10)])`` as ``sum(x ** 2 for x in range(10))``
-* Rewrite ``all([foo.bar for foo in foos])`` as ``all(foo.bar for foo in foos)``
-* Rewrite ``filter(lambda x: x % 2 == 0, [x ** 3 for x in range(10)])`` as ``filter(lambda x: x % 2 == 0, (x ** 3 for x in range(10)))``
-
-The list of builtins that are checked for are:
-
-* ``all``
-* ``any``
-* ``enumerate``
-* ``filter``
-* ``frozenset``
-* ``map``
-* ``max``
-* ``min``
-* ``sorted``
-* ``sum``
-* ``tuple``
-
 C408: Unnecessary ``<dict/list/tuple>`` call - rewrite as a literal.
 --------------------------------------------------------------------
 
@@ -145,15 +121,6 @@ For example:
 
 * Rewrite ``list([f(x) for x in foo])`` as ``[f(x) for x in foo]``
 
-C412: Unnecessary ``<dict/list/set>`` comprehension - 'in' can take a generator.
---------------------------------------------------------------------------------
-
-It's unnecessary to pass a ``dict``/``list``/``set`` comprehension to 'in', as it can take a generator instead.
-For example:
-
-* Rewrite ``y in [f(x) for x in foo]`` as ``y in (f(x) for x in foo)``
-* Rewrite ``y in {x ** 2 for x in foo}`` as ``y in (x ** 2 for x in foo)``
-
 C413: Unnecessary ``<list/reversed>`` call around sorted().
 -----------------------------------------------------------
 

diff --git a/src/flake8_comprehensions.py b/src/flake8_comprehensions.py
@@ -30,12 +30,10 @@ def __init__(self, tree, *args, **kwargs):
         ),
         "C405": "C405 Unnecessary {type} literal - ",
         "C406": "C406 Unnecessary {type} literal - ",
-        "C407": "C407 Unnecessary list comprehension - '{func}' can take a generator.",
         "C408": "C408 Unnecessary {type} call - rewrite as a literal.",
         "C409": "C409 Unnecessary {type} passed to tuple() - ",
         "C410": "C410 Unnecessary {type} passed to list() - ",
         "C411": "C411 Unnecessary list call - remove the outer call to list().",
-        "C412": "C412 Unnecessary {type} comprehension - 'in' can take a generator.",
         "C413": "C413 Unnecessary {outer} call around {inner}(){remediation}.",
         "C414": "C414 Unnecessary {inner} call within {outer}().",
         "C415": "C415 Unnecessary subscript reversal of iterable within {func}().",
@@ -132,76 +130,6 @@ def run(self):
                         type(self),
                     )
 
-                elif (
-                    num_positional_args == 1
-                    # These take 1 positional argument + some keyword arguments
-                    and (
-                        node.func.id
-                        in (
-                            "all",
-                            "any",
-                            "frozenset",
-                            "tuple",
-                            "max",
-                            "min",
-                            "sorted",
-                        )
-                    )
-                    and isinstance(node.args[0], ast.ListComp)
-                ):
-
-                    yield (
-                        node.lineno,
-                        node.col_offset,
-                        self.messages["C407"].format(func=node.func.id),
-                        type(self),
-                    )
-
-                elif (
-                    num_positional_args in (1, 2)
-                    # These can take a second positional argument
-                    and (
-                        node.func.id
-                        in (
-                            "enumerate",
-                            "sum",
-                        )
-                    )
-                    and isinstance(node.args[0], ast.ListComp)
-                ):
-
-                    yield (
-                        node.lineno,
-                        node.col_offset,
-                        self.messages["C407"].format(func=node.func.id),
-                        type(self),
-                    )
-
-                elif (
-                    num_positional_args == 2
-                    and node.func.id == "filter"
-                    and isinstance(node.args[1], ast.ListComp)
-                ):
-
-                    yield (
-                        node.lineno,
-                        node.col_offset,
-                        self.messages["C407"].format(func=node.func.id),
-                        type(self),
-                    )
-
-                elif (
-                    num_positional_args >= 2
-                    and node.func.id == "map"
-                    and any(isinstance(a, ast.ListComp) for a in node.args[1:])
-                ):
-                    yield (
-                        node.lineno,
-                        node.col_offset,
-                        self.messages["C407"].format(func=node.func.id),
-                        type(self),
-                    )
-
                 elif (
                     num_positional_args == 0
                     and not has_star_args(node)
@@ -312,24 +240,6 @@ def run(self):
                         type(self),
                     )
 
-            elif isinstance(node, ast.Compare):
-                if (
-                    len(node.ops) == 1
-                    and isinstance(node.ops[0], ast.In)
-                    and len(node.comparators) == 1
-                    and isinstance(
-                        node.comparators[0], (ast.DictComp, ast.ListComp, ast.SetComp)
-                    )
-                ):
-                    yield (
-                        node.lineno,
-                        node.col_offset,
-                        self.messages["C412"].format(
-                            type=comp_type[node.comparators[0].__class__]
-                        ),
-                        type(self),
-                    )
-
             elif isinstance(node, (ast.ListComp, ast.SetComp)):
                 if (
                     len(node.generators) == 1