Report undefined name for literal tuple unpacking; fixes lp:879941

4 files changed, 82 insertions, 9 deletions

diff --git a/NEWS.txt b/NEWS.txt
index 726f045..fcfa2bf 100644
--- a/NEWS.txt
+++ b/NEWS.txt
@@ -4,6 +4,8 @@ UNRELEASED:
     global name is used elsewhere in the module.
   - Catch undefined variable in loop generator when it is also used as
     loop variable.
+  - Report undefined name for `(a, b) = (1, 2)` but not for the general
+    unpacking `(a, b) = func()`.
 
 0.8.0 (2014-03-22):
   - Adapt for the AST in Python 3.4.
diff --git a/pyflakes/checker.py b/pyflakes/checker.py
index e4b5b1e..993a4fc 100644
--- a/pyflakes/checker.py
+++ b/pyflakes/checker.py
@@ -369,11 +369,14 @@ class Checker(object):
     def report(self, messageClass, *args, **kwargs):
         self.messages.append(messageClass(self.filename, *args, **kwargs))
 
-    def hasParent(self, node, kind):
+    def getParent(self, node):
         while True:
             node = node.parent
-            if not hasattr(node, 'elts'):
-                return isinstance(node, kind)
+            if not hasattr(node, 'elts') and not hasattr(node, 'ctx'):
+                return node
+
+    def hasParent(self, node, kind):
+        return isinstance(self.getParent(node), kind)
 
     def getCommonAncestor(self, lnode, rnode, stop=None):
         if not stop:
@@ -518,12 +521,15 @@ class Checker(object):
                                 scope[name].used[1], name, scope[name].source)
                     break
 
-        parent = getattr(node, 'parent', None)
-        if isinstance(parent, (ast.For, ast.comprehension, ast.Tuple, ast.List)):
+        parent_stmt = self.getParent(node)
+        # import pdb; pdb.set_trace()
+
+        if isinstance(parent_stmt, (ast.For, ast.comprehension)) or (
+                parent_stmt != node.parent and
+                not self.isLiteralTupleUnpacking(parent_stmt)):
             binding = Binding(name, node)
-        elif (parent is not None and name == '__all__' and
-              isinstance(self.scope, ModuleScope)):
-            binding = ExportBinding(name, parent, self.scope)
+        elif name == '__all__' and isinstance(self.scope, ModuleScope):
+            binding = ExportBinding(name, node.parent, self.scope)
         else:
             binding = Assignment(name, node)
         if name in self.scope:
@@ -546,6 +552,13 @@ class Checker(object):
         for node in iter_child_nodes(tree):
             self.handleNode(node, tree)
 
+    def isLiteralTupleUnpacking(self, node):
+        if isinstance(node, ast.Assign):
+            for child in node.targets + [node.value]:
+                if not hasattr(child, 'elts'):
+                    return False
+            return True
+
     def isDocstring(self, node):
         """
         Determine if the given node is a docstring, as long as it is at the
diff --git a/pyflakes/test/test_other.py b/pyflakes/test/test_other.py
index 54cc3c9..ff8716b 100644
--- a/pyflakes/test/test_other.py
+++ b/pyflakes/test/test_other.py
@@ -591,18 +591,40 @@ class TestUnusedAssignment(TestCase):
         in good Python code, so warning will only create false positives.
         """
         self.flakes('''
+        def f(tup):
+            (x, y) = tup
+        ''')
+        self.flakes('''
         def f():
             (x, y) = 1, 2
+        ''', m.UnusedVariable, m.UnusedVariable)
+        self.flakes('''
+        def f():
+            (x, y) = coords = 1, 2
+            if x > 1:
+                print(coords)
         ''')
+        self.flakes('''
+        def f():
+            (x, y) = coords = 1, 2
+        ''', m.UnusedVariable)
+        self.flakes('''
+        def f():
+            coords = (x, y) = 1, 2
+        ''', m.UnusedVariable)
 
     def test_listUnpacking(self):
         """
         Don't warn when a variable included in list unpacking is unused.
         """
         self.flakes('''
+        def f(tup):
+            [x, y] = tup
+        ''')
+        self.flakes('''
         def f():
             [x, y] = [1, 2]
-        ''')
+        ''', m.UnusedVariable, m.UnusedVariable)
 
     def test_closedOver(self):
         """
diff --git a/pyflakes/test/test_undefined_names.py b/pyflakes/test/test_undefined_names.py
index 6d731ee..29627b7 100644
--- a/pyflakes/test/test_undefined_names.py
+++ b/pyflakes/test/test_undefined_names.py
@@ -280,6 +280,42 @@ class Test(TestCase):
         ''')
 
     @skipIf(version_info < (3,), 'new in Python 3')
+    def test_usedAsStarUnpack(self):
+        """
+        Star names in unpack are used if RHS is not a tuple/list literal.
+        """
+        self.flakes('''
+        def f():
+            a, *b = range(10)
+        ''')
+        self.flakes('''
+        def f():
+            (*a, b) = range(10)
+        ''')
+        self.flakes('''
+        def f():
+            [a, *b, c] = range(10)
+        ''')
+
+    @skipIf(version_info < (3,), 'new in Python 3')
+    def test_unusedAsStarUnpack(self):
+        """
+        Star names in unpack are unused if RHS is a tuple/list literal.
+        """
+        self.flakes('''
+        def f():
+            a, *b = any, all, 4, 2, 'un'
+        ''', m.UnusedVariable, m.UnusedVariable)
+        self.flakes('''
+        def f():
+            (*a, b) = [bool, int, float, complex]
+        ''', m.UnusedVariable, m.UnusedVariable)
+        self.flakes('''
+        def f():
+            [a, *b, c] = 9, 8, 7, 6, 5, 4
+        ''', m.UnusedVariable, m.UnusedVariable, m.UnusedVariable)
+
+    @skipIf(version_info < (3,), 'new in Python 3')
     def test_keywordOnlyArgs(self):
         """Keyword-only arg names are defined."""
         self.flakes('''