SONARPY-2204 Migrate S5795 IdentityComparisonWithCachedTypesCheck to the new type model

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

@@ -19,24 +19,25 @@
  */
 package org.sonar.python.checks;
 
-import java.util.HashSet;
 import java.util.List;
-import java.util.Set;
 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.symbols.Usage;
+import org.sonar.plugins.python.api.quickfix.PythonQuickFix;
 import org.sonar.plugins.python.api.tree.CallExpression;
 import org.sonar.plugins.python.api.tree.Expression;
 import org.sonar.plugins.python.api.tree.IsExpression;
 import org.sonar.plugins.python.api.tree.Name;
 import org.sonar.plugins.python.api.tree.Tree;
-import org.sonar.plugins.python.api.types.InferredType;
-import org.sonar.plugins.python.api.quickfix.PythonQuickFix;
-import org.sonar.python.checks.utils.CheckUtils;
+import org.sonar.plugins.python.api.types.BuiltinTypes;
 import org.sonar.python.quickfix.TextEditUtils;
+import org.sonar.python.semantic.v2.SymbolV2;
+import org.sonar.python.semantic.v2.UsageV2;
 import org.sonar.python.tree.TreeUtils;
+import org.sonar.python.types.v2.PythonType;
+import org.sonar.python.types.v2.TriBool;
+import org.sonar.python.types.v2.TypeCheckBuilder;
+import org.sonar.python.types.v2.TypeChecker;
 
 import static java.util.Arrays.asList;
 
@@ -49,12 +50,40 @@ public class IdentityComparisonWithCachedTypesCheck extends PythonSubscriptionCh
   public static final String IS_QUICK_FIX_MESSAGE = "Replace with \"==\"";
   public static final String IS_NOT_QUICK_FIX_MESSAGE = "Replace with \"!=\"";
 
+  /**
+   * Fully qualified names of constructors and functions that are guaranteed to create fresh objects with
+   * references not shared anywhere else.
+   * <p>
+   * If a reference arises from a call to one of these functions, and it does not escape anywhere else, then an
+   * <code>is</code>-comparison with such a reference will always return <code>False</code>.
+   * <p>
+   * Note that these are fully qualified names of (value-level) expressions, not types.
+   */
+  private static final List<String> FQNS_CONSTRUCTORS_RETURNING_UNIQUE_REF =
+    asList("frozenset", "bytes", "int", "float", "str", "tuple", "hash");
+
+  /**
+   * Names of types that usually should not be compared with <code>is</code>.
+   * <p>
+   * Note that these are names of types, not `fqn`s of expressions.
+   */
+  private static final List<String> NAMES_OF_TYPES_UNSUITABLE_FOR_COMPARISON =
+    asList("frozenset", "bytes", "int", "float", "tuple");
+
+  private TypeChecker typeChecker;
+  private TypeCheckBuilder isNoneTypeChecker;
+
   @Override
   public void initialize(Context context) {
-    context.registerSyntaxNodeConsumer(Tree.Kind.IS, IdentityComparisonWithCachedTypesCheck::checkIsComparison);
+    context.registerSyntaxNodeConsumer(Tree.Kind.FILE_INPUT, ctx -> {
+      typeChecker = ctx.typeChecker();
+      isNoneTypeChecker = ctx.typeChecker().typeCheckBuilder().isBuiltinWithName(BuiltinTypes.NONE_TYPE);
+    });
+
+    context.registerSyntaxNodeConsumer(Tree.Kind.IS, this::checkIsComparison);
   }
 
-  private static void checkIsComparison(SubscriptionContext ctx) {
+  private void checkIsComparison(SubscriptionContext ctx) {
     IsExpression isExpr = (IsExpression) ctx.syntaxNode();
 
     // Comparison to none is checked by S5727
@@ -83,75 +112,71 @@ private static void checkIsComparison(SubscriptionContext ctx) {
     }
   }
 
-  private static boolean isComparisonToNone(IsExpression isExpr) {
-    return CheckUtils.isNone(isExpr.leftOperand().type()) || CheckUtils.isNone(isExpr.rightOperand().type());
+  private boolean isComparisonToNone(IsExpression isExpr) {
+    return isNone(isExpr.leftOperand().typeV2()) || isNone(isExpr.rightOperand().typeV2());
   }
 
-  /**
-   * Fully qualified names of constructors and functions that would are guaranteed to create fresh objects with
-   * references not shared anywhere else.
-   *
-   * If a reference arises from an call to one of these functions, and it does not escape anywhere else, then an
-   * <code>is</code>-comparison with such a reference will always return <code>False</code>.
-   *
-   * Note that these are fully qualified names of (value-level) expressions, not types.
-   */
-  private static final Set<String> FQNS_CONSTRUCTORS_RETURNING_UNIQUE_REF = new HashSet<>(
-    asList("frozenset", "bytes", "int", "float", "str", "tuple", "hash"));
-
-  /**
-   * Names of types that usually should not be compared with <code>is</code>.
-   *
-   * Note that these are names of types, not `fqn`s of expressions.
-   */
-  private static final Set<String> NAMES_OF_TYPES_UNSUITABLE_FOR_COMPARISON = new HashSet<>(
-    asList("frozenset", "bytes", "int", "float", "tuple"));
+  private boolean isNone(PythonType type) {
+    return isNoneTypeChecker.check(type) == TriBool.TRUE;
+  }
 
   /**
    * Checks whether an expression is either of a type that should generally be compared with <code>==</code>, or
    * whether it holds a non-escaping reference that stems from a function that guarantees that the
    * returned reference is not shared anywhere.
    */
-  private static boolean isUnsuitableOperand(Expression expr) {
-    InferredType tpe = expr.type();
-    if (isUnsuitableType(tpe)) {
-      if (expr.is(Tree.Kind.NAME)) {
-        Symbol symb = ((Name) expr).symbol();
-        // Impossible to cover. If the type could be inferred, then the `symb` cannot be null.
-        return symb != null && !isVariableThatCanEscape(symb);
+  private boolean isUnsuitableOperand(Expression expr) {
+    PythonType type = expr.typeV2();
+    if (isUnsuitableType(type)) {
+      if (expr instanceof Name name) {
+        SymbolV2 symbol = name.symbolV2();
+        return symbol != null && !isVariableThatCanEscape(symbol);
       } else {
         return true;
       }
     }
     if (expr.is(Tree.Kind.STRING_LITERAL)) {
       return true;
-    } else if (expr.is(Tree.Kind.CALL_EXPR)) {
-      Symbol calleeSymbol = ((CallExpression) expr).calleeSymbol();
-      if (calleeSymbol != null) {
-        String calleeFqn = calleeSymbol.fullyQualifiedName();
-        return FQNS_CONSTRUCTORS_RETURNING_UNIQUE_REF.contains(calleeFqn);
+    } else if (expr instanceof CallExpression callExpr) {
+      PythonType calleeType = callExpr.callee().typeV2();
+      return isConstructorReturningUniqueRef(calleeType);
+    }
+    return false;
+  }
+
+  private boolean isUnsuitableType(PythonType type) {
+    for (String builtinName : NAMES_OF_TYPES_UNSUITABLE_FOR_COMPARISON) {
+      TypeCheckBuilder builtinWithNameChecker = typeChecker.typeCheckBuilder().isBuiltinWithName(builtinName);
+      if (builtinWithNameChecker.check(type) == TriBool.TRUE) {
+        return true;
       }
     }
     return false;
   }
 
-  private static boolean isUnsuitableType(InferredType tpe) {
-    return NAMES_OF_TYPES_UNSUITABLE_FOR_COMPARISON.stream().anyMatch(tpe::canOnlyBe);
+  private boolean isConstructorReturningUniqueRef(PythonType type) {
+    for (String constructorName : FQNS_CONSTRUCTORS_RETURNING_UNIQUE_REF) {
+      TypeCheckBuilder constructorChecker = typeChecker.typeCheckBuilder().isTypeWithName(constructorName);
+      if (constructorChecker.check(type) == TriBool.TRUE) {
+        return true;
+      }
+    }
+    return false;
   }
 
   /**
    * Checks whether a variable is used anywhere except at the <code>is</code>-comparison site and in the defining
    * assignment. In such cases, we say that the reference to the value of the variable could have escaped,
    * and don't raise an issue. We do not check whether it could actually arrive in the other operand.
    */
-  private static boolean isVariableThatCanEscape(Symbol symb) {
-    List<Usage> usages = symb.usages();
+  private static boolean isVariableThatCanEscape(SymbolV2 symb) {
+    List<UsageV2> usages = symb.usages();
     if (usages.size() > 2) {
       // Check whether there are any usages except assignments and `is`-comparisons.
       // (Assignments override, they don't let the reference escape. Comparisons with `is` also don't let it escape,
       // because they only compare, they don't copy the reference anywhere.)
       return usages.stream().anyMatch(u -> !u.isBindingUsage() &&
-        TreeUtils.firstAncestorOfKind(u.tree(), Tree.Kind.IS) == null);
+                                           TreeUtils.firstAncestorOfKind(u.tree(), Tree.Kind.IS) == null);
     }
     // Must be definition and usage within <code>is</code>-comparison itself.
     return false;

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

@@ -18,6 +18,9 @@ def literal_comparison(param):
     (4, 5) is param # Noncompliant {{Replace this "is" operator with "=="; identity operator is not reliable here.}}
     #      ^^
 
+    param is -1 # Noncompliant
+    param is 1 + 1 # Noncompliant
+
 def literal_comparison_compliant(param):
     param is param # ok from the point of this rule
     param is someUnknownFunction(42) # ok
@@ -149,3 +152,7 @@ def comparison_to_none():
     "str" is None
     123 is None
     None is "str"
+
+def potential_null_symbols():
+    type(arr) is tuple
+    some_thing is other_thing

python-frontend/src/main/java/org/sonar/plugins/python/api/tree/Name.java

@@ -19,6 +19,7 @@
  */
 package org.sonar.plugins.python.api.tree;
 
+import javax.annotation.CheckForNull;
 import org.sonar.api.Beta;
 import org.sonar.python.semantic.v2.SymbolV2;
 
@@ -33,5 +34,6 @@ public interface Name extends Expression, HasSymbol {
   boolean isVariable();
 
   @Beta
+  @CheckForNull
   SymbolV2 symbolV2();
 }

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

@@ -54,9 +54,12 @@
 import org.sonar.plugins.python.api.tree.SetLiteral;
 import org.sonar.plugins.python.api.tree.StringLiteral;
 import org.sonar.plugins.python.api.tree.SubscriptionExpression;
+import org.sonar.plugins.python.api.tree.Token;
 import org.sonar.plugins.python.api.tree.Tree;
 import org.sonar.plugins.python.api.tree.Tuple;
 import org.sonar.plugins.python.api.tree.TypeAnnotation;
+import org.sonar.plugins.python.api.tree.UnaryExpression;
+import org.sonar.plugins.python.api.types.BuiltinTypes;
 import org.sonar.python.semantic.v2.ClassTypeBuilder;
 import org.sonar.python.semantic.v2.FunctionTypeBuilder;
 import org.sonar.python.semantic.v2.SymbolV2;
@@ -72,12 +75,15 @@
 import org.sonar.python.tree.SetLiteralImpl;
 import org.sonar.python.tree.StringLiteralImpl;
 import org.sonar.python.tree.TupleImpl;
+import org.sonar.python.tree.UnaryExpressionImpl;
 import org.sonar.python.types.v2.ClassType;
 import org.sonar.python.types.v2.FunctionType;
 import org.sonar.python.types.v2.Member;
 import org.sonar.python.types.v2.ModuleType;
 import org.sonar.python.types.v2.ObjectType;
 import org.sonar.python.types.v2.PythonType;
+import org.sonar.python.types.v2.TriBool;
+import org.sonar.python.types.v2.TypeCheckBuilder;
 import org.sonar.python.types.v2.TypeOrigin;
 import org.sonar.python.types.v2.TypeSource;
 import org.sonar.python.types.v2.UnionType;
@@ -152,6 +158,36 @@ public void visitNumericLiteral(NumericLiteral numericLiteral) {
     numericLiteralImpl.typeV2(new ObjectType(pythonType, new ArrayList<>(), new ArrayList<>()));
   }
 
+  @Override
+  public void visitUnaryExpression(UnaryExpression unaryExpr) {
+    super.visitUnaryExpression(unaryExpr);
+
+    var builtins = projectLevelTypeTable.getBuiltinsModule();
+    Token operator = unaryExpr.operator();
+    PythonType exprType = switch (operator.value()) {
+      case "~" -> builtins.resolveMember(BuiltinTypes.INT).orElse(PythonType.UNKNOWN);
+      case "not" -> builtins.resolveMember(BuiltinTypes.BOOL).orElse(PythonType.UNKNOWN);
+      case "+", "-" -> getTypeWhenUnaryPlusMinus(unaryExpr);
+      default -> unaryExpr.expression().typeV2();
+    };
+
+    if (unaryExpr instanceof UnaryExpressionImpl unaryExprImpl) {
+      unaryExprImpl.typeV2(exprType);
+    }
+  }
+
+  private PythonType getTypeWhenUnaryPlusMinus(UnaryExpression unaryExpr) {
+    var builtins = projectLevelTypeTable.getBuiltinsModule();
+    var isBooleanTypeCheck = new TypeCheckBuilder(projectLevelTypeTable).isBuiltinWithName(BuiltinTypes.BOOL);
+    var innerExprType = unaryExpr.expression().typeV2();
+
+    if (isBooleanTypeCheck.check(innerExprType) == TriBool.TRUE) {
+      return builtins.resolveMember(BuiltinTypes.INT).orElse(PythonType.UNKNOWN);
+    } else {
+      return innerExprType;
+    }
+  }
+
   @Override
   public void visitNone(NoneExpression noneExpression) {
     var builtins = this.projectLevelTypeTable.getBuiltinsModule();

python-frontend/src/main/java/org/sonar/python/tree/UnaryExpressionImpl.java

@@ -31,6 +31,7 @@
 import org.sonar.plugins.python.api.tree.UnaryExpression;
 import org.sonar.plugins.python.api.types.InferredType;
 import org.sonar.python.types.InferredTypes;
+import org.sonar.python.types.v2.PythonType;
 
 public class UnaryExpressionImpl extends PyTree implements UnaryExpression {
 
@@ -39,6 +40,7 @@ public class UnaryExpressionImpl extends PyTree implements UnaryExpression {
   private final Kind kind;
   private final Token operator;
   private final Expression expression;
+  private PythonType type = PythonType.UNKNOWN;
 
   private static Map<String, Kind> kindsByOperator() {
     Map<String, Kind> map = new HashMap<>();
@@ -90,4 +92,14 @@ public InferredType type() {
     }
     return InferredTypes.anyType();
   }
+
+  public UnaryExpression typeV2(PythonType type) {
+    this.type = type;
+    return this;
+  }
+
+  @Override
+  public PythonType typeV2() {
+    return type;
+  }
 }

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

@@ -24,14 +24,14 @@
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
-import org.sonar.python.semantic.v2.ProjectLevelTypeTable;
+import org.sonar.python.semantic.v2.TypeTable;
 
 public class TypeCheckBuilder {
 
-  ProjectLevelTypeTable projectLevelTypeTable;
+  TypeTable projectLevelTypeTable;
   List<TypePredicate> predicates = new ArrayList<>();
 
-  public TypeCheckBuilder(ProjectLevelTypeTable projectLevelTypeTable) {
+  public TypeCheckBuilder(TypeTable projectLevelTypeTable) {
     this.projectLevelTypeTable = projectLevelTypeTable;
   }