Python: Modernize 4 queries for missing/multiple calls to init/del methods

python/ql/lib/semmle/python/dataflow/new/internal/DataFlowDispatch.qll

@@ -851,9 +851,14 @@ Class getNextClassInMroKnownStartingClass(Class cls, Class startingClass) {
   )
 }
 
-private Function findFunctionAccordingToMroKnownStartingClass(
-  Class cls, Class startingClass, string name
-) {
+/**
+ * Gets a potential definition of the function `name` of the class `cls` according to our approximation of
+ * MRO for the class `startingCls` (see `getNextClassInMroKnownStartingClass` for more information).
+ *
+ * Note: this is almost the same as `findFunctionAccordingToMro`, except we know the
+ * `startingClass`, which can give slightly more precise results.
+ */
+Function findFunctionAccordingToMroKnownStartingClass(Class cls, Class startingClass, string name) {
   result = cls.getAMethod() and
   result.getName() = name and
   cls = getADirectSuperclass*(startingClass)
@@ -866,7 +871,7 @@ private Function findFunctionAccordingToMroKnownStartingClass(
 
 /**
  * Gets a potential definition of the function `name` according to our approximation of
- * MRO for the class `cls` (see `getNextClassInMroKnownStartingClass` for more information).
+ * MRO for the class `startingCls` (see `getNextClassInMroKnownStartingClass` for more information).
  *
  * Note: this is almost the same as `findFunctionAccordingToMro`, except we know the
  * `startingClass`, which can give slightly more precise results.

python/ql/src/Classes/CallsToInitDel/MethodCallOrder.qll

@@ -0,0 +1,167 @@
+/** Definitions for reasoning about multiple or missing calls to superclass methods. */
+
+import python
+import semmle.python.ApiGraphs
+import semmle.python.dataflow.new.internal.DataFlowDispatch
+import codeql.util.Option
+
+predicate multipleCallsToSuperclassMethod(Function meth, Function calledMulti, string name) {
+  exists(DataFlow::MethodCallNode call1, DataFlow::MethodCallNode call2, Class cls |
+    meth.getName() = name and
+    meth.getScope() = cls and
+    call1.asExpr() != call2.asExpr() and
+    calledMulti = getASuperCallTargetFromCall(cls, meth, call1, name) and
+    calledMulti = getASuperCallTargetFromCall(cls, meth, call2, name) and
+    nonTrivial(calledMulti)
+  )
+}
+
+Function getASuperCallTargetFromCall(
+  Class mroBase, Function meth, DataFlow::MethodCallNode call, string name
+) {
+  meth = call.getScope() and
+  getADirectSuperclass*(mroBase) = meth.getScope() and
+  meth.getName() = name and
+  call.calls(_, name) and
+  exists(Class targetCls | result = getASuperCallTargetFromClass(mroBase, targetCls, name) |
+    superCall(call, _) and
+    targetCls = getNextClassInMroKnownStartingClass(meth.getScope(), mroBase)
+    or
+    callsMethodOnClassWithSelf(meth, call, targetCls, _)
+  )
+}
+
+Function getASuperCallTargetFromClass(Class mroBase, Class cls, string name) {
+  exists(Function target |
+    target = findFunctionAccordingToMroKnownStartingClass(cls, mroBase, name) and
+    (result = target or result = getASuperCallTargetFromCall(mroBase, target, _, name))
+  )
+}
+
+predicate nonTrivial(Function meth) {
+  exists(Stmt s | s = meth.getAStmt() |
+    not s instanceof Pass and
+    not exists(DataFlow::Node call | call.asExpr() = s.(ExprStmt).getValue() |
+      superCall(call, meth.getName())
+      or
+      callsMethodOnClassWithSelf(meth, call, _, meth.getName())
+    )
+  ) and
+  exists(meth.getANormalExit()) // doesn't always raise an exception
+}
+
+predicate superCall(DataFlow::MethodCallNode call, string name) {
+  exists(DataFlow::Node sup |
+    call.calls(sup, name) and
+    sup = API::builtin("super").getACall()
+  )
+}
+
+predicate callsSuper(Function meth) {
+  exists(DataFlow::MethodCallNode call |
+    call.getScope() = meth and
+    superCall(call, meth.getName())
+  )
+}
+
+predicate callsMethodOnClassWithSelf(
+  Function meth, DataFlow::MethodCallNode call, Class target, string name
+) {
+  exists(DataFlow::Node callTarget, DataFlow::ParameterNode self |
+    call.calls(callTarget, name) and
+    self.getParameter() = meth.getArg(0) and
+    self.(DataFlow::LocalSourceNode).flowsTo(call.getArg(0)) and
+    callTarget = classTracker(target)
+  )
+}
+
+predicate callsMethodOnUnknownClassWithSelf(Function meth, string name) {
+  exists(DataFlow::MethodCallNode call, DataFlow::Node callTarget, DataFlow::ParameterNode self |
+    call.calls(callTarget, name) and
+    self.getParameter() = meth.getArg(0) and
+    self.(DataFlow::LocalSourceNode).flowsTo(call.getArg(0)) and
+    not exists(Class target | callTarget = classTracker(target))
+  )
+}
+
+predicate missingCallToSuperclassMethod(Class base, Function shouldCall, string name) {
+  shouldCall.getName() = name and
+  shouldCall.getScope() = getADirectSuperclass+(base) and
+  not shouldCall = getASuperCallTargetFromClass(base, base, name) and
+  nonTrivial(shouldCall) and
+  // "Benefit of the doubt" - if somewhere in the chain we call an unknown superclass, assume all the necessary parent methods are called from it
+  not callsMethodOnUnknownClassWithSelf(getASuperCallTargetFromClass(base, base, name), name)
+}
+
+predicate missingCallToSuperclassMethodRestricted(Class base, Function shouldCall, string name) {
+  missingCallToSuperclassMethod(base, shouldCall, name) and
+  not exists(Class superBase |
+    // Alert only on the highest base class that has the issue
+    superBase = getADirectSuperclass+(base) and
+    missingCallToSuperclassMethod(superBase, shouldCall, name)
+  ) and
+  not exists(Function subShouldCall |
+    // Mention in the alert only the lowest method we're missing the call to
+    subShouldCall.getScope() = getADirectSubclass+(shouldCall.getScope()) and
+    missingCallToSuperclassMethod(base, subShouldCall, name)
+  )
+}
+
+Function getPossibleMissingSuper(Class base, Function shouldCall, string name) {
+  missingCallToSuperclassMethod(base, shouldCall, name) and
+  exists(Function baseMethod |
+    baseMethod.getScope() = base and
+    baseMethod.getName() = name and
+    // the base method calls super, so is presumably expecting every method called in the MRO chain to do so
+    callsSuper(baseMethod) and
+    // result is something that does get called in the chain
+    result = getASuperCallTargetFromClass(base, base, name) and
+    // it doesn't call super
+    not callsSuper(result) and
+    // if it did call super, it would resolve to the missing method
+    shouldCall =
+      findFunctionAccordingToMroKnownStartingClass(getNextClassInMroKnownStartingClass(result
+              .getScope(), base), base, name)
+  )
+}
+
+private module FunctionOption = Option<Function>;
+
+class FunctionOption extends FunctionOption::Option {
+  /**
+   * Holds if this element is at the specified location.
+   * The location spans column `startcolumn` of line `startline` to
+   * column `endcolumn` of line `endline` in file `filepath`.
+   * For more information, see
+   * [Locations](https://codeql.github.com/docs/writing-codeql-queries/providing-locations-in-codeql-queries/).
+   */
+  predicate hasLocationInfo(
+    string filepath, int startline, int startcolumn, int endline, int endcolumn
+  ) {
+    this.asSome()
+        .getLocation()
+        .hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
+    or
+    this.isNone() and
+    filepath = "" and
+    startline = 0 and
+    startcolumn = 0 and
+    endline = 0 and
+    endcolumn = 0
+  }
+
+  string getQualifiedName() {
+    result = this.asSome().getQualifiedName()
+    or
+    this.isNone() and
+    result = ""
+  }
+}
+
+bindingset[name]
+FunctionOption getPossibleMissingSuperOption(Class base, Function shouldCall, string name) {
+  result.asSome() = getPossibleMissingSuper(base, shouldCall, name)
+  or
+  not exists(getPossibleMissingSuper(base, shouldCall, name)) and
+  result.isNone()
+}

python/ql/src/Classes/CallsToInitDel/MissingCallToDel.ql

@@ -0,0 +1,49 @@
+/**
+ * @name Missing call to superclass `__del__` during object destruction
+ * @description An omitted call to a superclass `__del__` method may lead to class instances not being cleaned up properly.
+ * @kind problem
+ * @tags quality
+ *       reliability
+ *       correctness
+ *       performance
+ * @problem.severity error
+ * @sub-severity low
+ * @precision high
+ * @id py/missing-call-to-delete
+ */
+
+import python
+import MethodCallOrder
+
+Function getDelMethod(Class c) {
+  result = c.getAMethod() and
+  result.getName() = "__del__"
+}
+
+from Class base, Function shouldCall, FunctionOption possibleIssue, string msg
+where
+  not exists(Function newMethod | newMethod = base.getAMethod() and newMethod.getName() = "__new__") and
+  exists(FunctionOption possiblyMissingSuper |
+    missingCallToSuperclassMethodRestricted(base, shouldCall, "__del__") and
+    possiblyMissingSuper = getPossibleMissingSuperOption(base, shouldCall, "__del__") and
+    (
+      not possiblyMissingSuper.isNone() and
+      possibleIssue = possiblyMissingSuper and
+      msg =
+        "This class does not call $@ during destruction. ($@ may be missing a call to super().__del__)"
+      or
+      possiblyMissingSuper.isNone() and
+      (
+        possibleIssue.asSome() = getDelMethod(base) and
+        msg =
+          "This class does not call $@ during destruction. ($@ may be missing a call to a base class __del__)"
+        or
+        not exists(getDelMethod(base)) and
+        possibleIssue.isNone() and
+        msg =
+          "This class does not call $@ during destruction. (The class lacks an __del__ method to ensure every base class __del__ is called.)"
+      )
+    )
+  )
+select base, msg, shouldCall, shouldCall.getQualifiedName(), possibleIssue,
+  possibleIssue.getQualifiedName()

python/ql/src/Classes/CallsToInitDel/MissingCallToInit.ql

@@ -0,0 +1,41 @@
+/**
+ * @name Missing call to superclass `__init__` during object initialization
+ * @description An omitted call to a superclass `__init__` method may lead to objects of this class not being fully initialized.
+ * @kind problem
+ * @tags quality
+ *       reliability
+ *       correctness
+ * @problem.severity error
+ * @sub-severity low
+ * @precision high
+ * @id py/missing-call-to-init
+ */
+
+import python
+import MethodCallOrder
+
+from Class base, Function shouldCall, FunctionOption possibleIssue, string msg
+where
+  exists(FunctionOption possiblyMissingSuper |
+    missingCallToSuperclassMethodRestricted(base, shouldCall, "__init__") and
+    possiblyMissingSuper = getPossibleMissingSuperOption(base, shouldCall, "__init__") and
+    (
+      possibleIssue.asSome() = possiblyMissingSuper.asSome() and
+      msg =
+        "This class does not call $@ during initialization. ($@ may be missing a call to super().__init__)"
+      or
+      possiblyMissingSuper.isNone() and
+      (
+        possibleIssue.asSome() = base.getInitMethod() and
+        msg =
+          "This class does not call $@ during initialization. ($@ may be missing a call to a base class __init__)"
+        or
+        not exists(base.getInitMethod()) and
+        possibleIssue.isNone() and
+        msg =
+          "This class does not call $@ during initialization. (The class lacks an __init__ method to ensure every base class __init__ is called.)"
+      )
+    )
+  )
+select base, msg, shouldCall, shouldCall.getQualifiedName(), possibleIssue,
+  possibleIssue.getQualifiedName()

python/ql/src/Classes/CallsToInitDel/SuperclassDelCalledMultipleTimes.ql

@@ -0,0 +1,30 @@
+/**
+ * @name Multiple calls to `__del__` during object destruction
+ * @description A duplicated call to a superclass `__del__` method may lead to class instances not be cleaned up properly.
+ * @kind problem
+ * @tags quality
+ *       reliability
+ *       correctness
+ * @problem.severity warning
+ * @sub-severity high
+ * @precision very-high
+ * @id py/multiple-calls-to-delete
+ */
+
+import python
+import MethodCallOrder
+
+predicate multipleCallsToSuperclassDel(Function meth, Function calledMulti) {
+  multipleCallsToSuperclassMethod(meth, calledMulti, "__del__")
+}
+
+from Function meth, Function calledMulti
+where
+  multipleCallsToSuperclassDel(meth, calledMulti) and
+  // Don't alert for multiple calls to a superclass del when a subclass will do.
+  not exists(Function subMulti |
+    multipleCallsToSuperclassDel(meth, subMulti) and
+    calledMulti.getScope() = getADirectSuperclass+(subMulti.getScope())
+  )
+select meth, "This delete method calls $@ multiple times.", calledMulti,
+  calledMulti.getQualifiedName()

python/ql/src/Classes/CallsToInitDel/SuperclassInitCalledMultipleTimes.ql

@@ -0,0 +1,30 @@
+/**
+ * @name Multiple calls to `__init__` during object initialization
+ * @description A duplicated call to a superclass `__init__` method may lead to objects of this class not being properly initialized.
+ * @kind problem
+ * @tags quality
+ *       reliability
+ *       correctness
+ * @problem.severity warning
+ * @sub-severity high
+ * @precision very-high
+ * @id py/multiple-calls-to-init
+ */
+
+import python
+import MethodCallOrder
+
+predicate multipleCallsToSuperclassInit(Function meth, Function calledMulti) {
+  multipleCallsToSuperclassMethod(meth, calledMulti, "__init__")
+}
+
+from Function meth, Function calledMulti
+where
+  multipleCallsToSuperclassInit(meth, calledMulti) and
+  // Don't alert for multiple calls to a superclass init when a subclass will do.
+  not exists(Function subMulti |
+    multipleCallsToSuperclassInit(meth, subMulti) and
+    calledMulti.getScope() = getADirectSuperclass+(subMulti.getScope())
+  )
+select meth, "This initialization method calls $@ multiple times.", calledMulti,
+  calledMulti.getQualifiedName()

python/ql/src/Classes/MethodCallOrder.qll

@@ -1,3 +1,5 @@
+deprecated module;
+
 import python
 
 // Helper predicates for multiple call to __init__/__del__ queries.