SONARPY-1881 S5756: NonCallableCalledCheck fix FP when a function parameter has reassignment to non callable type (#1812)

Author: maksim-grebeniuk-sonarsource

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

@@ -272,3 +272,31 @@ def using_isinstance_with_runtime_type():
         my_non_callable() # Noncompliant
     ...
     my_non_callable()  # Noncompliant
+
+def reassigned_param(a, param):
+    param = 1
+    if a:
+        param = [1,2,3]
+    param() # Noncompliant
+
+def conditionaly_reassigned_param(a, param):
+    if a:
+        param = [1,2,3]
+    param()
+
+def reassigned_param_try_except(a, param):
+    try:
+        param = 1
+        if a:
+            param = [1,2,3]
+        param() # FN
+    except:
+        ...
+
+def conditionally_reassigned_param_try_except(a, param):
+    try:
+        if a:
+            param = [1,2,3]
+        param()
+    except:
+        ...

python-frontend/src/main/java/org/sonar/python/semantic/v2/TypeInferenceV2.java

@@ -41,6 +41,7 @@
 import org.sonar.python.semantic.v2.types.TrivialTypeInferenceVisitor;
 import org.sonar.python.semantic.v2.types.TryStatementVisitor;
 import org.sonar.python.tree.TreeUtils;
+import org.sonar.python.types.v2.PythonType;
 
 public class TypeInferenceV2 {
 
@@ -130,11 +131,17 @@ private static void inferTypesAndMemberAccessSymbols(Tree scopeTree,
   }
 
   private static void flowSensitiveTypeInference(ControlFlowGraph cfg, Set<SymbolV2> trackedVars, PropagationVisitor propagationVisitor) {
+    // TODO: infer parameter type based on type hint or default value assignement
+    var parameterTypes = trackedVars
+      .stream()
+      .filter(s -> s.usages().stream().map(UsageV2::kind).anyMatch(UsageV2.Kind.PARAMETER::equals))
+      .collect(Collectors.toMap(SymbolV2::name, v -> PythonType.UNKNOWN));
+
     FlowSensitiveTypeInference flowSensitiveTypeInference = new FlowSensitiveTypeInference(
       trackedVars,
       propagationVisitor.assignmentsByAssignmentStatement(),
       propagationVisitor.definitionsByDefinitionStatement(),
-      Map.of());
+      parameterTypes);
 
     flowSensitiveTypeInference.compute(cfg);
     flowSensitiveTypeInference.compute(cfg);

python-frontend/src/main/java/org/sonar/python/semantic/v2/types/FlowSensitiveTypeInference.java

@@ -19,6 +19,7 @@
  */
 package org.sonar.python.semantic.v2.types;
 
+import java.util.HashSet;
 import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
@@ -28,6 +29,7 @@
 import org.sonar.plugins.python.api.tree.Expression;
 import org.sonar.plugins.python.api.tree.FunctionDef;
 import org.sonar.plugins.python.api.tree.Name;
+import org.sonar.plugins.python.api.tree.Parameter;
 import org.sonar.plugins.python.api.tree.Statement;
 import org.sonar.plugins.python.api.tree.Tree;
 import org.sonar.python.cfg.fixpoint.ForwardAnalysis;
@@ -57,11 +59,7 @@ public FlowSensitiveTypeInference(
   public ProgramState initialState() {
     TypeInferenceProgramState initialState = new TypeInferenceProgramState();
     for (SymbolV2 variable : trackedVars) {
-      var pythonTypeSet = Optional.of(variable.name())
-        .map(parameterTypesByName::get)
-        .map(Set::of)
-        .orElseGet(Set::of);
-      initialState.setTypes(variable, pythonTypeSet);
+      initialState.setTypes(variable, Set.of());
     }
     return initialState;
   }
@@ -88,12 +86,26 @@ public void updateProgramState(Tree element, ProgramState programState) {
       }
     } else if (element instanceof FunctionDef functionDef) {
       handleDefinition(functionDef, state);
+    } else if (element instanceof Parameter parameter) {
+      handleParameter(parameter, state);
     } else {
       // Here we should run "isinstance" visitor when we handle declared types, to avoid FPs when type guard checks are made
       updateTree(element, state);
     }
   }
 
+  private void handleParameter(Parameter parameter, TypeInferenceProgramState state) {
+    var name = parameter.name();
+
+    if (name == null || !trackedVars.contains(name.symbolV2())) {
+      return;
+    }
+
+    var type = parameterTypesByName.getOrDefault(name.name(), PythonType.UNKNOWN);
+    state.setTypes(name.symbolV2(), new HashSet<>(Set.of(type)));
+    updateTree(name, state);
+  }
+
   private static void updateTree(Tree tree, TypeInferenceProgramState state) {
     tree.accept(new ProgramStateTypeInferenceVisitor(state));
   }

python-frontend/src/main/java/org/sonar/python/semantic/v2/types/ParameterDefinition.java

@@ -0,0 +1,42 @@
+/*
+ * SonarQube Python Plugin
+ * Copyright (C) 2011-2024 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.semantic.v2.types;
+
+import org.sonar.plugins.python.api.tree.Name;
+import org.sonar.python.semantic.v2.SymbolV2;
+import org.sonar.python.types.v2.PythonType;
+
+public class ParameterDefinition extends Propagation {
+
+  protected ParameterDefinition(SymbolV2 lhsSymbol, Name lhsName) {
+    super(lhsSymbol, lhsName);
+  }
+
+  @Override
+  public Name lhsName() {
+    return lhsName;
+  }
+
+  @Override
+  public PythonType rhsType() {
+//    TODO: process type calculation based on default value assignment or type hint
+    return PythonType.UNKNOWN;
+  }
+}

python-frontend/src/main/java/org/sonar/python/semantic/v2/types/PropagationVisitor.java

@@ -24,6 +24,7 @@
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
 import java.util.Set;
 import org.sonar.plugins.python.api.tree.AnnotatedAssignment;
 import org.sonar.plugins.python.api.tree.AssignmentStatement;
@@ -32,6 +33,7 @@
 import org.sonar.plugins.python.api.tree.Expression;
 import org.sonar.plugins.python.api.tree.FunctionDef;
 import org.sonar.plugins.python.api.tree.Name;
+import org.sonar.plugins.python.api.tree.Parameter;
 import org.sonar.plugins.python.api.tree.Statement;
 import org.sonar.python.semantic.v2.SymbolV2;
 
@@ -69,6 +71,16 @@ public void visitFunctionDef(FunctionDef functionDef) {
     propagationsByLhs.computeIfAbsent(symbol, s -> new HashSet<>()).add(definition);
   }
 
+  @Override
+  public void visitParameter(Parameter parameter) {
+    Optional.ofNullable(parameter.name())
+      .ifPresent(name -> {
+        var symbol = name.symbolV2();
+        var parametedDefinition = new ParameterDefinition(symbol, name);
+        propagationsByLhs.computeIfAbsent(symbol, s -> new HashSet<>()).add(parametedDefinition);
+      });
+  }
+
   @Override
   public void visitAssignmentStatement(AssignmentStatement assignmentStatement) {
     super.visitAssignmentStatement(assignmentStatement);

python-frontend/src/test/java/org/sonar/python/semantic/v2/TypeInferenceV2Test.java

@@ -1226,6 +1226,54 @@ def f():
       .isSameAs(childClassType);
   }
 
+  @Test
+  void inferReassignedParameterType() {
+    var root = inferTypes("""
+      def reassigned_param(a, param):
+          param = 1
+          if a:
+              param = "1"
+          param()
+      """);
+
+    var paramType = TreeUtils.firstChild(root, CallExpression.class::isInstance)
+      .map(CallExpression.class::cast)
+      .map(CallExpression::callee)
+      .map(Expression::typeV2)
+      .map(UnionType.class::cast)
+      .get();
+
+
+    Assertions.assertThat(paramType).isInstanceOf(UnionType.class);
+    Assertions.assertThat(paramType.candidates())
+      .hasSize(2);
+
+    var candidatesUnwrappedType = paramType.candidates().stream()
+      .map(PythonType::unwrappedType)
+      .toList();
+
+    Assertions.assertThat(candidatesUnwrappedType)
+      .contains(INT_TYPE, STR_TYPE);
+  }
+
+  @Test
+  void inferConditionallyReassignedParameterType() {
+    var root = inferTypes("""
+      def reassigned_param(a, param):
+          if a:
+              param = "1"
+          param()
+      """);
+
+    var paramType = TreeUtils.firstChild(root, CallExpression.class::isInstance)
+      .map(CallExpression.class::cast)
+      .map(CallExpression::callee)
+      .map(Expression::typeV2)
+      .get();
+
+    Assertions.assertThat(paramType).isSameAs(PythonType.UNKNOWN);
+  }
+
   private static FileInput inferTypes(String lines) {
     return inferTypes(lines, new HashMap<>());
   }