JS: Refactor default import interop

Author: asgerf

javascript/ql/lib/semmle/javascript/ApiGraphs.qll

@@ -822,7 +822,7 @@ module API {
           or
           // special case: from `require('m')` to an export of `prop` in `m`
           exists(Import imp, Module m, string prop |
-            pred = imp.getImportedModuleNode() and
+            pred = imp.getImportedModuleNodeIfUnambiguous() and
             m = imp.getImportedModule() and
             lbl = Label::member(prop) and
             rhs = m.getAnExportedValue(prop)
@@ -1337,7 +1337,9 @@ module API {
       result = nd.getALocalSource()
       or
       // additional backwards step from `require('m')` to `exports` or `module.exports` in m
-      exists(Import imp | imp.getImportedModuleNode() = trackDefNode(nd, t.continue()) |
+      exists(Import imp |
+        imp.getImportedModuleNodeIfUnambiguous() = trackDefNode(nd, t.continue())
+      |
         result = DataFlow::exportsVarNode(imp.getImportedModule())
         or
         result = DataFlow::moduleVarNode(imp.getImportedModule()).getAPropertyRead("exports")

javascript/ql/lib/semmle/javascript/ES2015Modules.qll

@@ -130,18 +130,18 @@ class ImportDeclaration extends Stmt, Import, @import_declaration {
 
   override DataFlow::Node getImportedModuleNode() {
     // `import * as http from 'http'` or `import http from `http`'
+    not exists(DataFlow::destructuredModuleImportNode(this)) and
     exists(ImportSpecifier is |
       is = this.getASpecifier() and
       result = DataFlow::valueNode(is)
     |
-      is instanceof ImportNamespaceSpecifier and
-      count(this.getASpecifier()) = 1
+      is instanceof ImportNamespaceSpecifier
       or
       // For compatibility with the non-standard implementation of default imports,
-      // treat default imports as namespace imports in cases where it can't cause ambiguity
-      // between named exports and the properties of a default-exported object.
-      not this.getImportedModule().(ES2015Module).hasBothNamedAndDefaultExports() and
-      is.getImportedName() = "default"
+      // treat default imports as namespace imports. In cases where it causes ambiguity
+      // between named exports and the properties of a default-exported object, the caller
+      // of `getImportedModuleNode()` must check `isDefaultImport()` and correct the behaviour.
+      this.hasOnlyDefaultImport()
     )
     or
     // `import { createServer } from 'http'`
@@ -152,6 +152,12 @@ class ImportDeclaration extends Stmt, Import, @import_declaration {
   predicate isTypeOnly() { has_type_keyword(this) }
 
   override string getAPrimaryQlClass() { result = "ImportDeclaration" }
+
+  private predicate hasOnlyDefaultImport() {
+    unique( | | this.getASpecifier()) instanceof ImportDefaultSpecifier
+  }
+
+  override predicate isDefaultImport() { this.hasOnlyDefaultImport() }
 }
 
 /** A literal path expression appearing in an `import` declaration. */

javascript/ql/lib/semmle/javascript/Modules.qll

@@ -179,7 +179,58 @@ abstract class Import extends AstNode {
   }
 
   /**
-   * Gets the data flow node that the default import of this import is available at.
+   * Gets the data flow node referring to imported module object.
+   *
+   * For example:
+   * ```js
+   * // ES2015 style
+   * import * as foo from "bar"; // gets the node for `foo`
+   * import foo from "bar"; // gets the node for `foo` (see note on default imports below)
+   *
+   * // CommonJS style
+   * require("bar"); // gets the node for the `require` call
+   *
+   * // AMD style
+   * define(["bar"], function(foo) { // gets the node for the `foo` parameter
+   * })
+   * ```
+   *
+   * For statements of form `import foo from "bar'`, this gives the node corresponding to `foo`.
+   * Technically this should refer to the export binding named `"default"`, not the whole module, but for compatibility with non-standard
+   * interpretations of default imports, this node is usually treated as also referring to the whole module.
+   * If this behaviour is not wanted, use `isDefaultImport()` to handle that case differently.
    */
   abstract DataFlow::Node getImportedModuleNode();
+
+  /**
+   * Holds of the result of `getImportedModuleNode` actually refers to the export binding named `"default"`,
+   * as opposed an object whose properties correspond to the export bindings of the imported module.
+   *
+   * For compatibility with non-standard interpretations of `default` imports, the default
+   * import is usually returned by `getImportedModuleNode()`. If such behaviour is not wanted,
+   * this predicate can be used to handle that case differently.
+   *
+   * For example, `getImportedModuleNode()` returns `foo` in both of these imports, but `isDefaultImport()`
+   * only holds for the first one:
+   * ```js
+   * import foo from "bar";
+   * import * as foo from "bar";
+   * ```
+   */
+  predicate isDefaultImport() { none() }
+
+  /**
+   * Gets the same as `getImportedModuleNode()` expect this has no result for default imports when the target module
+   * has both default and named exports.
+   *
+   * This is to avoid ambiguity between named export bindings and the properties of the default-exported object.
+   */
+  pragma[nomagic]
+  final DataFlow::Node getImportedModuleNodeIfUnambiguous() {
+    if
+      this.isDefaultImport() and
+      this.getImportedModule().(ES2015Module).hasBothNamedAndDefaultExports()
+    then none()
+    else result = this.getImportedModuleNode()
+  }
 }