SONARPY-1800 NonCallableCallCheck migration: Use the display name of the new type model (#1777)

guillaume-dequenne · guillaume-dequenne · commit e7c6574cae6e · 2024-05-21T17:09:08.000+02:00
diff --git a/python-checks/src/main/java/org/sonar/python/checks/NonCallableCalledCheck.java b/python-checks/src/main/java/org/sonar/python/checks/NonCallableCalledCheck.java
@@ -27,7 +27,6 @@
 import org.sonar.plugins.python.api.tree.Expression;
 import org.sonar.plugins.python.api.tree.Tree;
 import org.sonar.plugins.python.api.types.InferredType;
-import org.sonar.python.types.InferredTypes;
 import org.sonar.python.types.v2.PythonType;
 import org.sonar.python.types.v2.TriBool;
 
@@ -43,10 +42,10 @@ public void initialize(Context context) {
       CallExpression callExpression = (CallExpression) ctx.syntaxNode();
       Expression callee = callExpression.callee();
       InferredType calleeType = callee.type();
-      PythonType typeV2 = callee.typeV2();
-      if (isNonCallableType(typeV2)) {
+      PythonType type = callee.typeV2();
+      if (isNonCallableType(type)) {
         String name = nameFromExpression(callee);
-        PreciseIssue preciseIssue = ctx.addIssue(callee, message(calleeType, name));
+        PreciseIssue preciseIssue = ctx.addIssue(callee, message(type, name));
         LocationInFile location = typeClassLocation(calleeType);
         if (location != null) {
           preciseIssue.secondary(location, "Definition.");
@@ -55,22 +54,20 @@ public void initialize(Context context) {
     });
   }
 
-  protected static String addTypeName(InferredType type) {
-    String typeName = InferredTypes.typeName(type);
-    if (typeName != null) {
-      return " has type " + typeName + " and it";
-    }
-    return "";
+  protected static String addTypeName(PythonType type) {
+    return type.displayName()
+      .map(d -> " has type " + d + " and it")
+      .orElse("");
   }
 
   public boolean isNonCallableType(PythonType type) {
     return type.hasMember("__call__") == TriBool.FALSE;
   }
 
-  public String message(InferredType calleeType, @Nullable String name) {
+  public String message(PythonType typeV2, @Nullable String name) {
     if (name != null) {
-      return String.format("Fix this call; \"%s\"%s is not callable.", name, addTypeName(calleeType));
+      return "Fix this call; \"%s\"%s is not callable.".formatted(name, addTypeName(typeV2));
     }
-    return String.format("Fix this call; this expression%s is not callable.", addTypeName(calleeType));
+    return "Fix this call; this expression%s is not callable.".formatted(addTypeName(typeV2));
   }
 }
diff --git a/python-checks/src/test/resources/checks/nonCallableCalled.py b/python-checks/src/test/resources/checks/nonCallableCalled.py
@@ -42,6 +42,10 @@ def call_noncallable(p):
       x = 'str'
     x() # FN: multiple assignment not handled
 
+
+def call_no_name():
+    42() # Noncompliant {{Fix this call; this expression has type int and it is not callable.}}
+
 def flow_sensitivity():
   my_var = "hello"
   my_var = 42
diff --git a/python-frontend/src/main/java/org/sonar/python/types/v2/ClassType.java b/python-frontend/src/main/java/org/sonar/python/types/v2/ClassType.java
@@ -47,12 +47,17 @@ public ClassType(String name, List<PythonType> attributes) {
   }
 
   @Override
-  public String displayName() {
+  public Optional<String> displayName() {
+    return Optional.of("type");
+  }
+
+  @Override
+  public Optional<String> instanceDisplayName() {
     var splits = name.split("\\.");
     if (splits.length > 0) {
-      return splits[splits.length - 1];
+      return Optional.of(splits[splits.length - 1]);
     }
-    return name;
+    return Optional.of(name);
   }
 
   @Override
diff --git a/python-frontend/src/main/java/org/sonar/python/types/v2/FunctionType.java b/python-frontend/src/main/java/org/sonar/python/types/v2/FunctionType.java
@@ -21,6 +21,7 @@
 
 import java.util.List;
 import java.util.Objects;
+import java.util.Optional;
 import javax.annotation.Nullable;
 
 /**
@@ -53,6 +54,11 @@ public boolean equals(Object o) {
       && Objects.equals(parameters, that.parameters);
   }
 
+  @Override
+  public Optional<String> displayName() {
+    return Optional.of("Callable");
+  }
+
   @Override
   public int hashCode() {
     return Objects.hash(name, attributes, parameters, returnType, isAsynchronous, hasDecorators, isInstanceMethod, hasVariadicParameter);
diff --git a/python-frontend/src/main/java/org/sonar/python/types/v2/ObjectType.java b/python-frontend/src/main/java/org/sonar/python/types/v2/ObjectType.java
@@ -31,8 +31,8 @@ public ObjectType(PythonType type) {
   }
 
   @Override
-  public String displayName() {
-    return type.displayName();
+  public Optional<String> displayName() {
+    return type.instanceDisplayName();
   }
 
   @Override
diff --git a/python-frontend/src/main/java/org/sonar/python/types/v2/PythonType.java b/python-frontend/src/main/java/org/sonar/python/types/v2/PythonType.java
@@ -31,8 +31,12 @@ default String name() {
     return this.toString();
   }
 
-  default String displayName() {
-    return this.toString();
+  default Optional<String> displayName() {
+    return Optional.empty();
+  }
+
+  default Optional<String> instanceDisplayName() {
+    return Optional.empty();
   }
 
   default boolean isCompatibleWith(PythonType another) {
diff --git a/python-frontend/src/main/java/org/sonar/python/types/v2/UnionType.java b/python-frontend/src/main/java/org/sonar/python/types/v2/UnionType.java
@@ -19,17 +19,26 @@
  */
 package org.sonar.python.types.v2;
 
+import java.util.ArrayList;
 import java.util.List;
+import java.util.Optional;
 
 public record UnionType(List<PythonType> candidates) implements PythonType {
   public UnionType() {
     this(List.of());
   }
 
   @Override
-  public String displayName() {
-    var candidatesName = candidates.stream().map(PythonType::displayName).toList();
-    return "Union[%s]".formatted(String.join(", ", candidatesName));
+  public Optional<String> displayName() {
+    List<String> candidateNames = new ArrayList<>();
+    for (PythonType candidate : candidates) {
+      Optional<String> s = candidate.displayName();
+      s.ifPresent(candidateNames::add);
+      if (s.isEmpty()) {
+        return Optional.empty();
+      }
+    }
+    return Optional.of("Union[%s]".formatted(String.join(", ", candidateNames)));
   }
 
   @Override
diff --git a/python-frontend/src/main/java/org/sonar/python/types/v2/UnknownType.java b/python-frontend/src/main/java/org/sonar/python/types/v2/UnknownType.java
@@ -27,9 +27,4 @@ public boolean isCompatibleWith(PythonType another) {
     return true;
   }
 
-  @Override
-  public String displayName() {
-    return "UnknownType";
-  }
-
 }
diff --git a/python-frontend/src/test/java/org/sonar/python/types/v2/ClassTypeTest.java b/python-frontend/src/test/java/org/sonar/python/types/v2/ClassTypeTest.java
@@ -57,6 +57,9 @@ void no_parents() {
     assertThat(classType.hasMember("unknown")).isEqualTo(TriBool.UNKNOWN);
     assertThat(classType.instancesHaveMember("__call__")).isEqualTo(TriBool.FALSE);
     assertThat(classType.instancesHaveMember("unknown")).isEqualTo(TriBool.FALSE);
+
+    assertThat(classType.displayName()).contains("type");
+    assertThat(classType.instanceDisplayName()).contains("C");
   }
 
   @Test
@@ -579,6 +582,21 @@ void builder() {
     assertThat(classTypeBuilder.build()).isEqualTo(classType);
   }
 
+  @Test
+  void displayName() {
+    ClassType classType = new ClassType("...");
+    assertThat(classType.instanceDisplayName()).contains("...");
+    assertThat(classType.displayName()).contains("type");
+
+    classType = new ClassType("MyClass");
+    assertThat(classType.instanceDisplayName()).contains("MyClass");
+    assertThat(classType.displayName()).contains("type");
+
+    classType = new ClassType("mymod.MyClass");
+    assertThat(classType.instanceDisplayName()).contains("MyClass");
+    assertThat(classType.displayName()).contains("type");
+  }
+
   public static ClassType classType(String... code) {
     return classTypes(code).get(0);
   }
diff --git a/python-frontend/src/test/java/org/sonar/python/types/v2/FunctionTypeTest.java b/python-frontend/src/test/java/org/sonar/python/types/v2/FunctionTypeTest.java
@@ -40,6 +40,8 @@ void arity() {
     FunctionType functionType = functionType("def fn(): pass");
     assertThat(functionType.isAsynchronous()).isFalse();
     assertThat(functionType.parameters()).isEmpty();
+    assertThat(functionType.displayName()).contains("Callable");
+    assertThat(functionType.instanceDisplayName()).isEmpty();
 
     functionType = functionType("async def fn(p1, p2, p3): pass");
     assertThat(functionType.parameters()).extracting(ParameterV2::name).containsExactly("p1", "p2", "p3");
diff --git a/python-frontend/src/test/java/org/sonar/python/types/v2/ObjectTypeTest.java b/python-frontend/src/test/java/org/sonar/python/types/v2/ObjectTypeTest.java
@@ -20,6 +20,7 @@
 package org.sonar.python.types.v2;
 
 import java.util.List;
+import java.util.Optional;
 import org.junit.jupiter.api.Test;
 import org.sonar.plugins.python.api.tree.ClassDef;
 import org.sonar.plugins.python.api.tree.DictionaryLiteral;
@@ -32,14 +33,10 @@
 import org.sonar.plugins.python.api.tree.StringLiteral;
 import org.sonar.plugins.python.api.tree.Tree;
 import org.sonar.plugins.python.api.tree.Tuple;
-import org.sonar.python.semantic.ProjectLevelSymbolTable;
-import org.sonar.python.semantic.v2.ProjectLevelTypeTable;
-import org.sonar.python.semantic.v2.SymbolTableBuilderV2;
-import org.sonar.python.semantic.v2.TypeInferenceV2;
 import org.sonar.python.tree.TreeUtils;
 
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.sonar.python.PythonTestUtils.parseWithoutSymbols;
+import static org.sonar.python.types.v2.TypesTestUtils.parseAndInferTypes;
 
 
 public class ObjectTypeTest {
@@ -55,7 +52,7 @@ class A: ...
     ClassType classType = (ClassType) ((ClassDef) fileInput.statements().statements().get(0)).name().typeV2();
     ObjectType objectType = (ObjectType) ((ExpressionStatement) fileInput.statements().statements().get(2)).expressions().get(0).typeV2();
 
-    assertThat(objectType.displayName()).isEqualTo("A");
+    assertThat(objectType.displayName()).contains("A");
     assertThat(objectType.isCompatibleWith(classType)).isTrue();
     assertThat(objectType.hasMember("foo")).isEqualTo(TriBool.FALSE);
   }
@@ -72,7 +69,7 @@ def foo(self): ...
     ClassType classType = (ClassType) ((ClassDef) fileInput.statements().statements().get(0)).name().typeV2();
     ObjectType objectType = (ObjectType) ((ExpressionStatement) fileInput.statements().statements().get(2)).expressions().get(0).typeV2();
 
-    assertThat(objectType.displayName()).isEqualTo("A");
+    assertThat(objectType.displayName()).contains("A");
     assertThat(objectType.isCompatibleWith(classType)).isTrue();
     assertThat(objectType.hasMember("foo")).isEqualTo(TriBool.TRUE);
   }
@@ -100,68 +97,68 @@ void literalTypes() {
     FileInput fileInput = parseAndInferTypes("\"hello\"");
     StringLiteral stringLiteral = (StringLiteral) TreeUtils.firstChild(fileInput, t -> t.is(Tree.Kind.STRING_LITERAL)).get();
     ObjectType stringLiteralType = (ObjectType) stringLiteral.typeV2();
-    assertThat(stringLiteralType.displayName()).isEqualTo("str");
+    assertThat(stringLiteralType.displayName()).contains("str");
 
     fileInput = parseAndInferTypes("(1, 2, 3)");
     Tuple intTuple = (Tuple) TreeUtils.firstChild(fileInput, t -> t.is(Tree.Kind.TUPLE)).get();
     ObjectType intTupleType = (ObjectType) intTuple.typeV2();
-    assertThat(intTupleType.displayName()).isEqualTo("tuple");
-    assertThat(intTupleType.attributes()).extracting(PythonType::displayName).containsExactly("int");
+    assertThat(intTupleType.displayName()).contains("tuple");
+    assertThat(intTupleType.attributes()).extracting(PythonType::displayName).extracting(Optional::get).containsExactly("int");
 
     fileInput = parseAndInferTypes("(1, \"hello\")");
     Tuple intStrTuple = (Tuple) TreeUtils.firstChild(fileInput, t -> t.is(Tree.Kind.TUPLE)).get();
     ObjectType intStrTupleType = (ObjectType) intStrTuple.typeV2();
-    assertThat(intStrTupleType.displayName()).isEqualTo("tuple");
+    assertThat(intStrTupleType.displayName()).contains("tuple");
     assertThat(intStrTupleType.attributes()).isEmpty();
 
     fileInput = parseAndInferTypes("(foo(),)");
     Tuple unknownTuple = (Tuple) TreeUtils.firstChild(fileInput, t -> t.is(Tree.Kind.TUPLE)).get();
     ObjectType unknownTupleType = (ObjectType) unknownTuple.typeV2();
-    assertThat(unknownTupleType.displayName()).isEqualTo("tuple");
+    assertThat(unknownTupleType.displayName()).contains("tuple");
     assertThat(unknownTupleType.attributes()).isEmpty();
 
     fileInput = parseAndInferTypes("{1, 2, 3}");
     SetLiteral setLiteral = (SetLiteral) TreeUtils.firstChild(fileInput, t -> t.is(Tree.Kind.SET_LITERAL)).get();
     ObjectType setLiteralType = (ObjectType) setLiteral.typeV2();
-    assertThat(setLiteralType.displayName()).isEqualTo("set");
+    assertThat(setLiteralType.displayName()).contains("set");
 
     fileInput = parseAndInferTypes("{\"my_key\": 42}");
     DictionaryLiteral dictionaryLiteral = (DictionaryLiteral) TreeUtils.firstChild(fileInput, t -> t.is(Tree.Kind.DICTIONARY_LITERAL)).get();
     ObjectType dictionaryLiteralType = (ObjectType) dictionaryLiteral.typeV2();
-    assertThat(dictionaryLiteralType.displayName()).isEqualTo("dict");
+    assertThat(dictionaryLiteralType.displayName()).contains("dict");
 
     fileInput = parseAndInferTypes("None");
     NoneExpression noneExpression = (NoneExpression) TreeUtils.firstChild(fileInput, t -> t.is(Tree.Kind.NONE)).get();
     ObjectType noneExpressionType = (ObjectType) noneExpression.typeV2();
-    assertThat(noneExpressionType.displayName()).isEqualTo("NoneType");
+    assertThat(noneExpressionType.displayName()).contains("NoneType");
 
     fileInput = parseAndInferTypes("unknown");
     Name name = (Name) TreeUtils.firstChild(fileInput, t -> t.is(Tree.Kind.NAME)).get();
     UnknownType nameType = (UnknownType) name.typeV2();
-    assertThat(nameType.displayName()).isEqualTo("UnknownType");
+    assertThat(nameType.displayName()).isEmpty();
   }
 
   @Test
   void numericLiteralTypes() {
     FileInput fileInput = parseAndInferTypes("42");
     NumericLiteral intLiteral = (NumericLiteral) TreeUtils.firstChild(fileInput, t -> t.is(Tree.Kind.NUMERIC_LITERAL)).get();
     ObjectType intLiteralType = (ObjectType) intLiteral.typeV2();
-    assertThat(intLiteralType.displayName()).isEqualTo("int");
+    assertThat(intLiteralType.displayName()).contains("int");
 
     fileInput = parseAndInferTypes("42.5");
     NumericLiteral floatLiteral = (NumericLiteral) TreeUtils.firstChild(fileInput, t -> t.is(Tree.Kind.NUMERIC_LITERAL)).get();
     ObjectType floatLiteralType = (ObjectType) floatLiteral.typeV2();
-    assertThat(floatLiteralType.displayName()).isEqualTo("float");
+    assertThat(floatLiteralType.displayName()).contains("float");
 
     fileInput = parseAndInferTypes("4e2");
     NumericLiteral exponentLiteral = (NumericLiteral) TreeUtils.firstChild(fileInput, t -> t.is(Tree.Kind.NUMERIC_LITERAL)).get();
     ObjectType exponentLiteralType = (ObjectType) exponentLiteral.typeV2();
-    assertThat(exponentLiteralType.displayName()).isEqualTo("float");
+    assertThat(exponentLiteralType.displayName()).contains("float");
 
     fileInput = parseAndInferTypes("42j");
     NumericLiteral complexLiteral = (NumericLiteral) TreeUtils.firstChild(fileInput, t -> t.is(Tree.Kind.NUMERIC_LITERAL)).get();
     ObjectType complexLiteralType = (ObjectType) complexLiteral.typeV2();
-    assertThat(complexLiteralType.displayName()).isEqualTo("complex");
+    assertThat(complexLiteralType.displayName()).contains("complex");
   }
 
   @Test
@@ -170,12 +167,4 @@ void objectType_of_unknown() {
     ObjectType objectType = new ObjectType(PythonType.UNKNOWN, List.of(), List.of());
     assertThat(objectType.hasMember("foo")).isEqualTo(TriBool.UNKNOWN);
   }
-
-
-  public static FileInput parseAndInferTypes(String... code) {
-    FileInput fileInput = parseWithoutSymbols(code);
-    fileInput.accept(new SymbolTableBuilderV2());
-    fileInput.accept(new TypeInferenceV2(new ProjectLevelTypeTable(ProjectLevelSymbolTable.empty())));
-    return fileInput;
-  }
 }
diff --git a/python-frontend/src/test/java/org/sonar/python/types/v2/TypesTestUtils.java b/python-frontend/src/test/java/org/sonar/python/types/v2/TypesTestUtils.java
@@ -0,0 +1,39 @@
+/*
+ * 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.types.v2;
+
+import org.sonar.plugins.python.api.tree.FileInput;
+import org.sonar.python.semantic.ProjectLevelSymbolTable;
+import org.sonar.python.semantic.v2.ProjectLevelTypeTable;
+import org.sonar.python.semantic.v2.SymbolTableBuilderV2;
+import org.sonar.python.semantic.v2.TypeInferenceV2;
+
+import static org.sonar.python.PythonTestUtils.parseWithoutSymbols;
+
+public class TypesTestUtils {
+
+
+  public static FileInput parseAndInferTypes(String... code) {
+    FileInput fileInput = parseWithoutSymbols(code);
+    fileInput.accept(new SymbolTableBuilderV2());
+    fileInput.accept(new TypeInferenceV2(new ProjectLevelTypeTable(ProjectLevelSymbolTable.empty())));
+    return fileInput;
+  }
+}
diff --git a/python-frontend/src/test/java/org/sonar/python/types/v2/UnionTypeTest.java b/python-frontend/src/test/java/org/sonar/python/types/v2/UnionTypeTest.java

Original file line number	Diff line number	Diff line change
`@@ -47,12 +47,17 @@ public ClassType(String name, List<PythonType> attributes) {`
`47`	`47`	`}`
`48`	`48`
`49`	`49`	`@Override`
`50`		`- public String displayName() {`
	`50`	`+ public Optional<String> displayName() {`
	`51`	`+ return Optional.of("type");`
	`52`	`+ }`
	`53`	`+`
	`54`	`+ @Override`
	`55`	`+ public Optional<String> instanceDisplayName() {`
`51`	`56`	`var splits = name.split("\\.");`
`52`	`57`	`if (splits.length > 0) {`
`53`		`- return splits[splits.length - 1];`
	`58`	`+ return Optional.of(splits[splits.length - 1]);`
`54`	`59`	`}`
`55`		`- return name;`
	`60`	`+ return Optional.of(name);`
`56`	`61`	`}`
`57`	`62`
`58`	`63`	`@Override`
Original file line number	Diff line number	Diff line change
`@@ -31,8 +31,8 @@ public ObjectType(PythonType type) {`
`31`	`31`	`}`
`32`	`32`
`33`	`33`	`@Override`
`34`		`- public String displayName() {`
`35`		`- return type.displayName();`
	`34`	`+ public Optional<String> displayName() {`
	`35`	`+ return type.instanceDisplayName();`
`36`	`36`	`}`
`37`	`37`
`38`	`38`	`@Override`
Original file line number	Diff line number	Diff line change
`@@ -31,8 +31,12 @@ default String name() {`
`31`	`31`	`return this.toString();`
`32`	`32`	`}`
`33`	`33`
`34`		`- default String displayName() {`
`35`		`- return this.toString();`
	`34`	`+ default Optional<String> displayName() {`
	`35`	`+ return Optional.empty();`
	`36`	`+ }`
	`37`	`+`
	`38`	`+ default Optional<String> instanceDisplayName() {`
	`39`	`+ return Optional.empty();`
`36`	`40`	`}`
`37`	`41`
`38`	`42`	`default boolean isCompatibleWith(PythonType another) {`
Original file line number	Diff line number	Diff line change
`@@ -27,9 +27,4 @@ public boolean isCompatibleWith(PythonType another) {`
`27`	`27`	`return true;`
`28`	`28`	`}`
`29`	`29`
`30`		`- @Override`
`31`		`- public String displayName() {`
`32`		`- return "UnknownType";`
`33`		`- }`
`34`		`-`
`35`	`30`	`}`