Skip to content

Commit

Permalink
Emit modified-iterating-list|dict|set for literals or usage of del
Browse files Browse the repository at this point in the history
  • Loading branch information
jacobtylerwalls committed Jun 1, 2022
1 parent a4b9ef5 commit 59fcee7
Show file tree
Hide file tree
Showing 6 changed files with 72 additions and 27 deletions.
5 changes: 5 additions & 0 deletions doc/whatsnew/2/2.15/index.rst
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,11 @@ False positives fixed
False negatives fixed
=====================

* Emit ``modified-iterating-list`` and analogous messages for dicts and sets when iterating
literals, or when using the ``del`` keyword.

Closes #6648


Other bug fixes
===============
Expand Down
35 changes: 31 additions & 4 deletions pylint/checkers/modified_iterating_checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,8 @@ class ModifiedIterationChecker(checkers.BaseChecker):
)
def visit_for(self, node: nodes.For) -> None:
iter_obj = node.iter
if isinstance(iter_obj, nodes.Name):
for body_node in node.body:
self._modified_iterating_check_on_node_and_children(body_node, iter_obj)
for body_node in node.body:
self._modified_iterating_check_on_node_and_children(body_node, iter_obj)

def _modified_iterating_check_on_node_and_children(
self, body_node: nodes.NodeNG, iter_obj: nodes.NodeNG
Expand All @@ -72,7 +71,19 @@ def _modified_iterating_check(
self, node: nodes.NodeNG, iter_obj: nodes.NodeNG
) -> None:
msg_id = None
if self._modified_iterating_list_cond(node, iter_obj):
if isinstance(node, nodes.Delete) and any(
self._deleted_iteration_target_cond(t, iter_obj) for t in node.targets
):
inferred = utils.safe_infer(iter_obj)
if isinstance(inferred, nodes.List):
msg_id = "modified-iterating-list"
elif isinstance(inferred, nodes.Dict):
msg_id = "modified-iterating-dict"
elif isinstance(inferred, nodes.Set):
msg_id = "modified-iterating-set"
elif not isinstance(iter_obj, nodes.Name):
pass
elif self._modified_iterating_list_cond(node, iter_obj):
msg_id = "modified-iterating-list"
elif self._modified_iterating_dict_cond(node, iter_obj):
msg_id = "modified-iterating-dict"
Expand Down Expand Up @@ -150,6 +161,22 @@ def _modified_iterating_set_cond(
and node.value.func.attrname in _SET_MODIFIER_METHODS
)

def _deleted_iteration_target_cond(
self, node: nodes.DelName, iter_obj: nodes.NodeNG
) -> bool:
if not isinstance(node, nodes.DelName):
return False
if not isinstance(iter_obj.parent, nodes.For):
return False
if not isinstance(
iter_obj.parent.target, (nodes.AssignName, nodes.BaseContainer)
):
return False
return any(
t == node.name
for t in utils.find_assigned_names_recursive(iter_obj.parent.target)
)


def register(linter: PyLinter) -> None:
linter.register_checker(ModifiedIterationChecker(linter))
14 changes: 13 additions & 1 deletion pylint/checkers/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
import re
import string
import warnings
from collections.abc import Iterable
from collections.abc import Iterable, Iterator
from functools import lru_cache, partial
from re import Match
from typing import TYPE_CHECKING, Callable, TypeVar
Expand Down Expand Up @@ -1792,3 +1792,15 @@ def in_for_else_branch(parent: nodes.NodeNG, stmt: nodes.Statement) -> bool:
return isinstance(parent, nodes.For) and any(
else_stmt.parent_of(stmt) or else_stmt == stmt for else_stmt in parent.orelse
)


def find_assigned_names_recursive(
target: nodes.AssignName | nodes.BaseContainer,
) -> Iterator[str]:
"""Yield the names of assignment targets, accounting for nested ones."""
if isinstance(target, nodes.AssignName):
if target.name is not None:
yield target.name
elif isinstance(target, nodes.BaseContainer):
for elt in target.elts:
yield from find_assigned_names_recursive(elt)
23 changes: 4 additions & 19 deletions pylint/checkers/variables.py
Original file line number Diff line number Diff line change
Expand Up @@ -1234,13 +1234,12 @@ def leave_functiondef(self, node: nodes.FunctionDef) -> None:

global_names = _flattened_scope_names(node.nodes_of_class(nodes.Global))
nonlocal_names = _flattened_scope_names(node.nodes_of_class(nodes.Nonlocal))
comprehension_target_names: list[str] = []
comprehension_target_names: set[str] = set()

for comprehension_scope in node.nodes_of_class(nodes.ComprehensionScope):
for generator in comprehension_scope.generators:
self._find_assigned_names_recursive(
generator.target, comprehension_target_names
)
for name in utils.find_assigned_names_recursive(generator.target):
comprehension_target_names.add(name)

for name, stmts in not_consumed.items():
self._check_is_unused(
Expand Down Expand Up @@ -1449,20 +1448,6 @@ def _should_node_be_skipped(

return False

def _find_assigned_names_recursive(
self,
target: nodes.AssignName | nodes.BaseContainer,
target_names: list[str],
) -> None:
"""Update `target_names` in place with the names of assignment
targets, recursively (to account for nested assignments).
"""
if isinstance(target, nodes.AssignName):
target_names.append(target.name)
elif isinstance(target, nodes.BaseContainer):
for elt in target.elts:
self._find_assigned_names_recursive(elt, target_names)

# pylint: disable=too-many-return-statements
def _check_consumer(
self,
Expand Down Expand Up @@ -2264,7 +2249,7 @@ def _check_is_unused(
stmt,
global_names,
nonlocal_names: Iterable[str],
comprehension_target_names: list[str],
comprehension_target_names: Iterable[str],
) -> None:
# Ignore some special names specified by user configuration.
if self._is_name_ignored(stmt, name):
Expand Down
12 changes: 12 additions & 0 deletions tests/functional/m/modified_iterating.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,18 @@
item_set.remove(4) # [modified-iterating-set]
item_list.remove(1) # [modified-iterating-list]

for item in [1, 2, 3]:
del item # [modified-iterating-list]

for inner_first, inner_second in [[1, 2], [1, 2]]:
del inner_second # [modified-iterating-list]

for k in my_dict:
del k # [modified-iterating-dict]

for element in item_set:
del element # [modified-iterating-set]

# Check for nested for loops and changes to iterators
for l in item_list:
item_list.append(1) # [modified-iterating-list]
Expand Down
10 changes: 7 additions & 3 deletions tests/functional/m/modified_iterating.txt
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ modified-iterating-set:39:4:39:27::Iterated set 'item_set' is being modified ins
modified-iterating-list:46:8:46:27::Iterated list 'item_list' is being modified inside for loop body, consider iterating through a copy of it instead.:INFERENCE
modified-iterating-set:47:8:47:26::Iterated set 'item_set' is being modified inside for loop body, iterate through a copy of it instead.:INFERENCE
modified-iterating-list:48:4:48:23::Iterated list 'item_list' is being modified inside for loop body, consider iterating through a copy of it instead.:INFERENCE
modified-iterating-list:52:4:52:23::Iterated list 'item_list' is being modified inside for loop body, consider iterating through a copy of it instead.:INFERENCE
modified-iterating-list:55:12:55:31::Iterated list 'item_list' is being modified inside for loop body, consider iterating through a copy of it instead.:INFERENCE
modified-iterating-list:57:16:57:35::Iterated list 'item_list' is being modified inside for loop body, consider iterating through a copy of it instead.:INFERENCE
modified-iterating-list:51:4:51:12::Iterated list 'list' is being modified inside for loop body, consider iterating through a copy of it instead.:INFERENCE
modified-iterating-list:54:4:54:20::Iterated list 'list' is being modified inside for loop body, consider iterating through a copy of it instead.:INFERENCE
modified-iterating-dict:57:4:57:9::Iterated dict 'my_dict' is being modified inside for loop body, iterate through a copy of it instead.:INFERENCE
modified-iterating-set:60:4:60:15::Iterated set 'item_set' is being modified inside for loop body, iterate through a copy of it instead.:INFERENCE
modified-iterating-list:64:4:64:23::Iterated list 'item_list' is being modified inside for loop body, consider iterating through a copy of it instead.:INFERENCE
modified-iterating-list:67:12:67:31::Iterated list 'item_list' is being modified inside for loop body, consider iterating through a copy of it instead.:INFERENCE
modified-iterating-list:69:16:69:35::Iterated list 'item_list' is being modified inside for loop body, consider iterating through a copy of it instead.:INFERENCE

0 comments on commit 59fcee7

Please sign in to comment.