Repository: adamchainz/flake8-comprehensions
Pull request #216 (2020-01-20 10:57:11)

Remove set comprehension check from C407

adamchainz: Thanks @pope1ni for spotting these in #215.

Patch:
diff --git a/HISTORY.rst b/HISTORY.rst
index 4189295..6e77bfb 100644
--- a/HISTORY.rst
+++ b/HISTORY.rst
@@ -2,6 +2,9 @@
 History
 =======
 
+* Remove check for set comprehensions in rule C407 as it would change the
+  results for certain builtins such as ``sum()``.
+
 3.2.0 (2020-01-20)
 ------------------
 
diff --git a/README.rst b/README.rst
index 59390fa..83eaa7b 100644
--- a/README.rst
+++ b/README.rst
@@ -47,7 +47,7 @@ C403 Unnecessary list comprehension - rewrite as a set comprehension.
 C404 Unnecessary list comprehension - rewrite as a dict comprehension.
 C405 Unnecessary (list/tuple) literal - rewrite as a set literal.
 C406 Unnecessary (list/tuple) literal - rewrite as a dict literal.
-C407 Unnecessary (dict/list/set) comprehension - '<builtin>' can take a generator.
+C407 Unnecessary (dict/list) comprehension - '<builtin>' can take a generator.
 C408 Unnecessary (dict/list/tuple) call - rewrite as a literal.
 C409 Unnecessary (list/tuple) passed to tuple() - (remove the outer call to tuple()/rewrite as a tuple literal).
 C410 Unnecessary (list/tuple) passed to list() - (remove the outer call to list()/rewrite as a list literal).
@@ -100,8 +100,8 @@ For example:
 * Rewrite ``dict([(1, 2)])`` or ``dict(((1, 2),))`` as ``{1: 2}``
 * Rewrite ``dict([])`` as ``{}``
 
-C407: Unnecessary (dict/list/set) comprehension - '<builtin>' can take a generator
-~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+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:
@@ -109,7 +109,7 @@ 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.bare for foo in foos)``
+  ``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)))``
 
@@ -153,7 +153,7 @@ It's unnecessary to pass a dict/list/set comprehension to 'in' that 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 (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
index dc3522e..c2b026f 100644
--- a/src/flake8_comprehensions.py
+++ b/src/flake8_comprehensions.py
@@ -138,9 +138,7 @@ class ComprehensionChecker:
                         node.func.id
                         in ("all", "any", "frozenset", "tuple", "max", "min", "sorted",)
                     )
-                    and isinstance(
-                        node.args[0], (ast.DictComp, ast.ListComp, ast.SetComp)
-                    )
+                    and isinstance(node.args[0], (ast.DictComp, ast.ListComp))
                 ):
 
                     yield (
@@ -156,9 +154,7 @@ class ComprehensionChecker:
                     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.DictComp, ast.ListComp, ast.SetComp)
-                    )
+                    and isinstance(node.args[0], (ast.DictComp, ast.ListComp))
                 ):
 
                     yield (
@@ -173,9 +169,7 @@ class ComprehensionChecker:
                 elif (
                     num_positional_args == 2
                     and node.func.id == "filter"
-                    and isinstance(
-                        node.args[1], (ast.DictComp, ast.ListComp, ast.SetComp)
-                    )
+                    and isinstance(node.args[1], (ast.DictComp, ast.ListComp))
                 ):
 
                     yield (
@@ -189,7 +183,7 @@ class ComprehensionChecker:
 
                 elif num_positional_args >= 2 and node.func.id == "map":
                     for arg in node.args[1:]:
-                        if isinstance(arg, (ast.DictComp, ast.ListComp, ast.SetComp)):
+                        if isinstance(arg, (ast.DictComp, ast.ListComp)):
                             yield (
                                 node.lineno,
                                 node.col_offset,


Test patch:
diff --git a/tests/test_flake8_comprehensions.py b/tests/test_flake8_comprehensions.py
index 14589e6..146c9fe 100644
--- a/tests/test_flake8_comprehensions.py
+++ b/tests/test_flake8_comprehensions.py
@@ -361,28 +361,25 @@ def test_C407_sum_pass_1(flake8dir):
     assert result.out_lines == []
 
 
-def test_C407_sum_fail_1(flake8dir):
+def test_C407_sum_pass_2(flake8dir):
     flake8dir.make_example_py(
         """
-        foo = sum([x + 1 for x in range(10)])
+        foo = sum({x % 2 for x in range(10)})
     """
     )
     result = flake8dir.run_flake8()
-    assert result.out_lines == [
-        "./example.py:1:7: C407 Unnecessary list comprehension - 'sum' can take "
-        + "a generator."
-    ]
+    assert result.out_lines == []
 
 
-def test_C407_sum_fail_2(flake8dir):
+def test_C407_sum_fail_1(flake8dir):
     flake8dir.make_example_py(
         """
-        foo = sum({x + 1 for x in range(10)})
+        foo = sum([x + 1 for x in range(10)])
     """
     )
     result = flake8dir.run_flake8()
     assert result.out_lines == [
-        "./example.py:1:7: C407 Unnecessary set comprehension - 'sum' can take "
+        "./example.py:1:7: C407 Unnecessary list comprehension - 'sum' can take "
         + "a generator."
     ]
 
@@ -395,24 +392,21 @@ def test_C407_filter_pass_1(flake8dir):
     assert result.out_lines == []
 
 
-def test_C407_filter_fail_1(flake8dir):
+def test_C407_filter_pass_2(flake8dir):
     flake8dir.make_example_py(
-        "filter(lambda x: x % 2 == 0, [x + 1 for x in range(10)])"
+        "filter(lambda x: x % 2 == 0, {x + 1 for x in range(10)})"
     )
     result = flake8dir.run_flake8()
-    assert result.out_lines == [
-        "./example.py:1:1: C407 Unnecessary list comprehension - 'filter' can take "
-        + "a generator."
-    ]
+    assert result.out_lines == []
 
 
-def test_C407_filter_fail_2(flake8dir):
+def test_C407_filter_fail_1(flake8dir):
     flake8dir.make_example_py(
-        "filter(lambda x: x % 2 == 0, {x + 1 for x in range(10)})"
+        "filter(lambda x: x % 2 == 0, [x + 1 for x in range(10)])"
     )
     result = flake8dir.run_flake8()
     assert result.out_lines == [
-        "./example.py:1:1: C407 Unnecessary set comprehension - 'filter' can take "
+        "./example.py:1:1: C407 Unnecessary list comprehension - 'filter' can take "
         + "a generator."
     ]
 
@@ -423,6 +417,12 @@ def test_C407_map_pass_1(flake8dir):
     assert result.out_lines == []
 
 
+def test_C407_map_pass_2(flake8dir):
+    flake8dir.make_example_py("map(lambda x: x + 1, {x + 1 for x in range(10)})")
+    result = flake8dir.run_flake8()
+    assert result.out_lines == []
+
+
 def test_C407_map_fail_1(flake8dir):
     flake8dir.make_example_py("map(lambda x: x + 1, [x + 1 for x in range(10)])")
     result = flake8dir.run_flake8()
@@ -443,15 +443,6 @@ def test_C407_map_fail_2(flake8dir):
     ]
 
 
-def test_C407_map_fail_3(flake8dir):
-    flake8dir.make_example_py("map(lambda x: x + 1, {x + 1 for x in range(10)})")
-    result = flake8dir.run_flake8()
-    assert result.out_lines == [
-        "./example.py:1:1: C407 Unnecessary set comprehension - 'map' can take "
-        + "a generator."
-    ]
-
-
 def test_C407_max_pass_1(flake8dir):
     flake8dir.make_example_py("max(x + 1 for x in range(10))")
     result = flake8dir.run_flake8()
@@ -472,6 +463,12 @@ def test_C407_max_pass_3(flake8dir):
     assert result.out_lines == []
 
 
+def test_C407_max_pass_4(flake8dir):
+    flake8dir.make_example_py("max({x + 1 for x in range(10)})")
+    result = flake8dir.run_flake8()
+    assert result.out_lines == []
+
+
 def test_C407_max_fail_1(flake8dir):
     flake8dir.make_example_py("max([x + 1 for x in range(10)])")
     result = flake8dir.run_flake8()
@@ -490,15 +487,6 @@ def test_C407_max_fail_2(flake8dir):
     ]
 
 
-def test_C407_max_fail_3(flake8dir):
-    flake8dir.make_example_py("max({x + 1 for x in range(10)})")
-    result = flake8dir.run_flake8()
-    assert result.out_lines == [
-        "./example.py:1:1: C407 Unnecessary set comprehension - 'max' can take "
-        + "a generator."
-    ]
-
-
 def test_C407_enumerate_pass_1(flake8dir):
     flake8dir.make_example_py("enumerate(1 for i in range(10))")
     result = flake8dir.run_flake8()