From 90424098b2b9ab982d581ca40d3825e1ff33ecf5 Mon Sep 17 00:00:00 2001
From: Asger F <asgerf@github.com>
Date: Fri, 5 Apr 2024 16:18:47 +0200
Subject: [PATCH] JS: Ensure MkClassInstance exists for base classes

---
 .../ql/lib/semmle/javascript/ApiGraphs.qll     | 18 ++++++++++++------
 .../ModelGeneration/ModelGeneration.expected   |  2 ++
 .../ModelGeneration/subclass/subclass.js       |  1 -
 3 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/javascript/ql/lib/semmle/javascript/ApiGraphs.qll b/javascript/ql/lib/semmle/javascript/ApiGraphs.qll
index 0345df3f00ece..5a03092df0c44 100644
--- a/javascript/ql/lib/semmle/javascript/ApiGraphs.qll
+++ b/javascript/ql/lib/semmle/javascript/ApiGraphs.qll
@@ -689,12 +689,7 @@ module API {
         any(Type t).hasUnderlyingType(m, _)
       } or
       MkClassInstance(DataFlow::ClassNode cls) {
-        hasSemantics(cls) and
-        (
-          cls = trackDefNode(_)
-          or
-          cls.getAnInstanceReference() = trackDefNode(_)
-        )
+        needsDefNode(cls)
       } or
       MkDef(DataFlow::Node nd) { rhs(_, _, nd) } or
       MkUse(DataFlow::Node nd) { use(_, _, nd) } or
@@ -708,6 +703,17 @@ module API {
         trackUseNode(src, true, bound, "").flowsTo(nd.getCalleeNode())
       }
 
+    private predicate needsDefNode(DataFlow::ClassNode cls) {
+      hasSemantics(cls) and
+      (
+        cls = trackDefNode(_)
+        or
+        cls.getAnInstanceReference() = trackDefNode(_)
+        or
+        needsDefNode(cls.getADirectSubClass())
+      )
+    }
+
     class TDef = MkModuleDef or TNonModuleDef;
 
     class TNonModuleDef = MkModuleExport or MkClassInstance or MkDef or MkSyntheticCallbackArg;
diff --git a/javascript/ql/test/library-tests/ModelGeneration/ModelGeneration.expected b/javascript/ql/test/library-tests/ModelGeneration/ModelGeneration.expected
index e3883154c3442..e6c93a1a0e8a8 100644
--- a/javascript/ql/test/library-tests/ModelGeneration/ModelGeneration.expected
+++ b/javascript/ql/test/library-tests/ModelGeneration/ModelGeneration.expected
@@ -37,6 +37,7 @@ typeModel
 | (subclass).A | subclass | Member[A] |
 | (subclass).A.prototype | (subclass).A | Instance |
 | (subclass).A.prototype | (subclass).B.prototype |  |
+| (subclass).A.prototype | (subclass).ExposedMidSubClass.prototype~expr1 |  |
 | (subclass).A.prototype.a | (subclass).A.prototype | Member[a] |
 | (subclass).B | subclass | Member[B] |
 | (subclass).B.prototype | (subclass).B | Instance |
@@ -51,6 +52,7 @@ typeModel
 | (subclass).ExposedMidSubClass | subclass | Member[ExposedMidSubClass] |
 | (subclass).ExposedMidSubClass.prototype | (subclass).ExposedMidSubClass | Instance |
 | (subclass).ExposedMidSubClass.prototype.m | (subclass).ExposedMidSubClass.prototype | Member[m] |
+| (subclass).ExposedMidSubClass.prototype~expr1 | (subclass).ExposedMidSubClass.prototype |  |
 | upstream-lib | (reexport).func | ReturnValue |
 | upstream-lib | reexport | Member[lib] |
 | upstream-lib.Type | (subclass).D.prototype |  |
diff --git a/javascript/ql/test/library-tests/ModelGeneration/subclass/subclass.js b/javascript/ql/test/library-tests/ModelGeneration/subclass/subclass.js
index 5ebab4f5fb2d9..6cbfbabad0e60 100644
--- a/javascript/ql/test/library-tests/ModelGeneration/subclass/subclass.js
+++ b/javascript/ql/test/library-tests/ModelGeneration/subclass/subclass.js
@@ -17,7 +17,6 @@ export class D extends upstream.Type {
 }
 
 // Test case where subclass chain goes through an internal class
-// TODO: we miss the subclass chain between ExposedMidSubClass and A
 class InternalMidClass extends A {}
 
 export class ExposedMidSubClass extends InternalMidClass {