Fix false negative for used-before-assignment when an Except block …

…intervenes between try/finally

ChangeLog

@@ -39,6 +39,10 @@ Release date: TBA
 
   Closes #4045
 
+* Fixed false negative for ``used-before-assignment`` in finally blocks
+  if an except handler did not define the assignment that might have failed
+  in the try block.
+
 * Fixed extremely long processing of long lines with comma's.
 
   Closes #5483

doc/whatsnew/2.13.rst

@@ -90,6 +90,10 @@ Other Changes
 
   Closes #4045
 
+* Fixed false negative for ``used-before-assignment`` in finally blocks
+  if an except handler did not define the assignment that might have failed
+  in the try block.
+
 * Fix a false positive for ``assigning-non-slot`` when the slotted class
   defined ``__setattr__``.
 

pylint/checkers/variables.py

@@ -652,23 +652,15 @@ def get_next_to_consume(self, node):
 
         # If this node is in a Finally block of a Try/Finally,
         # filter out assignments in the try portion, assuming they may fail
-        if (
-            found_nodes
-            and isinstance(node_statement.parent, nodes.TryFinally)
-            and node_statement in node_statement.parent.finalbody
-        ):
-            filtered_nodes = [
-                n
-                for n in found_nodes
-                if not (
-                    n.statement(future=True).parent is node_statement.parent
-                    and n.statement(future=True) in n.statement(future=True).parent.body
+        if found_nodes:
+            uncertain_nodes = (
+                self._uncertain_nodes_in_try_blocks_when_evaluating_finally_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]
 
         # If this node is in an ExceptHandler,
         # filter out assignments in the try portion, assuming they may fail
@@ -779,6 +771,43 @@ def _uncertain_nodes_in_try_blocks_when_evaluating_except_blocks(
             uncertain_nodes.append(other_node)
         return uncertain_nodes
 
+    @staticmethod
+    def _uncertain_nodes_in_try_blocks_when_evaluating_finally_blocks(
+        found_nodes: List[nodes.NodeNG], node_statement: nodes.Statement
+    ) -> List[nodes.NodeNG]:
+        uncertain_nodes: List[nodes.NodeNG] = []
+        (
+            closest_try_finally_ancestor,
+            child_of_closest_try_finally_ancestor,
+        ) = utils.get_node_first_ancestor_of_type_and_its_child(
+            node_statement, nodes.TryFinally
+        )
+        if closest_try_finally_ancestor is None:
+            return uncertain_nodes
+        if (
+            child_of_closest_try_finally_ancestor
+            not in closest_try_finally_ancestor.finalbody
+        ):
+            return uncertain_nodes
+        for other_node in found_nodes:
+            other_node_statement = other_node.statement(future=True)
+            (
+                other_node_try_finally_ancestor,
+                child_of_other_node_try_finally_ancestor,
+            ) = utils.get_node_first_ancestor_of_type_and_its_child(
+                other_node_statement, nodes.TryFinally
+            )
+            if other_node_try_finally_ancestor is not closest_try_finally_ancestor:
+                continue
+            if (
+                child_of_other_node_try_finally_ancestor
+                not in other_node_try_finally_ancestor.body
+            ):
+                continue
+            # Passed all tests for uncertain execution
+            uncertain_nodes.append(other_node)
+        return uncertain_nodes
+
 
 # pylint: disable=too-many-public-methods
 class VariablesChecker(BaseChecker):

tests/functional/c/consider/consider_using_with_open.py

@@ -137,7 +137,7 @@ def test_defined_in_try_and_finally(self):
         except FileNotFoundError:
             Path("foo").touch()
         finally:
-            file_handle.open("foo", encoding="utf")  # must not trigger
+            file_handle.open("foo", encoding="utf")  # [used-before-assignment] # must not trigger
         with file_handle:
             return file_handle.read()
 

tests/functional/c/consider/consider_using_with_open.txt

@@ -4,3 +4,4 @@ consider-using-with:46:4:46:33:test_open_outside_assignment:Consider using 'with
 consider-using-with:47:14:47:43:test_open_outside_assignment:Consider using 'with' for resource-allocating operations:UNDEFINED
 consider-using-with:52:8:52:37:test_open_inside_with_block:Consider using 'with' for resource-allocating operations:UNDEFINED
 consider-using-with:120:26:122:13:TestControlFlow.test_triggers_if_reassigned_after_if_else:Consider using 'with' for resource-allocating operations:UNDEFINED
+used-before-assignment:140:12:140:23:TestControlFlow.test_defined_in_try_and_finally:Using variable 'file_handle' before assignment:UNDEFINED

tests/functional/u/use/used_before_assignment_issue85.py

@@ -7,3 +7,61 @@ def main():
     finally:
         print(res)  # [used-before-assignment]
     print(res)
+
+
+def try_except_finally():
+    """When evaluating finally blocks, assume try statements fail."""
+    try:
+        res = 1 / 0
+        res = 42
+    except ZeroDivisionError:
+        print()
+    finally:
+        print(res)  # [used-before-assignment]
+    print(res)
+
+
+def try_except_finally_assignment_in_final_block():
+    """Assignment of the name in the final block does not warn."""
+    try:
+        res = 1 / 0
+        res = 42
+    except ZeroDivisionError:
+        print()
+    finally:
+        res = 999
+        print(res)
+    print(res)
+
+
+def try_except_finally_nested_try_finally_in_try():
+    """Don't confuse assignments in different finally statements where
+    one is nested inside a try.
+    """
+    try:
+        try:
+            res = 1 / 0
+        finally:
+            print(res)  # [used-before-assignment]
+        print(1 / 0)
+    except ZeroDivisionError:
+        print()
+    finally:
+        res = 999  # this assignment could be confused for that above
+        print(res)
+    print(res)
+
+
+def try_except_finally_nested_try_finally_in_finally():
+    """Don't confuse assignments in different finally statements where
+    one is nested inside a finally.
+    """
+    try:
+        pass
+    finally:
+        try:
+            times = 1
+        except TypeError:
+            pass
+        # Don't emit: only assume the outer try failed
+        print(times)

tests/functional/u/use/used_before_assignment_issue85.txt

@@ -1 +1,3 @@
 used-before-assignment:8:14:8:17:main:Using variable 'res' before assignment:UNDEFINED
+used-before-assignment:20:14:20:17:try_except_finally:Using variable 'res' before assignment:UNDEFINED
+used-before-assignment:45:18:45:21:try_except_finally_nested_try_finally_in_try:Using variable 'res' before assignment:UNDEFINED