JavaScript: Fix modelling of exported variables that have definitions without (syntactic) right-hand sides.

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

@@ -96,8 +96,7 @@ class AbstractProtoProperty extends AbstractProperty {
  * which in turn introduces a materialization.
  */
 private AbstractValue getAnAssignedValue(AbstractValue b, string p) {
-  exists (AnalyzedPropertyWrite apw, DataFlow::AnalyzedNode afn |
-    apw.writes(b, p, afn) and
-    result = afn.getALocalValue()
+  exists (AnalyzedPropertyWrite apw |
+    apw.writesValue(b, p, result)
   )
 }

javascript/ql/src/semmle/javascript/dataflow/internal/InterModuleTypeInference.qll

@@ -268,6 +268,12 @@ private class AnalyzedVariableExport extends AnalyzedPropertyWrite, DataFlow::Va
     propName = name and
     source = varDef.getSource().analyze()
   }
+
+  override predicate writesValue(AbstractValue baseVal, string propName, AbstractValue val) {
+    baseVal = TAbstractExportsObject(export.getEnclosingModule()) and
+    propName = name and
+    val = varDef.getAnAssignedValue()
+  }
 }
 
 /**

javascript/ql/src/semmle/javascript/dataflow/internal/InterProceduralTypeInference.qll

@@ -160,18 +160,26 @@ private class IIFEWithAnalyzedReturnFlow extends CallWithAnalyzedReturnFlow {
 
 }
 
+/**
+ * Gets the only access to `v`, which is the variable declared by `fn`.
+ *
+ * This predicate is not defined for global functions `fn`, or for
+ * local variables `v` that do not have exactly one access.
+ */
+private VarAccess getOnlyAccess(FunctionDeclStmt fn, LocalVariable v) {
+  v = fn.getVariable() and
+  result = v.getAnAccess() and
+  strictcount(v.getAnAccess()) = 1
+}
+
 /** A function that only is used locally, making it amenable to type inference. */
 class LocalFunction extends Function {
 
   DataFlow::Impl::ExplicitInvokeNode invk;
 
   LocalFunction() {
-    this instanceof FunctionDeclStmt and
-    exists (LocalVariable v, Expr callee |
-      callee = invk.getCalleeNode().asExpr() and
-      v = getVariable() and
-      v.getAnAccess() = callee and
-      forall(VarAccess o | o = v.getAnAccess() | o = callee) and
+    exists (LocalVariable v |
+      getOnlyAccess(this, v) = invk.getCalleeNode().asExpr() and
       not exists(v.getAnAssignedExpr()) and
       not exists(ExportDeclaration export | export.exportsAs(v, _))
     ) and

javascript/ql/src/semmle/javascript/dataflow/internal/PropertyTypeInference.qll

@@ -96,8 +96,24 @@ abstract class AnalyzedPropertyWrite extends DataFlow::Node {
   /**
    * Holds if this property write assigns `source` to property `propName` of one of the
    * concrete objects represented by `baseVal`.
+   *
+   * Note that not all property writes have an explicit `source` node; use predicate
+   * `writesValue` below to cover these cases.
    */
-  abstract predicate writes(AbstractValue baseVal, string propName, DataFlow::AnalyzedNode source);
+  predicate writes(AbstractValue baseVal, string propName, DataFlow::AnalyzedNode source) {
+    none()
+  }
+
+  /**
+   * Holds if this property write assigns `val` to property `propName` of one of the
+   * concrete objects represented by `baseVal`.
+   */
+  predicate writesValue(AbstractValue baseVal, string propName, AbstractValue val) {
+    exists (AnalyzedNode source |
+      writes(baseVal, propName, source) and
+      val = source.getALocalValue()
+    )
+  }
 
   /**
    * Holds if the flow information for the base node of this property write is incomplete

javascript/ql/test/library-tests/Flow/AbstractValues.expected

@@ -4,10 +4,12 @@
 | ChatListScreen.js:3:1:5:1 | instance of function foo |
 | a2.js:1:1:2:0 | exports object of module a2 |
 | a2.js:1:1:2:0 | module object of module a2 |
-| a.js:1:1:13:0 | exports object of module a |
-| a.js:1:1:13:0 | module object of module a |
+| a.js:1:1:18:0 | exports object of module a |
+| a.js:1:1:18:0 | module object of module a |
 | a.js:3:8:5:1 | function setX |
 | a.js:3:8:5:1 | instance of function setX |
+| a.js:15:1:17:1 | function bump |
+| a.js:15:1:17:1 | instance of function bump |
 | amd2.js:1:1:4:0 | exports object of module amd2 |
 | amd2.js:1:1:4:0 | module object of module amd2 |
 | amd2.js:1:8:3:1 | anonymous function |
@@ -36,8 +38,8 @@
 | arguments.js:30:2:33:1 | anonymous function |
 | arguments.js:30:2:33:1 | arguments object of anonymous function |
 | arguments.js:30:2:33:1 | instance of anonymous function |
-| b.js:1:1:55:0 | exports object of module b |
-| b.js:1:1:55:0 | module object of module b |
+| b.js:1:1:58:0 | exports object of module b |
+| b.js:1:1:58:0 | module object of module b |
 | backend.js:1:1:3:0 | exports object of module backend |
 | backend.js:1:1:3:0 | module object of module backend |
 | backend.js:1:17:1:18 | object literal |

javascript/ql/test/library-tests/Flow/a.js

@@ -10,3 +10,8 @@ let z = someGlobal;
 
 export let w;
 w = "w";
+
+export let notAlwaysZero = 0;
+function bump() {
+  ++notAlwaysZero;
+}

javascript/ql/test/library-tests/Flow/abseval.expected

@@ -6,6 +6,7 @@
 | a.js:9:5:9:5 | z | a.js:9:9:9:18 | someGlobal | file://:0:0:0:0 | indefinite value (global) |
 | a.js:9:5:9:5 | z | a.js:9:9:9:18 | someGlobal | file://:0:0:0:0 | non-zero value |
 | a.js:9:5:9:5 | z | a.js:9:9:9:18 | someGlobal | file://:0:0:0:0 | true |
+| a.js:14:12:14:24 | notAlwaysZero | a.js:14:28:14:28 | 0 | file://:0:0:0:0 | 0 |
 | amd.js:2:7:2:7 | m | amd.js:2:11:2:13 | mod | amd.js:1:1:7:0 | module object of module amd |
 | amd.js:2:7:2:7 | m | amd.js:2:11:2:13 | mod | file://:0:0:0:0 | indefinite value (call) |
 | amd.js:3:7:3:7 | e | amd.js:3:11:3:13 | exp | amd.js:1:1:7:0 | exports object of module amd |
@@ -58,6 +59,8 @@
 | b.js:48:5:48:7 | z13 | b.js:48:11:48:11 | w | file://:0:0:0:0 | non-empty, non-numeric string |
 | b.js:51:5:51:7 | z14 | b.js:51:11:51:24 | foo_reexported | file://:0:0:0:0 | indefinite value (import) |
 | b.js:54:5:54:7 | z15 | b.js:54:11:54:19 | something | file://:0:0:0:0 | indefinite value (import) |
+| b.js:57:5:57:7 | z16 | b.js:57:11:57:23 | notAlwaysZero | file://:0:0:0:0 | 0 |
+| b.js:57:5:57:7 | z16 | b.js:57:11:57:23 | notAlwaysZero | file://:0:0:0:0 | non-zero value |
 | backend.js:1:7:1:13 | Backend | backend.js:1:17:1:18 | {} | backend.js:1:17:1:18 | object literal |
 | classAccessors.js:10:9:10:11 | myX | classAccessors.js:10:15:10:20 | this.x | file://:0:0:0:0 | indefinite value (call) |
 | classAccessors.js:10:9:10:11 | myX | classAccessors.js:10:15:10:20 | this.x | file://:0:0:0:0 | indefinite value (heap) |

javascript/ql/test/library-tests/Flow/b.js

@@ -52,3 +52,6 @@ let z14 = foo_reexported;
 
 import { something } from './reexport-unknown';
 let z15 = something;
+
+import { notAlwaysZero } from './a';
+let z16 = notAlwaysZero;

javascript/ql/test/library-tests/Flow/types.expected

@@ -2,6 +2,7 @@
 | a.js:1:12:1:12 | x | a.js:1:16:1:16 | 0 | number |
 | a.js:1:19:1:19 | y | a.js:1:23:1:23 | 0 | number |
 | a.js:9:5:9:5 | z | a.js:9:9:9:18 | someGlobal | boolean, class, date, function, null, number, object, regular expression,string or undefined |
+| a.js:14:12:14:24 | notAlwaysZero | a.js:14:28:14:28 | 0 | number |
 | amd.js:2:7:2:7 | m | amd.js:2:11:2:13 | mod | boolean, class, date, function, null, number, object, regular expression,string or undefined |
 | amd.js:3:7:3:7 | e | amd.js:3:11:3:13 | exp | boolean, class, date, function, null, number, object, regular expression,string or undefined |
 | arguments.js:2:7:2:7 | y | arguments.js:2:11:2:11 | x | number |
@@ -32,6 +33,7 @@
 | b.js:48:5:48:7 | z13 | b.js:48:11:48:11 | w | string |
 | b.js:51:5:51:7 | z14 | b.js:51:11:51:24 | foo_reexported | boolean, class, date, function, null, number, object, regular expression,string or undefined |
 | b.js:54:5:54:7 | z15 | b.js:54:11:54:19 | something | boolean, class, date, function, null, number, object, regular expression,string or undefined |
+| b.js:57:5:57:7 | z16 | b.js:57:11:57:23 | notAlwaysZero | number |
 | backend.js:1:7:1:13 | Backend | backend.js:1:17:1:18 | {} | object |
 | classAccessors.js:10:9:10:11 | myX | classAccessors.js:10:15:10:20 | this.x | boolean, class, date, function, null, number, object, regular expression,string or undefined |
 | classAccessors.js:11:9:11:11 | myY | classAccessors.js:11:15:11:20 | this.y | boolean, class, date, function, null, number, object, regular expression,string or undefined |