SONARPY-2937 Rule S7500: Unnecessary <dict/list/set> comprehension - rewrite using <dict/list/set>() (#284)

GitOrigin-RevId: 768bf94a77a026b1ab76c1b9abe5b5d7ddb58165

Author: maksim-grebeniuk-sonarsource

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

@@ -398,6 +398,7 @@ public Stream<Class<?>> getChecks() {
       UnencryptedEbsVolumeCheck.class,
       UnionTypeExpressionCheck.class,
       UnnecessaryListComprehensionArgumentCheck.class,
+      UnnecessaryComprehensionCheck.class,
       UnnecessaryReversedCallCheck.class,
       UnnecessarySubscriptReversalCheck.class,
       UnquantifiedNonCapturingGroupCheck.class,

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

@@ -0,0 +1,84 @@
+/*
+ * 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.Objects;
+import java.util.Optional;
+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.ComprehensionExpression;
+import org.sonar.plugins.python.api.tree.ComprehensionFor;
+import org.sonar.plugins.python.api.tree.DictCompExpression;
+import org.sonar.plugins.python.api.tree.Name;
+import org.sonar.plugins.python.api.tree.Tree;
+import org.sonar.plugins.python.api.tree.Tuple;
+import org.sonar.python.tree.TreeUtils;
+
+@Rule(key = "S7500")
+public class UnnecessaryComprehensionCheck extends PythonSubscriptionCheck {
+
+  @Override
+  public void initialize(Context context) {
+    context.registerSyntaxNodeConsumer(Tree.Kind.GENERATOR_EXPR, UnnecessaryComprehensionCheck::checkComprehensionExpression);
+    context.registerSyntaxNodeConsumer(Tree.Kind.LIST_COMPREHENSION, UnnecessaryComprehensionCheck::checkComprehensionExpression);
+    context.registerSyntaxNodeConsumer(Tree.Kind.SET_COMPREHENSION, UnnecessaryComprehensionCheck::checkComprehensionExpression);
+    context.registerSyntaxNodeConsumer(Tree.Kind.DICT_COMPREHENSION, UnnecessaryComprehensionCheck::checkDictComprehensionExpression);
+  }
+
+  private static void checkComprehensionExpression(SubscriptionContext ctx) {
+    var comprehension = (ComprehensionExpression) ctx.syntaxNode();
+    var valueExpression = comprehension.resultExpression();
+    var loopExpression = Optional.of(comprehension)
+      .map(ComprehensionExpression::comprehensionFor)
+      .filter(comprehensionFor -> Objects.isNull(comprehensionFor.nestedClause()))
+      .map(ComprehensionFor::loopExpression)
+      .orElse(null);
+
+    if (valueExpression instanceof Name valueName
+        && loopExpression instanceof Name loopValueName
+        && valueName.name().equals(loopValueName.name())
+    ) {
+      ctx.addIssue(comprehension, "Replace this comprehension with passing the iterable to the collection constructor call");
+    }
+  }
+
+  private static void checkDictComprehensionExpression(SubscriptionContext ctx) {
+    var comprehension = (DictCompExpression) ctx.syntaxNode();
+    var keyExpression = comprehension.keyExpression();
+    var valueExpression = comprehension.valueExpression();
+    var loopExpressions = Optional.of(comprehension)
+      .map(DictCompExpression::comprehensionFor)
+      .filter(comprehensionFor -> Objects.isNull(comprehensionFor.nestedClause()))
+      .map(ComprehensionFor::loopExpression)
+      .map(TreeUtils.toInstanceOfMapper(Tuple.class))
+      .map(Tuple::elements)
+      .orElseGet(List::of);
+
+    if (keyExpression instanceof Name keyName
+        && valueExpression instanceof Name valueName
+        && loopExpressions.size() == 2
+        && loopExpressions.get(0) instanceof Name loopKeyName
+        && loopExpressions.get(1) instanceof Name loopValueName
+        && keyName.name().equals(loopKeyName.name())
+        && valueName.name().equals(loopValueName.name())
+    ) {
+      ctx.addIssue(comprehension, "Replace this comprehension with passing the iterable to the dict constructor call");
+    }
+  }
+}

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

@@ -0,0 +1,31 @@
+<p>This rule raises an issue when a comprehension is used to only copy a collection. Instead, the respective constructor should be used.</p>
+<h2>Why is this an issue?</h2>
+<p>Python comprehensions are a concise way to create new collections while transforming or filtering elements. However, using a comprehension that
+copies elements from one collection to another without any transformation is less readable than using the respective constructor directly.</p>
+<p>Therefore, when a comprehension is only copying elements, use the appropriate constructor instead:</p>
+<ul>
+  <li> Replace <code>[x for x in iterable]</code> with <code>list(iterable)</code> </li>
+  <li> Replace <code>{x for x in iterable}</code> with <code>set(iterable)</code> </li>
+  <li> Replace <code>{k: v for k, v in iterable.items()}</code> with <code>dict(iterable)</code> </li>
+</ul>
+<h2>How to fix it</h2>
+<p>Replace comprehensions that copy elements from one collection to another with the respective constructor.</p>
+<h3>Code examples</h3>
+<h4>Noncompliant code example</h4>
+<pre data-diff-id="1" data-diff-type="noncompliant">
+some_list = [1, 2, 3, 2]
+[x for x in some_list] # Noncompliant
+</pre>
+<h4>Compliant solution</h4>
+<pre data-diff-id="1" data-diff-type="compliant">
+some_list = [1, 2, 3, 2]
+list(some_list) # Compliant
+</pre>
+<h2>Resources</h2>
+<h3>Documentation</h3>
+<ul>
+  <li> Python Documentation - <a href="https://docs.python.org/3/library/stdtypes.html#dict">dict()</a> </li>
+  <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#set">set()</a> </li>
+</ul>
+

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

@@ -0,0 +1,21 @@
+{
+  "title": "Comprehensions only used to copy should be replaced with the respective constructor calls",
+  "type": "CODE_SMELL",
+  "status": "ready",
+  "remediation": {
+    "func": "Constant\/Issue",
+    "constantCost": "5min"
+  },
+  "tags": [],
+  "defaultSeverity": "Minor",
+  "ruleSpecification": "RSPEC-7500",
+  "sqKey": "S7500",
+  "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

@@ -266,6 +266,7 @@
     "S7494",
     "S7498",
     "S7499",
+    "S7500",
     "S7501",
     "S7502",
     "S7503",

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

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

@@ -0,0 +1,28 @@
+
+def case1():
+  iterable_single = [1, 2, 3, 2]
+  list_comp = [x for x in iterable_single] # Noncompliant
+  set_comp = {x for x in iterable_single} # Noncompliant
+  list_comp = list(x for x in iterable_single) # Noncompliant
+  set_comp = set(x for x in iterable_single) # Noncompliant
+
+  iterable_pairs = [('a', 1), ('b', 2)]
+  dict_comp = {k: v for k, v in iterable_pairs} # Noncompliant
+
+def case2():
+  iterable_single = [1, 2, 3, 2]
+  list_comp = [y for x in iterable_single]
+  tuple = tuple(x for x in iterable_single if not something(x))
+  list_comp = [x + 1 for x in iterable_single]
+  set_comp = {x + 1 for x in iterable_single}
+  list_comp = list(x + 1 for x in iterable_single)
+  set_comp = set(x + 1 for x in iterable_single)
+
+  iterable_pairs = [('a', 1), ('b', 2)]
+  dict_comp = {k: v + 1 for k, v in iterable_pairs}
+  dict_comp = {k: v for k, v.x in iterable_pairs}
+  dict_comp = {k: v for k.x, v in iterable_pairs}
+  dict_comp = {something(k): v for k, v in iterable_pairs}
+  dict_comp = {k: v for k, v in iterable_pairs if something(k)}
+  dict_comp = {k: v1 for k, v2 in iterable_pairs}
+  dict_comp = {k1: v for k2, v in iterable_pairs}