SONARPY-525 Rule S5655: Arguments given to functions should be of an expected type

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

@@ -0,0 +1,201 @@
+/*
+ * 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.List;
+import java.util.Optional;
+import java.util.function.Predicate;
+import org.sonar.check.Rule;
+import org.sonar.plugins.python.api.LocationInFile;
+import org.sonar.plugins.python.api.PythonSubscriptionCheck;
+import org.sonar.plugins.python.api.SubscriptionContext;
+import org.sonar.plugins.python.api.symbols.FunctionSymbol;
+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.Name;
+import org.sonar.plugins.python.api.tree.RegularArgument;
+import org.sonar.plugins.python.api.tree.Tree;
+import org.sonar.plugins.python.api.types.BuiltinTypes;
+import org.sonar.plugins.python.api.types.InferredType;
+
+@Rule(key = "S5655")
+public class ArgumentTypeCheck extends PythonSubscriptionCheck {
+
+  @Override
+  public void initialize(Context context) {
+    context.registerSyntaxNodeConsumer(Tree.Kind.CALL_EXPR, ctx -> {
+      CallExpression callExpression = (CallExpression) ctx.syntaxNode();
+      Symbol calleeSymbol = callExpression.calleeSymbol();
+      if (calleeSymbol == null) {
+        return;
+      }
+      if (!calleeSymbol.is(Symbol.Kind.FUNCTION)) {
+        // We might want to support ambiguous symbols for which every definition is a function
+        return;
+      }
+      FunctionSymbol functionSymbol = (FunctionSymbol) calleeSymbol;
+      if (functionSymbol.hasVariadicParameter()) {
+        return;
+      }
+      checkFunctionCall(ctx, callExpression, functionSymbol);
+    });
+  }
+
+  private static void checkFunctionCall(SubscriptionContext ctx, CallExpression callExpression, FunctionSymbol functionSymbol) {
+    boolean isKeyword = false;
+    int firstParameterOffset = firstParameterOffset(functionSymbol);
+    if (firstParameterOffset < 0) {
+      return;
+    }
+    for (int i = 0; i < callExpression.arguments().size(); i++) {
+      Argument argument = callExpression.arguments().get(i);
+      int parameterIndex = i + firstParameterOffset;
+      if (parameterIndex >= functionSymbol.parameters().size()) {
+        // S930 will raise the issue
+        return;
+      }
+      if (argument.is(Tree.Kind.REGULAR_ARGUMENT)) {
+        RegularArgument regularArgument = (RegularArgument) argument;
+        isKeyword |= regularArgument.keywordArgument() != null;
+        boolean shouldReport = isKeyword ? shouldReportKeywordArgument(regularArgument, functionSymbol)
+          : shouldReportPositionalArgument(regularArgument, functionSymbol, parameterIndex);
+        if (shouldReport) {
+          reportIssue(ctx, functionSymbol, regularArgument);
+        }
+      }
+    }
+  }
+
+  private static boolean shouldReportPositionalArgument(RegularArgument regularArgument, FunctionSymbol functionSymbol, int index) {
+    FunctionSymbol.Parameter functionParameter = functionSymbol.parameters().get(index);
+    InferredType argumentType = regularArgument.expression().type();
+    InferredType parameterType = functionParameter.declaredType();
+    if (parameterType.canOnlyBe("object")) {
+      // Avoid FPs as every Python 3 class implicitly inherits from object
+      return false;
+    }
+    return isIncompatibleTypes(argumentType, parameterType);
+  }
+
+  private static boolean shouldReportKeywordArgument(RegularArgument regularArgument, FunctionSymbol functionSymbol) {
+    Name keywordArgument = regularArgument.keywordArgument();
+    InferredType argumentType = regularArgument.expression().type();
+    if (keywordArgument == null) {
+      // Syntax error
+      return false;
+    }
+    String keywordName = keywordArgument.name();
+    Optional<FunctionSymbol.Parameter> correspondingParameter = functionSymbol.parameters().stream().filter(p -> keywordName.equals(p.name())).findFirst();
+    return correspondingParameter
+      .map(c -> {
+        InferredType parameterType = c.declaredType();
+        return (isIncompatibleTypes(argumentType, parameterType));
+      })
+      // If not present: S930 will raise the issue
+      .orElse(false);
+  }
+
+  private static void reportIssue(SubscriptionContext ctx, FunctionSymbol functionSymbol, RegularArgument regularArgument) {
+    PreciseIssue issue = ctx.addIssue(regularArgument, String.format("Change this argument; Function \"%s\" expects a different type", functionSymbol.name()));
+    LocationInFile locationInFile = functionSymbol.definitionLocation();
+    if (locationInFile != null) {
+      issue.secondary(locationInFile, "Function definition");
+    }
+  }
+
+  private static boolean isIncompatibleTypes(InferredType argumentType, InferredType parameterType) {
+    return isNotDuckTypeCompatible(argumentType, parameterType)
+      || (!argumentType.isCompatibleWith(parameterType) && !couldBeDuckTypeCompatible(argumentType, parameterType));
+  }
+
+  private static boolean isNotDuckTypeCompatible(InferredType argumentType, InferredType parameterType) {
+    // Avoid FNs if builtins have incomplete type hierarchy when we are certain of their type
+    String firstBuiltin = matchBuiltinCategory(argumentType::canOnlyBe);
+    String secondBuiltin = matchBuiltinCategory(parameterType::canOnlyBe);
+    return firstBuiltin != null && secondBuiltin != null && !firstBuiltin.equals(secondBuiltin);
+  }
+
+  private static boolean couldBeDuckTypeCompatible(InferredType firstType, InferredType secondType) {
+    // Here we'll return true if we cannot exclude possible duck typing because of unresolved type hierarchies or typing aliases
+    String firstPossibleBuiltin = matchBuiltinCategory(firstType::canBeOrExtend);
+    String secondPossibleBuiltin = matchBuiltinCategory(secondType::canBeOrExtend);
+    return firstPossibleBuiltin != null && firstPossibleBuiltin.equals(secondPossibleBuiltin);
+  }
+
+  public static String matchBuiltinCategory(Predicate<String> predicate) {
+    if (predicate.test(BuiltinTypes.STR)) {
+      return BuiltinTypes.STR;
+    }
+    if (predicate.test(BuiltinTypes.INT)
+      || predicate.test(BuiltinTypes.FLOAT)
+      || predicate.test(BuiltinTypes.COMPLEX)
+      || predicate.test(BuiltinTypes.BOOL)) {
+      return "number";
+    }
+    if (predicate.test(BuiltinTypes.LIST)) {
+      return BuiltinTypes.LIST;
+    }
+    if (predicate.test(BuiltinTypes.SET)) {
+      return BuiltinTypes.SET;
+    }
+    if (predicate.test(BuiltinTypes.DICT)) {
+      return BuiltinTypes.DICT;
+    }
+    if (predicate.test(BuiltinTypes.TUPLE)) {
+      return BuiltinTypes.TUPLE;
+    }
+    return null;
+  }
+
+  /*
+  He were return the offset between parameter position and argument position:
+  0 if there is no implicit first parameter (self, cls, etc...)
+  1 if there is an implicit first parameter
+  -1 if unknown (intent is not clear from function definition)
+   */
+  public static int firstParameterOffset(FunctionSymbol functionSymbol) {
+    List<FunctionSymbol.Parameter> parameters = functionSymbol.parameters();
+    if (parameters.isEmpty()) {
+      return 0;
+    }
+    String firstParamName = parameters.get(0).name();
+    if (firstParamName == null) {
+      // Should never happen
+      return -1;
+    }
+    if (functionSymbol.isInstanceMethod() && firstParamName.equals("self")) {
+      // If first param is not "self", we can't rule out the possibility of a static method missing the "@staticmethod" annotation
+      return 1;
+    }
+    List<String> decoratorNames = functionSymbol.decorators();
+    if (decoratorNames.size() > 1) {
+      // We want to avoid FP if there are many decorators
+      return -1;
+    }
+    if (decoratorNames.size() == 1 && decoratorNames.get(0).endsWith("classmethod")) {
+      return 1;
+    }
+    if (!functionSymbol.isInstanceMethod()) {
+      return 0;
+    }
+    return -1;
+  }
+}

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

@@ -58,6 +58,7 @@ public static Iterable<Class> getChecks() {
       AfterJumpStatementCheck.class,
       AllBranchesAreIdenticalCheck.class,
       ArgumentNumberCheck.class,
+      ArgumentTypeCheck.class,
       BackslashInStringCheck.class,
       BackticksUsageCheck.class,
       BareRaiseInFinallyCheck.class,

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

@@ -0,0 +1,16 @@
+<p>Python does not check the type of arguments provided to functions. However builtin functions and methods expect a specific type for each parameter.
+Providing an argument of the wrong type will make your program fail.</p>
+<p>This rule raises an issue when a builtin function is called with an argument of the wrong type.</p>
+<h2>Noncompliant Code Example</h2>
+<pre>
+round("42.3")  # Noncompliant
+</pre>
+<h2>Compliant Solution</h2>
+<pre>
+round(42.3)
+</pre>
+<h2>See</h2>
+<ul>
+  <li> <a href="https://docs.python.org/3/library/functions.html#built-in-funcs">Python documentation - builtins</a> </li>
+</ul>
+

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

@@ -0,0 +1,12 @@
+{
+  "title": "Arguments given to functions should be of an expected type",
+  "type": "BUG",
+  "status": "ready",
+  "tags": [
+
+  ],
+  "defaultSeverity": "Blocker",
+  "ruleSpecification": "RSPEC-5655",
+  "sqKey": "S5655",
+  "scope": "All"
+}

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

@@ -86,6 +86,7 @@
     "S5527",
     "S5603",
     "S5632",
+    "S5655",
     "S5685",
     "S5704",
     "S5706",

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