From c4f5924d84501fbc6ccdd9f22a22c8d77e09e0fe Mon Sep 17 00:00:00 2001
From: Rasmus Wriedt Larsen <rasmuswl@github.com>
Date: Fri, 20 Oct 2023 13:33:54 +0200
Subject: [PATCH] Python: Fix problem if import is used

I fixed it in both predicates... I think we might still be able to remove
`newDirectAlias` -- but with it being better, it will allow us to better test if `newImportAlias` actually cover everything we need!
---
 .../frameworks/internal/SubclassFinder.qll    | 53 ++++++++++---------
 .../library-tests/FindSubclass/Find.expected  |  2 +
 2 files changed, 30 insertions(+), 25 deletions(-)

diff --git a/python/ql/lib/semmle/python/frameworks/internal/SubclassFinder.qll b/python/ql/lib/semmle/python/frameworks/internal/SubclassFinder.qll
index 5275860e50f..b2036bef955 100644
--- a/python/ql/lib/semmle/python/frameworks/internal/SubclassFinder.qll
+++ b/python/ql/lib/semmle/python/frameworks/internal/SubclassFinder.qll
@@ -156,29 +156,28 @@ module NotExposed {
    * ```
    */
   predicate newDirectAlias(
-    FindSubclassesSpec spec, string newAliasFullyQualified, ImportMember importMember, Module mod,
-    Location loc
+    FindSubclassesSpec spec, string newAliasFullyQualified, Expr value, Module mod, Location loc
   ) {
-    importMember =
-      newOrExistingModeling(spec).getASubclass*().getAValueReachableFromSource().asExpr() and
-    importMember.getScope() = mod and
-    loc = importMember.getLocation() and
-    exists(Alias alias, string base |
-      mod.isPackageInit() and base = mod.getPackageName()
-      or
-      not mod.isPackageInit() and base = mod.getName()
-    |
-      alias.getValue() = importMember and
-      newAliasFullyQualified = base + "." + alias.getAsname()
-    ) and
-    (
-      not hasAllStatement(mod)
-      or
-      mod.declaredInAll(importMember.getName())
-    ) and
-    not alreadyExplicitlyModeled(spec, newAliasFullyQualified) and
-    not isTestCode(importMember) and
-    isAllowedModule(mod)
+    exists(Alias alias | value = alias.getValue() |
+      value = newOrExistingModeling(spec).getASubclass*().getAValueReachableFromSource().asExpr() and
+      value.getScope() = mod and
+      loc = value.getLocation() and
+      exists(string base |
+        mod.isPackageInit() and base = mod.getPackageName()
+        or
+        not mod.isPackageInit() and base = mod.getName()
+      |
+        newAliasFullyQualified = base + "." + alias.getAsname().(Name).getId()
+      ) and
+      (
+        not hasAllStatement(mod)
+        or
+        mod.declaredInAll(alias.getAsname().(Name).getId())
+      ) and
+      not alreadyExplicitlyModeled(spec, newAliasFullyQualified) and
+      not isTestCode(value) and
+      isAllowedModule(mod)
+    )
   }
 
   /**
@@ -191,11 +190,15 @@ module NotExposed {
     string relevantName, Location loc
   ) {
     loc = mod.getLocation() and
-    exists(API::Node relevantClass, Expr value |
+    exists(API::Node relevantClass, ControlFlowNode value |
       relevantClass = newOrExistingModeling(spec).getASubclass*() and
       ImportResolution::module_export(mod, relevantName, def) and
-      value = relevantClass.getAValueReachableFromSource().asExpr() and
-      value = def.asVar().getDefinition().(AssignmentDefinition).getValue().getNode()
+      value = relevantClass.getAValueReachableFromSource().asCfgNode() and
+      (
+        value = def.asVar().getDefinition().(AssignmentDefinition).getValue()
+        or
+        value = def.asCfgNode()
+      )
       // value could be a ClassExpr if a new class is defined, or a Name if defining an alias
     ) and
     (
diff --git a/python/ql/test/experimental/library-tests/FindSubclass/Find.expected b/python/ql/test/experimental/library-tests/FindSubclass/Find.expected
index d15e406b7b1..9fa7ab7b525 100644
--- a/python/ql/test/experimental/library-tests/FindSubclass/Find.expected
+++ b/python/ql/test/experimental/library-tests/FindSubclass/Find.expected
@@ -6,9 +6,11 @@
 | flask.View~Subclass | find_subclass_test | Member[B] |
 | flask.View~Subclass | find_subclass_test | Member[ViewAliasInExcept] |
 | flask.View~Subclass | find_subclass_test | Member[ViewAliasInTry] |
+| flask.View~Subclass | find_subclass_test | Member[ViewAlias] |
 | flask.View~Subclass | find_subclass_test | Member[ViewAlias_no_use] |
 | flask.View~Subclass | find_subclass_test | Member[View] |
 | flask.View~Subclass | find_subclass_test | Member[clash2] |
 | flask.View~Subclass | find_subclass_test | Member[clash3] |
 | flask.View~Subclass | find_subclass_test | Member[clash] |
+| flask.View~Subclass | find_subclass_test | Member[complete_module_alias] |
 | flask.View~Subclass | find_subclass_test | Member[complete_module_alias_no_use] |