Skip to content

Commit

Permalink
Emit used-before-assignment after if/else switches
Browse files Browse the repository at this point in the history
  • Loading branch information
jacobtylerwalls committed Aug 13, 2023
1 parent b02a5d7 commit f0be36d
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 22 deletions.
4 changes: 4 additions & 0 deletions doc/whatsnew/fragments/1727.false_negative
Original file line number Diff line number Diff line change
@@ -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
44 changes: 22 additions & 22 deletions pylint/checkers/variables.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()]
Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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.
"""
Expand Down Expand Up @@ -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(
Expand All @@ -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).
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit f0be36d

Please sign in to comment.