github · asgerf · May 6, 2020 · May 1, 2020 · May 1, 2020 · May 1, 2020
diff --git a/javascript/ql/src/semmle/javascript/AST.qll b/javascript/ql/src/semmle/javascript/AST.qll
@@ -3,6 +3,7 @@
  */
 
 import javascript
+private import internal.StmtContainers
 
 /**
  * A program element corresponding to JavaScript code, such as an expression
@@ -20,7 +21,7 @@ import javascript
  * abs(-42);
  * ```
  */
-class ASTNode extends @ast_node, Locatable {
+class ASTNode extends @ast_node, NodeInStmtContainer {
   override Location getLocation() { hasLocation(this, result) }
 
   override File getFile() {

diff --git a/javascript/ql/src/semmle/javascript/BasicBlocks.qll b/javascript/ql/src/semmle/javascript/BasicBlocks.qll
@@ -4,6 +4,7 @@
  */
 
 import javascript
+private import internal.StmtContainers
 
 /**
  * Holds if `nd` starts a new basic block.
@@ -114,7 +115,7 @@ private predicate bbIPostDominates(BasicBlock dom, BasicBlock bb) =
  *
  * At the database level, a basic block is represented by its first control flow node.
  */
-class BasicBlock extends @cfg_node, Locatable {
+class BasicBlock extends @cfg_node, NodeInStmtContainer {
   BasicBlock() { startsBB(this) }
 
   /** Gets a basic block succeeding this one. */
@@ -271,11 +272,6 @@ class BasicBlock extends @cfg_node, Locatable {
     exists(int n | defAt(n, v, d) | not exists(int m | m < n | defAt(m, v, _)))
   }
 
-  /**
-   * Gets the function or script to which this basic block belongs.
-   */
-  StmtContainer getContainer() { result = getFirstNode().getContainer() }
-
   /**
    * Gets the basic block that immediately dominates this basic block.
    */

diff --git a/javascript/ql/src/semmle/javascript/CFG.qll b/javascript/ql/src/semmle/javascript/CFG.qll
@@ -274,12 +274,13 @@
  */
 
 import javascript
+private import internal.StmtContainers
 
 /**
  * A node in the control flow graph, which is an expression, a statement,
  * or a synthetic node.
  */
-class ControlFlowNode extends @cfg_node, Locatable {
+class ControlFlowNode extends @cfg_node, Locatable, NodeInStmtContainer {
   /** Gets a node succeeding this node in the CFG. */
   ControlFlowNode getASuccessor() { successor(this, result) }
 
@@ -324,17 +325,6 @@ class ControlFlowNode extends @cfg_node, Locatable {
     // note the override in ControlFlowEntryNode below
   }
 
-  /** Gets the function or toplevel whose CFG this node belongs to. */
-  cached
-  StmtContainer getContainer() {
-    result = this.(Expr).getContainer() or
-    result = this.(Stmt).getContainer() or
-    result = this.(Property).getContainer() or
-    result = this.(PropertyPattern).getContainer() or
-    result = this.(ClassDefinition).getContainer() or
-    result = this.(MemberDeclaration).getContainer()
-  }
-
   /** Gets the basic block this node belongs to. */
   BasicBlock getBasicBlock() { this = result.getANode() }
 
@@ -364,17 +354,13 @@ class SyntheticControlFlowNode extends @synthetic_cfg_node, ControlFlowNode {
 
 /** A synthetic CFG node marking the entry point of a function or toplevel script. */
 class ControlFlowEntryNode extends SyntheticControlFlowNode, @entry_node {
-  override StmtContainer getContainer() { entry_cfg_node(this, result) }
-
   override predicate isUnreachable() { none() }
 
   override string toString() { result = "entry node of " + getContainer().toString() }
 }
 
 /** A synthetic CFG node marking the exit of a function or toplevel script. */
 class ControlFlowExitNode extends SyntheticControlFlowNode, @exit_node {
-  override StmtContainer getContainer() { exit_cfg_node(this, result) }
-
   override predicate isAFinalNode() { any() }
 
   override string toString() { result = "exit node of " + getContainer().toString() }
@@ -396,8 +382,6 @@ class GuardControlFlowNode extends SyntheticControlFlowNode, @guard_node {
     this = bb.getANode() or
     dominates(bb.getImmediateDominator())
   }
-
-  override StmtContainer getContainer() { result = getTest().getContainer() }
 }
 
 /**

diff --git a/javascript/ql/src/semmle/javascript/Classes.qll b/javascript/ql/src/semmle/javascript/Classes.qll
@@ -42,9 +42,6 @@ class ClassOrInterface extends @classorinterface, TypeParameterized {
     result = getIdentifier().getName() // Overridden in ClassExpr
   }
 
-  /** Gets the nearest enclosing function or toplevel in which this class or interface occurs. */
-  StmtContainer getContainer() { result = this.(ExprOrStmt).getContainer() }
-
   /** Gets a member declared in this class or interface. */
   MemberDeclaration getAMember() { result.getDeclaringType() = this }
 
@@ -278,9 +275,6 @@ class ClassDefinition extends @classdefinition, ClassOrInterface, AST::ValueNode
  * ```
  */
 class ClassDeclStmt extends @classdeclstmt, ClassDefinition, Stmt {
-  /** Gets the nearest enclosing function or toplevel in which this class declaration occurs. */
-  override StmtContainer getContainer() { result = Stmt.super.getContainer() }
-
   override ControlFlowNode getFirstControlFlowNode() {
     if hasDeclareKeyword(this) then result = this else result = getIdentifier()
   }
@@ -323,9 +317,6 @@ class ClassExpr extends @classexpr, ClassDefinition, Expr {
 
   override predicate isImpure() { none() }
 
-  /** Gets the nearest enclosing function or toplevel in which this class expression occurs. */
-  override StmtContainer getContainer() { result = Expr.super.getContainer() }
-
   override ControlFlowNode getFirstControlFlowNode() {
     if exists(getIdentifier())
     then result = getIdentifier()
@@ -545,9 +536,6 @@ class MemberDeclaration extends @property, Documentable {
   /** Gets the index of this member within its enclosing type. */
   int getMemberIndex() { properties(this, _, result, _, _) }
 
-  /** Gets the nearest enclosing function or toplevel in which this member occurs. */
-  StmtContainer getContainer() { result = getDeclaringType().getContainer() }
-
   /** Holds if the name of this member is computed by an impure expression. */
   predicate hasImpureNameExpr() { isComputed() and getNameExpr().isImpure() }
 

diff --git a/javascript/ql/src/semmle/javascript/Closure.qll b/javascript/ql/src/semmle/javascript/Closure.qll
@@ -115,18 +115,25 @@ module Closure {
     override DefaultClosureModuleDeclaration range;
   }
 
+  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()
+  }
+
   /**
    * A module using the Closure module system, declared using `goog.module()` or `goog.declareModuleId()`.
    */
   class ClosureModule extends Module {
-    ClosureModule() {
-      // Use AST-based predicate to cut recursive dependencies.
-      exists(MethodCallExpr call |
-        getAStmt().(ExprStmt).getExpr() = call and
-        call.getReceiver().(GlobalVarAccess).getName() = "goog" and
-        (call.getMethodName() = "module" or call.getMethodName() = "declareModuleId")
-      )
-    }
+    ClosureModule() { exists(googModuleDeclExprInContainer(this)) }
 
     /**
      * Gets the call to `goog.module` or `goog.declareModuleId` in this module.

diff --git a/javascript/ql/src/semmle/javascript/Expr.qll b/javascript/ql/src/semmle/javascript/Expr.qll
@@ -21,12 +21,6 @@ class ExprOrType extends @exprortype, Documentable {
   /** Gets the function in which this expression or type appears, if any. */
   Function getEnclosingFunction() { result = getContainer() }
 
-  /**
-   * Gets the statement container (function or toplevel) in which
-   * this expression or type appears.
-   */
-  StmtContainer getContainer() { exprContainers(this, result) }
-
   /**
    * Gets the JSDoc comment associated with this expression or type or its parent statement, if any.
    */
@@ -107,12 +101,6 @@ class ExprOrType extends @exprortype, Documentable {
  * ```
  */
 class Expr extends @expr, ExprOrStmt, ExprOrType, AST::ValueNode {
-  /**
-   * Gets the statement container (function or toplevel) in which
-   * this expression appears.
-   */
-  override StmtContainer getContainer() { exprContainers(this, result) }
-
   /** Gets this expression, with any surrounding parentheses removed. */
   override Expr stripParens() { result = this }
 
@@ -246,25 +234,32 @@ class Expr extends @expr, ExprOrStmt, ExprOrType, AST::ValueNode {
     )
   }
 
+  pragma[inline]
+  private Stmt getRawEnclosingStmt(Expr e) {
+    // For performance reasons, we need the enclosing statement without overrides
+    enclosingStmt(e, result)
+  }
+
   /**
    * Gets the data-flow node where exceptions thrown by this expression will
    * propagate if this expression causes an exception to be thrown.
    */
+  pragma[inline]
   DataFlow::Node getExceptionTarget() {
-    if exists(this.getEnclosingStmt().getEnclosingTryCatchStmt())
-    then
-      result =
-        DataFlow::parameterNode(this
-              .getEnclosingStmt()
-              .getEnclosingTryCatchStmt()
-              .getACatchClause()
-              .getAParameter())
-    else
-      result =
-        any(DataFlow::FunctionNode f | f.getFunction() = this.getContainer()).getExceptionalReturn()
+    result = getCatchParameterFromStmt(getRawEnclosingStmt(this))
+    or
+    not exists(getCatchParameterFromStmt(getRawEnclosingStmt(this))) and
+    result =
+      any(DataFlow::FunctionNode f | f.getFunction() = this.getContainer()).getExceptionalReturn()
   }
 }
 
+cached
+private DataFlow::Node getCatchParameterFromStmt(Stmt stmt) {
+  result =
+    DataFlow::parameterNode(stmt.getEnclosingTryCatchStmt().getACatchClause().getAParameter())
+}
+
 /**
  * An identifier.
  *
@@ -633,9 +628,6 @@ class Property extends @property, Documentable {
   /** Gets the (0-based) index at which this property appears in its enclosing literal. */
   int getIndex() { this = getObjectExpr().getProperty(result) }
 
-  /** Gets the function or toplevel in which this property occurs. */
-  StmtContainer getContainer() { result = getObjectExpr().getContainer() }
-
   /**
    * Holds if this property is impure, that is, the evaluation of its name or
    * its initializer expression could have side effects.

diff --git a/javascript/ql/src/semmle/javascript/JSDoc.qll b/javascript/ql/src/semmle/javascript/JSDoc.qll
@@ -191,8 +191,6 @@ class JSDocTypeExpr extends @jsdoc_type_expr, JSDocTypeExprParent, TypeAnnotatio
     )
   }
 
-  override StmtContainer getContainer() { result = getEnclosingStmt().getContainer() }
-
   override Function getEnclosingFunction() { result = getContainer() }
 
   override TopLevel getTopLevel() { result = getEnclosingStmt().getTopLevel() }

diff --git a/javascript/ql/src/semmle/javascript/Stmt.qll b/javascript/ql/src/semmle/javascript/Stmt.qll
@@ -18,9 +18,6 @@ import javascript
  * ```
  */
 class Stmt extends @stmt, ExprOrStmt, Documentable {
-  /** Gets the statement container (toplevel, function or namespace) to which this statement belongs. */
-  override StmtContainer getContainer() { stmtContainers(this, result) }
-
   /** Holds if this statement has an implicitly inserted semicolon. */
   predicate hasSemicolonInserted() {
     isSubjectToSemicolonInsertion() and

diff --git a/javascript/ql/src/semmle/javascript/TypeAnnotations.qll b/javascript/ql/src/semmle/javascript/TypeAnnotations.qll
@@ -3,11 +3,12 @@
  */
 
 import javascript
+private import internal.StmtContainers
 
 /**
  * A type annotation, either in the form of a TypeScript type or a JSDoc comment.
  */
-class TypeAnnotation extends @type_annotation, Locatable {
+class TypeAnnotation extends @type_annotation, NodeInStmtContainer {
   /** Holds if this is the `any` type. */
   predicate isAny() { none() }
 
@@ -89,11 +90,6 @@ class TypeAnnotation extends @type_annotation, Locatable {
   /** Gets the function in which this type appears, if any. */
   Function getEnclosingFunction() { none() }
 
-  /**
-   * Gets the statement container (function or toplevel) in which this type appears.
-   */
-  StmtContainer getContainer() { none() }
-
   /**
    * Gets the top-level containing this type annotation.
    */

diff --git a/javascript/ql/src/semmle/javascript/TypeScript.qll b/javascript/ql/src/semmle/javascript/TypeScript.qll
@@ -276,8 +276,6 @@ class InterfaceDeclaration extends Stmt, InterfaceDefinition, @interfacedeclarat
     )
   }
 
-  override StmtContainer getContainer() { result = Stmt.super.getContainer() }
-
   override string describe() { result = "interface " + getName() }
 
   /**
@@ -299,8 +297,6 @@ class InterfaceDeclaration extends Stmt, InterfaceDefinition, @interfacedeclarat
 class InterfaceTypeExpr extends TypeExpr, InterfaceDefinition, @interfacetypeexpr {
   override Identifier getIdentifier() { none() }
 
-  override StmtContainer getContainer() { result = TypeExpr.super.getContainer() }
-
   override string describe() { result = "anonymous interface" }
 }
 
@@ -535,8 +531,6 @@ class TypeExpr extends ExprOrType, @typeexpr, TypeAnnotation {
 
   override Function getEnclosingFunction() { result = ExprOrType.super.getEnclosingFunction() }
 
-  override StmtContainer getContainer() { result = ExprOrType.super.getContainer() }
-
   override TopLevel getTopLevel() { result = ExprOrType.super.getTopLevel() }
 
   override DataFlow::ClassNode getClass() { result.getAstNode() = getType().(ClassType).getClass() }

diff --git a/javascript/ql/src/semmle/javascript/Variables.qll b/javascript/ql/src/semmle/javascript/Variables.qll
@@ -600,9 +600,6 @@ class PropertyPattern extends @property, ASTNode {
   /** Gets the object pattern this property pattern belongs to. */
   ObjectPattern getObjectPattern() { properties(this, result, _, _, _) }
 
-  /** Gets the nearest enclosing function or toplevel in which this property pattern occurs. */
-  StmtContainer getContainer() { result = getObjectPattern().getContainer() }
-
   /** Holds if this pattern is impure, that is, if its evaluation could have side effects. */
   predicate isImpure() {
     isComputed() and getNameExpr().isImpure()

diff --git a/javascript/ql/src/semmle/javascript/dataflow/Configuration.qll b/javascript/ql/src/semmle/javascript/dataflow/Configuration.qll
@@ -926,10 +926,10 @@ private predicate callInputStep(
     argumentPassing(invk, pred, f, succ)
     or
     isRelevant(pred, cfg) and
-    exists(SsaDefinition prevDef, SsaDefinition def |
-      pred = DataFlow::ssaDefinitionNode(prevDef) and
+    exists(LocalVariable variable, SsaDefinition def |
+      pred = DataFlow::capturedVariableNode(variable) and
       calls(invk, f) and
-      captures(f, prevDef, def) and
+      captures(f, variable, def) and
       succ = DataFlow::ssaDefinitionNode(def)
     )
   ) and

diff --git a/javascript/ql/src/semmle/javascript/dataflow/TrackedNodes.qll b/javascript/ql/src/semmle/javascript/dataflow/TrackedNodes.qll
@@ -122,10 +122,10 @@ private module NodeTracking {
     (
       argumentPassing(invk, pred, f, succ)
       or
-      exists(SsaDefinition prevDef, SsaDefinition def |
-        pred = DataFlow::ssaDefinitionNode(prevDef) and
+      exists(LocalVariable variable, SsaDefinition def |
+        pred = DataFlow::capturedVariableNode(variable) and
         calls(invk, f) and
-        captures(f, prevDef, def) and
+        captures(f, variable, def) and
         succ = DataFlow::ssaDefinitionNode(def)
       )
     )

diff --git a/javascript/ql/src/semmle/javascript/dataflow/internal/FlowSteps.qll b/javascript/ql/src/semmle/javascript/dataflow/internal/FlowSteps.qll
@@ -78,11 +78,11 @@ predicate localExceptionStep(DataFlow::Node pred, DataFlow::Node succ) {
 cached
 private module CachedSteps {
   /**
-   * Holds if `f` captures the variable defined by `def` in `cap`.
+   * Holds if `f` captures the given `variable` in `cap`.
    */
   cached
-  predicate captures(Function f, SsaExplicitDefinition def, SsaVariableCapture cap) {
-    def.getSourceVariable() = cap.getSourceVariable() and
+  predicate captures(Function f, LocalVariable variable, SsaVariableCapture cap) {
+    variable = cap.getSourceVariable() and
     f = cap.getContainer()
   }
-Original file line number
+Diff line change
@@ Expand Up @@
         )
       }
-      override StmtContainer getContainer() { result = getEnclosingStmt().getContainer() }
       override Function getEnclosingFunction() { result = getContainer() }
       override TopLevel getTopLevel() { result = getEnclosingStmt().getTopLevel() }
@@ Expand Down @@