Python: Extend import resolution tests

Extends the tests to

1. Account parts of the test code that may be specific to Python 2 or 3,
2. Also track which arguments passed to `check` are references to
   modules.

The latter revealed a bunch of spurious results, which I have annotated
accordingly.

Author: tausbn

python/ql/test/experimental/import-resolution/importflow.ql

@@ -2,7 +2,9 @@ import python
 import semmle.python.dataflow.new.DataFlow
 import semmle.python.ApiGraphs
 import TestUtilities.InlineExpectationsTest
+import semmle.python.dataflow.new.internal.ImportResolution
 
+/** A string that appears on the right hand side of an assignment. */
 private class SourceString extends DataFlow::Node {
   string contents;
 
@@ -14,6 +16,45 @@ private class SourceString extends DataFlow::Node {
   string getContents() { result = contents }
 }
 
+/** An argument that is checked using the `check` function. */
+private class CheckArgument extends DataFlow::Node {
+  CheckArgument() { this = API::moduleImport("trace").getMember("check").getACall().getArg(1) }
+}
+
+/** A data-flow node that is a reference to a module. */
+private class ModuleRef extends DataFlow::Node {
+  Module mod;
+
+  ModuleRef() {
+    this = ImportResolution::getModuleReference(mod) and
+    not mod.getName() in ["__future__", "trace"]
+  }
+
+  string getName() { result = mod.getName() }
+}
+
+/**
+ * A data-flow node that is guarded by a version check. Only supports checks of the form `if
+ *sys.version_info[0] == ...` where the right hand side is either `2` or `3`.
+ */
+private class VersionGuardedNode extends DataFlow::Node {
+  int version;
+
+  VersionGuardedNode() {
+    version in [2, 3] and
+    exists(If parent, CompareNode c | parent.getBody().contains(this.asExpr()) |
+      c.operands(API::moduleImport("sys")
+            .getMember("version_info")
+            .getASubscript()
+            .asSource()
+            .asCfgNode(), any(Eq eq),
+        any(IntegerLiteral lit | lit.getValue() = version).getAFlowNode())
+    )
+  }
+
+  int getVersion() { result = version }
+}
+
 private class ImportConfiguration extends DataFlow::Configuration {
   ImportConfiguration() { this = "ImportConfiguration" }
 
@@ -30,12 +71,29 @@ class ResolutionTest extends InlineExpectationsTest {
   override string getARelevantTag() { result = "prints" }
 
   override predicate hasActualResult(Location location, string element, string tag, string value) {
-    exists(DataFlow::PathNode source, DataFlow::PathNode sink, ImportConfiguration config |
-      config.hasFlowPath(source, sink) and
-      tag = "prints" and
-      location = sink.getNode().getLocation() and
-      value = source.getNode().(SourceString).getContents() and
-      element = sink.getNode().toString()
+    (
+      exists(DataFlow::PathNode source, DataFlow::PathNode sink, ImportConfiguration config |
+        config.hasFlowPath(source, sink) and
+        correct_version(sink.getNode()) and
+        tag = "prints" and
+        location = sink.getNode().getLocation() and
+        value = source.getNode().(SourceString).getContents() and
+        element = sink.getNode().toString()
+      )
+      or
+      exists(ModuleRef ref |
+        correct_version(ref) and
+        ref instanceof CheckArgument and
+        tag = "prints" and
+        location = ref.getLocation() and
+        value = "\"<module " + ref.getName() + ">\"" and
+        element = ref.toString()
+      )
     )
   }
 }
+
+private predicate correct_version(DataFlow::Node n) {
+  not n instanceof VersionGuardedNode or
+  n.(VersionGuardedNode).getVersion() = major_version()
+}

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

@@ -39,15 +39,15 @@
 
 # A simple "import from" statement.
 from bar import bar_attr
-check("bar_attr", bar_attr, "bar_attr", globals()) #$ prints=bar_attr
+check("bar_attr", bar_attr, "bar_attr", globals()) #$ prints=bar_attr SPURIOUS: prints="<module bar>"
 
 # Importing an attribute from a subpackage of a package.
 from package.subpackage import subpackage_attr
-check("subpackage_attr", subpackage_attr, "subpackage_attr", globals()) #$ prints=subpackage_attr
+check("subpackage_attr", subpackage_attr, "subpackage_attr", globals()) #$ prints=subpackage_attr SPURIOUS: prints="<module package.subpackage.__init__>"
 
 # Importing a package attribute under an alias.
 from package import package_attr as package_attr_alias
-check("package_attr_alias", package_attr_alias,  "package_attr", globals()) #$ prints=package_attr
+check("package_attr_alias", package_attr_alias,  "package_attr", globals()) #$ prints=package_attr SPURIOUS: prints="<module package.__init__>"
 
 # Importing a subpackage under an alias.
 from package import subpackage as aliased_subpackage #$ imports=package.subpackage.__init__ as=aliased_subpackage
@@ -68,15 +68,15 @@ def local_import():
 import package.subpackage #$ imports=package.__init__ as=package
 check("package.package_attr", package.package_attr, "package_attr", globals()) #$ prints=package_attr
 
-if sys.version_info[0] >= 3:
+if sys.version_info[0] == 3:
     # Importing from a namespace module.
     from namespace_package.namespace_module import namespace_module_attr
-    check("namespace_module_attr", namespace_module_attr, "namespace_module_attr", globals()) #$ prints=namespace_module_attr
+    check("namespace_module_attr", namespace_module_attr, "namespace_module_attr", globals()) #$ prints=namespace_module_attr SPURIOUS: prints="<module namespace_package.namespace_module>"
 
 
 from attr_clash import clashing_attr, non_clashing_submodule #$ imports=attr_clash.clashing_attr as=clashing_attr imports=attr_clash.non_clashing_submodule as=non_clashing_submodule
-check("clashing_attr", clashing_attr, "clashing_attr", globals()) #$ prints=clashing_attr
-check("non_clashing_submodule", non_clashing_submodule, "<module attr_clash.non_clashing_submodule>", globals())
+check("clashing_attr", clashing_attr, "clashing_attr", globals()) #$ prints=clashing_attr SPURIOUS: prints="<module attr_clash.clashing_attr>" SPURIOUS: prints="<module attr_clash.__init__>"
+check("non_clashing_submodule", non_clashing_submodule, "<module attr_clash.non_clashing_submodule>", globals()) #$ prints="<module attr_clash.non_clashing_submodule>" SPURIOUS: prints="<module attr_clash.__init__>"
 
 exit(__file__)
 

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

@@ -5,7 +5,7 @@
 
 # Importing an attribute from the parent package.
 from .. import attr_used_in_subpackage as imported_attr
-check("imported_attr", imported_attr,  "attr_used_in_subpackage", globals()) #$ prints=attr_used_in_subpackage
+check("imported_attr", imported_attr,  "attr_used_in_subpackage", globals()) #$ prints=attr_used_in_subpackage SPURIOUS: prints="<module package.__init__>"
 
 # Importing an irrelevant attribute from a sibling module binds the name to the module.
 from .submodule import irrelevant_attr