Skip to content

Commit 5c59b48

Browse files
authored
Resolve used-before-assignment false negative when a function is defined under TYPE_CHECKING (#10034)
* Handle function def under type-checking * Handle case where name references a nonlocal binding from an outer frame * Add tests for nonlocal checks
1 parent f04761b commit 5c59b48

File tree

6 files changed

+74
-20
lines changed

6 files changed

+74
-20
lines changed
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Fix false negative for `used-before-assignment` when a function is defined inside a `TYPE_CHECKING` guard block and used later.
2+
3+
Closes #10028

pylint/checkers/variables.py

Lines changed: 41 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -303,6 +303,31 @@ def _assigned_locally(name_node: nodes.Name) -> bool:
303303
)
304304

305305

306+
def _is_before(node: nodes.NodeNG, reference_node: nodes.NodeNG) -> bool:
307+
"""Checks if node appears before reference_node."""
308+
if node.lineno < reference_node.lineno:
309+
return True
310+
if (
311+
node.lineno == reference_node.lineno
312+
and node.col_offset < reference_node.col_offset
313+
):
314+
return True
315+
return False
316+
317+
318+
def _is_nonlocal_name(node: nodes.Name, frame: nodes.LocalsDictNodeNG) -> bool:
319+
"""Checks if name node has a nonlocal declaration in the given frame."""
320+
if not isinstance(frame, nodes.FunctionDef):
321+
return False
322+
323+
return any(
324+
isinstance(stmt, nodes.Nonlocal)
325+
and node.name in stmt.names
326+
and _is_before(stmt, node)
327+
for stmt in frame.body
328+
)
329+
330+
306331
def _has_locals_call_after_node(stmt: nodes.NodeNG, scope: nodes.FunctionDef) -> bool:
307332
skip_nodes = (
308333
nodes.FunctionDef,
@@ -562,10 +587,7 @@ def get_next_to_consume(self, node: nodes.Name) -> list[nodes.NodeNG] | None:
562587
found_nodes = None
563588

564589
# Before filtering, check that this node's name is not a nonlocal
565-
if any(
566-
isinstance(child, nodes.Nonlocal) and node.name in child.names
567-
for child in node.frame().get_children()
568-
):
590+
if _is_nonlocal_name(node, node.frame()):
569591
return found_nodes
570592

571593
# And no comprehension is under the node's frame
@@ -723,7 +745,7 @@ def _uncertain_nodes_if_tests(
723745
name = other_node.name
724746
elif isinstance(other_node, (nodes.Import, nodes.ImportFrom)):
725747
name = node.name
726-
elif isinstance(other_node, nodes.ClassDef):
748+
elif isinstance(other_node, (nodes.FunctionDef, nodes.ClassDef)):
727749
name = other_node.name
728750
else:
729751
continue
@@ -1963,6 +1985,8 @@ def _report_unfound_name_definition(
19631985
return False
19641986
if self._is_variable_annotation_in_function(node):
19651987
return False
1988+
if self._has_nonlocal_binding(node):
1989+
return False
19661990
if (
19671991
node.name in self._reported_type_checking_usage_scopes
19681992
and node.scope() in self._reported_type_checking_usage_scopes[node.name]
@@ -2219,10 +2243,7 @@ def _is_variable_violation(
22192243
)
22202244
# check if we have a nonlocal
22212245
elif node.name in defframe.locals:
2222-
maybe_before_assign = not any(
2223-
isinstance(child, nodes.Nonlocal) and node.name in child.names
2224-
for child in defframe.get_children()
2225-
)
2246+
maybe_before_assign = not _is_nonlocal_name(node, defframe)
22262247

22272248
if (
22282249
base_scope_type == "lambda"
@@ -2295,19 +2316,11 @@ def _is_variable_violation(
22952316
# x = b if (b := True) else False
22962317
maybe_before_assign = False
22972318
elif (
2298-
isinstance( # pylint: disable=too-many-boolean-expressions
2299-
defnode, nodes.NamedExpr
2300-
)
2319+
isinstance(defnode, nodes.NamedExpr)
23012320
and frame is defframe
23022321
and defframe.parent_of(stmt)
23032322
and stmt is defstmt
2304-
and (
2305-
(
2306-
defnode.lineno == node.lineno
2307-
and defnode.col_offset < node.col_offset
2308-
)
2309-
or (defnode.lineno < node.lineno)
2310-
)
2323+
and _is_before(defnode, node)
23112324
):
23122325
# Relation of a name to the same name in a named expression
23132326
# Could be used before assignment if self-referencing:
@@ -2362,6 +2375,15 @@ def _maybe_used_and_assigned_at_once(defstmt: _base_nodes.Statement) -> bool:
23622375
def _is_builtin(self, name: str) -> bool:
23632376
return name in self.linter.config.additional_builtins or utils.is_builtin(name)
23642377

2378+
def _has_nonlocal_binding(self, node: nodes.Name) -> bool:
2379+
"""Checks if name node has a nonlocal binding in any enclosing frame."""
2380+
frame = node.frame()
2381+
while frame:
2382+
if _is_nonlocal_name(node, frame):
2383+
return True
2384+
frame = frame.parent.frame() if frame.parent else None
2385+
return False
2386+
23652387
@staticmethod
23662388
def _is_only_type_assignment(
23672389
node: nodes.Name,

tests/functional/u/used/used_before_assignment_nonlocal.py

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,3 +106,30 @@ def inner():
106106
print(args)
107107
inner()
108108
print(args)
109+
110+
111+
def nonlocal_in_outer_frame_fail():
112+
"""Nonlocal declared in outer frame, bad usage and assignment in inner frame."""
113+
num = 1
114+
def outer():
115+
nonlocal num
116+
def inner():
117+
print(num) # [used-before-assignment]
118+
num = 2
119+
inner()
120+
outer()
121+
122+
123+
def nonlocal_in_outer_frame_ok(callback, condition_a, condition_b):
124+
"""Nonlocal declared in outer frame, usage and definition in different frames."""
125+
def outer():
126+
nonlocal callback
127+
if condition_a:
128+
def inner():
129+
callback() # should not emit possibly-used-before-assignment
130+
inner()
131+
else:
132+
if condition_b:
133+
def callback():
134+
pass
135+
outer()

tests/functional/u/used/used_before_assignment_nonlocal.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,3 +6,4 @@ used-before-assignment:33:44:33:53:test_fail4:Using variable 'undefined' before
66
used-before-assignment:39:18:39:28:test_fail5:Using variable 'undefined1' before assignment:HIGH
77
used-before-assignment:90:10:90:18:type_annotation_never_gets_value_despite_nonlocal:Using variable 'some_num' before assignment:HIGH
88
used-before-assignment:96:14:96:18:inner_function_lacks_access_to_outer_args.inner:Using variable 'args' before assignment:HIGH
9+
used-before-assignment:117:18:117:21:nonlocal_in_outer_frame_fail.outer.inner:Using variable 'num' before assignment:HIGH

tests/functional/u/used/used_before_assignment_typing.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ def defined_in_elif_branch(self) -> calendar.Calendar: # [possibly-used-before-
173173

174174
def defined_in_else_branch(self) -> urlopen:
175175
print(zoneinfo) # [used-before-assignment]
176-
print(pprint())
176+
print(pprint()) # [used-before-assignment]
177177
print(collections()) # [used-before-assignment]
178178
return urlopen
179179

tests/functional/u/used/used_before_assignment_typing.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ used-before-assignment:153:20:153:28:VariableAnnotationsGuardedByTypeChecking:Us
66
possibly-used-before-assignment:170:40:170:48:TypeCheckingMultiBranch.defined_in_elif_branch:Possibly using variable 'calendar' before assignment:INFERENCE
77
possibly-used-before-assignment:171:14:171:20:TypeCheckingMultiBranch.defined_in_elif_branch:Possibly using variable 'bisect' before assignment:INFERENCE
88
used-before-assignment:175:14:175:22:TypeCheckingMultiBranch.defined_in_else_branch:Using variable 'zoneinfo' before assignment:INFERENCE
9+
used-before-assignment:176:14:176:20:TypeCheckingMultiBranch.defined_in_else_branch:Using variable 'pprint' before assignment:INFERENCE
910
used-before-assignment:177:14:177:25:TypeCheckingMultiBranch.defined_in_else_branch:Using variable 'collections' before assignment:INFERENCE
1011
possibly-used-before-assignment:180:43:180:48:TypeCheckingMultiBranch.defined_in_nested_if_else:Possibly using variable 'heapq' before assignment:INFERENCE
1112
used-before-assignment:184:39:184:44:TypeCheckingMultiBranch.defined_in_try_except:Using variable 'array' before assignment:INFERENCE

0 commit comments

Comments
 (0)