Repair Closure implementation

Author: asgerf

javascript/ql/lib/semmle/javascript/Closure.qll

@@ -5,6 +5,30 @@
 import javascript
 
 module Closure {
+  /** A call to `goog.require` */
+  class RequireCallExpr extends CallExpr {
+    RequireCallExpr() { this.getCallee().(PropAccess).getQualifiedName() = "goog.require" }
+
+    /** Gets the imported namespace name. */
+    string getClosureNamespace() { result = this.getArgument(0).getStringValue() }
+  }
+
+  /** A call to `goog.module` or `goog.declareModuleId`. */
+  private class ModuleDeclarationCall extends CallExpr {
+    private string kind;
+
+    ModuleDeclarationCall() {
+      this.getCallee().(PropAccess).getQualifiedName() = kind and
+      kind = ["goog.module", "goog.declareModuleId"]
+    }
+
+    /** Gets the declared namespace. */
+    string getClosureNamespace() { result = this.getArgument(0).getStringValue() }
+
+    /** Gets the string `goog.module` or `goog.declareModuleId` depending on which method is being called. */
+    string getModuleKind() { result = kind }
+  }
+
   /**
    * A reference to a Closure namespace.
    */
@@ -109,38 +133,26 @@ module Closure {
   class ClosureModuleDeclaration extends ClosureNamespaceRef, DataFlow::MethodCallNode instanceof DefaultClosureModuleDeclaration
   { }
 
-  private GlobalVariable googVariable() { variables(result, "goog", any(GlobalScope sc)) }
-
-  pragma[nomagic]
-  private MethodCallExpr googModuleDeclExpr() {
-    result.getReceiver() = googVariable().getAnAccess() and
-    result.getMethodName() = ["module", "declareModuleId"]
-  }
-
-  pragma[nomagic]
-  private MethodCallExpr googModuleDeclExprInContainer(StmtContainer container) {
-    result = googModuleDeclExpr() and
-    container = result.getContainer()
-  }
-
   pragma[noinline]
-  private ClosureRequireCall getARequireInTopLevel(ClosureModule m) { result.getTopLevel() = m }
+  private RequireCallExpr getARequireInTopLevel(ClosureModule m) { result.getTopLevel() = m }
 
   /**
    * A module using the Closure module system, declared using `goog.module()` or `goog.declareModuleId()`.
    */
   class ClosureModule extends Module {
-    ClosureModule() { exists(googModuleDeclExprInContainer(this)) }
+    private ModuleDeclarationCall decl;
+
+    ClosureModule() { decl.getTopLevel() = this }
 
     /**
      * Gets the call to `goog.module` or `goog.declareModuleId` in this module.
      */
-    ClosureModuleDeclaration getModuleDeclaration() { result.getTopLevel() = this }
+    deprecated ClosureModuleDeclaration getModuleDeclaration() { result.getTopLevel() = this }
 
     /**
      * Gets the namespace of this module.
      */
-    string getClosureNamespace() { result = this.getModuleDeclaration().getClosureNamespace() }
+    string getClosureNamespace() { result = decl.getClosureNamespace() }
 
     override Module getAnImportedModule() {
       result.(ClosureModule).getClosureNamespace() =
@@ -156,7 +168,7 @@ module Closure {
      * Has no result for ES6 modules using `goog.declareModuleId`.
      */
     Variable getExportsVariable() {
-      this.getModuleDeclaration().getMethodName() = "module" and
+      decl.getModuleKind() = "goog.module" and
       result = this.getScope().getVariable("exports")
     }
 

javascript/ql/lib/semmle/javascript/internal/NameResolution.qll

@@ -138,14 +138,13 @@ module NameResolution {
       node2 = type
     )
     or
-    exists(ClosureModuleDeclaration decl, ClosureImport imprt |
-      decl.getClosureNamespace() = imprt.getClosureNamespace() and
-      node1 = decl.getContainer() and
-      node2 = imprt
+    exists(Closure::RequireCallExpr req |
+      node1.(Closure::ClosureModule).getClosureNamespace() = req.getClosureNamespace() and
+      node2 = req
     )
     or
     exists(Closure::ClosureModule mod |
-      node1 = mod.getScope().getVariable("exports") and
+      node1 = mod.getExportsVariable() and
       node2 = mod
     )
     or
@@ -155,45 +154,6 @@ module NameResolution {
     )
   }
 
-  class ClosureImport extends CallExpr {
-    ClosureImport() { this.getCallee().(PropAccess).getQualifiedName() = "goog.require" }
-
-    string getClosureNamespace() { result = this.getArgument(0).getStringValue() }
-  }
-
-  class ClosureModuleDeclaration extends CallExpr {
-    private string kind;
-
-    ClosureModuleDeclaration() {
-      this.getCallee().(PropAccess).getQualifiedName() = kind and
-      kind = ["goog.module", "goog.declareModuleId"]
-    }
-
-    string getClosureNamespace() { result = this.getArgument(0).getStringValue() }
-
-    string getModuleKind() { result = kind }
-  }
-
-  class ClosureExport extends AssignExpr {
-    private PropAccess lhs;
-
-    ClosureExport() {
-      exists(ClosureModuleDeclaration decl, Module mod |
-        decl.getModuleKind() = "goog.module" and
-        decl.getContainer() = mod and
-        this.getTopLevel() = mod and
-        this.getLhs() = lhs and
-        lhs.getBase() = mod.getScope().getVariable("exports").getAnAccess()
-      )
-    }
-
-    string getName() { result = lhs.getPropertyName() }
-
-    Module getModule() { result = this.getTopLevel() }
-
-    Expr getValue() { result = this.getRhs() }
-  }
-
   /**
    * Holds if there is a read from `node1` to `node2` that accesses the member `name`.
    */
@@ -320,10 +280,9 @@ module NameResolution {
         result = enum.getMemberByName(name).getIdentifier()
       )
       or
-      exists(ClosureExport exprt |
-        mod = exprt.getModule() and
-        name = exprt.getName() and
-        result = exprt.getValue()
+      exists(AssignExpr assign, PropAccess lhs |
+        lhs.accesses(mod.(Closure::ClosureModule).getExportsVariable().getAnAccess(), name) and
+        result = assign.getRhs()
       )
     }
 
@@ -465,7 +424,7 @@ module NameResolution {
     )
     or
     mod = "global" and
-    qualifiedName = node.(ClosureImport).getClosureNamespace()
+    qualifiedName = node.(Closure::RequireCallExpr).getClosureNamespace()
     or
     // Additionally track through bulk re-exports (`export * from 'mod`).
     // These are normally handled by 'exportAs' which supports various shadowing rules,