SONARPY-2288 Fix FP on S5795 when checking if type is unsuitable (#2116)

SonarSource · Oct 30, 2024 · 01f6048 · 01f6048
1 parent fc16c0e
commit 01f6048
Show file tree

Hide file tree

Showing 4 changed files with 55 additions and 2 deletions.
diff --git a/...-checks/src/main/java/org/sonar/python/checks/IdentityComparisonWithCachedTypesCheck.java b/...-checks/src/main/java/org/sonar/python/checks/IdentityComparisonWithCachedTypesCheck.java
@@ -146,7 +146,7 @@ private boolean isUnsuitableOperand(Expression expr) {
 
   private boolean isUnsuitableType(PythonType type) {
     for (String builtinName : NAMES_OF_TYPES_UNSUITABLE_FOR_COMPARISON) {
-      TypeCheckBuilder builtinWithNameChecker = typeChecker.typeCheckBuilder().isBuiltinWithName(builtinName);
+      TypeCheckBuilder builtinWithNameChecker = typeChecker.typeCheckBuilder().isInstance().isBuiltinWithName(builtinName);
       if (builtinWithNameChecker.check(type) == TriBool.TRUE) {
         return true;
       }

diff --git a/python-checks/src/test/resources/checks/identityComparisonWithCachedTypes.py b/python-checks/src/test/resources/checks/identityComparisonWithCachedTypes.py
@@ -156,3 +156,7 @@ def comparison_to_none():
 def potential_null_symbols():
     type(arr) is tuple
     some_thing is other_thing
+
+def comparison_to_class(arg):
+    import typing
+    arg is not typing.Tuple
diff --git a/python-frontend/src/main/java/org/sonar/python/types/v2/TypeCheckBuilder.java b/python-frontend/src/main/java/org/sonar/python/types/v2/TypeCheckBuilder.java
@@ -61,6 +61,11 @@ public TypeCheckBuilder isBuiltinWithName(String name) {
     return this;
   }
 
+  public TypeCheckBuilder isInstance() {
+    predicates.add(new IsInstancePredicate());
+    return this;
+  }
+
   public TriBool check(PythonType pythonType) {
     TriBool result = TriBool.TRUE;
     for (TypePredicate predicate : predicates) {
@@ -161,7 +166,7 @@ public TriBool test(PythonType pythonType) {
     }
   }
 
-  record IsInstanceOfPredicate(PythonType expectedType)  implements TypePredicate {
+  record IsInstanceOfPredicate(PythonType expectedType) implements TypePredicate {
 
     @Override
     public TriBool test(PythonType pythonType) {
@@ -246,6 +251,22 @@ private static Set<PythonType> collectTypes(PythonType type) {
       }
       return result;
     }
+
+  }
+
+  record IsInstancePredicate() implements TypePredicate {
+
+    @Override
+    public TriBool test(PythonType pythonType) {
+      Set<PythonType> candiates = Set.of(pythonType);
+      if (pythonType instanceof UnionType unionType) {
+        candiates = unionType.candidates();
+      }
+      if (candiates.stream().allMatch(ObjectType.class::isInstance)) {
+        return TriBool.TRUE;
+      }
+      return TriBool.FALSE;
+    }
   }
 
   private static boolean containsUnknown(Set<PythonType> types) {

diff --git a/python-frontend/src/test/java/org/sonar/python/types/v2/TypeCheckerTest.java b/python-frontend/src/test/java/org/sonar/python/types/v2/TypeCheckerTest.java
@@ -294,4 +294,32 @@ class B: pass
     assertThat(localTypeChecker.typeCheckBuilder().isTypeOrInstanceWithName("my_package.mod.B").check(aType)).isEqualTo(TriBool.FALSE);
     assertThat(localTypeChecker.typeCheckBuilder().isTypeOrInstanceWithName("my_package.unknown.A").check(aType)).isEqualTo(TriBool.UNKNOWN);
   }
+
+
+  @Test
+  void testIsInstance() {
+    var symbolTable = ProjectLevelSymbolTable.empty();
+    var table = new ProjectLevelTypeTable(symbolTable);
+    var builder = new TypeCheckBuilder(table).isInstance();
+
+    var intType = table.getType("int");
+    var floatType = table.getType("float");
+
+    var unionOfAllObjects = UnionType.or(new ObjectType(intType), new ObjectType(floatType));
+    var unionOfSomeObjects = UnionType.or(new ObjectType(intType), floatType);
+    var unionOfNoObjects = UnionType.or(intType, floatType);
+
+    assertThat(builder.check(new ObjectType(intType)))
+      .isEqualTo(TriBool.TRUE);
+
+    assertThat(builder.check(intType))
+      .isEqualTo(TriBool.FALSE);
+
+    assertThat(builder.check(unionOfAllObjects))
+      .isEqualTo(TriBool.TRUE);
+    assertThat(builder.check(unionOfSomeObjects))
+      .isEqualTo(TriBool.FALSE);
+    assertThat(builder.check(unionOfNoObjects))
+      .isEqualTo(TriBool.FALSE);
+  }
 }