SONARPY-1437 Rule S6714: Passing a list to np.array should be preferred over passing a generator. (#1573)

Author: Jeremi Do Dinh

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

@@ -271,6 +271,7 @@ public static Iterable<Class> getChecks() {
       NotDiscoverableTestMethodCheck.class,
       NotImplementedErrorInOperatorMethodsCheck.class,
       NumpyIsNanCheck.class,
+      NumpyListOverGeneratorCheck.class,
       NumpyRandomSeedCheck.class,
       NumpyRandomStateCheck.class,
       NumpyWhereOneConditionCheck.class,

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

@@ -0,0 +1,100 @@
+/*
+ * SonarQube Python Plugin
+ * Copyright (C) 2011-2023 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 GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * 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 GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+package org.sonar.python.checks;
+
+import java.util.List;
+import java.util.Optional;
+import java.util.Set;
+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.symbols.Symbol;
+import org.sonar.plugins.python.api.tree.Argument;
+import org.sonar.plugins.python.api.tree.CallExpression;
+import org.sonar.plugins.python.api.tree.Expression;
+import org.sonar.plugins.python.api.tree.HasSymbol;
+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.python.cfg.fixpoint.ReachingDefinitionsAnalysis;
+import org.sonar.python.tree.TreeUtils;
+
+@Rule(key = "S6714")
+public class NumpyListOverGeneratorCheck extends PythonSubscriptionCheck {
+
+  public static final String MESSAGE = "Pass a list to \"np.array\" instead of passing a generator.";
+  private ReachingDefinitionsAnalysis reachingDefinitionsAnalysis;
+
+  @Override
+  public void initialize(Context context) {
+    context.registerSyntaxNodeConsumer(Tree.Kind.FILE_INPUT, ctx -> reachingDefinitionsAnalysis = new ReachingDefinitionsAnalysis((ctx.pythonFile())));
+    context.registerSyntaxNodeConsumer(Tree.Kind.CALL_EXPR, this::checkNumpyArrayCall);
+  }
+
+  private void checkNumpyArrayCall(SubscriptionContext ctx) {
+    CallExpression call = (CallExpression) ctx.syntaxNode();
+    Optional.ofNullable(call.calleeSymbol())
+      .map(Symbol::fullyQualifiedName)
+      .filter("numpy.array"::equals)
+      .ifPresent(fqn -> checkGeneratorCallee(call, ctx));
+  }
+
+  private void checkGeneratorCallee(CallExpression call, SubscriptionContext ctx) {
+    List<Argument> argList = call.arguments();
+    if (argList.isEmpty()) {
+      return;
+    }
+
+    if (Optional.of(argList.get(0))
+      .filter(arg -> arg.is(Tree.Kind.REGULAR_ARGUMENT))
+      .map(RegularArgument.class::cast)
+      .map(RegularArgument::expression)
+      .filter(regArg -> (regArg.is(Tree.Kind.GENERATOR_EXPR) || this.isNamedGeneratorExpression(regArg)))
+      .isEmpty()) {
+      return;
+    }
+
+    if (Optional.ofNullable(TreeUtils.nthArgumentOrKeyword(1, "dtype", argList))
+      .filter(regArg -> regArg.expression().is(Tree.Kind.NAME))
+      .map(regArg -> (Name) regArg.expression())
+      .map(HasSymbol::symbol)
+      .map(Symbol::fullyQualifiedName)
+      .filter("object"::equals)
+      .isEmpty()) {
+      ctx.addIssue(call, MESSAGE);
+    }
+  }
+
+  private boolean isNamedGeneratorExpression(Expression expression) {
+    return Optional.of(expression)
+      .flatMap(TreeUtils.toOptionalInstanceOfMapper(Name.class))
+      .map(name -> this.reachingDefinitionsAnalysis.valuesAtLocation(name))
+      .filter(NumpyListOverGeneratorCheck::checkSetProperties)
+      .isPresent();
+  }
+
+  private static boolean checkSetProperties(Set<Expression> set) {
+    return !set.isEmpty() && set.stream().allMatch(NumpyListOverGeneratorCheck::isGeneratorAndParentHasType);
+  }
+
+  private static boolean isGeneratorAndParentHasType(Expression expression) {
+    return expression.is(Tree.Kind.GENERATOR_EXPR) && !expression.parent().is(Tree.Kind.ANNOTATED_ASSIGNMENT);
+  }
+}

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

@@ -0,0 +1,47 @@
+<p>This rule raises an issue when a generator is passed to <code>np.array</code>.</p>
+<h2>Why is this an issue?</h2>
+<p>The creation of a NumPy array can be done in several ways, for example by passing a Python list to the <code>np.array</code> function. Another way
+would be to pass a generator to the <code>np.array</code> function, but doing so creates a 0-dimensional array of objects and may not be the intended
+goal. This NumPy array will have a have a data type (dtype) of <code>object</code> and could hold any Python objects.</p>
+<p>One of the characteristics of NumPy arrays is homogeneity, meaning all its elements are of the same type. Creating an array of objects allows the
+user to create heterogeneous array without raising any errors and creating such an array can lead to bugs further in the program.</p>
+<pre data-diff-id="1" data-diff-type="noncompliant">
+arr = np.array(x**2 for x in range(10))
+
+arr.reshape(1)
+arr.resize(2)
+arr.put(indices=1, values=3) # No issues raised.
+</pre>
+<p>The NumPy array <code>arr</code> shown above now holds 2 values: a generator and the number 3.</p>
+<h2>How to fix it</h2>
+<p>To fix this issue, either:</p>
+<ul>
+  <li> pass a Python list instead of a generator to the <code>np.array</code> function or, </li>
+  <li> explicitly show the intention to create a 0-dimensional array of objects by either adding <code>Any</code> as the type hint of the generator or
+  by specifying the <code>dtype</code> parameter of the NumPy array as <code>object</code>. </li>
+</ul>
+<h3>Code examples</h3>
+<h4>Noncompliant code example</h4>
+<pre data-diff-id="2" data-diff-type="noncompliant">
+arr = np.array(x**2 for x in range(10)) # Noncompliant: the resulting array will be of the data type: object.
+
+gen = (x*2 for x in range(5))
+arr = np.array(gen) # Noncompliant: the resulting array will be of the data type: object.
+</pre>
+<h4>Compliant solution</h4>
+<pre data-diff-id="2" data-diff-type="compliant">
+from typing import Any
+
+arr = np.array([x**2 for x in range(10)]) # Compliant: a list of 10 elements is passed to the np.array function.
+
+arr = np.array(x**2 for x in range(10), dtype=object) # Compliant: the dtype parameter of np.array is set to object.
+
+gen: Any = (x*2 for x in range(5))
+arr = np.array(gen) # Compliant: the generator is explicitly type hinted with Any.
+</pre>
+<h2>Resources</h2>
+<h3>Documentation</h3>
+<ul>
+  <li> NumPy Documentation - <a href="https://numpy.org/doc/stable/reference/typing.html#arraylike">ArrayLike</a> </li>
+</ul>
+

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

@@ -0,0 +1,25 @@
+{
+  "title": "Passing a list to np.array should be preferred over passing a generator",
+  "type": "CODE_SMELL",
+  "status": "ready",
+  "remediation": {
+    "func": "Constant\/Issue",
+    "constantCost": "5min"
+  },
+  "tags": [
+    "numpy",
+    "data-science"
+  ],
+  "defaultSeverity": "Major",
+  "ruleSpecification": "RSPEC-6714",
+  "sqKey": "S6714",
+  "scope": "All",
+  "quickfix": "unknown",
+  "code": {
+    "impacts": {
+      "MAINTAINABILITY": "MEDIUM",
+      "RELIABILITY": "LOW"
+    },
+    "attribute": "LOGICAL"
+  }
+}

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

@@ -206,6 +206,7 @@
     "S6663",
     "S6709",
     "S6711",
+    "S6714",
     "S6725",
     "S6727",
     "S6729",

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

@@ -0,0 +1,30 @@
+/*
+ * SonarQube Python Plugin
+ * Copyright (C) 2011-2023 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 GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * 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 GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+package org.sonar.python.checks;
+
+import org.junit.jupiter.api.Test;
+import org.sonar.python.checks.utils.PythonCheckVerifier;
+
+class NumpyListOverGeneratorCheckTest {
+  @Test
+  void test() {
+    PythonCheckVerifier.verify("src/test/resources/checks/numpyListOverGenerator.py", new NumpyListOverGeneratorCheck());
+  }
+}

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

@@ -0,0 +1,37 @@
+from typing import Any
+
+
+def import_1():
+    import numpy as np
+    from typing import Any
+
+    gen_1 = (x * 2 for x in range(5))
+    arr = np.array(gen_1)  # Noncompliant {{Pass a list to "np.array" instead of passing a generator.}}
+    #     ^^^^^^^^^^^^^^^
+    gen_2: Any = (x * 2 for x in range(5))
+    arr = np.array(gen_2)
+
+    gen_3: str = "Hello World"
+    gen_3 = (x * 2 for x in range(5))
+    arr = np.array(gen_3)  # Noncompliant
+
+    gen_4 = 42
+    np.array(gen_4)
+
+    gen_5: MyClass = (x * 2 for x in range(5))
+    np.array(gen_5)
+
+    def test(xx):
+        np.array(xx)
+        np.array(xx, gen_1)
+
+    arr = np.array((x ** 2 for x in range(10)), dtype=object)  # Compliant: the dtype parameter of np.array is set to object.
+
+    arr = np.array(x ** 2 for x in range(10))  # Noncompliant {{Pass a list to "np.array" instead of passing a generator.}}
+    #     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+    arr = np.array([x ** 2 for x in range(10)])  # Compliant: a list of 10 elements is passed to the np.array function.
+
+    arr = np.array((x ** 2 for x in range(10)), dtype=str)  # Noncompliant
+
+    arr = np.array()