Suppress unnecessary-list-index-lookup for whole loop on write

doc/whatsnew/2/2.14/full.rst

@@ -23,6 +23,11 @@ Release date: TBA
 
   Closes #6790
 
+* Fixed a false positive in ``unnecessary-list-index-lookup`` and ``unnecessary-dict-index-lookup``
+  when the subscript is updated in the body of a nested loop.
+
+  Closes #6818
+
 * Fixed the use of abbreviations for some special options on the command line.
 
   Closes #6810

pylint/checkers/refactoring/refactoring_checker.py

@@ -1921,14 +1921,30 @@ def _check_unnecessary_dict_index_lookup(
                 return
             iterating_object_name = node.iter.func.expr.as_string()
 
+            # Store potential violations. These will only be reported if we don't
+            # discover any writes to the collection during the loop.
+            messages = []
+
             # Verify that the body of the for loop uses a subscript
             # with the object that was iterated. This uses some heuristics
             # in order to make sure that the same object is used in the
             # for body.
 
             children = (
-                node.body if isinstance(node, nodes.For) else node.parent.get_children()
+                node.body
+                if isinstance(node, nodes.For)
+                else list(node.parent.get_children())
+            )
+
+            # Check if there are any for / while loops within the loop in question;
+            # If so, we will be more conservative about reporting errors as we
+            # can't yet do proper control flow analysis to be sure when
+            # reassignment will affect us
+            nested_loops = itertools.chain.from_iterable(
+                child.nodes_of_class((nodes.For, nodes.While)) for child in children
             )
+            has_nested_loops = next(nested_loops, None) is not None
+
             for child in children:
                 for subscript in child.nodes_of_class(nodes.Subscript):
                     if not isinstance(subscript.value, (nodes.Name, nodes.Attribute)):
@@ -1968,11 +1984,19 @@ def _check_unnecessary_dict_index_lookup(
                             # defined and compare that to the for loop's line number
                             continue
 
-                        self.add_message(
-                            "unnecessary-dict-index-lookup",
-                            node=subscript,
-                            args=(node.target.elts[1].as_string()),
-                        )
+                        if has_nested_loops:
+                            messages.append(
+                                {
+                                    "node": subscript,
+                                    "variable": node.target.elts[1].as_string(),
+                                }
+                            )
+                        else:
+                            self.add_message(
+                                "unnecessary-dict-index-lookup",
+                                node=subscript,
+                                args=(node.target.elts[1].as_string(),),
+                            )
 
                     # Case where .items is assigned to single var (i.e., for item in d.items())
                     elif isinstance(value, nodes.Subscript):
@@ -1999,11 +2023,31 @@ def _check_unnecessary_dict_index_lookup(
                         inferred = utils.safe_infer(value.slice)
                         if not isinstance(inferred, nodes.Const) or inferred.value != 0:
                             continue
-                        self.add_message(
-                            "unnecessary-dict-index-lookup",
-                            node=subscript,
-                            args=("1".join(value.as_string().rsplit("0", maxsplit=1)),),
-                        )
+
+                        if has_nested_loops:
+                            messages.append(
+                                {
+                                    "node": subscript,
+                                    "variable": "1".join(
+                                        value.as_string().rsplit("0", maxsplit=1)
+                                    ),
+                                }
+                            )
+                        else:
+                            self.add_message(
+                                "unnecessary-dict-index-lookup",
+                                node=subscript,
+                                args=(
+                                    "1".join(value.as_string().rsplit("0", maxsplit=1)),
+                                ),
+                            )
+
+            for message in messages:
+                self.add_message(
+                    "unnecessary-dict-index-lookup",
+                    node=message["node"],
+                    args=(message["variable"],),
+                )
 
     def _check_unnecessary_list_index_lookup(
         self, node: nodes.For | nodes.Comprehension
@@ -2029,9 +2073,25 @@ def _check_unnecessary_list_index_lookup(
         iterating_object_name = node.iter.args[0].name
         value_variable = node.target.elts[1]
 
+        # Store potential violations. These will only be reported if we don't
+        # discover any writes to the collection during the loop.
+        bad_nodes = []
+
         children = (
-            node.body if isinstance(node, nodes.For) else node.parent.get_children()
+            node.body
+            if isinstance(node, nodes.For)
+            else list(node.parent.get_children())
+        )
+
+        # Check if there are any for / while loops within the loop in question;
+        # If so, we will be more conservative about reporting errors as we
+        # can't yet do proper control flow analysis to be sure when
+        # reassignment will affect us
+        nested_loops = itertools.chain.from_iterable(
+            child.nodes_of_class((nodes.For, nodes.While)) for child in children
         )
+        has_nested_loops = next(nested_loops, None) is not None
+
         for child in children:
             for subscript in child.nodes_of_class(nodes.Subscript):
                 if isinstance(node, nodes.For) and _is_part_of_assignment_target(
@@ -2070,9 +2130,23 @@ def _check_unnecessary_list_index_lookup(
                         # reassigned on a later line, so it can't be used.
                         continue
 
-                    self.add_message(
-                        "unnecessary-list-index-lookup",
-                        node=subscript,
-                        args=(node.target.elts[1].name,),
-                        confidence=HIGH,
-                    )
+                    if has_nested_loops:
+                        # Have found a likely issue, but since there are nested
+                        # loops we don't want to report this unless we get to the
+                        # end fo the loop without updating the collection
+                        bad_nodes.append(subscript)
+                    else:
+                        self.add_message(
+                            "unnecessary-list-index-lookup",
+                            node=subscript,
+                            args=(node.target.elts[1].name,),
+                            confidence=HIGH,
+                        )
+
+        for subscript in bad_nodes:
+            self.add_message(
+                "unnecessary-list-index-lookup",
+                node=subscript,
+                args=(node.target.elts[1].name,),
+                confidence=HIGH,
+            )

tests/functional/u/unnecessary/unnecessary_dict_index_lookup.py

@@ -124,3 +124,9 @@ class Foo:
 d = {'a': 1, 'b': 2, 'c': 3}
 for key, val in d.items():
     ([d[key], x], y) = ([1, 2], 3)
+
+# Regression test for https://github.com/PyCQA/pylint/issues/6818
+d = {'a': 1, 'b': 2, 'c': 3}
+for key, val in d.items():
+    while d[key] > 0:
+        d[key] -= 1

tests/functional/u/unnecessary/unnecessary_list_index_lookup.py

@@ -62,3 +62,15 @@ def process_list_again(data):
 num_list = [1, 2, 3]
 for a, b in enumerate(num_list):
     ([x, num_list[a]], _) = ([5, 6], 1)
+
+# Regression test for https://github.com/PyCQA/pylint/issues/6818
+updated_list = [1, 2, 3]
+for idx, val in enumerate(updated_list):
+    while updated_list[idx] > 0:
+        updated_list[idx] -= 1
+
+updated_list = [1, 2, 3]
+for idx, val in enumerate(updated_list):
+    print(updated_list[idx]) # [unnecessary-list-index-lookup]
+    updated_list[idx] -= 1
+    print(updated_list[idx])

tests/functional/u/unnecessary/unnecessary_list_index_lookup.txt

@@ -1,3 +1,4 @@
 unnecessary-list-index-lookup:8:10:8:22::Unnecessary list index lookup, use 'val' instead:HIGH
 unnecessary-list-index-lookup:43:52:43:64::Unnecessary list index lookup, use 'val' instead:HIGH
 unnecessary-list-index-lookup:46:10:46:22::Unnecessary list index lookup, use 'val' instead:HIGH
+unnecessary-list-index-lookup:74:10:74:27::Unnecessary list index lookup, use 'val' instead:HIGH