Merge pull request #12193 from RasmusWL/import-resolution-fixup

Python: Fix `from <pkg> import *` import resolution

python/ql/lib/change-notes/2023-02-15-import-star-package.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* Fixed module resolution so we properly recognize that in `from <pkg> import *`, where `<pkg>` is a package, the actual imports are made from the `<pkg>/__init__.py` file.

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

@@ -167,8 +167,22 @@ module ImportResolution {
     )
   }
 
+  /**
+   * Gets the (most likely) module for the name `name`, if any.
+   *
+   * Handles the fact that for the name `<pkg>` representing a package the actual module
+   * is `<pkg>.__init__`.
+   *
+   * See `isPreferredModuleForName` for more details on what "most likely" module means.
+   */
+  pragma[inline]
+  private Module getModuleFromName(string name) {
+    isPreferredModuleForName(result.getFile(), name + ["", ".__init__"])
+  }
+
+  /** Gets the module from which attributes are imported by `i`. */
   Module getModuleImportedByImportStar(ImportStar i) {
-    isPreferredModuleForName(result.getFile(), i.getImportedModuleName())
+    result = getModuleFromName(i.getImportedModuleName())
   }
 
   /**
@@ -223,7 +237,7 @@ module ImportResolution {
     exists(string module_name | result = getReferenceToModuleName(module_name) |
       // Depending on whether the referenced module is a package or not, we may need to add a
       // trailing `.__init__` to the module name.
-      isPreferredModuleForName(m.getFile(), module_name + ["", ".__init__"])
+      m = getModuleFromName(module_name)
       or
       // Module defined via `sys.modules`
       m = sys_modules_module_with_name(module_name)
@@ -234,7 +248,7 @@ module ImportResolution {
       ar.accesses(getModuleReference(p), attr_name) and
       result = ar
     |
-      isPreferredModuleForName(m.getFile(), p.getPackageName() + "." + attr_name + ["", ".__init__"])
+      m = getModuleFromName(p.getPackageName() + "." + attr_name)
     )
     or
     // This is also true for attributes that come from reexports.
@@ -248,8 +262,7 @@ module ImportResolution {
     exists(string submodule, Module package |
       SsaSource::init_module_submodule_defn(result.asVar().getSourceVariable(),
         package.getEntryNode()) and
-      isPreferredModuleForName(m.getFile(),
-        package.getPackageName() + "." + submodule + ["", ".__init__"])
+      m = getModuleFromName(package.getPackageName() + "." + submodule)
     )
   }
 

python/ql/test/experimental/import-resolution/main.py

@@ -84,6 +84,12 @@ from attr_clash import clashing_attr, non_clashing_submodule #$ imports=attr_cla
 check("clashing_attr", clashing_attr, "clashing_attr", globals()) #$ prints=clashing_attr SPURIOUS: prints="<module attr_clash.clashing_attr>"
 check("non_clashing_submodule", non_clashing_submodule, "<module attr_clash.non_clashing_submodule>", globals()) #$ prints="<module attr_clash.non_clashing_submodule>"
 
+
+# check that import * from an __init__ file works
+from package.subpackage2 import *
+check("subpackage2_attr", subpackage2_attr, "subpackage2_attr", globals()) #$ prints=subpackage2_attr
+
+
 exit(__file__)
 
 print()
@@ -91,4 +97,4 @@ print()
 if status() == 0:
     print("PASS")
 else:
-    print("FAIL")
+    sys.exit("FAIL")

python/ql/test/experimental/import-resolution/package/subpackage2/__init__.py

@@ -0,0 +1,6 @@
+from trace import *
+enter(__file__)
+
+subpackage2_attr = "subpackage2_attr"
+
+exit(__file__)