Skip to content

Commit 0d7b0d7

Browse files
authored
Resolve false negative for used-before-assignment with TYPE_CHECKING guard (#9990)
* Handle case where class is defined under TYPE_CHECKING guard
1 parent 537e9da commit 0d7b0d7

File tree

6 files changed

+48
-35
lines changed

6 files changed

+48
-35
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 `TYPE_CHECKING` import is used as a type annotation prior to erroneous usage.
2+
3+
Refs #8893

pylint/checkers/variables.py

Lines changed: 37 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -723,6 +723,8 @@ def _uncertain_nodes_if_tests(
723723
name = other_node.name
724724
elif isinstance(other_node, (nodes.Import, nodes.ImportFrom)):
725725
name = node.name
726+
elif isinstance(other_node, nodes.ClassDef):
727+
name = other_node.name
726728
else:
727729
continue
728730

@@ -1263,7 +1265,7 @@ def __init__(self, linter: PyLinter) -> None:
12631265
tuple[nodes.ExceptHandler, nodes.AssignName]
12641266
] = []
12651267
"""This is a queue, last in first out."""
1266-
self._evaluated_type_checking_scopes: dict[
1268+
self._reported_type_checking_usage_scopes: dict[
12671269
str, list[nodes.LocalsDictNodeNG]
12681270
] = {}
12691271
self._postponed_evaluation_enabled = False
@@ -1763,12 +1765,12 @@ def _check_consumer(
17631765
if found_nodes is None:
17641766
return (VariableVisitConsumerAction.CONTINUE, None)
17651767
if not found_nodes:
1766-
self._report_unfound_name_definition(node, current_consumer)
1768+
is_reported = self._report_unfound_name_definition(node, current_consumer)
17671769
# Mark for consumption any nodes added to consumed_uncertain by
17681770
# get_next_to_consume() because they might not have executed.
17691771
nodes_to_consume = current_consumer.consumed_uncertain[node.name]
1770-
nodes_to_consume = self._filter_type_checking_import_from_consumption(
1771-
node, nodes_to_consume
1772+
nodes_to_consume = self._filter_type_checking_definitions_from_consumption(
1773+
node, nodes_to_consume, is_reported
17721774
)
17731775
return (
17741776
VariableVisitConsumerAction.RETURN,
@@ -1942,24 +1944,26 @@ def _report_unfound_name_definition(
19421944
self,
19431945
node: nodes.NodeNG,
19441946
current_consumer: NamesConsumer,
1945-
) -> None:
1946-
"""Reports used-before-assignment when all name definition nodes
1947-
get filtered out by NamesConsumer.
1947+
) -> bool:
1948+
"""Reports used-before-assignment error when all name definition nodes
1949+
are filtered out by NamesConsumer.
1950+
1951+
Returns True if an error is reported; otherwise, returns False.
19481952
"""
19491953
if (
19501954
self._postponed_evaluation_enabled
19511955
and utils.is_node_in_type_annotation_context(node)
19521956
):
1953-
return
1957+
return False
19541958
if self._is_builtin(node.name):
1955-
return
1959+
return False
19561960
if self._is_variable_annotation_in_function(node):
1957-
return
1961+
return False
19581962
if (
1959-
node.name in self._evaluated_type_checking_scopes
1960-
and node.scope() in self._evaluated_type_checking_scopes[node.name]
1963+
node.name in self._reported_type_checking_usage_scopes
1964+
and node.scope() in self._reported_type_checking_usage_scopes[node.name]
19611965
):
1962-
return
1966+
return False
19631967

19641968
confidence = HIGH
19651969
if node.name in current_consumer.names_under_always_false_test:
@@ -1979,31 +1983,33 @@ def _report_unfound_name_definition(
19791983
confidence=confidence,
19801984
)
19811985

1982-
def _filter_type_checking_import_from_consumption(
1986+
return True
1987+
1988+
def _filter_type_checking_definitions_from_consumption(
19831989
self,
19841990
node: nodes.NodeNG,
19851991
nodes_to_consume: list[nodes.NodeNG],
1992+
is_reported: bool,
19861993
) -> list[nodes.NodeNG]:
1987-
"""Do not consume type-checking import node as used-before-assignment
1988-
may invoke in different scopes.
1994+
"""Filters out type-checking definition nodes (e.g. imports, class definitions)
1995+
from consumption, as used-before-assignment may invoke in a different context.
1996+
1997+
If used-before-assignment is reported for the usage of a type-checking definition,
1998+
track the scope of that usage for future evaluation.
19891999
"""
1990-
type_checking_import = next(
1991-
(
1992-
n
1993-
for n in nodes_to_consume
1994-
if isinstance(n, (nodes.Import, nodes.ImportFrom))
1995-
and in_type_checking_block(n)
1996-
),
1997-
None,
1998-
)
1999-
# If used-before-assignment reported for usage of type checking import
2000-
# keep track of its scope
2001-
if type_checking_import and not self._is_variable_annotation_in_function(node):
2002-
self._evaluated_type_checking_scopes.setdefault(node.name, []).append(
2000+
type_checking_definitions = {
2001+
n
2002+
for n in nodes_to_consume
2003+
if isinstance(n, (nodes.Import, nodes.ImportFrom, nodes.ClassDef))
2004+
and in_type_checking_block(n)
2005+
}
2006+
2007+
if type_checking_definitions and is_reported:
2008+
self._reported_type_checking_usage_scopes.setdefault(node.name, []).append(
20032009
node.scope()
20042010
)
2005-
nodes_to_consume = [n for n in nodes_to_consume if n != type_checking_import]
2006-
return nodes_to_consume
2011+
2012+
return [n for n in nodes_to_consume if n not in type_checking_definitions]
20072013

20082014
@utils.only_required_for_messages("no-name-in-module")
20092015
def visit_import(self, node: nodes.Import) -> None:

tests/functional/u/used/used_before_assignment_postponed_evaluation.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,17 +20,17 @@ def f():
2020
return datetime.datetime.now() # [used-before-assignment]
2121

2222
def g() -> datetime.datetime:
23-
return datetime.datetime.now() # FALSE NEGATIVE
23+
return datetime.datetime.now() # [used-before-assignment]
2424

2525
if TYPE_CHECKING:
2626
class X:
2727
pass
2828

2929
def h():
30-
return X() # FALSE NEGATIVE
30+
return X() # [used-before-assignment]
3131

3232
def i() -> X:
33-
return X() # FALSE NEGATIVE
33+
return X() # [used-before-assignment]
3434

3535
if TYPE_CHECKING:
3636
from mod import Y
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,5 @@
11
used-before-assignment:10:6:10:9::Using variable 'var' before assignment:INFERENCE
22
used-before-assignment:20:11:20:19:f:Using variable 'datetime' before assignment:INFERENCE
3+
used-before-assignment:23:11:23:19:g:Using variable 'datetime' before assignment:INFERENCE
4+
used-before-assignment:30:11:30:12:h:Using variable 'X' before assignment:INFERENCE
5+
used-before-assignment:33:11:33:12:i:Using variable 'X' before assignment:INFERENCE

tests/functional/u/used/used_before_assignment_typing.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,7 @@ def defined_in_elif_branch(self) -> calendar.Calendar: # [possibly-used-before-
174174
def defined_in_else_branch(self) -> urlopen:
175175
print(zoneinfo) # [used-before-assignment]
176176
print(pprint())
177-
print(collections())
177+
print(collections()) # [used-before-assignment]
178178
return urlopen
179179

180180
def defined_in_nested_if_else(self) -> heapq: # [possibly-used-before-assignment]

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:177:14:177:25:TypeCheckingMultiBranch.defined_in_else_branch:Using variable 'collections' before assignment:INFERENCE
910
possibly-used-before-assignment:180:43:180:48:TypeCheckingMultiBranch.defined_in_nested_if_else:Possibly using variable 'heapq' before assignment:INFERENCE
1011
used-before-assignment:184:39:184:44:TypeCheckingMultiBranch.defined_in_try_except:Using variable 'array' before assignment:INFERENCE
1112
used-before-assignment:185:14:185:19:TypeCheckingMultiBranch.defined_in_try_except:Using variable 'types' before assignment:INFERENCE

0 commit comments

Comments
 (0)