SONARPY-3975 Rule S8519 "list(...)[0]" should not be used to get the first element (#1011)

Co-authored-by: Marc Jasper <marc.jasper@sonarsource.com>
GitOrigin-RevId: 8e7c0b37c56887d7ed66e8693af4504223faa0de

Author: 2 people

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

@@ -0,0 +1,98 @@
+/*
+ * 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.quickfix.PythonQuickFix;
+import org.sonar.plugins.python.api.tree.CallExpression;
+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.quickfix.TextEditUtils;
+import org.sonar.python.tree.NumericLiteralImpl;
+import org.sonar.python.tree.TreeUtils;
+
+@Rule(key = "S8519")
+public class ListIterableFirstElementCheck extends PythonSubscriptionCheck {
+
+  private static final String MESSAGE = "Replace \"list(...)[0]\" with \"next(iter(...))\" to avoid materializing the entire iterable.";
+  private static final TypeMatcher IS_LIST = TypeMatchers.isType("list");
+
+  @Override
+  public void initialize(Context context) {
+    context.registerSyntaxNodeConsumer(Tree.Kind.SUBSCRIPTION, ListIterableFirstElementCheck::checkSubscription);
+  }
+
+  private static void checkSubscription(SubscriptionContext ctx) {
+    SubscriptionExpression subscriptionExpression = (SubscriptionExpression) ctx.syntaxNode();
+
+    var subscripts = subscriptionExpression.subscripts();
+    if (!subscripts.commas().isEmpty() || subscripts.expressions().size() != 1) {
+      return;
+    }
+
+    var subscript = subscripts.expressions().get(0);
+    if (!(subscript instanceof NumericLiteral numericLiteral)) {
+      return;
+    }
+    if (!isIntegerLiteralSubscript(numericLiteral)) {
+      return;
+    }
+    long indexValue;
+    try {
+      indexValue = numericLiteral.valueAsLong();
+    } catch (NumberFormatException e) {
+      return;
+    }
+    if (indexValue != 0L) {
+      return;
+    }
+
+    if (!(subscriptionExpression.object() instanceof CallExpression listCall)) {
+      return;
+    }
+
+    if (!IS_LIST.isTrueFor(listCall.callee(), ctx)) {
+      return;
+    }
+
+    if (listCall.arguments().size() != 1 || !(listCall.arguments().get(0) instanceof RegularArgument regularArg)) {
+      return;
+    }
+
+    PreciseIssue issue = ctx.addIssue(listCall.callee(), MESSAGE);
+
+    String argText = TreeUtils.treeToString(regularArg.expression(), false);
+    if (argText != null && !argText.contains("\n")) {
+      PythonQuickFix quickFix = PythonQuickFix.newQuickFix("Replace with \"next(iter(...))\"",
+        TextEditUtils.replace(subscriptionExpression, "next(iter(" + argText + "))"));
+      issue.addQuickFix(quickFix);
+    }
+  }
+
+  private static boolean isIntegerLiteralSubscript(NumericLiteral literal) {
+    if (literal instanceof NumericLiteralImpl impl) {
+      return impl.numericKind() == NumericLiteralImpl.NumericKind.INT;
+    }
+    return false;
+  }
+}

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

@@ -306,6 +306,7 @@ public Stream<Class<?>> getChecks() {
       LambdaAssignmentCheck.class,
       LdapAuthenticationCheck.class,
       LineLengthCheck.class,
+      ListIterableFirstElementCheck.class,
       LocalVariableAndParameterNameConventionCheck.class,
       LoggersConfigurationCheck.class,
       LongIntegerWithLowercaseSuffixUsageCheck.class,

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

@@ -0,0 +1,56 @@
+<p>This rule raises an issue when code uses <code>list(iterable)[0]</code> to retrieve the first element of an iterable.</p>
+<h2>Why is this an issue?</h2>
+<p>Using <code>list(iterable)[0]</code> to get the first element of an iterable is inefficient because it materializes the entire iterable into a list
+in memory before accessing the first element.</p>
+<p>This approach has two main problems:</p>
+<ul>
+  <li><strong>Memory overhead</strong>: The entire iterable is converted to a list, consuming memory proportional to its size. For large iterables or
+  generators, this can waste significant memory.</li>
+  <li><strong>Time complexity</strong>: Creating the list is an O(n) operation, where n is the number of elements in the iterable. You pay the cost of
+  iterating through all elements even though you only need the first one.</li>
+</ul>
+<p>The pattern is particularly problematic with:</p>
+<ul>
+  <li>large datasets or database query results</li>
+  <li>generator expressions and generator functions</li>
+  <li>file readers or network streams</li>
+  <li>infinite iterables, which will cause the program to hang or crash</li>
+</ul>
+<h3>What is the potential impact?</h3>
+<ul>
+  <li>applications may become unresponsive or crash due to out-of-memory errors when processing large datasets</li>
+  <li>execution time increases unnecessarily, degrading user experience in performance-critical paths</li>
+  <li>infinite iterables cannot be consumed at all, causing the program to hang</li>
+  <li>infrastructure costs grow as data size increases due to wasted memory and CPU cycles</li>
+</ul>
+<h2>How to fix it</h2>
+<p>Replace <code>list(iterable)[0]</code> with <code>next(iter(iterable))</code> to retrieve only the first element without materializing the entire
+iterable into a list. The <code>iter()</code> function creates an iterator, and <code>next()</code> retrieves the next item from that iterator, in
+O(1) time and without allocating memory for the entire collection.</p>
+<p>Note that the two expressions differ in the exception raised when the iterable is empty: <code>list(iterable)[0]</code> raises
+<code>IndexError</code>, while <code>next(iter(iterable))</code> raises <code>StopIteration</code>. If the surrounding code catches
+<code>IndexError</code>, update the handler after applying this fix.</p>
+<h3>Code examples</h3>
+<h4>Noncompliant code example</h4>
+<pre data-diff-id="1" data-diff-type="noncompliant">
+def get_first_user(users):
+    return list(users)[0]  # Noncompliant
+</pre>
+<h4>Compliant solution</h4>
+<pre data-diff-id="1" data-diff-type="compliant">
+def get_first_user(users):
+    return next(iter(users))
+</pre>
+<h2>Resources</h2>
+<h3>Documentation</h3>
+<ul>
+  <li>Python Documentation - <a href="https://docs.python.org/3/library/functions.html#next"><code>next()</code> built-in function</a></li>
+  <li>Python Documentation - <a href="https://docs.python.org/3/library/functions.html#iter"><code>iter()</code> built-in function</a></li>
+  <li>Python Documentation - <a href="https://docs.python.org/3/glossary.html#term-iterator">Iterator</a></li>
+</ul>
+<h3>External coding guidelines</h3>
+<ul>
+  <li>Ruff - <a href="https://docs.astral.sh/ruff/rules/unnecessary-iterable-allocation-for-first-element/">RUF015 - Unnecessary iterable allocation
+  for first element</a></li>
+</ul>
+

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

@@ -0,0 +1,24 @@
+{
+  "title": "\"list(...)[0]\" should not be used to get the first element",
+  "type": "CODE_SMELL",
+  "status": "ready",
+  "remediation": {
+    "func": "Constant\/Issue",
+    "constantCost": "5min"
+  },
+  "tags": [
+    "performance",
+    "pythonic"
+  ],
+  "defaultSeverity": "Major",
+  "ruleSpecification": "RSPEC-8519",
+  "sqKey": "S8519",
+  "scope": "All",
+  "quickfix": "targeted",
+  "code": {
+    "impacts": {
+      "RELIABILITY": "MEDIUM"
+    },
+    "attribute": "EFFICIENT"
+  }
+}

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

@@ -334,6 +334,7 @@
     "S8504",
     "S8507",
     "S8517",
+    "S8519",
     "S8520",
     "S8521",
     "S8493"

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

@@ -0,0 +1,70 @@
+/*
+ * 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 java.util.stream.Stream;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+import org.sonar.python.checks.quickfix.PythonQuickFixVerifier;
+import org.sonar.python.checks.utils.PythonCheckVerifier;
+
+class ListIterableFirstElementCheckTest {
+
+  private static final ListIterableFirstElementCheck check = new ListIterableFirstElementCheck();
+
+  @Test
+  void test() {
+    PythonCheckVerifier.verify("src/test/resources/checks/listIterableFirstElement.py", check);
+  }
+
+  static Stream<Arguments> quickFixTestCases() {
+    return Stream.of(
+      Arguments.of("list(get_users())[0]", "next(iter(get_users()))"),
+      Arguments.of("list(users)[0]", "next(iter(users))"),
+      Arguments.of("list(x for x in range(10))[0]", "next(iter(x for x in range(10)))"),
+      Arguments.of("list(filter(None, items))[0]", "next(iter(filter(None, items)))")
+    );
+  }
+
+  @ParameterizedTest
+  @MethodSource("quickFixTestCases")
+  void test_quick_fix(String codeWithIssue, String correctCode) {
+    PythonQuickFixVerifier.verify(check, codeWithIssue, correctCode);
+  }
+
+  @Test
+  void test_quick_fix_message() {
+    String codeWithIssue = "list(get_users())[0]";
+
+    PythonQuickFixVerifier.verifyQuickFixMessages(check, codeWithIssue, "Replace with \"next(iter(...))\"");
+  }
+
+  @Test
+  void test_no_quick_fix_multiline_argument_expression() {
+    String codeWithIssue = """
+      list(
+          get_items(
+              arg
+          )
+      )[0]
+      """;
+
+    PythonQuickFixVerifier.verifyNoQuickFixes(check, codeWithIssue);
+  }
+}

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

@@ -0,0 +1,51 @@
+
+def noncompliant_materialize_first(users):
+    return list(users)[0]  # Noncompliant {{Replace "list(...)[0]" with "next(iter(...))" to avoid materializing the entire iterable.}}
+#          ^^^^
+
+
+def compliant_next_iter(users):
+    return next(iter(users))
+
+def compliant_nonzero_index(items):
+    return list(items)[1]
+
+def compliant_float_subscript(items):
+    return list(items)[0.0]
+
+def compliant_float_scientific_zero(items):
+    return list(items)[0e0]
+
+def compliant_complex_subscript(items):
+    return list(items)[0j]
+
+def compliant_negative_index(items):
+    return list(items)[-1]
+
+def compliant_slice(items):
+    return list(items)[0:1]
+
+def compliant_tuple_subscript(items):
+    return list(items)[0, 1]
+
+def compliant_starred(args):
+    return list(*args)[0]
+
+def compliant_multiple_args():
+    return list([], [])[0]
+
+def compliant_direct_index(items):
+    return items[0]
+
+def compliant_two_step_list(items):
+    all_items = list(items)
+    return all_items[0]  # out of scope: [0] applies to a Name, not to list(...) call
+
+def compliant_shadowed_list(items):
+    list = lambda x: x
+    return list(items)[0]
+
+
+def fn_subscript_not_literal_zero(items):
+    zero = 0
+    return list(items)[zero]  # FN: subscript must be numeric literal 0