From 4417b7730c3e42087d3adb56d13bd4756d05b0bc Mon Sep 17 00:00:00 2001 From: Florent Xicluna Date: Sun, 30 Mar 2014 04:22:34 +0200 Subject: Report undefined name for literal tuple unpacking; fixes lp:879941 --- NEWS.txt | 2 ++ pyflakes/checker.py | 29 ++++++++++++++++++++-------- pyflakes/test/test_other.py | 24 ++++++++++++++++++++++- pyflakes/test/test_undefined_names.py | 36 +++++++++++++++++++++++++++++++++++ 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 @@ -279,6 +279,42 @@ class Test(TestCase): print(a, b, c) ''') + @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.""" -- cgit v1.2.1