SONARPY-2017 Avoid type-related FPs in case of isinstance checks (#1885)

Author: maksim-grebeniuk-sonarsource

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

@@ -102,7 +102,7 @@ private void inferTypesAndMemberAccessSymbols(FunctionDef functionDef) {
   }
 
 
-  private static void inferTypesAndMemberAccessSymbols(Tree scopeTree,
+  private void inferTypesAndMemberAccessSymbols(Tree scopeTree,
     StatementList statements,
     Set<SymbolV2> declaredVariables,
     Set<Name> annotatedParameterNames,
@@ -130,7 +130,7 @@ private static void inferTypesAndMemberAccessSymbols(Tree scopeTree,
     }
   }
 
-  private static void flowSensitiveTypeInference(ControlFlowGraph cfg, Set<SymbolV2> trackedVars, PropagationVisitor propagationVisitor) {
+  private void flowSensitiveTypeInference(ControlFlowGraph cfg, Set<SymbolV2> trackedVars, PropagationVisitor propagationVisitor) {
     // TODO: infer parameter type based on default value assignement
     var parameterTypes = trackedVars
       .stream()
@@ -140,6 +140,7 @@ private static void flowSensitiveTypeInference(ControlFlowGraph cfg, Set<SymbolV
       .collect(Collectors.toMap(SymbolV2::name, TypeInferenceV2::getParameterType));
 
     FlowSensitiveTypeInference flowSensitiveTypeInference = new FlowSensitiveTypeInference(
+      projectLevelTypeTable,
       trackedVars,
       propagationVisitor.assignmentsByAssignmentStatement(),
       propagationVisitor.definitionsByDefinitionStatement(),

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

@@ -36,6 +36,7 @@
 import org.sonar.plugins.python.api.tree.Tree;
 import org.sonar.python.cfg.fixpoint.ForwardAnalysis;
 import org.sonar.python.cfg.fixpoint.ProgramState;
+import org.sonar.python.semantic.v2.ProjectLevelTypeTable;
 import org.sonar.python.semantic.v2.SymbolV2;
 import org.sonar.python.types.v2.PythonType;
 
@@ -44,9 +45,10 @@ public class FlowSensitiveTypeInference extends ForwardAnalysis {
   private final Map<Statement, Assignment> assignmentsByAssignmentStatement;
   private final Map<Statement, Set<Definition>> definitionsByDefinitionStatement;
   private final Map<String, PythonType> parameterTypesByName;
+  private final IsInstanceVisitor isInstanceVisitor;
 
   public FlowSensitiveTypeInference(
-    Set<SymbolV2> trackedVars,
+    ProjectLevelTypeTable projectLevelTypeTable, Set<SymbolV2> trackedVars,
     Map<Statement, Assignment> assignmentsByAssignmentStatement,
     Map<Statement, Set<Definition>> definitionsByDefinitionStatement,
     Map<String, PythonType> parameterTypesByName
@@ -55,6 +57,7 @@ public FlowSensitiveTypeInference(
     this.assignmentsByAssignmentStatement = assignmentsByAssignmentStatement;
     this.definitionsByDefinitionStatement = definitionsByDefinitionStatement;
     this.parameterTypesByName = parameterTypesByName;
+    this.isInstanceVisitor = new IsInstanceVisitor(projectLevelTypeTable);
   }
 
   @Override
@@ -95,7 +98,8 @@ public void updateProgramState(Tree element, ProgramState programState) {
     } else if (isForLoopAssignment(element)) {
       handleLoopAssignment(element, state);
     } else {
-      // Here we should run "isinstance" visitor when we handle declared types, to avoid FPs when type guard checks are made
+      isInstanceVisitor.setState(state);
+      element.accept(isInstanceVisitor);
       updateTree(element, state);
     }
   }

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

@@ -0,0 +1,68 @@
+/*
+ * 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 java.util.Set;
+import javax.annotation.CheckForNull;
+import org.sonar.plugins.python.api.tree.BaseTreeVisitor;
+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.python.semantic.v2.ProjectLevelTypeTable;
+import org.sonar.python.semantic.v2.SymbolV2;
+import org.sonar.python.types.v2.PythonType;
+import org.sonar.python.types.v2.TypeSource;
+
+public class IsInstanceVisitor extends BaseTreeVisitor {
+  private final PythonType isInstanceFunctionType;
+  private TypeInferenceProgramState state;
+
+  public IsInstanceVisitor(ProjectLevelTypeTable projectLevelTypeTable) {
+    isInstanceFunctionType = projectLevelTypeTable.getType("isinstance");
+  }
+
+  public void setState(TypeInferenceProgramState state) {
+    this.state = state;
+  }
+
+  @Override
+  public void visitCallExpression(CallExpression callExpression) {
+    if (callExpression.callee().typeV2() == isInstanceFunctionType && callExpression.arguments().size() == 2) {
+      var firstArgumentSymbol = getFirstArgumentSymbol(callExpression);
+      if (firstArgumentSymbol != null) {
+        state.setTypes(firstArgumentSymbol, Set.of(PythonType.UNKNOWN));
+      }
+    }
+    super.visitCallExpression(callExpression);
+  }
+
+  @CheckForNull
+  private SymbolV2 getFirstArgumentSymbol(CallExpression callExpression) {
+    var argument = callExpression.arguments().get(0);
+    if (argument instanceof RegularArgument regularArgument
+        && regularArgument.expression() instanceof Name variableName
+        && state.getTypes(variableName.symbolV2()).stream()
+          .anyMatch(type -> type != PythonType.UNKNOWN && type.typeSource() == TypeSource.TYPE_HINT)) {
+      return variableName.symbolV2();
+    }
+    return null;
+  }
+}

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

@@ -1843,6 +1843,51 @@ void basic_imported_symbols() {
     assertThat(((UnionType) mathModule.resolveMember("acos").get()).candidates()).allMatch(FunctionType.class::isInstance);
   }
 
+  @Test
+  void isInstanceTests() {
+    var xType = lastExpression("""
+      def foo(x: int):
+        if isinstance(x, Foo):
+          ...
+        x
+      """).typeV2();
+    Assertions.assertThat(xType).isSameAs(PythonType.UNKNOWN);
+
+    xType = lastExpression("""
+      def foo(x: int):
+        if isinstance(x):
+          ...
+        x
+      """).typeV2();
+    Assertions.assertThat(xType.unwrappedType()).isSameAs(INT_TYPE);
+
+    xType = lastExpression("""
+      def foo(x: int):
+        if isinstance(x.b, Foo):
+          ...
+        x
+      """).typeV2();
+    Assertions.assertThat(xType.unwrappedType()).isSameAs(INT_TYPE);
+
+    xType = lastExpression("""
+      def foo():
+        x = 10
+        if isinstance(x, Foo):
+          ...
+        x
+      """).typeV2();
+    Assertions.assertThat(xType.unwrappedType()).isSameAs(INT_TYPE);
+
+    xType = lastExpression("""
+      def foo(x: list):
+        if isinstance(**x, Foo):
+          ...
+        x
+      """).typeV2();
+
+    Assertions.assertThat(xType.unwrappedType()).isSameAs(LIST_TYPE);
+  }
+
   private static FileInput inferTypes(String lines) {
     return inferTypes(lines, PROJECT_LEVEL_TYPE_TABLE);
   }