From 6028f202c4b637f9edc54cef815586a54fd65958 Mon Sep 17 00:00:00 2001 From: Zen Lee <53538590+zenlyj@users.noreply.github.com> Date: Sun, 16 Apr 2023 02:01:37 +0800 Subject: Fix `used-before-assignment` TYPE_CHECKING false negatives (#8431) Co-authored-by: Pierre Sassoulas Co-authored-by: Jacob Walls --- doc/whatsnew/fragments/8198.bugfix | 4 ++ pylint/checkers/variables.py | 82 ++++++++++++++++------ .../u/used/used_before_assignment_scoping.py | 18 +++++ .../u/used/used_before_assignment_scoping.txt | 2 + 4 files changed, 86 insertions(+), 20 deletions(-) create mode 100644 doc/whatsnew/fragments/8198.bugfix create mode 100644 tests/functional/u/used/used_before_assignment_scoping.py create mode 100644 tests/functional/u/used/used_before_assignment_scoping.txt diff --git a/doc/whatsnew/fragments/8198.bugfix b/doc/whatsnew/fragments/8198.bugfix new file mode 100644 index 000000000..61b4028ce --- /dev/null +++ b/doc/whatsnew/fragments/8198.bugfix @@ -0,0 +1,4 @@ +Fix ``used-before-assignment`` false negative when TYPE_CHECKING imports +are used in multiple scopes. + +Closes #8198 diff --git a/pylint/checkers/variables.py b/pylint/checkers/variables.py index cad4c76ea..d82b5a8c8 100644 --- a/pylint/checkers/variables.py +++ b/pylint/checkers/variables.py @@ -1257,6 +1257,7 @@ class VariablesChecker(BaseChecker): 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( @@ -1717,30 +1718,16 @@ class VariablesChecker(BaseChecker): 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) @@ -1906,6 +1893,61 @@ class VariablesChecker(BaseChecker): 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.""" diff --git a/tests/functional/u/used/used_before_assignment_scoping.py b/tests/functional/u/used/used_before_assignment_scoping.py new file mode 100644 index 000000000..0d88210bd --- /dev/null +++ b/tests/functional/u/used/used_before_assignment_scoping.py @@ -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 diff --git a/tests/functional/u/used/used_before_assignment_scoping.txt b/tests/functional/u/used/used_before_assignment_scoping.txt new file mode 100644 index 000000000..32b6d3e1b --- /dev/null +++ b/tests/functional/u/used/used_before_assignment_scoping.txt @@ -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 -- cgit v1.2.1