Repository: adamchainz/flake8-comprehensions (python)

Issue:
A false positive for C416
Hi,

I saw some fixes were committed for C416 false positives before, and I believe I've found a new one.

I was working on some code, which converts a list of 2-element sequences to a list of 2-tuples:

```python
from typing import List, Tuple

a_list_of_2lists: List[List[int]] = [[i, i + 1] for i in range(10)]

a_list_of_2tuples: List[Tuple[int, int]] = [(a, b) for a, b in a_list_of_2lists]
```

Incidentally, the code passes if I create a 3-tuple instead:

```python
a_list_of_3tuples: List[Tuple[int, int, str]] = [(a, b, 'foobar') for a, b in a_list_of_2lists]
```

Then I had this idea:
```
a_list_of_2tuples: List[Tuple[int, int]] = [tuple((a, b)) for a, b in a_list_of_2lists]
```
but that gave me a C409

Cheers

Pull request:
Remove unpacking check from C416
Fixes #204.

Patch:
diff --git a/HISTORY.rst b/HISTORY.rst
index 1798e6c..7cb2d31 100644
--- a/HISTORY.rst
+++ b/HISTORY.rst
@@ -2,6 +2,9 @@
 History
 =======
 
+* Remove the tuple/unpacking check from C416 to prevent false positives where
+  the type of the iterable is changed from some iterable to a tuple.
+
 3.1.3 (2019-11-19)
 ------------------
 
diff --git a/README.rst b/README.rst
index 7024917..04fe4e2 100644
--- a/README.rst
+++ b/README.rst
@@ -201,8 +201,4 @@ It's unnecessary to use a list comprehension if the elements are unchanged. The
 iterable should be wrapped in ``list()`` or ``set()`` instead. For example:
 
 * Rewrite ``[x for x in iterable]`` as ``list(iterable)``
-* Rewrite ``[(x, y) for x, y in iterable]`` as ``list(iterable)``
-* Rewrite ``[(x, y) for (x, y) in iterable]`` as ``list(iterable)``
 * Rewrite ``{x for x in iterable}`` as ``set(iterable)``
-* Rewrite ``{(x, y) for x, y in iterable}`` as ``set(iterable)``
-* Rewrite ``{(x, y) for (x, y) in iterable}`` as ``set(iterable)``
diff --git a/src/flake8_comprehensions.py b/src/flake8_comprehensions.py
index 27f8ae2..71a8d97 100644
--- a/src/flake8_comprehensions.py
+++ b/src/flake8_comprehensions.py
@@ -288,27 +288,9 @@ class ComprehensionChecker:
                     and not node.generators[0].ifs
                     and not is_async_generator(node.generators[0])
                     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.elt, ast.Tuple)
-                            and isinstance(node.generators[0].target, ast.Tuple)
-                            and (
-                                len(node.elt.elts)
-                                == len(node.generators[0].target.elts)
-                            )
-                            and all(
-                                isinstance(a, ast.Name)
-                                and isinstance(b, ast.Name)
-                                and a.id == b.id
-                                for a, b in zip(
-                                    node.elt.elts, node.generators[0].target.elts
-                                )
-                            )
-                        )
+                        isinstance(node.elt, ast.Name)
+                        and isinstance(node.generators[0].target, ast.Name)
+                        and node.elt.id == node.generators[0].target.id
                     )
                 ):
                     lookup = {ast.ListComp: "list", ast.SetComp: "set"}


Test patch:
diff --git a/tests/test_flake8_comprehensions.py b/tests/test_flake8_comprehensions.py
index e33c638..27db4c3 100644
--- a/tests/test_flake8_comprehensions.py
+++ b/tests/test_flake8_comprehensions.py
@@ -964,29 +964,35 @@ def test_C416_pass_4_tuples(flake8dir):
     assert result.out_lines == []
 
 
-def test_C416_fail_1(flake8dir):
+def test_C416_fail_5_unpacking(flake8dir):
+    # We can't assume unpacking came from tuples, so these examples should pass
     flake8dir.make_example_py(
         """
-        [x for x in range(5)]
         [(x, y) for x, y in zip('abc', '123')]
         [(x, y) for (x, y) in zip('abc', '123')]
-        {x for x in range(5)}
         {(x, y) for x, y in zip('abc', '123')}
         {(x, y) for (x, y) in zip('abc', '123')}
     """
     )
     result = flake8dir.run_flake8()
+    assert result.out_lines == []
+
+
+def test_C416_fail_1_list(flake8dir):
+    flake8dir.make_example_py("[x for x in range(5)]")
+    result = flake8dir.run_flake8()
     # Column offset for list comprehensions was incorrect in Python < 3.8.
     # See https://bugs.python.org/issue31241 for details.
     col_offset = 1 if sys.version_info >= (3, 8) else 2
     assert result.out_lines == [
         "./example.py:1:%d: C416 Unnecessary list comprehension - rewrite using list()."
         % col_offset,
-        "./example.py:2:%d: C416 Unnecessary list comprehension - rewrite using list()."
-        % col_offset,
-        "./example.py:3:%d: C416 Unnecessary list comprehension - rewrite using list()."
-        % col_offset,
-        "./example.py:4:1: C416 Unnecessary set comprehension - rewrite using set().",
-        "./example.py:5:1: C416 Unnecessary set comprehension - rewrite using set().",
-        "./example.py:6:1: C416 Unnecessary set comprehension - rewrite using set().",
+    ]
+
+
+def test_C416_fail_2_set(flake8dir):
+    flake8dir.make_example_py("{x for x in range(5)}")
+    result = flake8dir.run_flake8()
+    assert result.out_lines == [
+        "./example.py:1:1: C416 Unnecessary set comprehension - rewrite using set().",
     ]