Fix used-before-assignment false positive for TYPE_CHECKING elif branch imports

doc/whatsnew/fragments/8437.bugfix

@@ -0,0 +1,4 @@
+Fix a ``used-before-assignment`` false positive when imports
+are made under the ``TYPE_CHECKING`` else if branch.
+
+Closes #8437

pylint/checkers/utils.py

@@ -2120,53 +2120,6 @@ def is_class_attr(name: str, klass: nodes.ClassDef) -> bool:
         return False
 
 
-def is_defined(name: str, node: nodes.NodeNG) -> bool:
-    """Searches for a tree node that defines the given variable name."""
-    is_defined_so_far = False
-
-    if isinstance(node, nodes.NamedExpr):
-        is_defined_so_far = node.target.name == name
-
-    if isinstance(node, (nodes.Import, nodes.ImportFrom)):
-        is_defined_so_far = any(node_name[0] == name for node_name in node.names)
-
-    if isinstance(node, nodes.With):
-        is_defined_so_far = any(
-            isinstance(item[1], nodes.AssignName) and item[1].name == name
-            for item in node.items
-        )
-
-    if isinstance(node, (nodes.ClassDef, nodes.FunctionDef)):
-        is_defined_so_far = node.name == name
-
-    if isinstance(node, nodes.AnnAssign):
-        is_defined_so_far = (
-            node.value
-            and isinstance(node.target, nodes.AssignName)
-            and node.target.name == name
-        )
-
-    if isinstance(node, nodes.Assign):
-        is_defined_so_far = any(
-            any(
-                (
-                    (
-                        isinstance(elt, nodes.Starred)
-                        and isinstance(elt.value, nodes.AssignName)
-                        and elt.value.name == name
-                    )
-                    or (isinstance(elt, nodes.AssignName) and elt.name == name)
-                )
-                for elt in get_all_elements(target)
-            )
-            for target in node.targets
-        )
-
-    return is_defined_so_far or any(
-        is_defined(name, child) for child in node.get_children()
-    )
-
-
 def get_inverse_comparator(op: str) -> str:
     """Returns the inverse comparator given a comparator.
 

pylint/checkers/variables.py

@@ -544,7 +544,6 @@ def __init__(self, node: nodes.NodeNG, scope_type: str) -> None:
             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,32 @@ def get_next_to_consume(self, node: nodes.Name) -> list[nodes.NodeNG] | None:
         return found_nodes
 
     @staticmethod
-    def _exhaustively_define_name_raise_or_return(
-        name: str, node: nodes.NodeNG
-    ) -> bool:
+    def _inferred_to_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.
         """
         # 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.get_children()
+                        if isinstance(child, 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,23 +733,43 @@ def _exhaustively_define_name_raise_or_return(
         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 else branch when test condition is inferred to be false
+        if all_inferred and all(
+            isinstance(inferred, nodes.Const) and not inferred.value
+            for inferred in all_inferred
+        ):
+            return NamesConsumer._branch_handles_name(name, node.orelse)
+        # Only search if branch when test condition is inferred to be true
+        if all_inferred and any(
+            isinstance(inferred, nodes.Const)
+            and inferred.value != NotImplemented
+            and inferred.value
+            for inferred in all_inferred
+        ):
+            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:
         return any(
             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
@@ -761,48 +786,36 @@ def _uncertain_nodes_in_false_tests(
         """
         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 +914,18 @@ def _defines_name_raises_or_returns(name: str, node: nodes.NodeNG) -> bool:
                 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 +944,10 @@ def _defines_name_raises_or_returns_recursive(
                     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
@@ -1687,16 +1716,22 @@ def _check_consumer(
         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 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 (
@@ -1838,7 +1873,9 @@ def _check_consumer(
                         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
@@ -2029,7 +2066,6 @@ def _in_lambda_or_comprehension_body(
             parent = parent.parent
         return False
 
-    # pylint: disable = too-many-branches
     @staticmethod
     def _is_variable_violation(
         node: nodes.Name,
@@ -2183,42 +2219,6 @@ def _is_variable_violation(
                         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
@@ -2258,18 +2258,15 @@ def _maybe_used_and_assigned_at_once(defstmt: nodes.Statement) -> bool:
             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)
 
@@ -2358,6 +2355,16 @@ def _is_never_evaluated(
                 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.