SONARPY-3928 Rule S8517 "sorted()" should not be used with indexing to find minimum or maximum values (#969)

Co-authored-by: Sonar Vibe Bot <vibe-bot@sonarsource.com>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: joke1196 <david.kunzmann@sonarsource.com>
Co-authored-by: Guillaume Dequenne <guillaume.dequenne@sonarsource.com>
GitOrigin-RevId: 4a75f134de001187116ffc77e881edb6f8c2ccdc

Author: 5 people

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

@@ -398,6 +398,7 @@ public Stream<Class<?>> getChecks() {
       SkippedTestNoReasonCheck.class,
       SkLearnEstimatorDontInitializeEstimatedValuesCheck.class,
       SleepZeroInAsyncCheck.class,
+      SortedMinMaxCheck.class,
       SpecialMethodParamListCheck.class,
       SpecialMethodReturnTypeCheck.class,
       SQLQueriesCheck.class,

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

@@ -0,0 +1,127 @@
+/*
+ * SonarQube Python Plugin
+ * Copyright (C) 2011-2025 SonarSource Sàrl
+ * mailto:info AT sonarsource DOT com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the Sonar Source-Available License Version 1, as published by SonarSource SA.
+ *
+ * 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 javax.annotation.Nullable;
+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.tree.CallExpression;
+import org.sonar.plugins.python.api.tree.Expression;
+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.tree.UnaryExpression;
+import org.sonar.plugins.python.api.types.v2.matchers.TypeMatcher;
+import org.sonar.plugins.python.api.types.v2.matchers.TypeMatchers;
+import org.sonar.python.checks.hotspots.CommonValidationUtils;
+import org.sonar.python.checks.utils.Expressions;
+import org.sonar.python.tree.TreeUtils;
+
+@Rule(key = "S8517")
+public class SortedMinMaxCheck extends PythonSubscriptionCheck {
+
+  private static final TypeMatcher IS_SORTED = TypeMatchers.isType("sorted");
+
+  @Override
+  public void initialize(Context context) {
+    context.registerSyntaxNodeConsumer(Tree.Kind.SUBSCRIPTION, SortedMinMaxCheck::check);
+  }
+
+  private static void check(SubscriptionContext ctx) {
+    SubscriptionExpression subscriptionExpression = (SubscriptionExpression) ctx.syntaxNode();
+
+    if (subscriptionExpression.subscripts().expressions().size() != 1) {
+      return;
+    }
+
+    Expression subscript = subscriptionExpression.subscripts().expressions().get(0);
+    Boolean isFirstElement = isIndexZeroOrMinusOne(subscript);
+    if (isFirstElement == null) {
+      return;
+    }
+
+    Expression object = subscriptionExpression.object();
+    if (!(object instanceof CallExpression callExpression)) {
+      return;
+    }
+
+    if (!IS_SORTED.isTrueFor(callExpression.callee(), ctx)) {
+      return;
+    }
+
+    Boolean isReversed = isReversed(callExpression);
+    if (isReversed == null) {
+      // Cannot determine reverse truthiness, skip to avoid wrong suggestions
+      return;
+    }
+
+    String replacement = getReplacementFunction(isFirstElement, isReversed);
+    ctx.addIssue(subscriptionExpression, "Use \"" + replacement + "()\" instead of sorting to find this value.");
+  }
+
+  /**
+   * Returns true if the expression is the integer literal 0 (first element),
+   * false if it is -1 (last element), or null if it is neither.
+   */
+  @Nullable
+  private static Boolean isIndexZeroOrMinusOne(Expression subscript) {
+    if (subscript instanceof NumericLiteral numericLiteral && CommonValidationUtils.isEqualTo(numericLiteral, 0)) {
+      return true;
+    }
+    if (subscript instanceof UnaryExpression unaryExpression
+      && unaryExpression.is(Tree.Kind.UNARY_MINUS)
+      && unaryExpression.expression() instanceof NumericLiteral numericLiteral
+      && CommonValidationUtils.isEqualTo(numericLiteral, 1)) {
+      return false;
+    }
+    return null;
+  }
+
+  /**
+   * Returns {@code Boolean.TRUE} if the reverse argument is truthy, {@code Boolean.FALSE} if it is absent or falsy,
+   * or {@code null} if the reverse argument is present but its truthiness cannot be determined.
+   */
+  @Nullable
+  private static Boolean isReversed(CallExpression callExpression) {
+    RegularArgument reverseArg = TreeUtils.argumentByKeyword("reverse", callExpression.arguments());
+    if (reverseArg == null) {
+      return Boolean.FALSE;
+    }
+    Expression value = reverseArg.expression();
+    if (Expressions.isTruthy(value)) {
+      return Boolean.TRUE;
+    }
+    if (Expressions.isFalsy(value)) {
+      return Boolean.FALSE;
+    }
+    return null;
+  }
+
+  private static String getReplacementFunction(boolean isFirstElement, boolean isReversed) {
+    // sorted()[0]          -> min (smallest first without reverse)
+    // sorted()[-1]         -> max (largest last without reverse)
+    // sorted(reverse=True)[0]  -> max (largest first with reverse)
+    // sorted(reverse=True)[-1] -> min (smallest last with reverse)
+    if (isFirstElement) {
+      return isReversed ? "max" : "min";
+    } else {
+      return isReversed ? "min" : "max";
+    }
+  }
+}

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

@@ -0,0 +1,64 @@
+<p>This rule raises an issue when <code>sorted()</code> with indexing (<code>[0]</code> or <code>[-1]</code>) is used to find the minimum or maximum
+value in a collection, including variants with <code>reverse=True</code>.</p>
+<h2>Why is this an issue?</h2>
+<p>When you need to find the smallest or largest element in a collection, you might be tempted to sort the entire collection and then access the first
+or last element. However, this approach is inefficient and makes your code harder to understand.</p>
+<p>The <code>sorted()</code> function creates a new sorted list from all elements, an O(n log n) operation in time, plus O(n) in memory. After
+sorting, you only use one element and discard the rest.</p>
+<p>Python provides built-in <code>min()</code> and <code>max()</code> functions that find the smallest or largest element in a single pass (O(n) time)
+with no extra memory allocation. They also make your intent immediately clear to anyone reading the code.</p>
+<p><code>min()</code>, <code>max()</code>, and <code>sorted()</code> all support an optional <code>key</code> parameter, so you can preserve custom
+comparison logic. Note that <code>min()</code> and <code>max()</code> do not have a <code>reverse</code> parameter, so when replacing
+<code>sorted(iterable, reverse=True)[0]</code>, you should use <code>max(iterable)</code> instead, and vice versa for <code>[-1]</code>.</p>
+<h3>What is the potential impact?</h3>
+<ul>
+  <li><strong>Performance degradation</strong>: sorting takes O(n log n) operations while <code>min()</code> or <code>max()</code> take only O(n). For
+  large collections, this difference results in noticeably slower execution.</li>
+  <li><strong>Memory overhead</strong>: <code>sorted()</code> creates a new list containing all elements. <code>min()</code> and <code>max()</code>
+  only store a single element during execution.</li>
+  <li><strong>Reduced code clarity</strong>: using <code>sorted()[0]</code> to find a minimum value obscures the intent of the code.</li>
+</ul>
+<h2>How to fix it</h2>
+<p>Replace <code>sorted(iterable)[0]</code> with <code>min(iterable)</code> and <code>sorted(iterable)[-1]</code> with <code>max(iterable)</code>.
+When <code>reverse=True</code> is used, the mapping is inverted: <code>sorted(iterable, reverse=True)[0]</code> becomes <code>max(iterable)</code> and
+<code>sorted(iterable, reverse=True)[-1]</code> becomes <code>min(iterable)</code>.</p>
+<h3>Code examples</h3>
+<h4>Noncompliant code example</h4>
+<pre data-diff-id="1" data-diff-type="noncompliant">
+numbers = [42, 17, 93, 8, 51]
+smallest = sorted(numbers)[0]  # Noncompliant
+</pre>
+<h4>Compliant solution</h4>
+<pre data-diff-id="1" data-diff-type="compliant">
+numbers = [42, 17, 93, 8, 51]
+smallest = min(numbers)
+</pre>
+<h4>Noncompliant code example</h4>
+<pre data-diff-id="2" data-diff-type="noncompliant">
+numbers = [42, 17, 93, 8, 51]
+largest = sorted(numbers)[-1]  # Noncompliant
+</pre>
+<h4>Compliant solution</h4>
+<pre data-diff-id="2" data-diff-type="compliant">
+numbers = [42, 17, 93, 8, 51]
+largest = max(numbers)
+</pre>
+<h4>Noncompliant code example</h4>
+<pre data-diff-id="3" data-diff-type="noncompliant">
+numbers = [42, 17, 93, 8, 51]
+largest = sorted(numbers, reverse=True)[0]  # Noncompliant
+</pre>
+<h4>Compliant solution</h4>
+<pre data-diff-id="3" data-diff-type="compliant">
+numbers = [42, 17, 93, 8, 51]
+largest = max(numbers)
+</pre>
+<h2>Resources</h2>
+<h3>Documentation</h3>
+<ul>
+  <li>Python Documentation - <a href="https://docs.python.org/3/library/functions.html#min"><code>min()</code> built-in function</a></li>
+  <li>Python Documentation - <a href="https://docs.python.org/3/library/functions.html#max"><code>max()</code> built-in function</a></li>
+  <li>Python Documentation - <a href="https://docs.python.org/3/library/functions.html#sorted"><code>sorted()</code> built-in function</a></li>
+  <li>Python Wiki - <a href="https://wiki.python.org/moin/TimeComplexity">Time Complexity</a></li>
+</ul>
+

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

@@ -0,0 +1,24 @@
+{
+  "title": "\"sorted()\" should not be used with indexing to find minimum or maximum values",
+  "type": "CODE_SMELL",
+  "status": "ready",
+  "remediation": {
+    "func": "Constant\/Issue",
+    "constantCost": "5 min"
+  },
+  "tags": [
+    "performance",
+    "pythonic"
+  ],
+  "defaultSeverity": "Major",
+  "ruleSpecification": "RSPEC-8517",
+  "sqKey": "S8517",
+  "scope": "All",
+  "quickfix": "targeted",
+  "code": {
+    "impacts": {
+      "MAINTAINABILITY": "MEDIUM"
+    },
+    "attribute": "EFFICIENT"
+  }
+}

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

@@ -327,6 +327,7 @@
     "S8412",
     "S8413",
     "S8414",
-    "S8415"
+    "S8415",
+    "S8517"
   ]
 }

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

@@ -0,0 +1,29 @@
+/*
+ * SonarQube Python Plugin
+ * Copyright (C) 2011-2025 SonarSource Sàrl
+ * mailto:info AT sonarsource DOT com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the Sonar Source-Available License Version 1, as published by SonarSource SA.
+ *
+ * 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 SortedMinMaxCheckTest {
+
+  @Test
+  void test() {
+    PythonCheckVerifier.verify("src/test/resources/checks/sortedMinMax.py", new SortedMinMaxCheck());
+  }
+
+}

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

@@ -0,0 +1,126 @@
+numbers = [42, 17, 93, 8, 51]
+words = ["banana", "fig", "apple", "date"]
+
+# Issues detected
+smallest = sorted(numbers)[0]  # Noncompliant {{Use "min()" instead of sorting to find this value.}}
+#          ^^^^^^^^^^^^^^^^^^
+
+largest = sorted(numbers)[-1]  # Noncompliant {{Use "max()" instead of sorting to find this value.}}
+#         ^^^^^^^^^^^^^^^^^^^
+
+also_largest = sorted(numbers, reverse=True)[0]  # Noncompliant {{Use "max()" instead of sorting to find this value.}}
+#              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+also_smallest = sorted(numbers, reverse=True)[-1]  # Noncompliant {{Use "min()" instead of sorting to find this value.}}
+#               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+# reverse=False is treated the same as no reverse argument
+x = sorted(numbers, reverse=False)[0]  # Noncompliant {{Use "min()" instead of sorting to find this value.}}
+y = sorted(numbers, reverse=False)[-1]  # Noncompliant {{Use "max()" instead of sorting to find this value.}}
+
+# key parameter does not suppress the issue
+shortest = sorted(words, key=len)[0]  # Noncompliant {{Use "min()" instead of sorting to find this value.}}
+#          ^^^^^^^^^^^^^^^^^^^^^^^^^
+
+longest_by_len = sorted(words, key=len, reverse=True)[0]  # Noncompliant {{Use "max()" instead of sorting to find this value.}}
+
+# inline collection literal
+inline_min = sorted([5, 3, 8, 1])[0]  # Noncompliant
+
+# used in a boolean expression
+def get_min(data):
+    if sorted(data)[0] < 0:  # Noncompliant
+        return True
+
+
+# Compliant cases
+
+# use min() and max() directly
+min_val = min(numbers)
+max_val = max(numbers)
+min_by_len = min(words, key=len)
+max_by_len = max(words, key=len)
+
+# sorted() without indexing
+sorted_numbers = sorted(numbers)
+
+# index other than 0 or -1
+second = sorted(numbers)[1]
+second_to_last = sorted(numbers)[-2]
+
+# slice, not a single index
+first_three = sorted(numbers)[0:3]
+
+# indexing a non-sorted() object
+first = numbers[0]
+last = numbers[-1]
+
+# sorted result stored in a variable first
+ordered = sorted(numbers)
+first_ordered = ordered[0]
+last_ordered = ordered[-1]
+
+# custom (shadowed) sorted function
+def case_custom_sorted():
+    def sorted(iterable):
+        return list(iterable)
+    result = sorted(numbers)[0]
+
+# indirect call through a variable
+def case_indirect():
+    sort_fn = sorted
+    result = sort_fn(numbers)[0] # Noncompliant
+#            ^^^^^^^^^^^^^^^^^^^
+
+# sorted result wrapped in another call
+def identity(x):
+    return x
+
+wrapped = identity(sorted(numbers))[0]
+
+# multiple subscripts (tuple index) - not a single subscript
+multi_subscript = sorted(numbers)[0, 1]
+
+# qualified name callee - TypeMatchers resolves builtins.sorted correctly
+import builtins
+qualified_sorted = builtins.sorted(numbers)[0]  # Noncompliant {{Use "min()" instead of sorting to find this value.}}
+
+# unary plus index - not UNARY_MINUS, so not -1
+unary_plus_index = sorted(numbers)[+1]
+
+# unary minus applied to a variable - not a NumericLiteral operand
+neg_var = 1
+unary_minus_var_index = sorted(numbers)[-neg_var]
+
+# reverse argument is a truthy integer literal
+reverse_int_literal = sorted(numbers, reverse=1)[0]  # Noncompliant {{Use "max()" instead of sorting to find this value.}}
+
+# reverse argument is a falsy integer literal
+reverse_zero = sorted(numbers, reverse=0)[0]  # Noncompliant {{Use "min()" instead of sorting to find this value.}}
+
+# reverse argument is a variable assigned to True - resolved through single assignment
+rev_flag = True
+reverse_var = sorted(numbers, reverse=rev_flag)[0]  # Noncompliant {{Use "max()" instead of sorting to find this value.}}
+
+# reverse argument is an unknown variable - cannot determine truthiness, skip
+def some_function(flag):
+    unknown_reverse = sorted(numbers, reverse=flag)[0]
+
+# sorted used inside a lambda expression (the sorted()[index] is still flagged)
+get_last = lambda lst: sorted(lst, key=lambda m: m)[-1]  # Noncompliant {{Use "max()" instead of sorting to find this value.}}
+
+# sorted result used as subscript key into another collection
+d = {"a": 1, "b": 2, "c": 3}
+value = d[sorted(d.keys())[-1]]  # Noncompliant {{Use "max()" instead of sorting to find this value.}}
+
+# negated key lambda - sorted(key=lambda x: -f(x))[0] is still noncompliant
+items = ["hi", "hello", "hey"]
+negated_key = sorted(items, key=lambda s: -len(s))[0]  # Noncompliant {{Use "min()" instead of sorting to find this value.}}
+
+# multi-line sorted call - same rule applies regardless of formatting
+multi_line_min = sorted(  # Noncompliant {{Use "min()" instead of sorting to find this value.}}
+    words,
+    key=len,
+)[0]
+
+test_float = sorted(items)[0.5]