Limit differentForks to the current scope

Alternate if & try forks are always in the same scope.

Rearranging the code a little allows for differentForks
to only be called on redefinitions within the same scope,
and inside differentForks the use of getCommonAncestor
can be limited to finding ancestors only when in current
scope.

Author: jayvdb

pyflakes/checker.py

@@ -394,9 +394,13 @@ def __init__(self, name, source, scope):
 class Scope(dict):
     importStarred = False       # set to True when import * is found
 
+    def __init__(self, node):
+        self.node = node
+
     def __repr__(self):
         scope_cls = self.__class__.__name__
-        return '<%s at 0x%x %s>' % (scope_cls, id(self), dict.__repr__(self))
+        return '<%s at 0x%x for %r containing: %s>' % (
+            scope_cls, id(self), self.node, dict.__repr__(self))
 
 
 class ClassScope(Scope):
@@ -413,8 +417,8 @@ class FunctionScope(Scope):
     alwaysUsed = {'__tracebackhide__', '__traceback_info__',
                   '__traceback_supplement__'}
 
-    def __init__(self):
-        super(FunctionScope, self).__init__()
+    def __init__(self, node):
+        super(FunctionScope, self).__init__(node)
         # Simplify: manage the special locals as globals
         self.globals = self.alwaysUsed.copy()
         self.returnValue = None     # First non-empty return
@@ -491,7 +495,8 @@ def __init__(self, tree, filename='(none)', builtins=None,
         if builtins:
             self.builtIns = self.builtIns.union(builtins)
         self.withDoctest = withDoctest
-        self.scopeStack = [ModuleScope()]
+        tree.depth = self.nodeDepth
+        self.scopeStack = [ModuleScope(tree)]
         self.exceptHandlers = [()]
         self.root = tree
         self.handleChildren(tree)
@@ -609,8 +614,8 @@ def checkDeadScopes(self):
                             messg = messages.RedefinedWhileUnused
                         self.report(messg, node, value.name, value.source)
 
-    def pushScope(self, scopeClass=FunctionScope):
-        self.scopeStack.append(scopeClass())
+    def pushScope(self, scopeClass=FunctionScope, node=None):
+        self.scopeStack.append(scopeClass(node))
 
     def report(self, messageClass, *args, **kwargs):
         self.messages.append(messageClass(self.filename, *args, **kwargs))
@@ -629,6 +634,9 @@ def getCommonAncestor(self, lnode, rnode, stop):
         if lnode is rnode:
             return lnode
 
+        if stop.depth in (lnode.depth, rnode.depth):
+            return None
+
         if (lnode.depth > rnode.depth):
             return self.getCommonAncestor(lnode.parent, rnode, stop)
         if (lnode.depth < rnode.depth):
@@ -643,7 +651,8 @@ def descendantOf(self, node, ancestors, stop):
 
     def differentForks(self, lnode, rnode):
         """True, if lnode and rnode are located on different forks of IF/TRY"""
-        ancestor = self.getCommonAncestor(lnode, rnode, self.root)
+        ancestor = self.getCommonAncestor(lnode, rnode,
+                                          self.scope.node or self.root)
         parts = getAlternatives(ancestor)
         if parts:
             for items in parts:
@@ -665,15 +674,19 @@ def addBinding(self, node, value):
                 break
         existing = scope.get(value.name)
 
-        if existing and not self.differentForks(node, existing.source):
+        if existing:
 
             parent_stmt = self.getParent(value.source)
             if isinstance(existing, Importation) and isinstance(parent_stmt, ast.For):
                 self.report(messages.ImportShadowedByLoopVar,
                             node, value.name, existing.source)
 
             elif scope is self.scope:
-                if (isinstance(parent_stmt, ast.comprehension) and
+                if self.differentForks(node, existing.source):
+                    # ignore redefinitions in different forks of `if` & `try`
+                    pass
+
+                elif (isinstance(parent_stmt, ast.comprehension) and
                         not isinstance(self.getParent(existing.source),
                                        (ast.For, ast.comprehension))):
                     self.report(messages.RedefinedInListComp,
@@ -903,7 +916,7 @@ def handleDoctests(self, node):
         saved_stack = self.scopeStack
         self.scopeStack = [self.scopeStack[0]]
         node_offset = self.offset or (0, 0)
-        self.pushScope(DoctestScope)
+        self.pushScope(DoctestScope, node)
         underscore_in_builtins = '_' in self.builtIns
         if not underscore_in_builtins:
             self.builtIns.add('_')
@@ -1079,7 +1092,7 @@ def GLOBAL(self, node):
     NONLOCAL = GLOBAL
 
     def GENERATOREXP(self, node):
-        self.pushScope(GeneratorScope)
+        self.pushScope(GeneratorScope, node)
         self.handleChildren(node)
         self.popScope()
 
@@ -1219,7 +1232,7 @@ def addArgs(arglist):
 
         def runFunction():
 
-            self.pushScope()
+            self.pushScope(FunctionScope, node)
             for name in args:
                 self.addBinding(node, Argument(name, node))
             if isinstance(node.body, list):
@@ -1265,7 +1278,7 @@ def CLASSDEF(self, node):
         if not PY2:
             for keywordNode in node.keywords:
                 self.handleNode(keywordNode, node)
-        self.pushScope(ClassScope)
+        self.pushScope(ClassScope, node)
         # doctest does not process doctest within a doctest
         # classes within classes are processed.
         if (self.withDoctest and