SONARPY-2963 Rule S7508: Unnecessary <list/reversed/set/sorted/tuple> call within <list/set/sorted/tuple>() (#289)

GitOrigin-RevId: 7a1efd44260b165eef9807e14ac23b4fc83210ea

Author: maksim-grebeniuk-sonarsource

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

@@ -0,0 +1,104 @@
+/*
+ * 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.List;
+import java.util.Map;
+import java.util.Optional;
+import java.util.Set;
+import java.util.stream.Collectors;
+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.RegularArgument;
+import org.sonar.plugins.python.api.tree.Tree;
+import org.sonar.plugins.python.api.types.v2.TriBool;
+import org.sonar.python.checks.utils.Expressions;
+import org.sonar.python.semantic.v2.SymbolV2;
+import org.sonar.python.tree.TreeUtils;
+import org.sonar.python.types.v2.TypeCheckBuilder;
+import org.sonar.python.types.v2.TypeCheckMap;
+
+@Rule(key = "S7508")
+public class NestedCollectionsCreationCheck extends PythonSubscriptionCheck {
+  public static final String LIST_FQN = "list";
+  public static final String SORTED_FQN = "sorted";
+  public static final String SET_FQN = "set";
+  public static final String TUPLE_FQN = "tuple";
+  public static final String REVERSED_FQN = "reversed";
+
+  private static final Map<String, Set<String>> SENSITIVE_NESTED_CALL_COMBINATIONS = Map.ofEntries(
+    Map.entry(LIST_FQN, Set.of(LIST_FQN, TUPLE_FQN, SORTED_FQN)),
+    Map.entry(SET_FQN, Set.of(LIST_FQN, SET_FQN, TUPLE_FQN, REVERSED_FQN, SORTED_FQN)),
+    Map.entry(SORTED_FQN, Set.of(LIST_FQN, TUPLE_FQN, SORTED_FQN)),
+    Map.entry(TUPLE_FQN, Set.of(LIST_FQN, TUPLE_FQN))
+  );
+  private TypeCheckMap<Set<TypeCheckBuilder>> sensitiveCallCombinationChecks;
+
+
+  @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) {
+    sensitiveCallCombinationChecks = new TypeCheckMap<>();
+    SENSITIVE_NESTED_CALL_COMBINATIONS.forEach((outerCallFqn, innerCallFqns) -> {
+      var outerCallTypeCheck = ctx.typeChecker().typeCheckBuilder().isTypeWithName(outerCallFqn);
+      var innerCallTypeChecks = innerCallFqns.stream()
+        .map(sensitiveMethodFqn -> ctx.typeChecker().typeCheckBuilder().isTypeWithName(sensitiveMethodFqn))
+        .collect(Collectors.toSet());
+      sensitiveCallCombinationChecks.put(outerCallTypeCheck, innerCallTypeChecks);
+    });
+  }
+
+  private void check(SubscriptionContext ctx) {
+    var callExpression = (CallExpression) ctx.syntaxNode();
+    sensitiveCallCombinationChecks.getOptionalForType(callExpression.callee().typeV2())
+      .ifPresent(nestedCallTypeChecks -> TreeUtils.nthArgumentOrKeywordOptional(0, "", callExpression.arguments())
+        .map(RegularArgument::expression)
+        .ifPresent(argumentExpression -> {
+          if (isSensitiveMethodCall(argumentExpression, nestedCallTypeChecks) || isAssignedToSensitiveMethodCall(argumentExpression, nestedCallTypeChecks)) {
+            ctx.addIssue(callExpression, "Remove this redundant call.");
+          }
+        }));
+  }
+
+  private static boolean isSensitiveMethodCall(@Nullable Expression expression, Set<TypeCheckBuilder> sensitiveMethodsTypeChecks) {
+    return expression instanceof CallExpression callExpression && sensitiveMethodsTypeChecks.stream()
+      .map(check -> check.check(callExpression.callee().typeV2()))
+      .anyMatch(TriBool.TRUE::equals);
+  }
+
+  private static boolean isAssignedToSensitiveMethodCall(Expression argumentExpression, Set<TypeCheckBuilder> sensitiveMethodsTypeChecks) {
+    return argumentExpression instanceof Name name
+           && getUsageCount(name) == 2
+           && isSensitiveMethodCall(Expressions.singleAssignedValue(name), sensitiveMethodsTypeChecks);
+  }
+
+  private static int getUsageCount(Name name) {
+    return Optional.ofNullable(name.symbolV2())
+      .map(SymbolV2::usages)
+      .map(List::size)
+      .orElse(0);
+  }
+}

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

@@ -279,6 +279,7 @@ public Stream<Class<?>> getChecks() {
       ModuleNameCheck.class,
       MultipleWhitespaceCheck.class,
       NeedlessPassCheck.class,
+      NestedCollectionsCreationCheck.class,
       NestedConditionalExpressionCheck.class,
       NestedControlFlowDepthCheck.class,
       NewStyleClassCheck.class,

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

@@ -0,0 +1,36 @@
+<p>This rule raises an issue when the functions <code>list()</code>, <code>tuple()</code>, <code>set()</code>, <code>sorted()</code>, or
+<code>reversed()</code> are unnecessarily wrapped around each other’s return values or used to convert values that don’t require conversion.</p>
+<h2>Why is this an issue?</h2>
+<p>Python’s built-in functions for processing iterables such as <code>list()</code>, <code>tuple()</code>, <code>set()</code>, <code>sorted()</code>,
+and <code>reversed()</code> are designed to accept any iterable as input. When these functions are unnecessarily nested within each other, it creates
+redundant operations that add unnecessary computational overhead by creating intermediate data structures, decrease code readability and make the
+intention less clear, and waste memory by duplicating data structures temporarily.</p>
+<h2>How to fix it</h2>
+<p>When the outer function is given a collection but could have been given an iterable, the unnecessary conversion should be removed. For example, in
+<code>sorted(list(iterable))</code>, the outer <code>sorted()</code> function can accept an iterable directly, so the inner <code>list()</code> call
+is redundant and should be removed.</p>
+<p>When the function <code>sorted()</code> is wrapped with <code>list()</code>, remove this conversion operation, since <code>sorted()</code> already
+returns a list.</p>
+<h3>Code examples</h3>
+<h4>Noncompliant code example</h4>
+<pre data-diff-id="1" data-diff-type="noncompliant">
+iterable = (3, 1, 4, 1)
+
+sorted_of_list = list(sorted(iterable)) # Noncompliant
+</pre>
+<h4>Compliant solution</h4>
+<pre data-diff-id="1" data-diff-type="compliant">
+iterable = (3, 1, 4, 1)
+
+sorted_of_list = sorted(iterable)
+</pre>
+<h2>Resources</h2>
+<h3>Documentation</h3>
+<ul>
+  <li> Python Documentation - <a href="https://docs.python.org/3/library/stdtypes.html#list">list</a> </li>
+  <li> Python Documentation - <a href="https://docs.python.org/3/library/stdtypes.html#tuple">tuple</a> </li>
+  <li> Python Documentation - <a href="https://docs.python.org/3/library/stdtypes.html#set">set</a> </li>
+  <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>
+</ul>
+

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

@@ -0,0 +1,21 @@
+{
+  "title": "Redundant collection functions should be avoided",
+  "type": "CODE_SMELL",
+  "status": "ready",
+  "remediation": {
+    "func": "Constant\/Issue",
+    "constantCost": "5min"
+  },
+  "tags": [],
+  "defaultSeverity": "Minor",
+  "ruleSpecification": "RSPEC-7508",
+  "sqKey": "S7508",
+  "scope": "All",
+  "quickfix": "targeted",
+  "code": {
+    "impacts": {
+      "MAINTAINABILITY": "LOW"
+    },
+    "attribute": "CONVENTIONAL"
+  }
+}

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

@@ -277,6 +277,7 @@
     "S7505",
     "S7506",
     "S7507",
+    "S7508",
     "S7510",
     "S7511",
     "S7512"

python-checks/src/test/java/org/sonar/python/checks/NestedCollectionsCreationCheckTest.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 NestedCollectionsCreationCheckTest {
+
+  @Test
+  void test() {
+    PythonCheckVerifier.verify("src/test/resources/checks/nestedCollectionsCreation.py", new NestedCollectionsCreationCheck());
+  }
+
+}

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

@@ -0,0 +1,50 @@
+def case1():
+  iterable = (3, 1, 4, 1)
+  list(list(iterable)) # Noncompliant
+  list(tuple(iterable)) # Noncompliant
+  list(sorted(iterable)) # Noncompliant
+
+def case2():
+  iterable = (3, 1, 4, 1)
+  set(list(iterable)) # Noncompliant
+  set(set(iterable)) # Noncompliant
+  set(tuple(iterable)) # Noncompliant
+  set(reversed(iterable)) # Noncompliant
+  set(sorted(iterable)) # Noncompliant
+
+def case3():
+  iterable = (3, 1, 4, 1)
+  sorted(list(iterable)) # Noncompliant
+  sorted(tuple(iterable)) # Noncompliant
+  sorted(sorted(iterable)) # Noncompliant
+
+def case4():
+  iterable = (3, 1, 4, 1)
+  tuple(list(iterable)) # Noncompliant
+  tuple(tuple(iterable)) # Noncompliant
+
+def case5():
+  iterable = (3, 1, 4, 1)
+  list_of_list = list(iterable)
+  tuple_of_list = tuple(iterable)
+  set_of_list = set(iterable)
+  sorted_of_list = sorted(iterable)
+  set_of_set = set(iterable)
+  sorted_of_sorted = sorted(iterable)
+  set_of_sorted = set(iterable)
+
+def case6():
+  iterable = (3, 1, 4, 1)
+  single_usage_assigned_list = list(iterable)
+  list_of_list = list(single_usage_assigned_list) # Noncompliant
+  single_usage_assigned_not_sensitive = something_else(iterable)
+  list_of_list = list(single_usage_assigned_not_sensitive)
+  multiple_usage_assigned_list = list(iterable)
+  multiple_usage_assigned_list.append(5)
+  list_of_list = list(multiple_usage_assigned_list)
+
+def case7():
+  iterable = (3, 1, 4, 1)
+  sorted(reversed(iterable)) # OK, it should be raided by S7510
+  reversed(sorted(iterable)) # OK, it should be raided by S7510
+  list(set(iterable))