Don't assume direct parentage when emitting `used-before-assignment` (#5582)

* Fix #4045: Don't assume try ancestors are immediate parents when emitting `used-before-assignment`

Co-authored-by: Daniël van Noord <13665637+DanielNoord@users.noreply.github.com>
Co-authored-by: Pierre Sassoulas <pierre.sassoulas@gmail.com>

6 files changed, 142 insertions, 16 deletions

diff --git a/ChangeLog b/ChangeLog
index 2bf37a089..df0a7313b 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -33,6 +33,12 @@ Release date: TBA
 
   Closes #85, #2615
 
+* Fixed false negative for ``used-before-assignment`` when a conditional
+  or context manager intervened before the try statement that suggested
+  it might fail.
+
+  Closes #4045
+
 * Fixed extremely long processing of long lines with comma's.
 
   Closes #5483
diff --git a/doc/whatsnew/2.13.rst b/doc/whatsnew/2.13.rst
index c1b592237..10970a1dd 100644
--- a/doc/whatsnew/2.13.rst
+++ b/doc/whatsnew/2.13.rst
@@ -84,6 +84,12 @@ Other Changes
 
   Closes #85, #2615
 
+* Fixed false negative for ``used-before-assignment`` when a conditional
+  or context manager intervened before the try statement that suggested
+  it might fail.
+
+  Closes #4045
+
 * Fix a false positive for ``assigning-non-slot`` when the slotted class
   defined ``__setattr__``.
 
diff --git a/pylint/checkers/utils.py b/pylint/checkers/utils.py
index c20342c82..f42123048 100644
--- a/pylint/checkers/utils.py
+++ b/pylint/checkers/utils.py
@@ -1713,3 +1713,19 @@ def get_node_first_ancestor_of_type(
         if isinstance(ancestor, ancestor_type):
             return ancestor
     return None
+
+
+def get_node_first_ancestor_of_type_and_its_child(
+    node: nodes.NodeNG, ancestor_type: Union[Type[T_Node], Tuple[Type[T_Node]]]
+) -> Union[Tuple[None, None], Tuple[T_Node, nodes.NodeNG]]:
+    """Modified version of get_node_first_ancestor_of_type to also return the
+    descendant visited directly before reaching the sought ancestor. Useful
+    for extracting whether a statement is guarded by a try, except, or finally
+    when searching for a TryFinally ancestor.
+    """
+    child = node
+    for ancestor in node.node_ancestors():
+        if isinstance(ancestor, ancestor_type):
+            return (ancestor, child)
+        child = ancestor
+    return None, None
diff --git a/pylint/checkers/variables.py b/pylint/checkers/variables.py
index ae01a65d1..f01866276 100644
--- a/pylint/checkers/variables.py
+++ b/pylint/checkers/variables.py
@@ -672,26 +672,24 @@ scope_type : {self._atomic.scope_type}
 
         # If this node is in an ExceptHandler,
         # filter out assignments in the try portion, assuming they may fail
-        if found_nodes and isinstance(node_statement.parent, nodes.ExceptHandler):
-            filtered_nodes = [
-                n
-                for n in found_nodes
-                if not (
-                    isinstance(n.statement(future=True).parent, nodes.TryExcept)
-                    and n.statement(future=True) in n.statement(future=True).parent.body
-                    and node_statement.parent
-                    in n.statement(future=True).parent.handlers
+        if found_nodes:
+            uncertain_nodes = (
+                self._uncertain_nodes_in_try_blocks_when_evaluating_except_blocks(
+                    found_nodes, node_statement
                 )
-            ]
-            filtered_nodes_set = set(filtered_nodes)
-            difference = [n for n in found_nodes if n not in filtered_nodes_set]
-            self.consumed_uncertain[node.name] += difference
-            found_nodes = filtered_nodes
+            )
+            self.consumed_uncertain[node.name] += uncertain_nodes
+            uncertain_nodes_set = set(uncertain_nodes)
+            found_nodes = [n for n in found_nodes if n not in uncertain_nodes_set]
 
         return found_nodes
 
     @staticmethod
-    def _uncertain_nodes_in_except_blocks(found_nodes, node, node_statement):
+    def _uncertain_nodes_in_except_blocks(
+        found_nodes: List[nodes.NodeNG],
+        node: nodes.NodeNG,
+        node_statement: nodes.Statement,
+    ) -> List[nodes.NodeNG]:
         """Return any nodes in ``found_nodes`` that should be treated as uncertain
         because they are in an except block.
         """
@@ -734,6 +732,53 @@ scope_type : {self._atomic.scope_type}
             uncertain_nodes.append(other_node)
         return uncertain_nodes
 
+    @staticmethod
+    def _uncertain_nodes_in_try_blocks_when_evaluating_except_blocks(
+        found_nodes: List[nodes.NodeNG], node_statement: nodes.Statement
+    ) -> List[nodes.NodeNG]:
+        """Return any nodes in ``found_nodes`` that should be treated as uncertain
+        because they are in a try block and the ``node_statement`` being evaluated
+        is in one of its except handlers.
+        """
+        uncertain_nodes: List[nodes.NodeNG] = []
+        closest_except_handler = utils.get_node_first_ancestor_of_type(
+            node_statement, nodes.ExceptHandler
+        )
+        if closest_except_handler is None:
+            return uncertain_nodes
+        for other_node in found_nodes:
+            other_node_statement = other_node.statement(future=True)
+            # If the other statement is the except handler guarding `node`, it executes
+            if other_node_statement is closest_except_handler:
+                continue
+            # Ensure other_node is in a try block
+            (
+                other_node_try_ancestor,
+                other_node_try_ancestor_visited_child,
+            ) = utils.get_node_first_ancestor_of_type_and_its_child(
+                other_node_statement, nodes.TryExcept
+            )
+            if other_node_try_ancestor is None:
+                continue
+            if (
+                other_node_try_ancestor_visited_child
+                not in other_node_try_ancestor.body
+            ):
+                continue
+            # Make sure nesting is correct -- there should be at least one
+            # except handler that is a sibling attached to the try ancestor,
+            # or is an ancestor of the try ancestor.
+            if not any(
+                closest_except_handler in other_node_try_ancestor.handlers
+                or other_node_try_ancestor_except_handler
+                in closest_except_handler.node_ancestors()
+                for other_node_try_ancestor_except_handler in other_node_try_ancestor.handlers
+            ):
+                continue
+            # Passed all tests for uncertain execution
+            uncertain_nodes.append(other_node)
+        return uncertain_nodes
+
 
 # pylint: disable=too-many-public-methods
 class VariablesChecker(BaseChecker):
diff --git a/tests/functional/u/use/used_before_assignment_issue2615.py b/tests/functional/u/use/used_before_assignment_issue2615.py
index 912c71387..bce073bf3 100644
--- a/tests/functional/u/use/used_before_assignment_issue2615.py
+++ b/tests/functional/u/use/used_before_assignment_issue2615.py
@@ -4,6 +4,57 @@ def main():
     try:
         res = 1 / 0
         res = 42
+        if main():
+            res = None
+        with open(__file__, encoding="utf-8") as opened_file:
+            res = opened_file.readlines()
     except ZeroDivisionError:
         print(res)  # [used-before-assignment]
     print(res)
+
+
+def nested_except_blocks():
+    """Assignments in an except are tested against possibly failing
+    assignments in try blocks at two different nesting levels."""
+    try:
+        res = 1 / 0
+        res = 42
+        if main():
+            res = None
+        with open(__file__, encoding="utf-8") as opened_file:
+            res = opened_file.readlines()
+    except ZeroDivisionError:
+        try:
+            more_bad_division = 1 / 0
+        except ZeroDivisionError:
+            print(more_bad_division)  # [used-before-assignment]
+            print(res)  # [used-before-assignment]
+    print(res)
+
+
+def consecutive_except_blocks():
+    """An assignment assumed to execute in one TryExcept should continue to be
+    assumed to execute in a consecutive TryExcept.
+    """
+    try:
+        res = 100
+    except ZeroDivisionError:
+        pass
+    try:
+        pass
+    except ValueError:
+        print(res)
+
+
+def name_earlier_in_except_block():
+    """Permit the name that might not have been assigned during the try block
+    to be defined inside a conditional inside the except block.
+    """
+    try:
+        res = 1 / 0
+    except ZeroDivisionError:
+        if main():
+            res = 10
+        else:
+            res = 11
+        print(res)
diff --git a/tests/functional/u/use/used_before_assignment_issue2615.txt b/tests/functional/u/use/used_before_assignment_issue2615.txt
index ce6e4b9d0..0da3892c9 100644
--- a/tests/functional/u/use/used_before_assignment_issue2615.txt
+++ b/tests/functional/u/use/used_before_assignment_issue2615.txt
@@ -1 +1,3 @@
-used-before-assignment:8:14:8:17:main:Using variable 'res' before assignment:UNDEFINED
+used-before-assignment:12:14:12:17:main:Using variable 'res' before assignment:UNDEFINED
+used-before-assignment:30:18:30:35:nested_except_blocks:Using variable 'more_bad_division' before assignment:UNDEFINED
+used-before-assignment:31:18:31:21:nested_except_blocks:Using variable 'res' before assignment:UNDEFINED