Rewrite the comprehension-escape and exception-escape to work only on the corresponding nodes

These two checks were implemented in terms of visit_namne, that is, for every name in the tree,
we looked to see if that name leaked. This was resulting in a couple of false positives
and also in a performance issue, since we were calling nodes_of_class() and scope() for each
name node. Instead, this approach uses the visit methods for exception and comprehension nodes
and looks to see from there if the current scope uses leaked names.
This is not the ideal situation as well, ideal would be to have access to the definition
frame of each name, but that requires some extra engineering effort in astroid to get it right.
Close #2106

2 files changed, 73 insertions, 48 deletions

diff --git a/pylint/checkers/python3.py b/pylint/checkers/python3.py
index f2efa18d1..da7f58eb6 100644
--- a/pylint/checkers/python3.py
+++ b/pylint/checkers/python3.py
@@ -627,6 +627,34 @@ class Python3Checker(checkers.BaseChecker):
             if isinstance(arg, astroid.Tuple):
                 self.add_message('parameter-unpacking', node=arg)
 
+    @utils.check_messages('comprehension-escape')
+    def visit_listcomp(self, node):
+        names = {
+            generator.target.name for generator in node.generators
+            if isinstance(generator.target, astroid.AssignName)
+        }
+        scope = node.parent.scope()
+        scope_names = scope.nodes_of_class(
+            astroid.Name,
+            skip_klass=astroid.FunctionDef,
+        )
+        has_redefined_assign_name = any(
+            assign_name
+            for assign_name in
+            scope.nodes_of_class(
+                astroid.AssignName,
+                skip_klass=astroid.FunctionDef,
+            )
+            if assign_name.name in names and assign_name.lineno > node.lineno
+        )
+        if has_redefined_assign_name:
+            return
+        scope_names = list(scope_names)
+        for scope_name in scope_names:
+            if scope_name.name not in names or scope_name.lineno <= node.lineno:
+                continue
+            self.add_message('comprehension-escape', node=scope_name)
+
     def visit_name(self, node):
         """Detect when a "bad" built-in is referenced."""
         found_node, _ = node.lookup(node.name)
@@ -635,38 +663,6 @@ class Python3Checker(checkers.BaseChecker):
                 message = node.name.lower() + '-builtin'
                 self.add_message(message, node=node)
 
-        # On Python 3 we don't find the leaked objects as
-        # they are already deleted, so instead look for them manually.
-        scope = node.scope()
-        assigned = None
-        for node_ in scope.nodes_of_class(astroid.AssignName):
-            if node_.name == node.name and node_.lineno < node.lineno:
-                assigned = node_
-        if not assigned:
-            return
-
-        assign_statement = assigned.statement()
-        assigns = (node_ for node_ in scope.nodes_of_class(astroid.Assign)
-                   if node_.lineno < node.lineno)
-
-        for assign in assigns:
-            for target in assign.targets:
-                if getattr(target, 'name', None) == node.name:
-                    return
-
-        if isinstance(assign_statement, astroid.ExceptHandler):
-            current = node
-            while current and not isinstance(current.parent, astroid.ExceptHandler):
-                current = current.parent
-            if current and isinstance(current.parent, astroid.ExceptHandler):
-                return
-            self.add_message('exception-escape', node=node)
-
-        if isinstance(assign_statement, (astroid.Expr, astroid.Assign)):
-            if (isinstance(assign_statement.value, astroid.ListComp)
-                    and not assign_statement.parent_of(node)):
-                self.add_message('comprehension-escape', node=node)
-
     @utils.check_messages('print-statement')
     def visit_print(self, node):
         self.add_message('print-statement', node=node, always_warn=True)
@@ -903,11 +899,38 @@ class Python3Checker(checkers.BaseChecker):
         except astroid.InferenceError:
             return
 
-    @utils.check_messages('unpacking-in-except')
+    @utils.check_messages('unpacking-in-except', 'comprehension-escape')
     def visit_excepthandler(self, node):
         """Visit an except handler block and check for exception unpacking."""
         if isinstance(node.name, (astroid.Tuple, astroid.List)):
             self.add_message('unpacking-in-except', node=node)
+            return
+
+        # Find any names
+        scope = node.parent.scope()
+        scope_names = scope.nodes_of_class(
+            astroid.Name,
+            skip_klass=astroid.FunctionDef,
+        )
+        scope_names = list(scope_names)
+        potential_leaked_names = [
+            scope_name
+            for scope_name in scope_names
+            if scope_name.name == node.name.name and scope_name.lineno > node.lineno
+        ]
+        reassignments_for_same_name = {
+            assign_name.lineno
+            for assign_name in
+            scope.nodes_of_class(
+                astroid.AssignName,
+                skip_klass=astroid.FunctionDef,
+            )
+            if assign_name.name == node.name.name
+        }
+        for leaked_name in potential_leaked_names:
+            if any(node.lineno < elem < leaked_name.lineno for elem in reassignments_for_same_name):
+                continue
+            self.add_message('exception-escape', node=leaked_name)
 
     @utils.check_messages('backtick')
     def visit_repr(self, node):
diff --git a/pylint/test/unittest_checker_python3.py b/pylint/test/unittest_checker_python3.py
index 8e1bce493..e61f9cd25 100644
--- a/pylint/test/unittest_checker_python3.py
+++ b/pylint/test/unittest_checker_python3.py
@@ -34,6 +34,7 @@ python2_only = pytest.mark.skipif(sys.version_info[0] > 2, reason='Python 2 only
 # TODO(cpopa): Port these to the functional test framework instead.
 
 class TestPython3Checker(testutils.CheckerTestCase):
+
     CHECKER_CLASS = checker.Python3Checker
 
     def check_bad_builtin(self, builtin_name):
@@ -719,21 +720,22 @@ class TestPython3Checker(testutils.CheckerTestCase):
             self.checker.visit_attribute(node)
 
     def test_comprehension_escape(self):
-        list_comp, set_comp, dict_comp = astroid.extract_node('''
-        [i for i in range(10)]
+        assign, escaped_node = astroid.extract_node('''
+        a = [i for i in range(10)] #@
         i #@
-        {c for c in range(10)}
-        c #@
-        {j:j for j in range(10)}
-        j #@
         ''')
-        message = testutils.Message('comprehension-escape', node=list_comp)
+        good_module = astroid.parse('''
+        {c for c in range(10)} #@
+        {j:j for j in range(10)} #@
+        [image_child] = [x for x in range(10)]
+        thumbnail = func(__(image_child))
+        ''')
+        message = testutils.Message('comprehension-escape', node=escaped_node)
         with self.assertAddsMessages(message):
-            self.checker.visit_name(list_comp)
+            self.checker.visit_listcomp(assign.value)
 
-        for node in (set_comp, dict_comp):
-            with self.assertNoMessages():
-                self.checker.visit_name(node)
+        with self.assertNoMessages():
+            self.walk(good_module)
 
     def test_comprehension_escape_newly_introduced(self):
         node = astroid.extract_node('''
@@ -745,7 +747,7 @@ class TestPython3Checker(testutils.CheckerTestCase):
             self.walk(node)
 
     def test_exception_escape(self):
-        bad, good = astroid.extract_node('''
+        module = astroid.parse('''
         try: 1/0
         except ValueError as exc:
             pass
@@ -756,11 +758,11 @@ class TestPython3Checker(testutils.CheckerTestCase):
            exc = 2
         exc #@
         ''')
-        message = testutils.Message('exception-escape', node=bad)
+        message = testutils.Message('exception-escape', node=module.body[1].value)
         with self.assertAddsMessages(message):
-            self.checker.visit_name(bad)
+            self.checker.visit_excepthandler(module.body[0].handlers[0])
         with self.assertNoMessages():
-            self.checker.visit_name(good)
+            self.checker.visit_excepthandler(module.body[2].handlers[0])
 
     def test_bad_sys_attribute(self):
         node = astroid.extract_node('''