From f0be36da0f761041904d94a454c3c36e80797656 Mon Sep 17 00:00:00 2001 From: Jacob Walls Date: Sun, 13 Aug 2023 13:13:59 -0400 Subject: [PATCH] Emit `used-before-assignment` after if/else switches --- doc/whatsnew/fragments/1727.false_negative | 4 ++ pylint/checkers/variables.py | 44 +++++++++++----------- 2 files changed, 26 insertions(+), 22 deletions(-) create mode 100644 doc/whatsnew/fragments/1727.false_negative diff --git a/doc/whatsnew/fragments/1727.false_negative b/doc/whatsnew/fragments/1727.false_negative new file mode 100644 index 0000000000..7055606bf0 --- /dev/null +++ b/doc/whatsnew/fragments/1727.false_negative @@ -0,0 +1,4 @@ +Emit ``used-before-assignment`` when relying on names after an ``if/else`` +switch when one branch failed to define the name, raise, or return. + +Closes #1727 diff --git a/pylint/checkers/variables.py b/pylint/checkers/variables.py index 6d929a0710..5ae7fbb895 100644 --- a/pylint/checkers/variables.py +++ b/pylint/checkers/variables.py @@ -534,6 +534,7 @@ def __init__(self, node: nodes.NodeNG, scope_type: str) -> None: copy.copy(node.locals), {}, collections.defaultdict(list), scope_type ) self.node = node + self.names_under_always_false_test = set() def __repr__(self) -> str: _to_consumes = [f"{k}->{v}" for k, v in self._atomic.to_consume.items()] @@ -635,7 +636,7 @@ def get_next_to_consume(self, node: nodes.Name) -> list[nodes.NodeNG] | None: # Filter out assignments guarded by always false conditions if found_nodes: - uncertain_nodes = self._uncertain_nodes_in_false_tests(found_nodes, node) + uncertain_nodes = self._uncertain_nodes_if_tests(found_nodes, node) self.consumed_uncertain[node.name] += uncertain_nodes uncertain_nodes_set = set(uncertain_nodes) found_nodes = [n for n in found_nodes if n not in uncertain_nodes_set] @@ -685,8 +686,9 @@ def get_next_to_consume(self, node: nodes.Name) -> list[nodes.NodeNG] | None: return found_nodes - @staticmethod - def _inferred_to_define_name_raise_or_return(name: str, node: nodes.NodeNG) -> bool: + def _inferred_to_define_name_raise_or_return( + self, name: str, node: nodes.NodeNG + ) -> bool: """Return True if there is a path under this `if_node` that is inferred to define `name`, raise, or return. """ @@ -736,17 +738,14 @@ def _inferred_to_define_name_raise_or_return(name: str, node: nodes.NodeNG) -> b # Only search else branch when test condition is inferred to be false if all_inferred and only_search_else: - return NamesConsumer._branch_handles_name(name, node.orelse) - # Only search if branch when test condition is inferred to be true - if all_inferred and only_search_if: - return NamesConsumer._branch_handles_name(name, node.body) + self.names_under_always_false_test.add(node.name) + return self._branch_handles_name(name, node.orelse) # Search both if and else branches - return NamesConsumer._branch_handles_name( - name, node.body - ) or NamesConsumer._branch_handles_name(name, node.orelse) + return self._branch_handles_name(name, node.body) and self._branch_handles_name( + name, node.orelse + ) - @staticmethod - def _branch_handles_name(name: str, body: Iterable[nodes.NodeNG]) -> bool: + def _branch_handles_name(self, name: str, body: Iterable[nodes.NodeNG]) -> bool: return any( NamesConsumer._defines_name_raises_or_returns(name, if_body_stmt) or isinstance( @@ -759,17 +758,15 @@ def _branch_handles_name(name: str, body: Iterable[nodes.NodeNG]) -> bool: nodes.While, ), ) - and NamesConsumer._inferred_to_define_name_raise_or_return( - name, if_body_stmt - ) + and self._inferred_to_define_name_raise_or_return(name, if_body_stmt) for if_body_stmt in body ) - def _uncertain_nodes_in_false_tests( + def _uncertain_nodes_if_tests( self, found_nodes: list[nodes.NodeNG], node: nodes.NodeNG ) -> list[nodes.NodeNG]: - """Identify nodes of uncertain execution because they are defined under - tests that evaluate false. + """Identify nodes of uncertain execution because they are defined under if + tests. Don't identify a node if there is a path that is inferred to define the name, raise, or return (e.g. any executed if/elif/else branch). @@ -805,7 +802,7 @@ def _uncertain_nodes_in_false_tests( continue # Name defined in the if/else control flow - if NamesConsumer._inferred_to_define_name_raise_or_return(name, outer_if): + if self._inferred_to_define_name_raise_or_return(name, outer_if): continue uncertain_nodes.append(other_node) @@ -1949,9 +1946,12 @@ def _report_unfound_name_definition( ): return - confidence = ( - CONTROL_FLOW if node.name in current_consumer.consumed_uncertain else HIGH - ) + confidence = HIGH + if node.name in current_consumer.consumed_uncertain: + confidence = CONTROL_FLOW + if node.name in current_consumer.names_under_always_false_test: + confidence = INFERENCE + self.add_message( "used-before-assignment", args=node.name,