SONARPY-2964 Rule S7511: Unnecessary subscript reversal of iterable within <reversed/set/sorted>() (#269)

GitOrigin-RevId: baf7e5b39a48e9f59c1bb8eaa0ac7864879b2f46

Author: maksim-grebeniuk-sonarsource

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

@@ -391,6 +391,7 @@ public Stream<Class<?>> getChecks() {
       UnencryptedEbsVolumeCheck.class,
       UnionTypeExpressionCheck.class,
       UnnecessaryReversedCallCheck.class,
+      UnnecessarySubscriptReversalCheck.class,
       UnquantifiedNonCapturingGroupCheck.class,
       UnreachableExceptCheck.class,
       UnreadPrivateAttributesCheck.class,

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

@@ -0,0 +1,107 @@
+/*
+ * SonarQube Python Plugin
+ * Copyright (C) 2011-2025 SonarSource SA
+ * 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 java.util.Optional;
+import java.util.stream.Stream;
+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.Name;
+import org.sonar.plugins.python.api.tree.NumericLiteral;
+import org.sonar.plugins.python.api.tree.RegularArgument;
+import org.sonar.plugins.python.api.tree.SliceExpression;
+import org.sonar.plugins.python.api.tree.SliceItem;
+import org.sonar.plugins.python.api.tree.Tree;
+import org.sonar.plugins.python.api.tree.UnaryExpression;
+import org.sonar.plugins.python.api.types.v2.TriBool;
+import org.sonar.python.checks.utils.Expressions;
+import org.sonar.python.tree.TreeUtils;
+import org.sonar.python.types.v2.TypeCheckBuilder;
+
+@Rule(key = "S7511")
+public class UnnecessarySubscriptReversalCheck extends PythonSubscriptionCheck {
+  private TypeCheckBuilder isReversedTypeCheck;
+  private TypeCheckBuilder isSortedTypeCheck;
+  private TypeCheckBuilder isSetTypeCheck;
+
+  @Override
+  public void initialize(Context context) {
+    context.registerSyntaxNodeConsumer(Tree.Kind.FILE_INPUT, this::initChecks);
+    context.registerSyntaxNodeConsumer(Tree.Kind.CALL_EXPR, this::check);
+  }
+
+  private void initChecks(SubscriptionContext ctx) {
+    isReversedTypeCheck = ctx.typeChecker().typeCheckBuilder().isTypeWithName("reversed");
+    isSortedTypeCheck = ctx.typeChecker().typeCheckBuilder().isTypeWithName("sorted");
+    isSetTypeCheck = ctx.typeChecker().typeCheckBuilder().isTypeWithName("set");
+  }
+
+  private void check(SubscriptionContext ctx) {
+    TreeUtils.toOptionalInstanceOf(CallExpression.class, ctx.syntaxNode())
+      .filter(this::isSensitiveCall)
+      .flatMap(callExpression -> getArgumentSubscriptReversal(callExpression)
+        .or(() -> getOuterSubscriptReversal(callExpression)))
+      .ifPresent(redundantSubscriptReversal -> ctx.addIssue(redundantSubscriptReversal, "Remove this redundant subscript reversal"));
+  }
+
+  private static Optional<Tree> getOuterSubscriptReversal(CallExpression callExpression) {
+    return Optional.of(callExpression)
+      .map(Tree::parent)
+      .filter(UnnecessarySubscriptReversalCheck::isSubscriptReversal);
+  }
+
+  private static Optional<Tree> getArgumentSubscriptReversal(CallExpression callExpression) {
+    return TreeUtils.nthArgumentOrKeywordOptional(0, "", callExpression.arguments())
+      .map(RegularArgument::expression)
+      .filter(arg -> isSubscriptReversal(arg) || isAssignedToSubscriptReversal(arg))
+      .map(Tree.class::cast);
+  }
+
+  private static boolean isAssignedToSubscriptReversal(Expression argumentExpression) {
+    return argumentExpression instanceof Name name
+           && getUsageCount(name) == 2
+           && isSubscriptReversal(Expressions.singleAssignedValue(name));
+  }
+
+  private boolean isSensitiveCall(CallExpression callExpression) {
+    return Stream.of(isReversedTypeCheck, isSortedTypeCheck, isSetTypeCheck)
+      .map(check -> check.check(callExpression.callee().typeV2()))
+      .anyMatch(TriBool.TRUE::equals);
+  }
+
+  private static boolean isSubscriptReversal(@Nullable Tree expression) {
+    return expression instanceof SliceExpression sliceExpression
+           && sliceExpression.sliceList().slices().size() == 1
+           && sliceExpression.sliceList().slices().get(0) instanceof SliceItem sliceItem
+           && sliceItem.lowerBound() == null
+           && sliceItem.upperBound() == null
+           && sliceItem.stride() instanceof UnaryExpression unaryExpression
+           && unaryExpression.is(Tree.Kind.UNARY_MINUS)
+           && unaryExpression.expression() instanceof NumericLiteral numericLiteral
+           && numericLiteral.valueAsLong() == 1;
+  }
+
+  private static int getUsageCount(Name name) {
+    var symbol = name.symbolV2();
+    if (symbol == null) return 0;
+    return symbol.usages().size();
+  }
+}

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

@@ -0,0 +1,38 @@
+<p>This rule raises an issue when an iterable is reversed using slicing, like <code>[::-1]</code>, before being passed to <code>set()</code>,
+<code>sorted()</code>, or <code>reversed()</code>.</p>
+<h2>Why is this an issue?</h2>
+<p>Reversing an iterable using slicing, likle <code>[::-1]</code>, before passing it to <code>set()</code>, <code>sorted()</code>, or
+<code>reversed()</code> is unnecessary and inefficient. The slicing operation creates a new copy of the iterable in reverse order, which is not needed
+for the following reasons:</p>
+<ul>
+  <li> <code>set()</code>: The order of elements in a set is inherently undefined, so reversing the iterable before creating the set has no effect on
+  the final set, and it introduces unnecessary computation. </li>
+  <li> <code>sorted()</code>: The <code>sorted()</code> function has a <code>reverse</code> parameter that provides a more efficient way to sort in
+  descending order. Using slicing to reverse the result of <code>sorted()</code> is less efficient and less readable. </li>
+  <li> <code>reversed()</code>: Applying <code>reversed()</code> twice on the same iterable effectively returns the original iterable, if it supports
+  direct iteration. If the iterable is a one-time iterator, then you will need to create an iterator from the original iterable using
+  <code>iter(iterable)</code>. Using slicing adds unnecessary overhead. </li>
+</ul>
+<h2>How to fix it</h2>
+<p>To fix these issues remove the redundant slicing operation: * <code>set(iterable[::-1])</code>: Replace with <code>set(iterable)</code>. *
+<code>sorted(iterable)[::-1]</code>: Replace with <code>sorted(iterable, reverse=True)</code>. * <code>reversed(iterable[::-1])</code>: Replace with
+<code>iterable</code> if it supports direct iteration, or <code>iter(iterable)</code> if it is a one-time iterator.</p>
+<h3>Code examples</h3>
+<h4>Noncompliant code example</h4>
+<pre data-diff-id="1" data-diff-type="noncompliant">
+iterable = [1, 3, 2]
+result = set(iterable[::-1]) # Noncompliant
+</pre>
+<h4>Compliant solution</h4>
+<pre data-diff-id="1" data-diff-type="compliant">
+iterable = [1, 3, 2]
+result = set(iterable)
+</pre>
+<h2>Resources</h2>
+<h3>Documentation</h3>
+<ul>
+  <li> Python Documentation - <a href="https://docs.python.org/3/library/functions.html#sorted">sorted</a> </li>
+  <li> Python Documentation - <a href="https://docs.python.org/3/library/functions.html#reversed">reversed</a> </li>
+  <li> Python Documentation - <a href="https://docs.python.org/3/library/functions.html#func-set">set</a> </li>
+</ul>
+

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

@@ -0,0 +1,21 @@
+{
+  "title": "Passing a reversed iterable to \"set()\", \"sorted()\", or \"reversed()\" should be avoided",
+  "type": "CODE_SMELL",
+  "status": "ready",
+  "remediation": {
+    "func": "Constant\/Issue",
+    "constantCost": "1min"
+  },
+  "tags": [],
+  "defaultSeverity": "Major",
+  "ruleSpecification": "RSPEC-7511",
+  "sqKey": "S7511",
+  "scope": "All",
+  "quickfix": "unknown",
+  "code": {
+    "impacts": {
+      "MAINTAINABILITY": "MEDIUM"
+    },
+    "attribute": "CONVENTIONAL"
+  }
+}

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

@@ -255,6 +255,7 @@
     "S6984",
     "S6985",
     "S7483",
+    "S7486",
     "S7488",
     "S7491",
     "S7493",
@@ -263,7 +264,6 @@
     "S7499",
     "S7501",
     "S7510",
-    "S7486",
-    "S7501"
+    "S7511"
   ]
 }

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

@@ -0,0 +1,29 @@
+/*
+ * SonarQube Python Plugin
+ * Copyright (C) 2011-2025 SonarSource SA
+ * 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 UnnecessarySubscriptReversalCheckTest {
+
+  @Test
+  void test() {
+    PythonCheckVerifier.verify("src/test/resources/checks/unnecessarySubscriptReversal.py", new UnnecessarySubscriptReversalCheck());
+  }
+
+}

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

@@ -0,0 +1,45 @@
+
+def case1():
+  data = [3, 1, 4, 1, 5, 9]
+  reversed(data[::-1]) # Noncompliant
+  sorted(data[::-1]) # Noncompliant
+  set(data[::-1]) # Noncompliant
+  reversed(data[1::-1])
+  sorted(data[:1:-1])
+  set(data[1:1:-1])
+  set(data[::])
+  reversed(data)
+  sorted(data)
+  set(data)
+
+def case2():
+  data = [3, 1, 4, 1, 5, 9]
+  reversed_data = data[::-1]
+  sorted(reversed_data) # Noncompliant
+  sorted(data[:3,3:])
+  reversed_modified_data = data[::-1]
+  reversed_modified_data.append(10)
+  sorted(reversed_modified_data)
+  some_data = data
+  sorted(some_data)
+
+def case3():
+  data = [3, 1, 4, 1, 5, 9]
+  reversed(data)[::-1] # Noncompliant
+  sorted(data)[::-1] # Noncompliant
+  set(data)[::-1] # Noncompliant
+
+def case4():
+  data = [3, 1, 4, 1, 5, 9]
+  set(data[::+1])
+  set(data[::-2])
+  set(data[::1])
+  set(data[::-b])
+  list(data[::-1])
+
+def case5():
+  data = [3, 1, 4, 1, 5, 9]
+  reversed()
+  sorted(data)[1]
+  sorted(reverse=True)
+  set()