SONARPY-1789 Resolve inherited class members (#1770)

guillaume-dequenne · guillaume-dequenne · commit f8c6c2a9ff98 · 2024-05-21T17:09:08.000+02:00
diff --git a/its/ruling/src/test/resources/expected/python-S5756.json b/its/ruling/src/test/resources/expected/python-S5756.json
@@ -1,25 +1,4 @@
 {
-"project:buildbot-0.8.6p1/buildbot/test/unit/test_revlinks.py": [
-81
-],
-"project:django-2.2.3/django/contrib/gis/geos/prototypes/io.py": [
-151,
-153,
-238,
-249,
-258,
-263,
-270,
-276,
-281,
-285
-],
-"project:django-2.2.3/django/db/transaction.py": [
-297
-],
-"project:django-2.2.3/django/test/utils.py": [
-789
-],
 "project:pecos/examples/qp2q/preprocessing/session_data_processing.py": [
 34
 ],
diff --git a/its/ruling/src/test/resources/expected_extended/calibre/python-S5756.json b/its/ruling/src/test/resources/expected_extended/calibre/python-S5756.json
diff --git a/its/ruling/src/test/resources/expected_extended/salt/python-S5756.json b/its/ruling/src/test/resources/expected_extended/salt/python-S5756.json
diff --git a/python-checks/src/test/resources/checks/nonCallableCalled.py b/python-checks/src/test/resources/checks/nonCallableCalled.py
@@ -181,3 +181,27 @@ def typing_named_tuple_no_fp():
     from typing import NamedTuple
     Employee = NamedTuple('Employee', [('name', str), ('id', int)])
     employee = Employee("Sam", 42)
+
+
+class Parent:
+    def __call__(self):
+        ...
+
+class Child(Parent):
+    ...
+
+
+def inherited_call_method():
+    child = Child()
+    child()  # OK
+
+
+some_global_func = None
+
+def assigning_global(my_func):
+    global some_global_func
+    some_global_func = my_func
+
+def calling_global_func():
+    # FP
+    some_global_func() # Noncompliant
diff --git a/python-frontend/src/main/java/org/sonar/python/semantic/v2/ProjectLevelTypeTable.java b/python-frontend/src/main/java/org/sonar/python/semantic/v2/ProjectLevelTypeTable.java
@@ -23,6 +23,7 @@
 import java.util.stream.IntStream;
 import org.sonar.python.semantic.ProjectLevelSymbolTable;
 import org.sonar.python.types.v2.ModuleType;
+import org.sonar.python.types.v2.PythonType;
 
 public class ProjectLevelTypeTable {
 
@@ -42,7 +43,7 @@ public ModuleType getModule(String... moduleName) {
   public ModuleType getModule(List<String> moduleNameParts) {
     var parent = rootModule;
     for (int i = 0; i < moduleNameParts.size(); i++) {
-      var existing = parent.resolveMember(moduleNameParts.get(i));
+      var existing = parent.resolveMember(moduleNameParts.get(i)).orElse(PythonType.UNKNOWN);
 
       if (existing instanceof ModuleType existingModule) {
         parent = existingModule;
diff --git a/python-frontend/src/main/java/org/sonar/python/semantic/v2/TypeInferenceV2.java b/python-frontend/src/main/java/org/sonar/python/semantic/v2/TypeInferenceV2.java
@@ -80,7 +80,8 @@ public void visitFileInput(FileInput fileInput) {
   public void visitStringLiteral(StringLiteral stringLiteral) {
     ModuleType builtins = this.projectLevelTypeTable.getModule(BUILTINS);
     // TODO: multiple object types to represent str instance?
-    ((StringLiteralImpl) stringLiteral).typeV2(new ObjectType(builtins.resolveMember("str"), List.of(), List.of()));
+    PythonType strType = builtins.resolveMember("str").orElse(PythonType.UNKNOWN);
+    ((StringLiteralImpl) stringLiteral).typeV2(new ObjectType(strType, List.of(), List.of()));
   }
 
   @Override
@@ -89,15 +90,17 @@ public void visitNumericLiteral(NumericLiteral numericLiteral) {
     InferredType type = numericLiteral.type();
     String memberName = ((RuntimeType) type).getTypeClass().fullyQualifiedName();
     if (memberName != null) {
-      ((NumericLiteralImpl) numericLiteral).typeV2(new ObjectType(builtins.resolveMember(memberName), List.of(), List.of()));
+      PythonType pythonType = builtins.resolveMember(memberName).orElse(PythonType.UNKNOWN);
+      ((NumericLiteralImpl) numericLiteral).typeV2(new ObjectType(pythonType, List.of(), List.of()));
     }
   }
 
   @Override
   public void visitNone(NoneExpression noneExpression) {
     ModuleType builtins = this.projectLevelTypeTable.getModule(BUILTINS);
     // TODO: multiple object types to represent str instance?
-    ((NoneExpressionImpl) noneExpression).typeV2(new ObjectType(builtins.resolveMember("NoneType"), List.of(), List.of()));
+    PythonType noneType = builtins.resolveMember("NoneType").orElse(PythonType.UNKNOWN);
+    ((NoneExpressionImpl) noneExpression).typeV2(new ObjectType(noneType, List.of(), List.of()));
   }
 
   @Override
@@ -106,7 +109,8 @@ public void visitListLiteral(ListLiteral listLiteral) {
     scan(listLiteral.elements());
     List<PythonType> pythonTypes = listLiteral.elements().expressions().stream().map(Expression::typeV2).distinct().toList();
     // TODO: cleanly reduce attributes
-    ((ListLiteralImpl) listLiteral).typeV2(new ObjectType(builtins.resolveMember("list"), pythonTypes, List.of()));
+    PythonType listType = builtins.resolveMember("list").orElse(PythonType.UNKNOWN);
+    ((ListLiteralImpl) listLiteral).typeV2(new ObjectType(listType, pythonTypes, List.of()));
   }
 
   @Override
@@ -233,7 +237,7 @@ public void visitImportFrom(ImportFrom importFrom) {
           .stream()
           .findFirst()
           .ifPresent(name -> {
-            var type = module.resolveMember(name.name());
+            var type = module.resolveMember(name.name()).orElse(PythonType.UNKNOWN);
 
             var boundName = Optional.ofNullable(aliasedName.alias())
               .orElse(name);
@@ -265,7 +269,9 @@ public void visitAssignmentStatement(AssignmentStatement assignmentStatement) {
   public void visitQualifiedExpression(QualifiedExpression qualifiedExpression) {
     scan(qualifiedExpression.qualifier());
     if (qualifiedExpression.name() instanceof NameImpl name) {
-      var nameType = qualifiedExpression.qualifier().typeV2().resolveMember(qualifiedExpression.name().name());
+      var nameType = qualifiedExpression.qualifier().typeV2()
+        .resolveMember(qualifiedExpression.name().name())
+        .orElse(PythonType.UNKNOWN);
       name.typeV2(nameType);
     }
   }
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
@@ -71,9 +71,9 @@ public boolean isCompatibleWith(PythonType another) {
       var isASubClass = this.isASubClassFrom(other);
       var areAttributeCompatible = this.areAttributesCompatible(other);
       var isDuckTypeCompatible = !this.members.isEmpty() && other.members.stream().allMatch(member -> this.members.contains(member));
-      return Objects.equals(this, another) || "builtins.object".equals(other.name()) || 
+      return Objects.equals(this, another) || "builtins.object".equals(other.name()) ||
         isDuckTypeCompatible ||
-        ( isASubClass && areAttributeCompatible) ;
+        (isASubClass && areAttributeCompatible);
     }
     return true;
   }
@@ -96,20 +96,33 @@ public String key() {
   }
 
   @Override
-  public PythonType resolveMember(String memberName) {
+  public Optional<PythonType> resolveMember(String memberName) {
+    return localMember(memberName)
+      .or(() -> inheritedMember(memberName));
+  }
+
+  private Optional<PythonType> localMember(String memberName) {
     return members.stream()
       .filter(m -> m.name().equals(memberName))
       .map(Member::type)
-      .findFirst().orElse(PythonType.UNKNOWN);
+      .findFirst();
+  }
+
+  private Optional<PythonType> inheritedMember(String memberName) {
+    return superClasses.stream()
+      .map(s -> s.resolveMember(memberName))
+      .filter(Optional::isPresent)
+      .map(Optional::get)
+      .findFirst();
   }
 
   public boolean hasUnresolvedHierarchy() {
     return superClasses.stream().anyMatch(s -> {
-      if (s instanceof ClassType parentClassType) {
-        return parentClassType.hasUnresolvedHierarchy();
+        if (s instanceof ClassType parentClassType) {
+          return parentClassType.hasUnresolvedHierarchy();
+        }
+        return true;
       }
-      return true;
-    }
     );
   }
 
@@ -138,8 +151,7 @@ public TriBool instancesHaveMember(String memberName) {
       // TODO: instances of NamedTuple are type
       return TriBool.TRUE;
     }
-    // TODO: look at parents
-    return resolveMember(memberName) != PythonType.UNKNOWN ? TriBool.TRUE : TriBool.FALSE;
+    return resolveMember(memberName).isPresent() ? TriBool.TRUE : TriBool.FALSE;
   }
 
   @Override
diff --git a/python-frontend/src/main/java/org/sonar/python/types/v2/ModuleType.java b/python-frontend/src/main/java/org/sonar/python/types/v2/ModuleType.java
@@ -22,6 +22,7 @@
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Objects;
+import java.util.Optional;
 import javax.annotation.Nullable;
 
 public record ModuleType(@Nullable String name, @Nullable ModuleType parent, Map<String, PythonType> members) implements PythonType {
@@ -33,9 +34,9 @@ public ModuleType(@Nullable String name, @Nullable ModuleType parent) {
     this(name, parent, new HashMap<>());
   }
 
-  public PythonType resolveMember(String memberName) {
-    // FIXME: handle case where type is missing
-    return members.getOrDefault(memberName, PythonType.UNKNOWN);
+  @Override
+  public Optional<PythonType> resolveMember(String memberName) {
+    return Optional.ofNullable(members.get(memberName));
   }
 
   @Override
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
@@ -22,6 +22,7 @@
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Objects;
+import java.util.Optional;
 
 public record ObjectType(PythonType type, List<PythonType> attributes, List<Member> members) implements PythonType {
 
@@ -40,17 +41,17 @@ public boolean isCompatibleWith(PythonType another) {
   }
 
   @Override
-  public PythonType resolveMember(String memberName) {
+  public Optional<PythonType> resolveMember(String memberName) {
     return members().stream()
       .filter(member -> Objects.equals(member.name(), memberName))
       .map(Member::type)
       .findFirst()
-      .orElseGet(() -> type.resolveMember(memberName));
+      .or(() -> type.resolveMember(memberName));
   }
 
   @Override
   public TriBool hasMember(String memberName) {
-    if (resolveMember(memberName) != PythonType.UNKNOWN) {
+    if (resolveMember(memberName).isPresent()) {
       return TriBool.TRUE;
     }
     if (type instanceof ClassType classType) {
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
@@ -19,6 +19,8 @@
  */
 package org.sonar.python.types.v2;
 
+import java.util.Optional;
+
 /**
  * PythonType
  */
@@ -41,8 +43,8 @@ default String key() {
     return name();
   }
 
-  default PythonType resolveMember(String memberName) {
-    return PythonType.UNKNOWN;
+  default Optional<PythonType> resolveMember(String memberName) {
+    return Optional.empty();
   }
 
   default TriBool hasMember(String memberName) {
diff --git a/python-frontend/src/test/java/org/sonar/python/semantic/v2/TypeInferenceV2Test.java b/python-frontend/src/test/java/org/sonar/python/semantic/v2/TypeInferenceV2Test.java
@@ -147,7 +147,7 @@ void testProjectLevelSymbolTableImports() {
       .extracting(Expression::typeV2)
       .isInstanceOf(ModuleType.class)
       .matches(type -> {
-        assertThat(type.resolveMember("C"))
+        assertThat(type.resolveMember("C").get())
           .isInstanceOf(ClassType.class);
         return true;
       })
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
@@ -59,6 +59,18 @@ void no_parents() {
     assertThat(classType.instancesHaveMember("unknown")).isEqualTo(TriBool.FALSE);
   }
 
+  @Test
+  void simple_member() {
+    ClassType classType = classType("""
+          class C:
+            def foo(): ...
+          """
+    );
+
+    assertThat(classType.instancesHaveMember("foo")).isEqualTo(TriBool.TRUE);
+    assertThat(classType.instancesHaveMember("bar")).isEqualTo(TriBool.FALSE);
+  }
+
   @Test
   void equals_test() {
     ClassType classType1 = classType("class C: ...");
@@ -411,15 +423,42 @@ void duplicated_class_member_self() {
 
   @Test
   void class_members_with_inheritance() {
-    ClassType classType = classTypes(
+    ClassType classB = classTypes(
       "class A:",
       "  def meth(): pass",
       "class B(A): ",
       "  def foo(): pass").get(1);
 
-    assertThat(classType.members()).hasSize(1);
-    ClassType classA = (ClassType) classType.superClasses().get(0);
+    assertThat(classB.members()).hasSize(1);
+    ClassType classA = (ClassType) classB.superClasses().get(0);
     assertThat(classA.members()).hasSize(1);
+
+    assertThat(classB.resolveMember("foo")).isPresent();
+    assertThat(classB.resolveMember("meth")).isPresent();
+    assertThat(classB.resolveMember("unkown")).isNotPresent();
+  }
+
+  @Test
+  void class_members_multiple_inheritance() {
+    // See https://docs.python.org/3/howto/mro.html#python-2-3-mro
+    List<ClassType> classTypes = classTypes(
+      """
+        class A:
+          def foo(param): ...
+        class B:
+          def foo(): ...
+        class C(A, B): ...
+        """
+    );
+    ClassType classA = classTypes.get(0);
+    ClassType classB = classTypes.get(1);
+    ClassType classC = classTypes.get(2);
+
+    PythonType fooA = classA.resolveMember("foo").get();
+    PythonType fooB = classB.resolveMember("foo").get();
+    PythonType fooC = classC.resolveMember("foo").get();
+
+    assertThat(fooC).isSameAs(fooA).isNotSameAs(fooB);
   }
 
   @Test
@@ -433,9 +472,8 @@ void static_member_usages() {
       "    A.foo = 0",
       "    A.bar"
     );
-    PythonType foo = classType.resolveMember("foo");
-    assertThat(foo).isNotNull();
-    assertThat(classType.resolveMember("bar")).isNull();
+    assertThat((classType.resolveMember("foo"))).isPresent();
+    assertThat(classType.resolveMember("bar")).isNotPresent();
   }
 
   @Test
@@ -449,8 +487,7 @@ void inherited_static_member() {
     );
 
     assertThat(classType.instancesHaveMember("foo")).isEqualTo(TriBool.TRUE);
-    PythonType foo = classType.resolveMember("foo");
-    assertThat(foo).isNotNull();
+    assertThat(classType.resolveMember("foo")).isPresent();
   }
 
   @Test
diff --git a/python-frontend/src/test/java/org/sonar/python/types/v2/ModuleTypeTest.java b/python-frontend/src/test/java/org/sonar/python/types/v2/ModuleTypeTest.java
