SONARPY-215 Rule S3699: The output of functions that don't return anything should not be used (#1100)

Author: andrea-guarino-sonarsource

its/ruling/src/test/java/org/sonar/python/it/RulingHelper.java

@@ -62,6 +62,7 @@ static List<String> bugRuleKeys() {
       "S2757",
       "S2823",
       "S3403",
+      "S3699",
       "S3827",
       "S3862",
       "S3923",

its/ruling/src/test/resources/expected/python-S3699.json

@@ -0,0 +1,16 @@
+{
+'project:django-2.2.3/django/core/files/locks.py':[
+108,
+112,
+],
+'project:numpy-1.16.4/numpy/core/records.py':[
+482,
+],
+'project:numpy-1.16.4/numpy/ma/mrecords.py':[
+259,
+],
+'project:tensorflow/python/data/kernel_tests/interleave_test.py':[
+297,
+297,
+],
+}

its/ruling/src/test/resources/expected_extended/calibre/python-S3699.json

@@ -0,0 +1,14 @@
+{
+'calibre:src/calibre/ebooks/metadata/xisbn.py':[
+106,
+],
+'calibre:src/calibre/ebooks/oeb/polish/tests/container.py':[
+100,
+],
+'calibre:src/calibre/ebooks/snb/snbfile.py':[
+89,
+],
+'calibre:src/calibre/gui2/open_with.py':[
+325,
+],
+}

its/ruling/src/test/resources/expected_extended/nltk/python-S3699.json

@@ -0,0 +1,8 @@
+{
+'nltk:nltk/app/collocations_app.py':[
+119,
+],
+'nltk:nltk/app/concordance_app.py':[
+277,
+],
+}

its/ruling/src/test/resources/expected_extended/salt/python-S3699.json

@@ -0,0 +1,8 @@
+{
+'salt:salt/cloud/clouds/linode.py':[
+1814,
+],
+'salt:salt/utils/http.py':[
+453,
+],
+}

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

@@ -252,6 +252,7 @@ public static Iterable<Class> getChecks() {
       UselessParenthesisAfterKeywordCheck.class,
       UselessParenthesisCheck.class,
       UselessStatementCheck.class,
+      UseOfEmptyReturnValueCheck.class,
       VerifiedSslTlsCertificateCheck.class,
       WeakSSLProtocolCheck.class,
       WildcardImportCheck.class,

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

@@ -0,0 +1,54 @@
+/*
+ * SonarQube Python Plugin
+ * Copyright (C) 2011-2022 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.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.AssignmentStatement;
+import org.sonar.plugins.python.api.tree.CallExpression;
+import org.sonar.plugins.python.api.tree.Expression;
+import org.sonar.plugins.python.api.tree.RegularArgument;
+import org.sonar.python.types.InferredTypes;
+
+import static org.sonar.plugins.python.api.tree.Tree.Kind.ASSIGNMENT_STMT;
+import static org.sonar.plugins.python.api.tree.Tree.Kind.CALL_EXPR;
+import static org.sonar.plugins.python.api.tree.Tree.Kind.REGULAR_ARGUMENT;
+
+@Rule(key = "S3699")
+public class UseOfEmptyReturnValueCheck extends PythonSubscriptionCheck {
+
+  private static final String MESSAGE = "Remove this use of the output from \"%s\"; \"%s\" doesn’t return anything.";
+
+  @Override
+  public void initialize(Context context) {
+    context.registerSyntaxNodeConsumer(ASSIGNMENT_STMT, ctx -> checkReturnValue(((AssignmentStatement) ctx.syntaxNode()).assignedValue(), ctx));
+    context.registerSyntaxNodeConsumer(REGULAR_ARGUMENT, ctx -> checkReturnValue(((RegularArgument) ctx.syntaxNode()).expression(), ctx));
+  }
+
+  private static void checkReturnValue(Expression expression, SubscriptionContext ctx) {
+    if (expression.is(CALL_EXPR) && expression.type().equals(InferredTypes.NONE)) {
+      CallExpression callExpression = (CallExpression) expression;
+      Optional.ofNullable(callExpression.calleeSymbol())
+        .ifPresent(symbol -> ctx.addIssue(expression, String.format(MESSAGE, symbol.name(), symbol.name())));
+    }
+  }
+}

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

@@ -0,0 +1,17 @@
+<p>If a function does not return anything, it makes no sense to use its output. Specifically, passing it to another function, or assigning its
+"result" to a variable is probably a bug because such functions return nothing, which is probably not what was intended.</p>
+<h2>Noncompliant Code Example</h2>
+<pre>
+def foo():
+  l = [1, 2, 3]
+  result = l.append(42) # Noncompliant, `append` mutates list l
+  return result
+</pre>
+<h2>Compliant Solution</h2>
+<pre>
+def foo():
+  l = [1, 2, 3]
+  l.append(42)
+  return l
+</pre>
+

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

@@ -0,0 +1,15 @@
+{
+  "title": "The output of functions that don\u0027t return anything should not be used",
+  "type": "BUG",
+  "status": "ready",
+  "remediation": {
+    "func": "Constant\/Issue",
+    "constantCost": "5min"
+  },
+  "tags": [],
+  "defaultSeverity": "Major",
+  "ruleSpecification": "RSPEC-3699",
+  "sqKey": "S3699",
+  "scope": "All",
+  "quickfix": "unknown"
+}

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

@@ -72,6 +72,7 @@
     "S3457",
     "S3516",
     "S3626",
+    "S3699",
     "S3752",
     "S3776",
     "S3827",