SONARPY-3971 Rule S8518 "enumerate()" values should be unpacked instead of using index lookups (#1010)

GitOrigin-RevId: 00eba0c1ece4f51bda64e9a76c74a5445ffe476f

Author: sonar-nigel[bot]

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

@@ -0,0 +1,136 @@
+/*
+ * SonarQube Python Plugin
+ * Copyright (C) SonarSource Sàrl
+ * mailto:info AT sonarsource DOT com
+ *
+ * You can redistribute and/or modify this program under the terms of
+ * the Sonar Source-Available License Version 1, as published by SonarSource Sàrl.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
+ * See the Sonar Source-Available License for more details.
+ *
+ * You should have received a copy of the Sonar Source-Available License
+ * along with this program; if not, see https://sonarsource.com/license/ssal/
+ */
+package org.sonar.python.checks;
+
+import org.sonar.check.Rule;
+import org.sonar.plugins.python.api.PythonSubscriptionCheck;
+import org.sonar.plugins.python.api.SubscriptionContext;
+import org.sonar.plugins.python.api.symbols.v2.SymbolV2;
+import org.sonar.plugins.python.api.tree.AnnotatedAssignment;
+import org.sonar.plugins.python.api.tree.AssignmentStatement;
+import org.sonar.plugins.python.api.tree.CallExpression;
+import org.sonar.plugins.python.api.tree.CompoundAssignmentStatement;
+import org.sonar.plugins.python.api.tree.DelStatement;
+import org.sonar.plugins.python.api.tree.ExpressionList;
+import org.sonar.plugins.python.api.tree.ForStatement;
+import org.sonar.plugins.python.api.tree.Name;
+import org.sonar.plugins.python.api.tree.NumericLiteral;
+import org.sonar.plugins.python.api.tree.RegularArgument;
+import org.sonar.plugins.python.api.tree.SubscriptionExpression;
+import org.sonar.plugins.python.api.tree.Tree;
+import org.sonar.plugins.python.api.types.v2.matchers.TypeMatcher;
+import org.sonar.plugins.python.api.types.v2.matchers.TypeMatchers;
+import org.sonar.python.tree.TreeUtils;
+
+@Rule(key = "S8518")
+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 TypeMatcher ENUMERATE_MATCHER = TypeMatchers.isType("enumerate");
+
+  @Override
+  public void initialize(Context context) {
+    context.registerSyntaxNodeConsumer(Tree.Kind.FOR_STMT, EnumerateUnpackingCheck::check);
+  }
+
+  private static void check(SubscriptionContext ctx) {
+    ForStatement forStatement = (ForStatement) ctx.syntaxNode();
+
+    if (forStatement.testExpressions().size() != 1) {
+      return;
+    }
+    if (!(forStatement.testExpressions().get(0) instanceof CallExpression call)) {
+      return;
+    }
+    if (!ENUMERATE_MATCHER.isTrueFor(call.callee(), ctx)) {
+      return;
+    }
+    if (forStatement.expressions().size() != 2) {
+      return;
+    }
+    if (!(forStatement.expressions().get(0) instanceof Name indexName)) {
+      return;
+    }
+    SymbolV2 indexSymbol = indexName.symbolV2();
+    if (indexSymbol == null) {
+      return;
+    }
+
+    RegularArgument startArg = TreeUtils.nthArgumentOrKeyword(1, "start", call.arguments());
+    if (startArg != null && !(startArg.expression() instanceof NumericLiteral num && "0".equals(num.valueAsString()))) {
+      return;
+    }
+
+    RegularArgument iterableArg = TreeUtils.nthArgumentOrKeyword(0, "iterable", call.arguments());
+    if (iterableArg == null || !(iterableArg.expression() instanceof Name iterableName)) {
+      return;
+    }
+    SymbolV2 iterableSymbol = iterableName.symbolV2();
+    if (iterableSymbol == null) {
+      return;
+    }
+
+    boolean hasSubscriptWriteTarget = TreeUtils.hasDescendant(forStatement.body(),
+      node -> isMatchingSubscript(node, indexSymbol, iterableSymbol)
+        && isSubscriptWriteTarget((SubscriptionExpression) node));
+
+    if (hasSubscriptWriteTarget) {
+      return;
+    }
+
+    boolean hasRedundantSubscript = TreeUtils.hasDescendant(forStatement.body(),
+      node -> isMatchingSubscript(node, indexSymbol, iterableSymbol));
+
+    if (hasRedundantSubscript) {
+      ctx.addIssue(call, MESSAGE);
+    }
+  }
+
+  private static boolean isMatchingSubscript(Tree node, SymbolV2 indexSymbol, SymbolV2 iterableSymbol) {
+    if (!(node instanceof SubscriptionExpression subscription)) {
+      return false;
+    }
+    if (subscription.subscripts().expressions().size() != 1) {
+      return false;
+    }
+    if (!(subscription.subscripts().expressions().get(0) instanceof Name subscriptName)) {
+      return false;
+    }
+    if (!indexSymbol.equals(subscriptName.symbolV2())) {
+      return false;
+    }
+    if (!(subscription.object() instanceof Name objectName)) {
+      return false;
+    }
+    return iterableSymbol.equals(objectName.symbolV2());
+  }
+
+  private static boolean isSubscriptWriteTarget(SubscriptionExpression subscription) {
+    Tree parent = subscription.parent();
+    if (parent instanceof DelStatement) {
+      return true;
+    }
+    if (parent instanceof CompoundAssignmentStatement compound) {
+      return compound.lhsExpression() == subscription;
+    }
+    if (parent instanceof AnnotatedAssignment annotated) {
+      return annotated.variable() == subscription;
+    }
+    return parent instanceof ExpressionList exprList
+      && exprList.parent() instanceof AssignmentStatement;
+  }
+}

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

@@ -217,6 +217,7 @@ public Stream<Class<?>> getChecks() {
       EmptyFunctionCheck.class,
       EmptyNestedBlockCheck.class,
       EmptyStringRepetitionCheck.class,
+      EnumerateUnpackingCheck.class,
       ExceptionGroupCheck.class,
       ExceptionCauseTypeCheck.class,
       ExceptionNotThrownCheck.class,

python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S8518.html

@@ -0,0 +1,24 @@
+<p>This rule raises an issue when a loop iterates over <code>enumerate()</code> but accesses list elements by index instead of unpacking the value directly from the enumeration.</p>
+<h2>Why is this an issue?</h2>
+<p>The <code>enumerate()</code> function provides both the index and the value of each element when iterating over a sequence. When you only unpack the index and then use it to subscript the original list, the value is already available from <code>enumerate()</code> and the subscript operation is redundant.</p>
+<p>This pattern results in unnecessary operations and reduced readability.</p>
+<h3>Code examples</h3>
+<h4>Noncompliant code example</h4>
+<pre data-diff-id="1" data-diff-type="noncompliant">
+fruits = ['apple', 'banana', 'cherry']
+
+for i, _ in enumerate(fruits):
+    print(f"Index {i}: {fruits[i]}")  # Noncompliant
+</pre>
+<h4>Compliant solution</h4>
+<pre data-diff-id="1" data-diff-type="compliant">
+fruits = ['apple', 'banana', 'cherry']
+
+for i, fruit in enumerate(fruits):
+    print(f"Index {i}: {fruit}")
+</pre>
+<h2>Resources</h2>
+<h3>Documentation</h3>
+<ul>
+  <li>Python Documentation - <a href="https://docs.python.org/3/library/functions.html#enumerate">enumerate()</a></li>
+</ul>

python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S8518.json

@@ -0,0 +1,21 @@
+{
+  "title": "\"enumerate()\" values should be unpacked instead of using index lookups",
+  "type": "CODE_SMELL",
+  "status": "ready",
+  "remediation": {
+    "func": "Constant\/Issue",
+    "constantCost": "2min"
+  },
+  "tags": [],
+  "defaultSeverity": "Minor",
+  "ruleSpecification": "RSPEC-8518",
+  "sqKey": "S8518",
+  "scope": "All",
+  "quickfix": "unknown",
+  "code": {
+    "impacts": {
+      "MAINTAINABILITY": "LOW"
+    },
+    "attribute": "CONVENTIONAL"
+  }
+}

python-checks/src/test/java/org/sonar/python/checks/EnumerateUnpackingCheckTest.java

@@ -0,0 +1,29 @@
+/*
+ * SonarQube Python Plugin
+ * Copyright (C) SonarSource Sàrl
+ * mailto:info AT sonarsource DOT com
+ *
+ * You can redistribute and/or modify this program under the terms of
+ * the Sonar Source-Available License Version 1, as published by SonarSource Sàrl.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
+ * See the Sonar Source-Available License for more details.
+ *
+ * You should have received a copy of the Sonar Source-Available License
+ * along with this program; if not, see https://sonarsource.com/license/ssal/
+ */
+package org.sonar.python.checks;
+
+import org.junit.jupiter.api.Test;
+import org.sonar.python.checks.utils.PythonCheckVerifier;
+
+class EnumerateUnpackingCheckTest {
+
+  @Test
+  void test() {
+    PythonCheckVerifier.verify("src/test/resources/checks/enumerateUnpacking.py", new EnumerateUnpackingCheck());
+  }
+
+}

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

@@ -0,0 +1,141 @@
+
+def noncompliant_basic():
+    lst = ['apple', 'banana', 'cherry']
+    for i, _ in enumerate(lst):  # Noncompliant {{Unpack the value from 'enumerate()' directly instead of using an index lookup.}}
+#               ^^^^^^^^^^^^^^
+        print(lst[i])
+
+
+def noncompliant_start_zero_keyword():
+    colors = ['red', 'green', 'blue']
+    for i, _ in enumerate(colors, start=0):  # Noncompliant
+        print(colors[i])
+
+
+def noncompliant_rhs_of_augmented_assignment():
+    items = "abcdef"
+    result = ""
+    for ind, _ in enumerate(items):  # Noncompliant
+        result += items[ind]
+
+
+def compliant_proper_unpacking():
+    fruits = ['apple', 'banana', 'cherry']
+    for i, fruit in enumerate(fruits):
+        print(f"Index {i}: {fruit}")
+
+
+def compliant_multiple_iterables():
+    lst = [1, 2, 3]
+    for i in lst, lst:
+        print(i)
+
+
+def compliant_plain_iterable_not_call():
+    pairs = [(1, 'a'), (2, 'b'), (3, 'c')]
+    for i, v in pairs:
+        print(i, v)
+
+
+def compliant_non_enumerate_call():
+    items = [1, 2, 3]
+    others = [4, 5, 6]
+    for i, v in zip(items, others):
+        print(items[i])
+
+
+def compliant_single_loop_variable():
+    items = ['a', 'b', 'c']
+    for item in enumerate(items):
+        print(item)
+
+
+def compliant_tuple_unpacking_first_var():
+    pairs = [(1, 'a'), (2, 'b')]
+    for (a, b), value in enumerate(pairs):
+        print(a, b, value)
+
+
+def compliant_nonzero_start():
+    items = ['a', 'b', 'c']
+    for i, _ in enumerate(items, start=1):
+        print(items[i - 1])
+
+
+def compliant_variable_start():
+    items = ['a', 'b', 'c']
+    n = 0
+    for i, _ in enumerate(items, n):
+        print(items[i])
+
+
+def compliant_no_arguments():
+    for i, _ in enumerate():
+        print(i)
+
+
+def compliant_inline_literal_iterable():
+    for i, _ in enumerate([1, 2, 3]):
+        pass
+
+
+def compliant_undeclared_iterable():
+    for i, _ in enumerate(undeclared_var):
+        print(undeclared_var[i])
+
+
+def compliant_write_assignment():
+    items = ['a', 'b', 'c']
+    for i, item in enumerate(items):
+        items[i] = item.upper()
+
+
+def compliant_write_augmented_assignment():
+    counts = [0, 0, 0]
+    for i, val in enumerate(counts):
+        counts[i] += 1
+
+
+def compliant_write_del():
+    items = ['a', 'b', 'c']
+    for i, item in enumerate(items):
+        if item == 'b':
+            del items[i]
+            break
+
+
+def compliant_different_list():
+    keys = ['x', 'y', 'z']
+    values = [1, 2, 3]
+    for i, key in enumerate(keys):
+        print(values[i])
+
+
+def compliant_different_index_var():
+    lst = ['a', 'b', 'c']
+    j = 0
+    for i, _ in enumerate(lst):
+        print(lst[j])
+
+
+def compliant_modified_index():
+    items = ['a', 'b', 'c']
+    for i, _ in enumerate(items):
+        if i + 1 < len(items):
+            print(items[i + 1])
+
+
+def compliant_multi_index_subscript():
+    import numpy as np
+    matrix = np.array([[1, 2], [3, 4]])
+    items = [10, 20]
+    for i, _ in enumerate(items):
+        print(matrix[i, 0])
+
+
+def compliant_subscript_object_not_name():
+    def get_list():
+        return ['x', 'y', 'z']
+    lst = ['a', 'b', 'c']
+    for i, _ in enumerate(lst):
+        print(get_list()[i])