Fix `used-before-assignment` false positive for `TYPE_CHECKING` elif branch imports (#8441)

Co-authored-by: Pierre Sassoulas <pierre.sassoulas@gmail.com>

1 files changed, 119 insertions, 110 deletions

diff --git a/pylint/checkers/variables.py b/pylint/checkers/variables.py
index 130724fa5..422aedf9c 100644
--- a/pylint/checkers/variables.py
+++ b/pylint/checkers/variables.py
@@ -544,7 +544,6 @@ class NamesConsumer:
             copy.copy(node.locals), {}, collections.defaultdict(list), scope_type
         )
         self.node = node
-        self._if_nodes_deemed_uncertain: set[nodes.If] = set()
 
     def __repr__(self) -> str:
         _to_consumes = [f"{k}->{v}" for k, v in self._atomic.to_consume.items()]
@@ -695,26 +694,28 @@ scope_type : {self._atomic.scope_type}
         return found_nodes
 
     @staticmethod
-    def _exhaustively_define_name_raise_or_return(
-        name: str, node: nodes.NodeNG
-    ) -> bool:
-        """Return True if there is a collectively exhaustive set of paths under
-        this `if_node` that define `name`, raise, or return.
+    def _inferred_to_define_name_raise_or_return(name: str, node: nodes.NodeNG) -> bool:
+        """Return True if there is a path under this `if_node`
+        that is inferred to define `name`, raise, or return.
         """
         # Handle try and with
         if isinstance(node, (nodes.TryExcept, nodes.TryFinally)):
             # Allow either a path through try/else/finally OR a path through ALL except handlers
-            return (
-                NamesConsumer._defines_name_raises_or_returns_recursive(name, node)
-                or isinstance(node, nodes.TryExcept)
-                and all(
-                    NamesConsumer._defines_name_raises_or_returns_recursive(
-                        name, handler
-                    )
-                    for handler in node.handlers
+            try_except_node = node
+            if isinstance(node, nodes.TryFinally):
+                try_except_node = next(
+                    (child for child in node.nodes_of_class(nodes.TryExcept)),
+                    None,
                 )
+            handlers = try_except_node.handlers if try_except_node else []
+            return NamesConsumer._defines_name_raises_or_returns_recursive(
+                name, node
+            ) or all(
+                NamesConsumer._defines_name_raises_or_returns_recursive(name, handler)
+                for handler in handlers
             )
-        if isinstance(node, nodes.With):
+
+        if isinstance(node, (nodes.With, nodes.For, nodes.While)):
             return NamesConsumer._defines_name_raises_or_returns_recursive(name, node)
 
         if not isinstance(node, nodes.If):
@@ -728,13 +729,29 @@ scope_type : {self._atomic.scope_type}
         if NamesConsumer._defines_name_raises_or_returns(name, node):
             return True
 
-        # If there is no else, then there is no collectively exhaustive set of paths
-        if not node.orelse:
-            return False
+        test = node.test.value if isinstance(node.test, nodes.NamedExpr) else node.test
+        all_inferred = utils.infer_all(test)
+        only_search_if = False
+        only_search_else = True
 
+        for inferred in all_inferred:
+            if not isinstance(inferred, nodes.Const):
+                only_search_else = False
+                continue
+            val = inferred.value
+            only_search_if = only_search_if or (val != NotImplemented and val)
+            only_search_else = only_search_else and not val
+
+        # Only search else branch when test condition is inferred to be false
+        if all_inferred and only_search_else:
+            return NamesConsumer._branch_handles_name(name, node.orelse)
+        # Only search if branch when test condition is inferred to be true
+        if all_inferred and only_search_if:
+            return NamesConsumer._branch_handles_name(name, node.body)
+        # Search both if and else branches
         return NamesConsumer._branch_handles_name(
             name, node.body
-        ) and NamesConsumer._branch_handles_name(name, node.orelse)
+        ) or NamesConsumer._branch_handles_name(name, node.orelse)
 
     @staticmethod
     def _branch_handles_name(name: str, body: Iterable[nodes.NodeNG]) -> bool:
@@ -742,9 +759,16 @@ scope_type : {self._atomic.scope_type}
             NamesConsumer._defines_name_raises_or_returns(name, if_body_stmt)
             or isinstance(
                 if_body_stmt,
-                (nodes.If, nodes.TryExcept, nodes.TryFinally, nodes.With),
+                (
+                    nodes.If,
+                    nodes.TryExcept,
+                    nodes.TryFinally,
+                    nodes.With,
+                    nodes.For,
+                    nodes.While,
+                ),
             )
-            and NamesConsumer._exhaustively_define_name_raise_or_return(
+            and NamesConsumer._inferred_to_define_name_raise_or_return(
                 name, if_body_stmt
             )
             for if_body_stmt in body
@@ -756,53 +780,41 @@ scope_type : {self._atomic.scope_type}
         """Identify nodes of uncertain execution because they are defined under
         tests that evaluate false.
 
-        Don't identify a node if there is a collectively exhaustive set of paths
-        that define the name, raise, or return (e.g. every if/else branch).
+        Don't identify a node if there is a path that is inferred to
+        define the name, raise, or return (e.g. any executed if/elif/else branch).
         """
         uncertain_nodes = []
         for other_node in found_nodes:
-            if in_type_checking_block(other_node):
-                continue
-
-            if not isinstance(other_node, nodes.AssignName):
+            if isinstance(other_node, nodes.AssignName):
+                name = other_node.name
+            elif isinstance(other_node, (nodes.Import, nodes.ImportFrom)):
+                name = node.name
+            else:
                 continue
 
-            closest_if = utils.get_node_first_ancestor_of_type(other_node, nodes.If)
-            if closest_if is None:
-                continue
-            if node.frame() is not closest_if.frame():
-                continue
-            if closest_if is not None and closest_if.parent_of(node):
+            all_if = [
+                n
+                for n in other_node.node_ancestors()
+                if isinstance(n, nodes.If) and not n.parent_of(node)
+            ]
+            if not all_if:
                 continue
 
-            # Name defined in every if/else branch
-            if NamesConsumer._exhaustively_define_name_raise_or_return(
-                other_node.name, closest_if
+            closest_if = all_if[0]
+            if (
+                isinstance(node, nodes.AssignName)
+                and node.frame() is not closest_if.frame()
             ):
                 continue
-
-            # Higher-level if already determined to be always false
-            if any(
-                if_node.parent_of(closest_if)
-                for if_node in self._if_nodes_deemed_uncertain
-            ):
-                uncertain_nodes.append(other_node)
+            if closest_if.parent_of(node):
                 continue
 
-            # All inferred values must test false
-            if isinstance(closest_if.test, nodes.NamedExpr):
-                test = closest_if.test.value
-            else:
-                test = closest_if.test
-            all_inferred = utils.infer_all(test)
-            if not all_inferred or not all(
-                isinstance(inferred, nodes.Const) and not inferred.value
-                for inferred in all_inferred
-            ):
+            outer_if = all_if[-1]
+            # Name defined in the if/else control flow
+            if NamesConsumer._inferred_to_define_name_raise_or_return(name, outer_if):
                 continue
 
             uncertain_nodes.append(other_node)
-            self._if_nodes_deemed_uncertain.add(closest_if)
 
         return uncertain_nodes
 
@@ -901,6 +913,18 @@ scope_type : {self._atomic.scope_type}
                 for child_named_expr in node.nodes_of_class(nodes.NamedExpr)
             ):
                 return True
+        if isinstance(node, (nodes.Import, nodes.ImportFrom)) and any(
+            (node_name[1] and node_name[1] == name) or (node_name[0] == name)
+            for node_name in node.names
+        ):
+            return True
+        if isinstance(node, nodes.With) and any(
+            isinstance(item[1], nodes.AssignName) and item[1].name == name
+            for item in node.items
+        ):
+            return True
+        if isinstance(node, (nodes.ClassDef, nodes.FunctionDef)) and node.name == name:
+            return True
         return False
 
     @staticmethod
@@ -919,6 +943,10 @@ scope_type : {self._atomic.scope_type}
                     for nested_stmt in stmt.get_children()
                 ):
                     return True
+            if isinstance(
+                stmt, nodes.TryExcept
+            ) and NamesConsumer._defines_name_raises_or_returns_recursive(name, stmt):
+                return True
         return False
 
     @staticmethod
@@ -1688,16 +1716,25 @@ class VariablesChecker(BaseChecker):
         if found_nodes is None:
             return (VariableVisitConsumerAction.CONTINUE, None)
         if not found_nodes:
-            if node.name in current_consumer.consumed_uncertain:
-                confidence = CONTROL_FLOW
-            else:
-                confidence = HIGH
-            self.add_message(
-                "used-before-assignment",
-                args=node.name,
-                node=node,
-                confidence=confidence,
-            )
+            if (
+                not (
+                    self._postponed_evaluation_enabled
+                    and utils.is_node_in_type_annotation_context(node)
+                )
+                and not self._is_builtin(node.name)
+                and not self._is_variable_annotation_in_function(node)
+            ):
+                confidence = (
+                    CONTROL_FLOW
+                    if node.name in current_consumer.consumed_uncertain
+                    else HIGH
+                )
+                self.add_message(
+                    "used-before-assignment",
+                    args=node.name,
+                    node=node,
+                    confidence=confidence,
+                )
             # Mark for consumption any nodes added to consumed_uncertain by
             # get_next_to_consume() because they might not have executed.
             return (
@@ -1839,7 +1876,9 @@ class VariablesChecker(BaseChecker):
                         confidence=HIGH,
                     )
 
-        elif self._is_only_type_assignment(node, defstmt):
+        elif not self._is_builtin(node.name) and self._is_only_type_assignment(
+            node, defstmt
+        ):
             if node.scope().locals.get(node.name):
                 self.add_message(
                     "used-before-assignment", args=node.name, node=node, confidence=HIGH
@@ -2030,7 +2069,6 @@ class VariablesChecker(BaseChecker):
             parent = parent.parent
         return False
 
-    # pylint: disable = too-many-branches
     @staticmethod
     def _is_variable_violation(
         node: nodes.Name,
@@ -2184,42 +2222,6 @@ class VariablesChecker(BaseChecker):
                         anc is defnode.value for anc in node.node_ancestors()
                     )
 
-            # Look for type checking definitions inside a type checking guard.
-            # Relevant for function annotations only, not variable annotations (AnnAssign)
-            if (
-                isinstance(defstmt, (nodes.Import, nodes.ImportFrom))
-                and isinstance(defstmt.parent, nodes.If)
-                and in_type_checking_block(defstmt)
-                and not in_type_checking_block(node)
-            ):
-                defstmt_parent = defstmt.parent
-
-                maybe_annotation = utils.get_node_first_ancestor_of_type(
-                    node, nodes.AnnAssign
-                )
-                if not (
-                    maybe_annotation
-                    and utils.get_node_first_ancestor_of_type(
-                        maybe_annotation, nodes.FunctionDef
-                    )
-                ):
-                    # Exempt those definitions that are used inside the type checking
-                    # guard or that are defined in any elif/else type checking guard branches.
-                    used_in_branch = defstmt_parent.parent_of(node)
-                    if not used_in_branch:
-                        if defstmt_parent.has_elif_block():
-                            defined_in_or_else = utils.is_defined(
-                                node.name, defstmt_parent.orelse[0]
-                            )
-                        else:
-                            defined_in_or_else = any(
-                                utils.is_defined(node.name, content)
-                                for content in defstmt_parent.orelse
-                            )
-
-                        if not defined_in_or_else:
-                            maybe_before_assign = True
-
         return maybe_before_assign, annotation_return, use_outer_definition
 
     @staticmethod
@@ -2259,18 +2261,15 @@ class VariablesChecker(BaseChecker):
             for call in value.nodes_of_class(klass=nodes.Call)
         )
 
-    def _is_only_type_assignment(
-        self, node: nodes.Name, defstmt: nodes.Statement
-    ) -> bool:
+    def _is_builtin(self, name: str) -> bool:
+        return name in self.linter.config.additional_builtins or utils.is_builtin(name)
+
+    @staticmethod
+    def _is_only_type_assignment(node: nodes.Name, defstmt: nodes.Statement) -> bool:
         """Check if variable only gets assigned a type and never a value."""
         if not isinstance(defstmt, nodes.AnnAssign) or defstmt.value:
             return False
 
-        if node.name in self.linter.config.additional_builtins or utils.is_builtin(
-            node.name
-        ):
-            return False
-
         defstmt_frame = defstmt.frame(future=True)
         node_frame = node.frame(future=True)
 
@@ -2359,6 +2358,16 @@ class VariablesChecker(BaseChecker):
                 return True
         return False
 
+    @staticmethod
+    def _is_variable_annotation_in_function(node: nodes.NodeNG) -> bool:
+        is_annotation = utils.get_node_first_ancestor_of_type(node, nodes.AnnAssign)
+        return (
+            is_annotation
+            and utils.get_node_first_ancestor_of_type(  # type: ignore[return-value]
+                is_annotation, nodes.FunctionDef
+            )
+        )
+
     def _ignore_class_scope(self, node: nodes.NodeNG) -> bool:
         """Return True if the node is in a local class scope, as an assignment.