SONARPY-2926 Rule S7494: Unnecessary generator - rewrite as a comprehension (#265)

GitOrigin-RevId: 3e22edf0ae5c95d70da198aa13099d30a481b758

Author: Seppli11

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

@@ -382,6 +382,7 @@ public Stream<Class<?>> getChecks() {
       ReferencedBeforeAssignmentCheck.class,
       RegexComplexityCheck.class,
       RegexLookaheadCheck.class,
+      RewriteCollectionConstructorAsComprehensionCheck.class,
       UnconditionalAssertionCheck.class,
       UndefinedNameAllPropertyCheck.class,
       UnencryptedSageMakerNotebookCheck.class,

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

@@ -0,0 +1,159 @@
+/*
+ * 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 javax.annotation.Nullable;
+import org.sonar.check.Rule;
+import org.sonar.plugins.python.api.PythonSubscriptionCheck;
+import org.sonar.plugins.python.api.tree.Argument;
+import org.sonar.plugins.python.api.tree.CallExpression;
+import org.sonar.plugins.python.api.tree.ComprehensionClause;
+import org.sonar.plugins.python.api.tree.ComprehensionExpression;
+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.tree.Tuple;
+import org.sonar.plugins.python.api.types.v2.PythonType;
+import org.sonar.plugins.python.api.types.v2.TriBool;
+import org.sonar.python.semantic.v2.SymbolV2;
+import org.sonar.python.types.v2.TypeCheckBuilder;
+import org.sonar.python.types.v2.TypeCheckMap;
+
+@Rule(key = "S7494")
+public class RewriteCollectionConstructorAsComprehensionCheck extends PythonSubscriptionCheck {
+
+  private static final Map<String, String> COLLECTION_MESSAGES = Map.of(
+    "list", "Replace list constructor call with a list comprehension.",
+    "set", "Replace set constructor call with a set comprehension.",
+    "dict", "Replace dict constructor call with a dictionary comprehension."
+  );
+
+  private TypeCheckMap<String> collectionTypeCheckerMap = null;
+  private TypeCheckBuilder dictTypeChecker = null;
+
+  @Override
+  public void initialize(Context context) {
+    collectionTypeCheckerMap = new TypeCheckMap<>();
+    context.registerSyntaxNodeConsumer(Tree.Kind.FILE_INPUT, ctx -> {
+      for (var collectionEntry : COLLECTION_MESSAGES.entrySet()) {
+        TypeCheckBuilder typeChecker = ctx.typeChecker().typeCheckBuilder().isTypeWithFqn(collectionEntry.getKey());
+        collectionTypeCheckerMap.put(typeChecker, collectionEntry.getValue());
+
+        if ("dict".equals(collectionEntry.getKey())) {
+          dictTypeChecker = typeChecker;
+        }
+      }
+    });
+
+    context.registerSyntaxNodeConsumer(Tree.Kind.CALL_EXPR, ctx -> {
+      CallExpression callExpression = (CallExpression) ctx.syntaxNode();
+      List<Argument> arguments = callExpression.arguments();
+
+      String message = getMessageForConstructor(callExpression).orElse(null);
+      if (message == null) {
+        return;
+      }
+
+      ComprehensionExpression generator = getSingleGeneratorArg(arguments).orElse(null);
+      if (generator == null) {
+        return;
+      }
+
+      if (isGeneratorTransformingData(generator, callExpression.callee().typeV2())) {
+        ctx.addIssue(callExpression.callee(), message);
+      }
+    });
+  }
+
+  private Optional<String> getMessageForConstructor(CallExpression callExpression) {
+    return collectionTypeCheckerMap.getOptionalForType(callExpression.callee().typeV2());
+  }
+
+  private static Optional<ComprehensionExpression> getSingleGeneratorArg(List<Argument> arguments) {
+    if (arguments.size() != 1) {
+      return Optional.empty();
+    }
+
+    if (!(arguments.get(0) instanceof RegularArgument regularArg)) {
+      return Optional.empty();
+    }
+
+    Expression argument = regularArg.expression();
+    if (!argument.is(Tree.Kind.GENERATOR_EXPR)) {
+      return Optional.empty();
+    }
+
+    return Optional.of((ComprehensionExpression) argument);
+  }
+
+  private boolean isGeneratorTransformingData(ComprehensionExpression generator, PythonType methodType) {
+    if (hasMultipleNestedClauses(generator.comprehensionFor())) {
+      return true;
+    }
+    if (dictTypeChecker.check(methodType) == TriBool.TRUE) {
+      return isDictTransformingData(generator);
+    } else {
+      return isListOrSetComprehensionTransformingData(generator);
+    }
+  }
+
+  private static boolean hasMultipleNestedClauses(ComprehensionClause clause) {
+    return clause.nestedClause() != null;
+  }
+
+  private static boolean isDictTransformingData(ComprehensionExpression generator) {
+    Expression resultExpr = generator.resultExpression();
+    Expression loopVarExpr = generator.comprehensionFor().loopExpression();
+
+    var resultTuple = getTwoNamedTuple(resultExpr);
+    var varTuple = getTwoNamedTuple(loopVarExpr);
+
+    if (resultTuple != null && varTuple != null) {
+      return !resultTuple.equals(varTuple);
+    }
+
+    return true;
+  }
+
+  private static TwoNamedTuple getTwoNamedTuple(Expression expr) {
+    if (expr instanceof Tuple tuple && tuple.elements().size() == 2) {
+      var elements = tuple.elements();
+      if (elements.get(0) instanceof Name name1 && elements.get(1) instanceof Name name2) {
+        return new TwoNamedTuple(name1.symbolV2(), name2.symbolV2());
+      }
+    }
+    return null;
+  }
+
+  private record TwoNamedTuple(@Nullable SymbolV2 symbol1, @Nullable SymbolV2 symbol2) {
+  }
+
+  private static boolean isListOrSetComprehensionTransformingData(ComprehensionExpression generator) {
+    Expression elementExpr = generator.resultExpression();
+    Expression loopVarExpr = generator.comprehensionFor().loopExpression();
+
+    if (elementExpr instanceof Name elementName && loopVarExpr instanceof Name loopVarName) {
+      return elementName.symbolV2() != loopVarName.symbolV2();
+    }
+
+    return true;
+  }
+}

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

@@ -0,0 +1,40 @@
+<p>This rule raises an issue when <code>list()</code>, <code>set()</code> or <code>dict()</code> is unnecessarily used around a generator
+expression.</p>
+<h2>Why is this an issue?</h2>
+<p>Using <code>list()</code>, <code>set()</code>, or <code>dict()</code> around a generator expression is redundant when a corresponding comprehension
+can directly express the same operation. Comprehensions are clearer, more concise, and often more readable than the equivalent constructor/generator
+expression combination.</p>
+<p>This principle applies to all three built-in collection types: <code>list</code>, <code>set</code>, and <code>dict</code>:</p>
+<ul>
+  <li> Use <code>[f(x) for x in foo]</code> instead of <code>list(f(x) for x in foo)</code> </li>
+  <li> Use <code>{f(x) for x in foo}</code> instead of <code>set(f(x) for x in foo)</code> </li>
+  <li> Use <code>{k: v for k, v in items}</code> instead of <code>dict((k, v) for k, v in items)</code> </li>
+</ul>
+<h3>Exceptions</h3>
+<p>If the generator expression doesn’t filter or modify the collection, the rule does not raise. For example, <code>list(x for x in foo)</code> is
+simply copying the iterable <code>foo</code> into a list, which is equivalent to <code>list(foo)</code> and covered by a different rule.</p>
+<h2>How to fix it</h2>
+<p>Replace the collection constructor with the appropriate comprehension syntax.</p>
+<h3>Code examples</h3>
+<h4>Noncompliant code example</h4>
+<pre data-diff-id="1" data-diff-type="noncompliant">
+def f(x):
+    return x * 2
+
+list(f(x) for x in range(5))  # Noncompliant
+</pre>
+<h4>Compliant solution</h4>
+<pre data-diff-id="1" data-diff-type="compliant">
+def f(x):
+    return x * 2
+
+[f(x) for x in range(5)] # Compliant
+</pre>
+<h2>Resources</h2>
+<h3>Documentation</h3>
+<ul>
+  <li> Python Documentation - <a href="https://docs.python.org/3/tutorial/datastructures.html#list-comprehensions">List Comprehensions</a> </li>
+  <li> Python Documentation - <a href="https://docs.python.org/3/tutorial/datastructures.html#dictionaries">Dictionaries</a> </li>
+  <li> Python Documentation - <a href="https://docs.python.org/3/tutorial/datastructures.html#sets">Sets</a> </li>
+</ul>
+

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

@@ -0,0 +1,21 @@
+{
+  "title": "Comprehensions should be used instead of constructors around generator expressions",
+  "type": "CODE_SMELL",
+  "status": "ready",
+  "remediation": {
+    "func": "Constant\/Issue",
+    "constantCost": "5min"
+  },
+  "tags": [],
+  "defaultSeverity": "Minor",
+  "ruleSpecification": "RSPEC-7494",
+  "sqKey": "S7494",
+  "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

@@ -258,6 +258,7 @@
     "S7488",
     "S7491",
     "S7493",
+    "S7494",
     "S7498",
     "S7499",
     "S7501",

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

@@ -0,0 +1,28 @@
+/*
+ * 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 RewriteCollectionConstructorAsComprehensionCheckTest {
+
+  @Test
+  void test() {
+    PythonCheckVerifier.verify("src/test/resources/checks/rewriteCollectionConstructorAsComprehension.py", new RewriteCollectionConstructorAsComprehensionCheck());
+  }
+}

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

@@ -0,0 +1,59 @@
+
+def func(x):
+    return x * 2
+
+def lists():
+    list(func(x) for x in range(5))  # Noncompliant {{Replace list constructor call with a list comprehension.}}
+   #^^^^
+
+    list(x + 1 for x in range(10))  # Noncompliant
+    list((x, y) for x in range(3) for y in range(2))  # Noncompliant
+    list(x for x in range(10) if x % 2 == 0)  # Noncompliant
+
+    list(x for x in range(10))
+    list(range(10))
+    list()
+    list([1, 2, 3])
+
+def sets():
+    set(func(x) for x in range(5))  # Noncompliant {{Replace set constructor call with a set comprehension.}}
+   #^^^
+    set(x * 2 for x in items)  # Noncompliant
+    set(x for x in range(10) if x > 5)  # Noncompliant
+
+    set(x for x in range(10))
+    set()
+    set([1, 2, 3])
+    set(x for x,y in range(10)) # Noncompliant
+    set(y for x in range(10)) # Noncompliant
+
+def dicts():
+    dict((k, v) for k, v in items.items())
+    dict((k, func(v)) for k, v in items.items())  # Noncompliant {{Replace dict constructor call with a dictionary comprehension.}}
+   #^^^^
+    dict((v, k) for k, v in items.items())  # Noncompliant
+    dict((k.upper(), v) for k, v in items.items())  # Noncompliant
+    dict((k, v) for k, v in items.items() if k.startswith('a'))  # Noncompliant
+    dict(tuple(k, v) for k, v in items.items() if k.startswith('a'))  # Noncompliant
+    dict(tuple([k, v]) for k, v in items.items()) # Noncompliant
+    dict((t, t) for t in items) # Noncompliant
+    dict((t, t, t) for t in items) # Noncompliant
+    dict((t,) for t in items) # Noncompliant
+
+def compliant_examples():
+    [func(x) for x in range(5)]
+    {func(x) for x in range(5)}
+    {k: func(v) for k, v in items.items()}
+    {v: k for k, v in items.items()}
+
+def other():
+    list(map(func, range(10)))
+    list([x for x in range(10)])
+    result = list(func(x) for x in range(5))  # Noncompliant
+
+    list(*args)
+    list(**args)
+
+def multiple_clauses():
+    list((i, j) for i in range(3) for j in range(2))  # Noncompliant
+    [(i, j) for i in range(3) for j in range(2)]