Skip to content

Don't assume direct parentage when emitting used-before-assignment #5582

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 17 commits into from
Jan 10, 2022
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions ChangeLog
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,12 @@ Release date: TBA

Closes #85, #2615

* Fixed false negative for ``used-before-assignment`` when a conditional
or context manager intervened before the try statement that suggested
it might fail.

Closes #4045

* Fixed extremely long processing of long lines with comma's.

Closes #5483
Expand Down
6 changes: 6 additions & 0 deletions doc/whatsnew/2.13.rst
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,12 @@ Other Changes

Closes #85, #2615

* Fixed false negative for ``used-before-assignment`` when a conditional
or context manager intervened before the try statement that suggested
it might fail.

Closes #4045

* Fix a false positive for ``assigning-non-slot`` when the slotted class
defined ``__setattr__``.

Expand Down
58 changes: 44 additions & 14 deletions pylint/checkers/variables.py
Original file line number Diff line number Diff line change
Expand Up @@ -677,21 +677,15 @@ def get_next_to_consume(self, node):

# If this node is in an ExceptHandler,
# filter out assignments in the try portion, assuming they may fail
if found_nodes and isinstance(node_statement.parent, nodes.ExceptHandler):
filtered_nodes = [
n
for n in found_nodes
if not (
isinstance(n.statement(future=True).parent, nodes.TryExcept)
and n.statement(future=True) in n.statement(future=True).parent.body
and node_statement.parent
in n.statement(future=True).parent.handlers
if found_nodes:
uncertain_nodes = (
self._uncertain_nodes_in_try_blocks_when_evaluating_except_blocks(
found_nodes, node_statement
)
]
filtered_nodes_set = set(filtered_nodes)
difference = [n for n in found_nodes if n not in filtered_nodes_set]
self.consumed_uncertain[node.name] += difference
found_nodes = filtered_nodes
)
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]

return found_nodes

Expand Down Expand Up @@ -740,6 +734,42 @@ def _uncertain_nodes_in_except_blocks(found_nodes, node, node_statement):
uncertain_nodes.append(other_node)
return uncertain_nodes

@staticmethod
def _uncertain_nodes_in_try_blocks_when_evaluating_except_blocks(
found_nodes, node_statement
):
"""
Return any nodes in ``found_nodes`` that should be treated as uncertain
because they are in a try block and the ``node_statement`` being evaluated
is in one of its except handlers.
"""
closest_except_handler = utils.get_node_first_ancestor_of_type(
node_statement, nodes.ExceptHandler
)
if closest_except_handler is None:
return []
closest_try_ancestor = utils.get_node_first_ancestor_of_type(
node_statement, nodes.TryExcept
)
uncertain_nodes = []
for other_node in found_nodes:
other_node_statement = other_node.statement(future=True)
if other_node_statement is closest_except_handler:
continue
other_node_try_ancestor = utils.get_node_first_ancestor_of_type(
other_node_statement, nodes.TryExcept
)
if other_node_try_ancestor is not closest_try_ancestor:
continue
other_node_except_handler = utils.get_node_first_ancestor_of_type(
other_node_statement, nodes.ExceptHandler
)
if other_node_except_handler is closest_except_handler:
continue
# Passed all tests for uncertain execution
uncertain_nodes.append(other_node)
return uncertain_nodes


# pylint: disable=too-many-public-methods
class VariablesChecker(BaseChecker):
Expand Down
36 changes: 36 additions & 0 deletions tests/functional/u/use/used_before_assignment_issue2615.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,42 @@ def main():
try:
res = 1 / 0
res = 42
if main():
res = None
with open(__file__, encoding="utf-8") as opened_file:
res = opened_file.readlines()
except ZeroDivisionError:
print(res) # [used-before-assignment]
print(res)


def nested_except_blocks():
"""Don't confuse except blocks with each other."""
try:
res = 1 / 0
res = 42
if main():
res = None
with open(__file__, encoding="utf-8") as opened_file:
res = opened_file.readlines()
except ZeroDivisionError:
try:
more_bad_division = 1 / 0
except ZeroDivisionError:
print(more_bad_division) # [used-before-assignment]
print(res) # don't emit: too nested to make confident inferences
print(res)


def name_earlier_in_except_block():
"""Permit the name that might not have been assigned during the try block
to be defined inside a conditional inside the except block.
"""
try:
res = 1 / 0
except ZeroDivisionError:
if main():
res = 10
else:
res = 11
print(res)
3 changes: 2 additions & 1 deletion tests/functional/u/use/used_before_assignment_issue2615.txt
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
used-before-assignment:8:14:8:17:main:Using variable 'res' before assignment:UNDEFINED
used-before-assignment:12:14:12:17:main:Using variable 'res' before assignment:UNDEFINED
used-before-assignment:29:18:29:35:nested_except_blocks:Using variable 'more_bad_division' before assignment:UNDEFINED