JavaScript: Avoid a few bad join orders

javascript/ql/src/semmle/javascript/Promises.qll

@@ -184,6 +184,7 @@ module PromiseTypeTracking {
    * Gets the result from a single step through a promise, from `pred` with tracker `t2` to `result` with tracker `t`.
    * This can be loading a resolved value from a promise, storing a value in a promise, or copying a resolved value from one promise to another.
    */
+  pragma[inline]
   DataFlow::SourceNode promiseStep(
     DataFlow::SourceNode pred, DataFlow::TypeTracker t, DataFlow::TypeTracker t2
   ) {

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

@@ -945,18 +945,31 @@ private predicate reachableFromStoreBase(
         s2.getEndLabel())
   )
   or
-  exists(DataFlow::Node mid, PathSummary oldSummary, PathSummary newSummary |
-    reachableFromStoreBase(prop, rhs, mid, cfg, oldSummary) and
-    (
-      flowStep(mid, cfg, nd, newSummary)
-      or
-      isAdditionalLoadStoreStep(mid, nd, prop, cfg) and
-      newSummary = PathSummary::level()
-    ) and
+  exists(PathSummary oldSummary, PathSummary newSummary |
+    reachableFromStoreBaseStep(prop, rhs, nd, cfg, oldSummary, newSummary) and
     summary = oldSummary.appendValuePreserving(newSummary)
   )
 }
 
+/**
+ * Holds if `rhs` is the right-hand side of a write to property `prop`, and `nd` is reachable
+ * from the base of that write under configuration `cfg` (possibly through callees) along a
+ * path whose last step is summarized by `newSummary`, and the previous steps are summarized
+ * by `oldSummary`.
+ */
+pragma[noinline]
+private predicate reachableFromStoreBaseStep(
+  string prop, DataFlow::Node rhs, DataFlow::Node nd, DataFlow::Configuration cfg,
+  PathSummary oldSummary, PathSummary newSummary
+) {
+  exists(DataFlow::Node mid | reachableFromStoreBase(prop, rhs, mid, cfg, oldSummary) |
+    flowStep(mid, cfg, nd, newSummary)
+    or
+    isAdditionalLoadStoreStep(mid, nd, prop, cfg) and
+    newSummary = PathSummary::level()
+  )
+}
+
 /**
  * Holds if the value of `pred` is written to a property of some base object, and that base
  * object may flow into the base of property read `succ` under configuration `cfg` along
@@ -968,13 +981,29 @@ pragma[noinline]
 private predicate flowThroughProperty(
   DataFlow::Node pred, DataFlow::Node succ, DataFlow::Configuration cfg, PathSummary summary
 ) {
-  exists(string prop, DataFlow::Node base, PathSummary oldSummary, PathSummary newSummary |
-    reachableFromStoreBase(prop, pred, base, cfg, oldSummary) and
-    loadStep(base, succ, prop, cfg, newSummary) and
+  exists(PathSummary oldSummary, PathSummary newSummary |
+    storeToLoad(pred, succ, cfg, oldSummary, newSummary) and
     summary = oldSummary.append(newSummary)
   )
 }
 
+/**
+ * Holds if the value of `pred` is written to a property of some base object, and that base
+ * object may flow into the base of property read `succ` under configuration `cfg` along
+ * a path whose last step is summarized by `newSummary`, and the previous steps are summarized
+ * by `oldSummary`.
+ */
+pragma[noinline]
+private predicate storeToLoad(
+  DataFlow::Node pred, DataFlow::Node succ, DataFlow::Configuration cfg, PathSummary oldSummary,
+  PathSummary newSummary
+) {
+  exists(string prop, DataFlow::Node base |
+    reachableFromStoreBase(prop, pred, base, cfg, oldSummary) and
+    loadStep(base, succ, prop, cfg, newSummary)
+  )
+}
+
 /**
  * Holds if `arg` and `cb` are passed as arguments to a function which in turn
  * invokes `cb`, passing `arg` as its `i`th argument.

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

@@ -157,12 +157,12 @@ class InvokeNode extends DataFlow::SourceNode {
    * `name` is set to `result`.
    */
   DataFlow::ValueNode getOptionArgument(int i, string name) {
-    exists(ObjectLiteralNode obj |
-      obj.flowsTo(getArgument(i)) and
-      obj.hasPropertyWrite(name, result)
-    )
+    getOptionsArgument(i).hasPropertyWrite(name, result)
   }
 
+  pragma[noinline]
+  private ObjectLiteralNode getOptionsArgument(int i) { result.flowsTo(getArgument(i)) }
+
   /** Gets an abstract value representing possible callees of this call site. */
   final AbstractValue getACalleeValue() { result = getCalleeNode().analyze().getAValue() }