SONARPY-751 Rule S5886: Function return types should be consistent with their type hint

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

@@ -5,6 +5,10 @@
 'project:buildbot-0.8.6p1/buildbot/status/web/feeds.py':[
 240,
 ],
+'project:django-2.2.3/django/core/files/locks.py':[
+109,
+113,
+],
 'project:tensorflow/python/keras/layers/normalization.py':[
 1131,
 ],

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

@@ -0,0 +1,9 @@
+{
+'project:biopython/Bio/PDB/PICIO.py':[
+178,
+191,
+205,
+246,
+250,
+],
+}

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

@@ -113,6 +113,7 @@ public static Iterable<Class> getChecks() {
       FixmeCommentCheck.class,
       FunctionComplexityCheck.class,
       FunctionNameCheck.class,
+      FunctionReturnTypeCheck.class,
       FunctionUsingLoopVariableCheck.class,
       GenericExceptionRaisedCheck.class,
       HardCodedCredentialsCheck.class,

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

@@ -0,0 +1,131 @@
+/*
+ * SonarQube Python Plugin
+ * Copyright (C) 2011-2020 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.ArrayList;
+import java.util.List;
+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.BaseTreeVisitor;
+import org.sonar.plugins.python.api.tree.Expression;
+import org.sonar.plugins.python.api.tree.FunctionDef;
+import org.sonar.plugins.python.api.tree.ReturnStatement;
+import org.sonar.plugins.python.api.tree.Tree;
+import org.sonar.plugins.python.api.tree.TypeAnnotation;
+import org.sonar.plugins.python.api.tree.YieldStatement;
+import org.sonar.plugins.python.api.types.InferredType;
+import org.sonar.python.semantic.FunctionSymbolImpl;
+import org.sonar.python.types.InferredTypes;
+
+@Rule(key = "S5886")
+public class FunctionReturnTypeCheck extends PythonSubscriptionCheck {
+
+  private static final String MESSAGE = "Return a value of type \"%s\" instead of \"%s\" or update function \"%s\" type hint.";
+
+  @Override
+  public void initialize(Context context) {
+    context.registerSyntaxNodeConsumer(Tree.Kind.FUNCDEF, ctx -> {
+      FunctionDef functionDef = (FunctionDef) ctx.syntaxNode();
+      Symbol symbol = functionDef.name().symbol();
+      if (symbol == null || !symbol.is(Symbol.Kind.FUNCTION)) {
+        return;
+      }
+      FunctionSymbolImpl functionSymbol = (FunctionSymbolImpl) symbol;
+      InferredType declaredReturnType = functionSymbol.declaredReturnType();
+      if (declaredReturnType == InferredTypes.anyType()) {
+        return;
+      }
+      ReturnTypeVisitor returnTypeVisitor = new ReturnTypeVisitor(declaredReturnType);
+      functionDef.accept(returnTypeVisitor);
+      raiseIssues(ctx, functionDef, declaredReturnType, returnTypeVisitor);
+    });
+  }
+
+  private static void raiseIssues(SubscriptionContext ctx, FunctionDef functionDef, InferredType declaredReturnType, ReturnTypeVisitor returnTypeVisitor) {
+    String functionName = functionDef.name().name();
+    String returnTypeName = InferredTypes.typeName(declaredReturnType);
+    if (!returnTypeVisitor.yieldStatements.isEmpty()) {
+      if (declaredReturnType.mustBeOrExtend("typing.Generator")) {
+        return;
+      }
+      returnTypeVisitor.yieldStatements
+        .forEach(y -> ctx.addIssue(y, String.format("Remove this yield statement or annotate function \"%s\" with \"typing.Generator\".", functionName)));
+    }
+    returnTypeVisitor.invalidReturnStatements.forEach(i -> {
+      PreciseIssue issue;
+      if (i.expressions().size() > 1) {
+        issue = ctx.addIssue(i, String.format(MESSAGE, returnTypeName, "tuple", functionName));
+      } else if (i.expressions().size() == 1 && InferredTypes.typeName(i.expressions().get(0).type()) != null) {
+        issue = ctx.addIssue(i.expressions().get(0), String.format(MESSAGE, returnTypeName, InferredTypes.typeName(i.expressions().get(0).type()), functionName));
+      } else {
+        issue = ctx.addIssue(i, String.format("Return a value of type \"%s\" or update function \"%s\" type hint.", returnTypeName, functionName));
+      }
+      addSecondaries(issue, functionDef);
+    });
+  }
+
+  private static void addSecondaries(PreciseIssue issue, FunctionDef functionDef) {
+    issue.secondary(functionDef.name(), "Function definition.");
+    TypeAnnotation returnTypeAnnotation = functionDef.returnTypeAnnotation();
+    if (returnTypeAnnotation != null) {
+      issue.secondary(returnTypeAnnotation.expression(), "Type hint.");
+    }
+  }
+
+  private static class ReturnTypeVisitor extends BaseTreeVisitor {
+
+    InferredType returnType;
+    List<YieldStatement> yieldStatements = new ArrayList<>();
+    List<ReturnStatement> invalidReturnStatements = new ArrayList<>();
+
+    ReturnTypeVisitor(InferredType returnType) {
+      this.returnType = returnType;
+    }
+
+    @Override
+    public void visitReturnStatement(ReturnStatement returnStatement) {
+      List<Expression> expressions = returnStatement.expressions();
+      if (expressions.isEmpty()) {
+        if (!returnType.mustBeOrExtend("NoneType")) {
+          invalidReturnStatements.add(returnStatement);
+        }
+      } else if (expressions.size() > 1) {
+        if (!returnType.canBeOrExtend("tuple")) {
+          invalidReturnStatements.add(returnStatement);
+        }
+      } else {
+        Expression expression = expressions.get(0);
+        InferredType inferredType = expression.type();
+        if (!inferredType.isCompatibleWith(returnType)) {
+          invalidReturnStatements.add(returnStatement);
+        }
+      }
+      super.visitReturnStatement(returnStatement);
+    }
+
+    @Override
+    public void visitYieldStatement(YieldStatement yieldStatement) {
+      yieldStatements.add(yieldStatement);
+      super.visitYieldStatement(yieldStatement);
+    }
+  }
+}

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

@@ -0,0 +1,27 @@
+<p>Developers can use type hints to specify which type a function is expected to return. These annotations are not enforced at runtime and returning a
+different type might not fail. It is however likely to be unintended and will lead to maintainability issues, if not bugs.</p>
+<p>This rule raises an issue when a function or method returns a value that contradicts its type hint.</p>
+<h2>Noncompliant Code Example</h2>
+<pre>
+def hello() -&gt; str:
+    return 42  # Noncompliant. Function's type hint asks for a string return value
+
+def should_return_a_string(condition) -&gt; str:
+    if condition:
+        return "a string"
+    # Noncompliant. The function returns None if the condition is not met
+</pre>
+<h2>Compliant Solution</h2>
+<pre>
+def hello() -&gt; str:
+    return "Hello"
+
+def should_return_a_string() -&gt; Optional[str]:
+    if condition:
+        return "a string"
+</pre>
+<h2>See</h2>
+<ul>
+  <li> <a href="https://docs.python.org/3/library/typing.html">Python documentation - Support for type hints</a> </li>
+</ul>
+

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

@@ -0,0 +1,16 @@
+{
+  "title": "Function return types should be consistent with their type hint",
+  "type": "CODE_SMELL",
+  "status": "ready",
+  "remediation": {
+    "func": "Constant\/Issue",
+    "constantCost": "5min"
+  },
+  "tags": [
+
+  ],
+  "defaultSeverity": "Major",
+  "ruleSpecification": "RSPEC-5886",
+  "sqKey": "S5886",
+  "scope": "All"
+}

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

@@ -127,6 +127,7 @@
     "S5799",
     "S5806",
     "S5807",
-    "S5828"
+    "S5828",
+    "S5886"
   ]
 }

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

@@ -0,0 +1,31 @@
+/*
+ * SonarQube Python Plugin
+ * Copyright (C) 2011-2020 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.Test;
+import org.sonar.python.checks.utils.PythonCheckVerifier;
+
+public class FunctionReturnTypeCheckTest {
+
+  @Test
+  public void test() {
+    PythonCheckVerifier.verify("src/test/resources/checks/functionReturnType.py", new FunctionReturnTypeCheck());
+  }
+}