SONARPY-4113 Improve issue reporting for S8518 (#1075)

GitOrigin-RevId: faaa6ec657ff23b9b2a50f4d4651444ad5d1bbf1

Author: guillaume-dequenne

python-checks/src/main/java/org/sonar/python/checks/EnumerateUnpackingCheck.java

@@ -16,6 +16,8 @@
  */
 package org.sonar.python.checks;
 
+import java.util.ArrayList;
+import java.util.List;
 import org.sonar.check.Rule;
 import org.sonar.plugins.python.api.PythonSubscriptionCheck;
 import org.sonar.plugins.python.api.SubscriptionContext;
@@ -40,6 +42,7 @@
 public class EnumerateUnpackingCheck extends PythonSubscriptionCheck {
 
   private static final String MESSAGE = "Unpack the value from 'enumerate()' directly instead of using an index lookup.";
+  private static final String SECONDARY_MESSAGE = "Replace this index lookup with the unpacked value.";
   private static final TypeMatcher ENUMERATE_MATCHER = TypeMatchers.isType("enumerate");
 
   @Override
@@ -84,26 +87,30 @@ private static void check(SubscriptionContext ctx) {
       return;
     }
 
-    boolean hasSubscriptWriteTarget = TreeUtils.hasDescendant(forStatement.body(),
-      node -> isMatchingSubscript(node, indexSymbol, iterableSymbol)
-        && isSubscriptWriteTarget((SubscriptionExpression) node));
+    List<SubscriptionExpression> matchingSubscripts = new ArrayList<>();
+    collectMatchingSubscripts(forStatement.body(), indexSymbol, iterableSymbol, matchingSubscripts);
 
-    if (hasSubscriptWriteTarget) {
+    if (matchingSubscripts.isEmpty()) {
+      return;
+    }
+    if (matchingSubscripts.stream().anyMatch(EnumerateUnpackingCheck::isSubscriptWriteTarget)) {
       return;
     }
 
-    boolean hasRedundantSubscript = TreeUtils.hasDescendant(forStatement.body(),
-      node -> isMatchingSubscript(node, indexSymbol, iterableSymbol));
+    PreciseIssue issue = ctx.addIssue(call, MESSAGE);
+    matchingSubscripts.forEach(subscript -> issue.secondary(subscript, SECONDARY_MESSAGE));
+  }
 
-    if (hasRedundantSubscript) {
-      ctx.addIssue(call, MESSAGE);
+  private static void collectMatchingSubscripts(Tree tree, SymbolV2 indexSymbol, SymbolV2 iterableSymbol, List<SubscriptionExpression> result) {
+    for (Tree child : tree.children()) {
+      if (child instanceof SubscriptionExpression subscription && isMatchingSubscript(subscription, indexSymbol, iterableSymbol)) {
+        result.add(subscription);
+      }
+      collectMatchingSubscripts(child, indexSymbol, iterableSymbol, result);
     }
   }
 
-  private static boolean isMatchingSubscript(Tree node, SymbolV2 indexSymbol, SymbolV2 iterableSymbol) {
-    if (!(node instanceof SubscriptionExpression subscription)) {
-      return false;
-    }
+  private static boolean isMatchingSubscript(SubscriptionExpression subscription, SymbolV2 indexSymbol, SymbolV2 iterableSymbol) {
     if (subscription.subscripts().expressions().size() != 1) {
       return false;
     }

python-checks/src/test/resources/checks/enumerateUnpacking.py

@@ -4,19 +4,34 @@ def noncompliant_basic():
     for i, _ in enumerate(lst):  # Noncompliant {{Unpack the value from 'enumerate()' directly instead of using an index lookup.}}
 #               ^^^^^^^^^^^^^^
         print(lst[i])
+#             ^^^^^^< {{Replace this index lookup with the unpacked value.}}
 
 
 def noncompliant_start_zero_keyword():
     colors = ['red', 'green', 'blue']
     for i, _ in enumerate(colors, start=0):  # Noncompliant
+#               ^^^^^^^^^^^^^^^^^^^^^^^^^^
         print(colors[i])
+#             ^^^^^^^^^< {{Replace this index lookup with the unpacked value.}}
 
 
 def noncompliant_rhs_of_augmented_assignment():
     items = "abcdef"
     result = ""
     for ind, _ in enumerate(items):  # Noncompliant
+#                 ^^^^^^^^^^^^^^^^
         result += items[ind]
+#                 ^^^^^^^^^^< {{Replace this index lookup with the unpacked value.}}
+
+
+def noncompliant_multiple_usages():
+    lst = ['a', 'b', 'c']
+    for i, _ in enumerate(lst):  # Noncompliant
+#               ^^^^^^^^^^^^^^
+        print(lst[i])
+#             ^^^^^^< {{Replace this index lookup with the unpacked value.}}
+        print(lst[i].upper())
+#             ^^^^^^< {{Replace this index lookup with the unpacked value.}}
 
 
 def compliant_proper_unpacking():
@@ -90,6 +105,13 @@ def compliant_write_assignment():
         items[i] = item.upper()
 
 
+def compliant_mixed_read_and_write():
+    items = ['a', 'b', 'c']
+    for i, _ in enumerate(items):
+        print(items[i])
+        items[i] = items[i].upper()
+
+
 def compliant_write_augmented_assignment():
     counts = [0, 0, 0]
     for i, val in enumerate(counts):