Merge pull request #1163 from xiemaisi/js/more-type-tracking

Approved by asger-semmle

Author: semmle-qlci

javascript/ql/src/semmle/javascript/AMD.qll

@@ -170,20 +170,19 @@ class AMDModuleDefinition extends CallExpr {
   }
 }
 
-/** A path expression appearing in the list of dependencies of an AMD module. */
-private class AMDDependencyPath extends PathExprInModule, ConstantString {
-  AMDDependencyPath() {
-    exists(AMDModuleDefinition amd | this.getParentExpr*() = amd.getDependencies().getAnElement())
+/** An AMD dependency, considered as a path expression. */
+private class AmdDependencyPath extends PathExprCandidate {
+  AmdDependencyPath() {
+    exists(AMDModuleDefinition amd |
+      this = amd.getDependencies().getAnElement() or
+      this = amd.getARequireCall().getAnArgument()
+    )
   }
-
-  override string getValue() { result = this.(ConstantString).getStringValue() }
 }
 
-/** A path expression appearing in a `require` call in an AMD module. */
-private class AMDRequirePath extends PathExprInModule, ConstantString {
-  AMDRequirePath() {
-    exists(AMDModuleDefinition amd | this.getParentExpr*() = amd.getARequireCall().getAnArgument())
-  }
+/** A constant path element appearing in an AMD dependency expression. */
+private class ConstantAmdDependencyPathElement extends PathExprInModule, ConstantString {
+  ConstantAmdDependencyPathElement() { this = any(AmdDependencyPath amd).getAPart() }
 
   override string getValue() { result = this.(ConstantString).getStringValue() }
 }

javascript/ql/src/semmle/javascript/Closure.qll

@@ -64,7 +64,7 @@ module Closure {
    * a top-level expression statement.
    */
   private predicate isTopLevelExpr(DataFlow::Node node) {
-    node.getTopLevel().getAChildStmt().(ExprStmt).getExpr().flow() = node
+    any(TopLevel tl).getAChildStmt().(ExprStmt).getExpr().flow() = node
   }
 
   /**

javascript/ql/src/semmle/javascript/NodeJS.qll

@@ -239,22 +239,22 @@ class Require extends CallExpr, Import {
   }
 }
 
-/** A literal path expression appearing in a `require` import. */
-private class LiteralRequiredPath extends PathExprInModule, ConstantString {
-  LiteralRequiredPath() { exists(Require req | this.getParentExpr*() = req.getArgument(0)) }
-
-  override string getValue() { result = this.getStringValue() }
-}
-
-/** A literal path expression appearing in a call to `require.resolve`. */
-private class LiteralRequireResolvePath extends PathExprInModule, ConstantString {
-  LiteralRequireResolvePath() {
+/** An argument to `require` or `require.resolve`, considered as a path expression. */
+private class RequirePath extends PathExprCandidate {
+  RequirePath() {
+    this = any(Require req).getArgument(0)
+    or
     exists(RequireVariable req, MethodCallExpr reqres |
       reqres.getReceiver() = req.getAnAccess() and
       reqres.getMethodName() = "resolve" and
-      this.getParentExpr*() = reqres.getArgument(0)
+      this = reqres.getArgument(0)
     )
   }
+}
+
+/** A constant path element appearing in a call to `require` or `require.resolve`. */
+private class ConstantRequirePathElement extends PathExprInModule, ConstantString {
+  ConstantRequirePathElement() { this = any(RequirePath rp).getAPart() }
 
   override string getValue() { result = this.getStringValue() }
 }

javascript/ql/src/semmle/javascript/Paths.qll

@@ -236,3 +236,20 @@ private class ConcatPath extends PathExpr {
     result = this.(AddExpr).getAnOperand().(PathExpr).getSearchRoot(priority)
   }
 }
+
+/**
+ * An expression that appears in a syntactic position where it may represent a path.
+ *
+ * Examples include arguments to the CommonJS `require` function or AMD dependency arguments.
+ */
+abstract class PathExprCandidate extends Expr {
+  /**
+   * Gets an expression that is nested inside this expression.
+   *
+   * Equivalent to `getAChildExpr*()`, but useful to enforce a better join order (in spite of
+   * what the optimizer thinks, there are generally far fewer `PathExprCandidate`s than
+   * `ConstantString`s).
+   */
+  pragma[nomagic]
+  Expr getAPart() { result = this or result = getAPart().getAChildExpr() }
+}

javascript/ql/src/semmle/javascript/dataflow/Sources.qll

@@ -162,10 +162,11 @@ class SourceNode extends DataFlow::Node {
    *
    * See `TypeTracker` for more details about how to use this.
    */
+  pragma[inline]
   DataFlow::SourceNode track(TypeTracker t2, TypeTracker t) {
     exists(StepSummary summary |
       StepSummary::step(this, result, summary) and
-      t = StepSummary::append(t2, summary)
+      t = t2.append(summary)
     )
   }
 
@@ -176,10 +177,11 @@ class SourceNode extends DataFlow::Node {
    *
    * See `TypeBackTracker` for more details about how to use this.
    */
+  pragma[inline]
   DataFlow::SourceNode backtrack(TypeBackTracker t2, TypeBackTracker t) {
     exists(StepSummary summary |
       StepSummary::step(result, this, summary) and
-      t = StepSummary::prepend(summary, t2)
+      t = t2.prepend(summary)
     )
   }
 }