From 48eebfb7e3f69db1aa3fb167b9dc8879f15050e9 Mon Sep 17 00:00:00 2001 From: Zen Date: Tue, 22 Oct 2024 18:42:45 +0800 Subject: [PATCH] Handle functions defined under type-checking --- pylint/checkers/variables.py | 60 ++++++++++++------- .../u/used/used_before_assignment_typing.py | 2 +- .../u/used/used_before_assignment_typing.txt | 1 + 3 files changed, 42 insertions(+), 21 deletions(-) diff --git a/pylint/checkers/variables.py b/pylint/checkers/variables.py index 150dcba838..862ef6679d 100644 --- a/pylint/checkers/variables.py +++ b/pylint/checkers/variables.py @@ -303,6 +303,17 @@ def _assigned_locally(name_node: nodes.Name) -> bool: ) +def _is_before(node: nodes.NodeNG, reference_node: nodes.NodeNG) -> bool: + """ + Returns True if node appears before reference_node in the source code, False otherwise. + """ + if node.lineno < reference_node.lineno: + return True + if node.lineno == reference_node.lineno: + return node.col_offset < reference_node.col_offset + return False + + def _has_locals_call_after_node(stmt: nodes.NodeNG, scope: nodes.FunctionDef) -> bool: skip_nodes = ( nodes.FunctionDef, @@ -531,7 +542,9 @@ def mark_as_consumed(self, name: str, consumed_nodes: list[nodes.NodeNG]) -> Non else: del self.to_consume[name] - def get_next_to_consume(self, node: nodes.Name) -> list[nodes.NodeNG] | None: + def get_next_to_consume( + self, node: nodes.Name, is_nonlocal: bool + ) -> list[nodes.NodeNG] | None: """Return a list of the nodes that define `node` from this scope. If it is uncertain whether a node will be consumed, such as for statements in @@ -542,6 +555,12 @@ def get_next_to_consume(self, node: nodes.Name) -> list[nodes.NodeNG] | None: parent_node = node.parent found_nodes = self.to_consume.get(name) node_statement = node.statement() + + # Filter out all nodes that define `node`, as it is a nonlocal + if is_nonlocal: + self.consumed_uncertain[node.name] += found_nodes if found_nodes else [] + return [] + if ( found_nodes and isinstance(parent_node, nodes.Assign) @@ -561,13 +580,6 @@ def get_next_to_consume(self, node: nodes.Name) -> list[nodes.NodeNG] | None: ): found_nodes = None - # Before filtering, check that this node's name is not a nonlocal - if any( - isinstance(child, nodes.Nonlocal) and node.name in child.names - for child in node.frame().get_children() - ): - return found_nodes - # And no comprehension is under the node's frame if VariablesChecker._comprehension_between_frame_and_node(node): return found_nodes @@ -723,7 +735,7 @@ def _uncertain_nodes_if_tests( name = other_node.name elif isinstance(other_node, (nodes.Import, nodes.ImportFrom)): name = node.name - elif isinstance(other_node, nodes.ClassDef): + elif isinstance(other_node, (nodes.FunctionDef, nodes.ClassDef)): name = other_node.name else: continue @@ -1268,6 +1280,7 @@ def __init__(self, linter: PyLinter) -> None: self._reported_type_checking_usage_scopes: dict[ str, list[nodes.LocalsDictNodeNG] ] = {} + self._nonlocal_nodes_stack: list[list[nodes.Nonlocal]] = [] self._postponed_evaluation_enabled = False @utils.only_required_for_messages( @@ -1434,6 +1447,9 @@ def leave_setcomp(self, _: nodes.SetComp) -> None: def visit_functiondef(self, node: nodes.FunctionDef) -> None: """Visit function: update consumption analysis variable and check locals.""" self._to_consume.append(NamesConsumer(node, "function")) + self._nonlocal_nodes_stack.append( + [n for n in node.body if isinstance(n, nodes.Nonlocal)] + ) if not ( self.linter.is_message_enabled("redefined-outer-name") or self.linter.is_message_enabled("redefined-builtin") @@ -1483,6 +1499,7 @@ def visit_functiondef(self, node: nodes.FunctionDef) -> None: def leave_functiondef(self, node: nodes.FunctionDef) -> None: """Leave function: check function's locals are consumed.""" self._check_metaclasses(node) + self._nonlocal_nodes_stack.pop() if node.type_comment_returns: self._store_type_annotation_node(node.type_comment_returns) @@ -1761,7 +1778,9 @@ def _check_consumer( self._check_late_binding_closure(node) return (VariableVisitConsumerAction.RETURN, None) - found_nodes = current_consumer.get_next_to_consume(node) + found_nodes = current_consumer.get_next_to_consume( + node, self._is_nonlocal(node) + ) if found_nodes is None: return (VariableVisitConsumerAction.CONTINUE, None) if not found_nodes: @@ -1940,6 +1959,13 @@ def _check_consumer( return (VariableVisitConsumerAction.RETURN, found_nodes) + def _is_nonlocal(self, node: nodes.Name) -> bool: + return any( + node.name in nonlocal_node.names and _is_before(nonlocal_node, node) + for nonlocal_scope in self._nonlocal_nodes_stack + for nonlocal_node in nonlocal_scope + ) + def _report_unfound_name_definition( self, node: nodes.NodeNG, @@ -1964,6 +1990,8 @@ def _report_unfound_name_definition( and node.scope() in self._reported_type_checking_usage_scopes[node.name] ): return False + if self._is_nonlocal(node): + return False confidence = HIGH if node.name in current_consumer.names_under_always_false_test: @@ -2291,19 +2319,11 @@ def _is_variable_violation( # x = b if (b := True) else False maybe_before_assign = False elif ( - isinstance( # pylint: disable=too-many-boolean-expressions - defnode, nodes.NamedExpr - ) + isinstance(defnode, nodes.NamedExpr) and frame is defframe and defframe.parent_of(stmt) and stmt is defstmt - and ( - ( - defnode.lineno == node.lineno - and defnode.col_offset < node.col_offset - ) - or (defnode.lineno < node.lineno) - ) + and _is_before(defnode, node) ): # Relation of a name to the same name in a named expression # Could be used before assignment if self-referencing: diff --git a/tests/functional/u/used/used_before_assignment_typing.py b/tests/functional/u/used/used_before_assignment_typing.py index 585a26a628..5229260d2f 100644 --- a/tests/functional/u/used/used_before_assignment_typing.py +++ b/tests/functional/u/used/used_before_assignment_typing.py @@ -173,7 +173,7 @@ def defined_in_elif_branch(self) -> calendar.Calendar: # [possibly-used-before- def defined_in_else_branch(self) -> urlopen: print(zoneinfo) # [used-before-assignment] - print(pprint()) + print(pprint()) # [used-before-assignment] print(collections()) # [used-before-assignment] return urlopen diff --git a/tests/functional/u/used/used_before_assignment_typing.txt b/tests/functional/u/used/used_before_assignment_typing.txt index 886ac70759..06e162e9c4 100644 --- a/tests/functional/u/used/used_before_assignment_typing.txt +++ b/tests/functional/u/used/used_before_assignment_typing.txt @@ -6,6 +6,7 @@ used-before-assignment:153:20:153:28:VariableAnnotationsGuardedByTypeChecking:Us possibly-used-before-assignment:170:40:170:48:TypeCheckingMultiBranch.defined_in_elif_branch:Possibly using variable 'calendar' before assignment:INFERENCE possibly-used-before-assignment:171:14:171:20:TypeCheckingMultiBranch.defined_in_elif_branch:Possibly using variable 'bisect' before assignment:INFERENCE used-before-assignment:175:14:175:22:TypeCheckingMultiBranch.defined_in_else_branch:Using variable 'zoneinfo' before assignment:INFERENCE +used-before-assignment:176:14:176:20:TypeCheckingMultiBranch.defined_in_else_branch:Using variable 'pprint' before assignment:INFERENCE used-before-assignment:177:14:177:25:TypeCheckingMultiBranch.defined_in_else_branch:Using variable 'collections' before assignment:INFERENCE possibly-used-before-assignment:180:43:180:48:TypeCheckingMultiBranch.defined_in_nested_if_else:Possibly using variable 'heapq' before assignment:INFERENCE used-before-assignment:184:39:184:44:TypeCheckingMultiBranch.defined_in_try_except:Using variable 'array' before assignment:INFERENCE