Emit modified-iterating-list|dict|set for literals or usage of del (#6652)

Author: jacobtylerwalls

doc/whatsnew/2/2.15/index.rst

@@ -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
 ===============

pylint/checkers/modified_iterating_checker.py

@@ -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
@@ -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"
@@ -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))

pylint/checkers/utils.py

@@ -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
@@ -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)

pylint/checkers/variables.py

@@ -1245,13 +1245,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(
@@ -1460,20 +1459,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,
@@ -2275,7 +2260,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):

tests/functional/m/modified_iterating.py

@@ -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]

tests/functional/m/modified_iterating.txt

@@ -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