Skip to content

Commit

Permalink
Remove assumption of direct parentage in used-before-assignment hom…
Browse files Browse the repository at this point in the history
…onym handling

The previous fixes for false positives involving homonyms with variables in comprehension tests in
#5586 and #5817 still relied on assumptions of direct parentage.
  • Loading branch information
jacobtylerwalls authored and Pierre-Sassoulas committed Mar 31, 2022
1 parent 5ea03af commit 8812626
Show file tree
Hide file tree
Showing 3 changed files with 78 additions and 10 deletions.
6 changes: 6 additions & 0 deletions ChangeLog
Expand Up @@ -20,6 +20,12 @@ What's New in Pylint 2.13.4?
============================
Release date: TBA

* Fix false positive regression in 2.13.0 for ``used-before-assignment`` for
homonyms between variable assignments in try/except blocks and variables in
a comprehension's filter.

Closes #6035

* Include ``testing_pylintrc`` in source and wheel distributions.

Closes #6028
Expand Down
40 changes: 30 additions & 10 deletions pylint/checkers/variables.py
Expand Up @@ -623,13 +623,12 @@ def get_next_to_consume(self, node: nodes.Name) -> Optional[List[nodes.NodeNG]]:
):
return found_nodes

# And is not part of a test in a filtered comprehension
if VariablesChecker._has_homonym_in_comprehension_test(node):
return found_nodes

# Filter out assignments in ExceptHandlers that node is not contained in
# unless this is a test in a filtered comprehension
# Example: [e for e in range(3) if e] <--- followed by except e:
if found_nodes and (
not isinstance(parent_node, nodes.Comprehension)
or node not in parent_node.ifs
):
if found_nodes:
found_nodes = [
n
for n in found_nodes
Expand Down Expand Up @@ -1500,10 +1499,7 @@ def _check_consumer(
# (like "if x" in "[x for x in expr() if x]")
# https://github.com/PyCQA/pylint/issues/5586
and not (
(
isinstance(node.parent.parent, nodes.Comprehension)
and node.parent in node.parent.parent.ifs
)
self._has_homonym_in_comprehension_test(node)
# Or homonyms against values to keyword arguments
# (like "var" in "[func(arg=var) for var in expr()]")
or (
Expand Down Expand Up @@ -2476,6 +2472,30 @@ def _has_homonym_in_upper_function_scope(
for _consumer in self._to_consume[index - 1 :: -1]
)

@staticmethod
def _has_homonym_in_comprehension_test(node: nodes.Name) -> bool:
"""Return True if `node`'s frame contains a comprehension employing an
identical name in a test.
The name in the test could appear at varying depths:
Examples:
[x for x in range(3) if name]
[x for x in range(3) if name.num == 1]
[x for x in range(3)] if call(name.num)]
"""
closest_comprehension = utils.get_node_first_ancestor_of_type(
node, nodes.Comprehension
)
return (
closest_comprehension is not None
and node.frame(future=True).parent_of(closest_comprehension)
and any(
test is node or test.parent_of(node)
for test in closest_comprehension.ifs
)
)

def _store_type_annotation_node(self, type_annotation):
"""Given a type annotation, store all the name nodes it refers to."""
if isinstance(type_annotation, nodes.Name):
Expand Down
Expand Up @@ -7,3 +7,45 @@ def func():
except ZeroDivisionError:
value = 1
print(value)


def func2():
"""Same, but with attribute access."""
try:
print(value for value in range(1 / 0) if isinstance(value.num, int))
except ZeroDivisionError:
value = 1
print(value)


def func3():
"""Same, but with no call."""
try:
print(value for value in range(1 / 0) if value)
except ZeroDivisionError:
value = 1
print(value)


def func4():
"""https://github.com/PyCQA/pylint/issues/6035"""
assets = [asset for asset in range(3) if asset.name == "filename"]

try:
raise ValueError
except ValueError:
asset = assets[0]
print(asset)


def func5():
"""Similar, but with subscript notation"""
results = {}
# pylint: disable-next=consider-using-dict-items
filtered = [k for k in results if isinstance(results[k], dict)]

try:
1 / 0
except ZeroDivisionError:
k = None
print(k, filtered)

0 comments on commit 8812626

Please sign in to comment.