Skip to content

Commit

Permalink
Fix used-before-assignment TYPE_CHECKING false negatives (#8431)
Browse files Browse the repository at this point in the history
Co-authored-by: Pierre Sassoulas <pierre.sassoulas@gmail.com>
Co-authored-by: Jacob Walls <jacobtylerwalls@gmail.com>
  • Loading branch information
3 people authored Apr 15, 2023
1 parent 2db55f6 commit 6028f20
Show file tree
Hide file tree
Showing 4 changed files with 86 additions and 20 deletions.
4 changes: 4 additions & 0 deletions doc/whatsnew/fragments/8198.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Fix ``used-before-assignment`` false negative when TYPE_CHECKING imports
are used in multiple scopes.

Closes #8198
82 changes: 62 additions & 20 deletions pylint/checkers/variables.py
Original file line number Diff line number Diff line change
Expand Up @@ -1257,6 +1257,7 @@ def __init__(self, linter: PyLinter) -> None:
tuple[nodes.ExceptHandler, nodes.AssignName]
] = []
"""This is a queue, last in first out."""
self._evaluated_type_checking_scopes: dict[str, list[nodes.NodeNG]] = {}
self._postponed_evaluation_enabled = False

@utils.only_required_for_messages(
Expand Down Expand Up @@ -1717,30 +1718,16 @@ def _check_consumer(
if found_nodes is None:
return (VariableVisitConsumerAction.CONTINUE, None)
if not found_nodes:
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,
)
self._report_unfound_name_definition(node, current_consumer)
# Mark for consumption any nodes added to consumed_uncertain by
# get_next_to_consume() because they might not have executed.
nodes_to_consume = current_consumer.consumed_uncertain[node.name]
nodes_to_consume = self._filter_type_checking_import_from_consumption(
node, nodes_to_consume
)
return (
VariableVisitConsumerAction.RETURN,
current_consumer.consumed_uncertain[node.name],
nodes_to_consume,
)

self._check_late_binding_closure(node)
Expand Down Expand Up @@ -1906,6 +1893,61 @@ def _check_consumer(

return (VariableVisitConsumerAction.RETURN, found_nodes)

def _report_unfound_name_definition(
self, node: nodes.NodeNG, current_consumer: NamesConsumer
) -> None:
"""Reports used-before-assignment when all name definition nodes
get filtered out by NamesConsumer.
"""
if (
self._postponed_evaluation_enabled
and utils.is_node_in_type_annotation_context(node)
):
return
if self._is_builtin(node.name):
return
if self._is_variable_annotation_in_function(node):
return
if (
node.name in self._evaluated_type_checking_scopes
and node.scope() in self._evaluated_type_checking_scopes[node.name]
):
return

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,
)

def _filter_type_checking_import_from_consumption(
self, node: nodes.NodeNG, nodes_to_consume: list[nodes.NodeNG]
) -> list[nodes.NodeNG]:
"""Do not consume type-checking import node as used-before-assignment
may invoke in different scopes.
"""
type_checking_import = next(
(
n
for n in nodes_to_consume
if isinstance(n, (nodes.Import, nodes.ImportFrom))
and in_type_checking_block(n)
),
None,
)
# If used-before-assignment reported for usage of type checking import
# keep track of its scope
if type_checking_import and not self._is_variable_annotation_in_function(node):
self._evaluated_type_checking_scopes.setdefault(node.name, []).append(
node.scope()
)
nodes_to_consume = [n for n in nodes_to_consume if n != type_checking_import]
return nodes_to_consume

@utils.only_required_for_messages("no-name-in-module")
def visit_import(self, node: nodes.Import) -> None:
"""Check modules attribute accesses."""
Expand Down
18 changes: 18 additions & 0 deletions tests/functional/u/used/used_before_assignment_scoping.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# pylint: disable=missing-function-docstring, missing-module-docstring

from typing import TYPE_CHECKING

if TYPE_CHECKING:
from datetime import datetime


def func_two():
second = datetime.now() # [used-before-assignment]
return second


def func():
first: datetime
first = datetime.now() # [used-before-assignment]
second = datetime.now()
return first, second
2 changes: 2 additions & 0 deletions tests/functional/u/used/used_before_assignment_scoping.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
used-before-assignment:10:13:10:21:func_two:Using variable 'datetime' before assignment:CONTROL_FLOW
used-before-assignment:16:12:16:20:func:Using variable 'datetime' before assignment:CONTROL_FLOW

0 comments on commit 6028f20

Please sign in to comment.