Merge unused-variable-flakes-2718-2

Author: jml
Reviewer: exarkun
Fixes: #2718

Add support for detecting local variables which are bound but then never
used.

Author: exarkun

pyflakes/checker.py

@@ -8,25 +8,37 @@
 from pyflakes import messages
 
 
+
 class Binding(object):
     """
+    Represents the binding of a value to a name.
+
+    The checker uses this to keep track of which names have been bound and
+    which names have not. See L{Assignment} for a special type of binding that
+    is checked with stricter rules.
+
     @ivar used: pair of (L{Scope}, line-number) indicating the scope and
                 line number that this binding was last used
     """
+
     def __init__(self, name, source):
         self.name = name
         self.source = source
         self.used = False
 
+
     def __str__(self):
         return self.name
 
+
     def __repr__(self):
         return '<%s object %r from line %r at 0x%x>' % (self.__class__.__name__,
                                                         self.name,
                                                         self.source.lineno,
                                                         id(self))
 
+
+
 class UnBinding(Binding):
     '''Created by the 'del' operator.'''
 
@@ -35,22 +47,44 @@ def __init__(self, name, source):
         name = name.split('.')[0]
         super(Importation, self).__init__(name, source)
 
+
+
+class Argument(Binding):
+    """
+    Represents binding a name as an argument.
+    """
+
+
+
 class Assignment(Binding):
-    pass
+    """
+    Represents binding a name with an explicit assignment.
+
+    The checker will raise warnings for any Assignment that isn't used. Also,
+    the checker does not consider assignments in tuple/list unpacking to be
+    Assignments, rather it treats them as simple Bindings.
+    """
+
+
 
 class FunctionDefinition(Binding):
     pass
 
 
+
 class Scope(dict):
     importStarred = False       # set to True when import * is found
 
+
     def __repr__(self):
         return '<%s at 0x%x %s>' % (self.__class__.__name__, id(self), dict.__repr__(self))
 
+
     def __init__(self):
         super(Scope, self).__init__()
 
+
+
 class ClassScope(Scope):
     pass
 
@@ -72,41 +106,77 @@ class ModuleScope(Scope):
     pass
 
 
-
 # Globally defined names which are not attributes of the __builtin__ module.
 _MAGIC_GLOBALS = ['__file__', '__builtins__']
 
 
 
 class Checker(object):
+    """
+    I check the cleanliness and sanity of Python code.
+
+    @ivar _deferredFunctions: Tracking list used by L{deferFunction}.  Elements
+        of the list are two-tuples.  The first element is the callable passed
+        to L{deferFunction}.  The second element is a copy of the scope stack
+        at the time L{deferFunction} was called.
+
+    @ivar _deferredAssignments: Similar to C{_deferredFunctions}, but for
+        callables which are deferred assignment checks.
+    """
+
     nodeDepth = 0
     traceTree = False
 
     def __init__(self, tree, filename='(none)'):
-        self.deferred = []
+        self._deferredFunctions = []
+        self._deferredAssignments = []
         self.dead_scopes = []
         self.messages = []
         self.filename = filename
         self.scopeStack = [ModuleScope()]
         self.futuresAllowed = True
-
         self.handleChildren(tree)
-        for handler, scope in self.deferred:
-            self.scopeStack = scope
-            handler()
+        self._runDeferred(self._deferredFunctions)
+        # Set _deferredFunctions to None so that deferFunction will fail
+        # noisily if called after we've run through the deferred functions.
+        self._deferredFunctions = None
+        self._runDeferred(self._deferredAssignments)
+        # Set _deferredAssignments to None so that deferAssignment will fail
+        # noisly if called after we've run through the deferred assignments.
+        self._deferredAssignments = None
         del self.scopeStack[1:]
         self.popScope()
         self.check_dead_scopes()
 
-    def defer(self, callable):
-        '''Schedule something to be called after just before completion.
+
+    def deferFunction(self, callable):
+        '''
+        Schedule a function handler to be called just before completion.
 
         This is used for handling function bodies, which must be deferred
         because code later in the file might modify the global scope. When
         `callable` is called, the scope at the time this is called will be
         restored, however it will contain any new bindings added to it.
         '''
-        self.deferred.append( (callable, self.scopeStack[:]) )
+        self._deferredFunctions.append((callable, self.scopeStack[:]))
+
+
+    def deferAssignment(self, callable):
+        """
+        Schedule an assignment handler to be called just after deferred
+        function handlers.
+        """
+        self._deferredAssignments.append((callable, self.scopeStack[:]))
+
+
+    def _runDeferred(self, deferred):
+        """
+        Run the callables in C{deferred} using their associated scope stack.
+        """
+        for handler, scope in deferred:
+            self.scopeStack = scope
+            handler()
+
 
     def scope(self):
         return self.scopeStack[-1]
@@ -132,9 +202,10 @@ def report(self, messageClass, *args, **kwargs):
 
     def handleChildren(self, tree):
         for node in tree.getChildNodes():
-            self.handleNode(node)
+            self.handleNode(node, tree)
 
-    def handleNode(self, node):
+    def handleNode(self, node, parent):
+        node.parent = parent
         if self.traceTree:
             print '  ' * self.nodeDepth + node.__class__.__name__
         self.nodeDepth += 1
@@ -208,7 +279,7 @@ def WITH(self, node):
         # Of course these are assignments, not references, so we have to
         # handle them as a special case here.
 
-        self.handleNode(node.expr)
+        self.handleNode(node.expr, node)
 
         if isinstance(node.vars, ast.AssTuple):
             varNodes = node.vars.nodes
@@ -232,8 +303,8 @@ def GLOBAL(self, node):
 
     def LISTCOMP(self, node):
         for qual in node.quals:
-            self.handleNode(qual)
-        self.handleNode(node.expr)
+            self.handleNode(qual, node)
+        self.handleNode(node.expr, node)
 
     GENEXPRINNER = LISTCOMP
 
@@ -305,7 +376,7 @@ def FUNCTION(self, node):
 
     def LAMBDA(self, node):
         for default in node.defaults:
-            self.handleNode(default)
+            self.handleNode(default, node)
 
         def runFunction():
             args = []
@@ -322,11 +393,21 @@ def addArgs(arglist):
             self.pushFunctionScope()
             addArgs(node.argnames)
             for name in args:
-                self.addBinding(node.lineno, Assignment(name, node), reportRedef=False)
-            self.handleNode(node.code)
+                self.addBinding(node.lineno, Argument(name, node), reportRedef=False)
+            self.handleNode(node.code, node)
+            def checkUnusedAssignments():
+                """
+                Check to see if any assignments have not been used.
+                """
+                for name, binding in self.scope.iteritems():
+                    if (not binding.used and not name in self.scope.globals
+                        and isinstance(binding, Assignment)):
+                        self.report(messages.UnusedVariable,
+                                    binding.source.lineno, name)
+            self.deferAssignment(checkUnusedAssignments)
             self.popScope()
 
-        self.defer(runFunction)
+        self.deferFunction(runFunction)
 
 
     def CLASS(self, node):
@@ -337,9 +418,9 @@ def CLASS(self, node):
         """
         if getattr(node, "decorators", None) is not None:
             self.handleChildren(node.decorators)
-        self.addBinding(node.lineno, Assignment(node.name, node))
+        self.addBinding(node.lineno, Binding(node.name, node))
         for baseNode in node.bases:
-            self.handleNode(baseNode)
+            self.handleNode(baseNode, node)
         self.pushClassScope()
         self.handleChildren(node.code)
         self.popScope()
@@ -372,12 +453,20 @@ def ASSNAME(self, node):
                                     scope[node.name].source.lineno)
                         break
 
-            self.addBinding(node.lineno, Assignment(node.name, node))
+            if isinstance(node.parent,
+                          (ast.For, ast.ListCompFor, ast.GenExprFor,
+                           ast.AssTuple, ast.AssList)):
+                binding = Binding(node.name, node)
+            else:
+                binding = Assignment(node.name, node)
+            if node.name in self.scope:
+                binding.used = self.scope[node.name].used
+            self.addBinding(node.lineno, binding)
 
     def ASSIGN(self, node):
-        self.handleNode(node.expr)
+        self.handleNode(node.expr, node)
         for subnode in node.nodes[::-1]:
-            self.handleNode(subnode)
+            self.handleNode(subnode, node)
 
     def IMPORT(self, node):
         for name, alias in node.names:

pyflakes/messages.py

@@ -71,3 +71,15 @@ class LateFutureImport(Message):
     def __init__(self, filename, lineno, names):
         Message.__init__(self, filename, lineno)
         self.message_args = (names,)
+
+
+class UnusedVariable(Message):
+    """
+    Indicates that a variable has been explicity assigned to but not actually
+    used.
+    """
+
+    message = 'local variable %r is assigned to but never used'
+    def __init__(self, filename, lineno, names):
+        Message.__init__(self, filename, lineno)
+        self.message_args = (names,)

pyflakes/test/test_imports.py

@@ -401,6 +401,7 @@ def test_nonGlobalDoesNotRedefine(self):
         import fu
         def a():
             fu = 3
+            return fu
         fu
         ''')