From bfbf686d7be9c4e0c2aa7cd46ea2ad836ced3bc0 Mon Sep 17 00:00:00 2001
From: Esben Sparre Andreasen <esben@semmle.com>
Date: Tue, 12 Feb 2019 15:14:12 +0100
Subject: [PATCH 01/16] JS: fixup changenote for
 js/unbound-event-handler-receiver

---
 change-notes/1.20/analysis-javascript.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/change-notes/1.20/analysis-javascript.md b/change-notes/1.20/analysis-javascript.md
index 88b8ec66d495..f0d5435e98bd 100644
--- a/change-notes/1.20/analysis-javascript.md
+++ b/change-notes/1.20/analysis-javascript.md
@@ -26,7 +26,6 @@
 | Incomplete URL substring sanitization | correctness, security, external/cwe/cwe-020 | Highlights URL sanitizers that are likely to be incomplete, indicating a violation of [CWE-020](https://cwe.mitre.org/data/definitions/20.html). Results shown on LGTM by default. |
 | Incorrect suffix check (`js/incorrect-suffix-check`) | correctness, security, external/cwe/cwe-020 | Highlights error-prone suffix checks based on `indexOf`, indicating a potential violation of [CWE-20](https://cwe.mitre.org/data/definitions/20.html). Results are shown on LGTM by default. |
 | Loop iteration skipped due to shifting (`js/loop-iteration-skipped-due-to-shifting`) | correctness | Highlights code that removes an element from an array while iterating over it, causing the loop to skip over some elements. Results are shown on LGTM by default. |
-| Unbound event handler receiver (`js/unbound-event-handler-receiver`) | Fewer false positive results | Additional ways that class methods can be bound are recognized. |
 | Useless comparison test (`js/useless-comparison-test`) | correctness | Highlights code that is unreachable due to a numeric comparison that is always true or always false. Results are shown on LGTM by default. |
 
 ## Changes to existing queries
@@ -39,6 +38,7 @@
 | Insecure randomness | More results | This rule now flags insecure uses of `crypto.pseudoRandomBytes`. |
 | Reflected cross-site scripting             | Fewer false-positive results. | This rule now recognizes custom sanitizers. |
 | Stored cross-site scripting                | Fewer false-positive results. | This rule now recognizes custom sanitizers. |
+| Unbound event handler receiver (`js/unbound-event-handler-receiver`) | Fewer false positive results | Additional ways that class methods can be bound are recognized. |
 | Uncontrolled data used in network request  | More results                 | This rule now recognizes host values that are vulnerable to injection. |
 | Unused parameter                           | Fewer false-positive results | This rule no longer flags parameters with leading underscore. |
 | Unused variable, import, function or class | Fewer false-positive results | This rule now flags fewer variables that are implictly used by JSX elements, and no longer flags variables with leading underscore. |

From 0cf2eaec5ee3e6c053f5b685c6f9d89020a1dbe2 Mon Sep 17 00:00:00 2001
From: Esben Sparre Andreasen <esben@semmle.com>
Date: Tue, 12 Feb 2019 15:57:52 +0100
Subject: [PATCH 02/16] JS: introduce CapturedSource

---
 .../javascript/dataflow/CapturedNodes.qll     | 74 +++++++++++++++
 .../CapturedNodes/CapturedSource.expected     | 25 +++++
 .../CapturedNodes/CapturedSource.ql           |  4 +
 .../CapturedSource_hasOwnProperty.expected    | 22 +++++
 .../CapturedSource_hasOwnProperty.ql          |  6 ++
 .../MethodCallTypeInference.expected          | 12 +++
 .../CapturedNodes/MethodCallTypeInference.ql  |  4 +
 .../MethodCallTypeInferenceUsage.expected     |  4 +
 .../MethodCallTypeInferenceUsage.ql           |  5 +
 .../CapturedNodes/method-calls.js             | 57 ++++++++++++
 .../test/library-tests/CapturedNodes/tst.js   | 93 +++++++++++++++++++
 .../test/library-tests/CapturedNodes/tst.ts   |  6 ++
 12 files changed, 312 insertions(+)
 create mode 100644 javascript/ql/src/semmle/javascript/dataflow/CapturedNodes.qll
 create mode 100644 javascript/ql/test/library-tests/CapturedNodes/CapturedSource.expected
 create mode 100644 javascript/ql/test/library-tests/CapturedNodes/CapturedSource.ql
 create mode 100644 javascript/ql/test/library-tests/CapturedNodes/CapturedSource_hasOwnProperty.expected
 create mode 100644 javascript/ql/test/library-tests/CapturedNodes/CapturedSource_hasOwnProperty.ql
 create mode 100644 javascript/ql/test/library-tests/CapturedNodes/MethodCallTypeInference.expected
 create mode 100644 javascript/ql/test/library-tests/CapturedNodes/MethodCallTypeInference.ql
 create mode 100644 javascript/ql/test/library-tests/CapturedNodes/MethodCallTypeInferenceUsage.expected
 create mode 100644 javascript/ql/test/library-tests/CapturedNodes/MethodCallTypeInferenceUsage.ql
 create mode 100644 javascript/ql/test/library-tests/CapturedNodes/method-calls.js
 create mode 100644 javascript/ql/test/library-tests/CapturedNodes/tst.js
 create mode 100644 javascript/ql/test/library-tests/CapturedNodes/tst.ts

diff --git a/javascript/ql/src/semmle/javascript/dataflow/CapturedNodes.qll b/javascript/ql/src/semmle/javascript/dataflow/CapturedNodes.qll
new file mode 100644
index 000000000000..bcb9c44abe5a
--- /dev/null
+++ b/javascript/ql/src/semmle/javascript/dataflow/CapturedNodes.qll
@@ -0,0 +1,74 @@
+/**
+ * Provides classes for the nodes that the dataflow library can reason about soundly.
+ */
+
+import javascript
+
+/**
+ * Holds if the dataflow library can not track flow through `escape` due to `cause`.
+ */
+private predicate isEscape(DataFlow::Node escape, string cause) {
+  escape = any(DataFlow::InvokeNode invk).getAnArgument() and cause = "argument"
+  or
+  escape = any(DataFlow::FunctionNode fun).getAReturn() and cause = "return"
+  or
+  escape = any(ThrowStmt t).getExpr().flow() and cause = "throw"
+  or
+  escape = any(DataFlow::GlobalVariable v).getAnAssignedExpr().flow() and cause = "global"
+  or
+  escape = any(DataFlow::PropWrite write).getRhs() and cause = "heap"
+  or
+  escape = any(ExportDeclaration e).getSourceNode(_) and cause = "export"
+  or
+  any(WithStmt with).mayAffect(escape.asExpr()) and cause = "heap"
+}
+
+private DataFlow::Node getAnEscape() {
+  isEscape(result, _)
+}
+
+/**
+ * Holds if `n` can flow to a `this`-variable.
+ */
+private predicate exposedAsReceiver(DataFlow::SourceNode n) {
+  // pragmatic limitation: guarantee for object literals only
+  not n instanceof DataFlow::ObjectLiteralNode
+  or
+  exists(AbstractValue v | n.getAPropertyWrite().getRhs().analyze().getALocalValue() = v |
+    v.isIndefinite(_) or
+    exists(ThisExpr dis | dis.getBinder() = v.(AbstractCallable).getFunction())
+  )
+  or
+  n.flowsToExpr(any(FunctionBindExpr bind).getObject())
+  or
+  // technically, the builtin prototypes could have a `this`-using function through which this node escapes, but we ignore that here
+  // (we also ignore `o['__' + 'proto__"] = ...`)
+  exists(n.getAPropertyWrite("__proto__"))
+  or
+  // could check the assigned value of all affected variables, but it is unlikely to matter in practice
+  exists(WithStmt with | n.flowsToExpr(with.getExpr()))
+}
+
+/**
+ * A source for which the flow is entirely captured by the dataflow library.
+ * All uses of the node is represented by `this.flowsTo(_)` and friends.
+ */
+class CapturedSource extends DataFlow::SourceNode {
+  CapturedSource() {
+    // pragmatic limitation: object literals only
+    this instanceof DataFlow::ObjectLiteralNode and
+    not flowsTo(getAnEscape()) and
+    not exposedAsReceiver(this)
+  }
+
+  predicate hasOwnProperty(string name) {
+    // the property is defined in the initializer,
+    any(DataFlow::PropWrite write).writes(this, name, _) and
+    // and it is never deleted
+    not exists(DeleteExpr del, DataFlow::PropRef ref |
+      del.getOperand().flow() = ref and
+      flowsTo(ref.getBase()) and
+      (ref.getPropertyName() = name or not exists(ref.getPropertyName()))
+    )
+  }
+}
diff --git a/javascript/ql/test/library-tests/CapturedNodes/CapturedSource.expected b/javascript/ql/test/library-tests/CapturedNodes/CapturedSource.expected
new file mode 100644
index 000000000000..d78dd6e228ff
--- /dev/null
+++ b/javascript/ql/test/library-tests/CapturedNodes/CapturedSource.expected
@@ -0,0 +1,25 @@
+| method-calls.js:2:11:7:2 | {\\n\\t\\tm1: ... }; }\\n\\t} |
+| method-calls.js:11:23:11:24 | {} |
+| method-calls.js:16:11:16:12 | {} |
+| method-calls.js:23:11:23:17 | {m: f1} |
+| method-calls.js:27:11:27:17 | {m: f2} |
+| method-calls.js:32:12:32:18 | {m: f3} |
+| method-calls.js:36:15:36:21 | {m: f2} |
+| method-calls.js:42:16:42:28 | {m: () => 42} |
+| method-calls.js:46:17:46:29 | {m: () => 42} |
+| method-calls.js:50:16:50:28 | {m: () => 42} |
+| method-calls.js:53:16:53:28 | {m: () => 42} |
+| tst.js:3:18:3:19 | {} |
+| tst.js:4:18:4:36 | { f: function(){} } |
+| tst.js:5:18:5:34 | { [unknown]: 42 } |
+| tst.js:38:18:38:26 | { p: 42 } |
+| tst.js:42:18:42:33 | { p: 42, q: 42 } |
+| tst.js:52:23:52:24 | {} |
+| tst.js:56:20:56:35 | { p: 42, p: 42 } |
+| tst.js:59:20:59:28 | { p: 42 } |
+| tst.js:63:20:63:28 | { p: 42 } |
+| tst.js:68:19:68:27 | { p: 42 } |
+| tst.js:73:18:73:20 | { } |
+| tst.js:76:18:76:26 | { p: 42 } |
+| tst.js:77:18:77:28 | { p: true } |
+| tst.js:82:20:82:21 | {} |
diff --git a/javascript/ql/test/library-tests/CapturedNodes/CapturedSource.ql b/javascript/ql/test/library-tests/CapturedNodes/CapturedSource.ql
new file mode 100644
index 000000000000..0731e04afee1
--- /dev/null
+++ b/javascript/ql/test/library-tests/CapturedNodes/CapturedSource.ql
@@ -0,0 +1,4 @@
+import javascript
+import semmle.javascript.dataflow.CapturedNodes
+
+select any(CapturedSource n)
diff --git a/javascript/ql/test/library-tests/CapturedNodes/CapturedSource_hasOwnProperty.expected b/javascript/ql/test/library-tests/CapturedNodes/CapturedSource_hasOwnProperty.expected
new file mode 100644
index 000000000000..76e494cf1ad0
--- /dev/null
+++ b/javascript/ql/test/library-tests/CapturedNodes/CapturedSource_hasOwnProperty.expected
@@ -0,0 +1,22 @@
+| method-calls.js:2:11:7:2 | {\\n\\t\\tm1: ... }; }\\n\\t} | m1 |
+| method-calls.js:2:11:7:2 | {\\n\\t\\tm1: ... }; }\\n\\t} | m2 |
+| method-calls.js:2:11:7:2 | {\\n\\t\\tm1: ... }; }\\n\\t} | m3 |
+| method-calls.js:2:11:7:2 | {\\n\\t\\tm1: ... }; }\\n\\t} | m4 |
+| method-calls.js:23:11:23:17 | {m: f1} | m |
+| method-calls.js:27:11:27:17 | {m: f2} | m |
+| method-calls.js:32:12:32:18 | {m: f3} | m |
+| method-calls.js:36:15:36:21 | {m: f2} | m |
+| method-calls.js:42:16:42:28 | {m: () => 42} | m |
+| method-calls.js:46:17:46:29 | {m: () => 42} | m |
+| method-calls.js:50:16:50:28 | {m: () => 42} | m |
+| method-calls.js:53:16:53:28 | {m: () => 42} | m |
+| tst.js:4:18:4:36 | { f: function(){} } | f |
+| tst.js:38:18:38:26 | { p: 42 } | p |
+| tst.js:42:18:42:33 | { p: 42, q: 42 } | p |
+| tst.js:42:18:42:33 | { p: 42, q: 42 } | q |
+| tst.js:56:20:56:35 | { p: 42, p: 42 } | p |
+| tst.js:59:20:59:28 | { p: 42 } | p |
+| tst.js:63:20:63:28 | { p: 42 } | p |
+| tst.js:68:19:68:27 | { p: 42 } | p |
+| tst.js:76:18:76:26 | { p: 42 } | p |
+| tst.js:77:18:77:28 | { p: true } | p |
diff --git a/javascript/ql/test/library-tests/CapturedNodes/CapturedSource_hasOwnProperty.ql b/javascript/ql/test/library-tests/CapturedNodes/CapturedSource_hasOwnProperty.ql
new file mode 100644
index 000000000000..14911b329daa
--- /dev/null
+++ b/javascript/ql/test/library-tests/CapturedNodes/CapturedSource_hasOwnProperty.ql
@@ -0,0 +1,6 @@
+import javascript
+import semmle.javascript.dataflow.CapturedNodes
+
+from CapturedSource src, string name
+where src.hasOwnProperty(name)
+select src, name
diff --git a/javascript/ql/test/library-tests/CapturedNodes/MethodCallTypeInference.expected b/javascript/ql/test/library-tests/CapturedNodes/MethodCallTypeInference.expected
new file mode 100644
index 000000000000..4c11147ddead
--- /dev/null
+++ b/javascript/ql/test/library-tests/CapturedNodes/MethodCallTypeInference.expected
@@ -0,0 +1,12 @@
+| method-calls.js:8:2:8:8 | o1.m1() | boolean, class, date, function, null, number, object, regular expression,string or undefined |
+| method-calls.js:9:2:9:8 | o1.m2() | boolean, class, date, function, null, number, object, regular expression,string or undefined |
+| method-calls.js:12:2:12:7 | o.m3() | boolean, class, date, function, null, number, object, regular expression,string or undefined |
+| method-calls.js:19:2:19:7 | o2.m() | boolean, class, date, function, null, number, object, regular expression,string or undefined |
+| method-calls.js:23:11:23:21 | {m: f1}.m() | boolean, class, date, function, null, number, object, regular expression,string or undefined |
+| method-calls.js:28:11:28:16 | o2.m() | boolean, class, date, function, null, number, object, regular expression,string or undefined |
+| method-calls.js:32:11:32:23 | ({m: f3}).m() | boolean, class, date, function, null, number, object, regular expression,string or undefined |
+| method-calls.js:37:11:37:16 | o4.m() | boolean, class, date, function, null, number, object, regular expression,string or undefined |
+| method-calls.js:43:12:43:16 | o.m() | boolean, class, date, function, null, number, object, regular expression,string or undefined |
+| method-calls.js:47:12:47:16 | o.m() | boolean, class, date, function, null, number, object, regular expression,string or undefined |
+| method-calls.js:51:12:51:16 | o.m() | boolean, class, date, function, null, number, object, regular expression,string or undefined |
+| method-calls.js:54:12:54:16 | o.m() | boolean, class, date, function, null, number, object, regular expression,string or undefined |
diff --git a/javascript/ql/test/library-tests/CapturedNodes/MethodCallTypeInference.ql b/javascript/ql/test/library-tests/CapturedNodes/MethodCallTypeInference.ql
new file mode 100644
index 000000000000..b5224bed0676
--- /dev/null
+++ b/javascript/ql/test/library-tests/CapturedNodes/MethodCallTypeInference.ql
@@ -0,0 +1,4 @@
+import javascript
+
+from DataFlow::MethodCallNode call
+select call, call.analyze().ppTypes()
diff --git a/javascript/ql/test/library-tests/CapturedNodes/MethodCallTypeInferenceUsage.expected b/javascript/ql/test/library-tests/CapturedNodes/MethodCallTypeInferenceUsage.expected
new file mode 100644
index 000000000000..8d3ed2834a6f
--- /dev/null
+++ b/javascript/ql/test/library-tests/CapturedNodes/MethodCallTypeInferenceUsage.expected
@@ -0,0 +1,4 @@
+| method-calls.js:23:11:23:21 | {m: f1}.m() | method-calls.js:24:2:24:3 | v1 | file://:0:0:0:0 | indefinite value (call) |
+| method-calls.js:28:11:28:16 | o2.m() | method-calls.js:29:2:29:3 | v2 | file://:0:0:0:0 | indefinite value (call) |
+| method-calls.js:32:11:32:23 | ({m: f3}).m() | method-calls.js:33:2:33:3 | v3 | file://:0:0:0:0 | indefinite value (call) |
+| method-calls.js:37:11:37:16 | o4.m() | method-calls.js:38:2:38:3 | v4 | file://:0:0:0:0 | indefinite value (call) |
diff --git a/javascript/ql/test/library-tests/CapturedNodes/MethodCallTypeInferenceUsage.ql b/javascript/ql/test/library-tests/CapturedNodes/MethodCallTypeInferenceUsage.ql
new file mode 100644
index 000000000000..e35d82a37370
--- /dev/null
+++ b/javascript/ql/test/library-tests/CapturedNodes/MethodCallTypeInferenceUsage.ql
@@ -0,0 +1,5 @@
+import javascript
+
+from DataFlow::MethodCallNode call, DataFlow::Node use
+where call.flowsTo(use) and use != call and not exists(use.getASuccessor())
+select call, use, use.analyze().getAValue()
\ No newline at end of file
diff --git a/javascript/ql/test/library-tests/CapturedNodes/method-calls.js b/javascript/ql/test/library-tests/CapturedNodes/method-calls.js
new file mode 100644
index 000000000000..f17f72e58962
--- /dev/null
+++ b/javascript/ql/test/library-tests/CapturedNodes/method-calls.js
@@ -0,0 +1,57 @@
+(function() {
+	var o1 = {
+		m1: function(){ return {}; },
+		m2: function(){ return {}; },
+		m3: function(){ return {}; },
+		m4: function(){ return {}; }
+	};
+	o1.m1();
+	o1.m2();
+	unknown(o1.m2);
+	var o = unknown? o1: {};
+	o.m3(); // NOT supported
+	var m4 = o.m4;
+	m4();
+
+	var o2 = {};
+	o2.m = function() { return {}; };
+	o2[unknown] = function() { return true; }; // could be __proto__
+	o2.m();
+});
+(function(){
+	function f1(){return {};}
+	var v1 = {m: f1}.m();
+	v1 === true;
+
+	function f2(){return {};}
+	var o2 = {m: f2};
+	var v2 = o2.m();
+	v2 === true;
+
+	function f3(){return {};}
+	var v3 = ({m: f3}).m();
+	v3 === true;
+
+	function f4(){return {};}
+	var { o4 } = {m: f2};
+	var v4 = o4.m();
+	v4 === true;
+});
+
+(function(){
+	(function(o = {m: () => 42}){
+		var v1 = o.m();
+	})(unknown);
+
+	function f(o = {m: () => 42}){
+		var v2 = o.m();
+	};
+	f(unknown);
+	(function(o = {m: () => 42}){
+		var v3 = o.m();
+	})({m: unknown});
+	(function(o = {m: () => 42}){
+		var v4 = o.m();
+	})({m: () => true});
+
+});
diff --git a/javascript/ql/test/library-tests/CapturedNodes/tst.js b/javascript/ql/test/library-tests/CapturedNodes/tst.js
new file mode 100644
index 000000000000..dee138c0f270
--- /dev/null
+++ b/javascript/ql/test/library-tests/CapturedNodes/tst.js
@@ -0,0 +1,93 @@
+(function capturedSource(){
+
+	let captured1 = {};
+	let captured2 = { f: function(){} };
+	let captured3 = { [unknown]: 42 };
+
+	unknown({});
+
+	function known(){}
+	known({});
+
+	function known_escaping(e){unknown(e)}
+	known_escaping({});
+
+	(function(){return {}});
+
+	(function(){throw {}});
+
+	global = {};
+
+	this.p = {};
+
+	let local_in_with;
+	with (unknown) {
+		local_in_with = {};
+	}
+
+	with({}){}
+
+	({ m: function(){ this; } });
+	({ m: unknown });
+	let indirectlyUnknown = unknown? unknown: function(){};
+	({ m: indirectlyUnknown });
+});
+
+(function capturedProperty(){
+
+	let captured1 = { p: 42 };
+	captured1.p;
+	captured1.p;
+
+	let captured2 = { p: 42, q: 42 };
+	captured2.p;
+	captured2.p;
+	captured2.q = 42;
+	captured2 = 42;
+
+	let nonObject = function(){}
+	nonObject.p = 42;
+	nonObject.p;
+
+	let nonInitializer = {};
+	nonInitializer.p = 42;
+	nonInitializer.p;
+
+	let overridden1 = { p: 42, p: 42 };
+	overridden1.p;
+
+	let overridden2 = { p: 42 };
+	overridden2.p = 42;
+	overridden2.p;
+
+	let overridden3 = { p: 42 };
+	overridden3[x] = 42;
+	overridden3.p;
+
+	function f(o) {
+		let captured3 = { p: 42 };
+		o = o || captured3;
+		o.p;
+	}
+
+	let captured4 = { };
+	captured4.p;
+
+	let captured5 = { p: 42 },
+	    captured6 = { p: true };
+	(unknown? captured5: captured6).p; // could support this with a bit of extra work
+
+	(function(semiCaptured7){
+		if(unknown)
+			semiCaptured7 = {};
+		semiCaptured7.p = 42;
+	});
+
+});
+
+(function (){
+	let bound = {};
+	bound::unknown();
+});
+
+// semmle-extractor-options: --experimental
diff --git a/javascript/ql/test/library-tests/CapturedNodes/tst.ts b/javascript/ql/test/library-tests/CapturedNodes/tst.ts
new file mode 100644
index 000000000000..1f2794c0e5e1
--- /dev/null
+++ b/javascript/ql/test/library-tests/CapturedNodes/tst.ts
@@ -0,0 +1,6 @@
+class C  {
+    constructor(
+        private readonly F: { timeout: number } = { timeout: 1500 }
+    ) {
+    }
+}

From 91dccc3356df024b7f90a5d6abca77c79e75d363 Mon Sep 17 00:00:00 2001
From: Esben Sparre Andreasen <esben@semmle.com>
Date: Tue, 12 Feb 2019 15:02:03 +0100
Subject: [PATCH 03/16] JS: add query js/unused-property

---
 .../suites/javascript/maintainability-more    |  1 +
 .../ql/src/Declarations/UnusedProperty.qhelp  | 34 ++++++++
 .../ql/src/Declarations/UnusedProperty.ql     | 74 +++++++++++++++++
 .../ql/src/Declarations/UnusedVariable.ql     | 18 +---
 .../ql/src/Declarations/UnusedVariable.qll    | 22 +++++
 .../Declarations/examples/UnusedProperty.js   |  8 ++
 .../ql/src/Expressions/ExprHasNoEffect.ql     | 35 +-------
 .../ql/src/Expressions/ExprHasNoEffect.qll    | 39 +++++++++
 .../UnusedProperty/UnusedProperty.expected    |  9 ++
 .../UnusedProperty/UnusedProperty.qlref       |  1 +
 .../Declarations/UnusedProperty/tst.js        | 83 +++++++++++++++++++
 .../Declarations/UnusedProperty/tst.ts        | 28 +++++++
 12 files changed, 301 insertions(+), 51 deletions(-)
 create mode 100644 javascript/ql/src/Declarations/UnusedProperty.qhelp
 create mode 100644 javascript/ql/src/Declarations/UnusedProperty.ql
 create mode 100644 javascript/ql/src/Declarations/UnusedVariable.qll
 create mode 100644 javascript/ql/src/Declarations/examples/UnusedProperty.js
 create mode 100644 javascript/ql/src/Expressions/ExprHasNoEffect.qll
 create mode 100644 javascript/ql/test/query-tests/Declarations/UnusedProperty/UnusedProperty.expected
 create mode 100644 javascript/ql/test/query-tests/Declarations/UnusedProperty/UnusedProperty.qlref
 create mode 100644 javascript/ql/test/query-tests/Declarations/UnusedProperty/tst.js
 create mode 100644 javascript/ql/test/query-tests/Declarations/UnusedProperty/tst.ts

diff --git a/javascript/config/suites/javascript/maintainability-more b/javascript/config/suites/javascript/maintainability-more
index f887a26e1797..0f09b9ee4e6b 100644
--- a/javascript/config/suites/javascript/maintainability-more
+++ b/javascript/config/suites/javascript/maintainability-more
@@ -5,6 +5,7 @@
 + semmlecode-javascript-queries/Declarations/DeadStoreOfProperty.ql: /Maintainability/Declarations
 + semmlecode-javascript-queries/Declarations/DuplicateVarDecl.ql: /Maintainability/Declarations
 + semmlecode-javascript-queries/Declarations/UnusedParameter.ql: /Maintainability/Declarations
++ semmlecode-javascript-queries/Declarations/UnusedProperty.ql: /Maintainability/Declarations
 + semmlecode-javascript-queries/Declarations/UnusedVariable.ql: /Maintainability/Declarations
 + semmlecode-javascript-queries/Expressions/UnneededDefensiveProgramming.ql: /Maintainability/Expressions
 + semmlecode-javascript-queries/LanguageFeatures/ArgumentsCallerCallee.ql: /Maintainability/Language Features
diff --git a/javascript/ql/src/Declarations/UnusedProperty.qhelp b/javascript/ql/src/Declarations/UnusedProperty.qhelp
new file mode 100644
index 000000000000..6da2b7d16f40
--- /dev/null
+++ b/javascript/ql/src/Declarations/UnusedProperty.qhelp
@@ -0,0 +1,34 @@
+<!DOCTYPE qhelp PUBLIC
+"-//Semmle//qhelp//EN"
+"qhelp.dtd">
+<qhelp>
+	<overview>
+		<p>
+			Unused object properties make code harder to maintain and use. Clients that are unaware that a
+			property is unused may perform nontrivial computations to compute a value that is ultimately
+			unused.
+		</p>
+
+	</overview>
+	<recommendation>
+		<p>Remove the unused property.</p>
+
+	</recommendation>
+	<example>
+
+		<p>
+			In this code, the function <code>f</code> initializes a property <code>prop_a</code> with a
+			call to the function <code>expensiveComputation</code>, but later on this property is never read.
+			Removing <code>prop</code> would improve code quality and performance.
+		</p>
+
+		<sample src="examples/UnusedProperty.js" />
+
+	</example>
+	<references>
+
+		<li>Coding Horror: <a href="http://blog.codinghorror.com/code-smells/">Code Smells</a>.</li>
+
+
+	</references>
+</qhelp>
diff --git a/javascript/ql/src/Declarations/UnusedProperty.ql b/javascript/ql/src/Declarations/UnusedProperty.ql
new file mode 100644
index 000000000000..b364a2b21483
--- /dev/null
+++ b/javascript/ql/src/Declarations/UnusedProperty.ql
@@ -0,0 +1,74 @@
+/**
+ * @name Unused property
+ * @description Unused properties may be a symptom of a bug and should be examined carefully.
+ * @kind problem
+ * @problem.severity recommendation
+ * @id js/unused-property
+ * @tags maintainability
+ * @precision high
+ */
+
+import javascript
+import semmle.javascript.dataflow.CapturedNodes
+import UnusedVariable
+import UnusedParameter
+import Expressions.ExprHasNoEffect
+
+predicate hasUnknownPropertyRead(CapturedSource obj) {
+  // dynamic reads
+  exists(DataFlow::PropRead r | obj.getAPropertyRead() = r | not exists(r.getPropertyName()))
+  or
+  // reflective reads
+  obj.flowsToExpr(any(EnhancedForLoop l).getIterationDomain())
+  or
+  obj.flowsToExpr(any(InExpr l).getRightOperand())
+  or
+  obj.flowsToExpr(any(SpreadElement e).getOperand())
+  or
+  exists(obj.getAPropertyRead("hasOwnProperty"))
+  or
+  exists(obj.getAPropertyRead("propertyIsEnumerable"))
+}
+
+predicate flowsToTypeRestrictedExpression(CapturedSource n) {
+  exists (Expr restricted, TypeExpr type |
+    n.flowsToExpr(restricted) and
+    not type.isAny() |
+    exists (TypeAssertion assertion |
+      type = assertion.getTypeAnnotation() and
+      restricted = assertion.getExpression()
+    )
+    or
+    exists (BindingPattern v |
+      type = v.getTypeAnnotation() and
+      restricted = v.getAVariable().getAnAssignedExpr()
+    )
+    // no need to reason about writes to typed fields, captured nodes do not reach them
+  )
+}
+
+from DataFlow::PropWrite w, CapturedSource n, string name
+where
+  w = n.getAPropertyWrite(name) and
+  not exists(n.getAPropertyRead(name)) and
+  not w.getBase().analyze().getAValue() != n.analyze().getAValue() and
+  not hasUnknownPropertyRead(n) and
+  // avoid reporting if the definition is unreachable
+  w.getAstNode().getFirstControlFlowNode().getBasicBlock() instanceof ReachableBasicBlock and
+  // avoid implicitly read properties
+  not (
+    name = "toString" or
+    name = "valueOf" or
+    name.matches("@@%") // @@iterator, for example
+  ) and
+  // avoid flagging properties that a type system requires
+  not flowsToTypeRestrictedExpression(n) and
+  // flagged by js/unused-local-variable
+  not exists(UnusedLocal l | l.getAnAssignedExpr().getUnderlyingValue().flow() = n) and
+  // flagged by js/unused-parameter
+  not exists(Parameter p | isAnAccidentallyUnusedParameter(p) |
+    p.getDefault().getUnderlyingValue().flow() = n
+  ) and
+  // flagged by js/useless-expression
+  not inVoidContext(n.asExpr())
+select w, "Unused property " + name + "."
diff --git a/javascript/ql/src/Declarations/UnusedVariable.ql b/javascript/ql/src/Declarations/UnusedVariable.ql
index aa361582c10c..10eb914701eb 100644
--- a/javascript/ql/src/Declarations/UnusedVariable.ql
+++ b/javascript/ql/src/Declarations/UnusedVariable.ql
@@ -10,23 +10,7 @@
  */
 
 import javascript
-
-/**
- * A local variable that is neither used nor exported, and is not a parameter
- * or a function name.
- */
-class UnusedLocal extends LocalVariable {
-  UnusedLocal() {
-    not exists(getAnAccess()) and
-    not exists(Parameter p | this = p.getAVariable()) and
-    not exists(FunctionExpr fe | this = fe.getVariable()) and
-    not exists(ClassExpr ce | this = ce.getVariable()) and
-    not exists(ExportDeclaration ed | ed.exportsAs(this, _)) and
-    not exists(LocalVarTypeAccess type | type.getVariable() = this) and
-    // common convention: variables with leading underscore are intentionally unused
-    getName().charAt(0) != "_"
-  }
-}
+import UnusedVariable
 
 /**
  * Holds if `v` is mentioned in a JSDoc comment in the same file, and that file
diff --git a/javascript/ql/src/Declarations/UnusedVariable.qll b/javascript/ql/src/Declarations/UnusedVariable.qll
new file mode 100644
index 000000000000..98d69b80938c
--- /dev/null
+++ b/javascript/ql/src/Declarations/UnusedVariable.qll
@@ -0,0 +1,22 @@
+/**
+ * This library contains parts of the 'js/unused-local-variable' query implementation.
+ */
+
+import javascript
+
+/**
+ * A local variable that is neither used nor exported, and is not a parameter
+ * or a function name.
+ */
+class UnusedLocal extends LocalVariable {
+  UnusedLocal() {
+    not exists(getAnAccess()) and
+    not exists(Parameter p | this = p.getAVariable()) and
+    not exists(FunctionExpr fe | this = fe.getVariable()) and
+    not exists(ClassExpr ce | this = ce.getVariable()) and
+    not exists(ExportDeclaration ed | ed.exportsAs(this, _)) and
+    not exists(LocalVarTypeAccess type | type.getVariable() = this) and
+    // common convention: variables with leading underscore are intentionally unused
+    getName().charAt(0) != "_"
+  }
+}
diff --git a/javascript/ql/src/Declarations/examples/UnusedProperty.js b/javascript/ql/src/Declarations/examples/UnusedProperty.js
new file mode 100644
index 000000000000..3d1441d430b2
--- /dev/null
+++ b/javascript/ql/src/Declarations/examples/UnusedProperty.js
@@ -0,0 +1,8 @@
+function f() {
+	var o = {
+		prop_a: expensiveComputation(),
+		prop_b: anotherComputation()
+	};
+
+	return o.prop_b;
+}
diff --git a/javascript/ql/src/Expressions/ExprHasNoEffect.ql b/javascript/ql/src/Expressions/ExprHasNoEffect.ql
index c2dbe038963d..3ceda6fa29bc 100644
--- a/javascript/ql/src/Expressions/ExprHasNoEffect.ql
+++ b/javascript/ql/src/Expressions/ExprHasNoEffect.ql
@@ -16,40 +16,7 @@ import javascript
 import DOMProperties
 import semmle.javascript.frameworks.xUnit
 import semmle.javascript.RestrictedLocations
-
-/**
- * Holds if `e` appears in a syntactic context where its value is discarded.
- */
-predicate inVoidContext(Expr e) {
-  exists(ExprStmt parent |
-    // e is a toplevel expression in an expression statement
-    parent = e.getParent() and
-    // but it isn't an HTML attribute or a configuration object
-    not exists(TopLevel tl | tl = parent.getParent() |
-      tl instanceof CodeInAttribute
-      or
-      // if the toplevel in its entirety is of the form `({ ... })`,
-      // it is probably a configuration object (e.g., a require.js build configuration)
-      tl.getNumChildStmt() = 1 and e.stripParens() instanceof ObjectExpr
-    )
-  )
-  or
-  exists(SeqExpr seq, int i, int n |
-    e = seq.getOperand(i) and
-    n = seq.getNumOperands()
-  |
-    i < n - 1 or inVoidContext(seq)
-  )
-  or
-  exists(ForStmt stmt | e = stmt.getUpdate())
-  or
-  exists(ForStmt stmt | e = stmt.getInit() |
-    // Allow the pattern `for(i; i < 10; i++)`
-    not e instanceof VarAccess
-  )
-  or
-  exists(LogicalBinaryExpr logical | e = logical.getRightOperand() and inVoidContext(logical))
-}
+import ExprHasNoEffect
 
 /**
  * Holds if `e` is of the form `x;` or `e.p;` and has a JSDoc comment containing a tag.
diff --git a/javascript/ql/src/Expressions/ExprHasNoEffect.qll b/javascript/ql/src/Expressions/ExprHasNoEffect.qll
new file mode 100644
index 000000000000..3b6f4703ad6e
--- /dev/null
+++ b/javascript/ql/src/Expressions/ExprHasNoEffect.qll
@@ -0,0 +1,39 @@
+/**
+ * This library contains parts of the 'js/useless-expression' query implementation.
+ */
+
+import javascript
+
+/**
+ * Holds if `e` appears in a syntactic context where its value is discarded.
+ */
+predicate inVoidContext(Expr e) {
+  exists(ExprStmt parent |
+    // e is a toplevel expression in an expression statement
+    parent = e.getParent() and
+    // but it isn't an HTML attribute or a configuration object
+    not exists(TopLevel tl | tl = parent.getParent() |
+      tl instanceof CodeInAttribute
+      or
+      // if the toplevel in its entirety is of the form `({ ... })`,
+      // it is probably a configuration object (e.g., a require.js build configuration)
+      tl.getNumChildStmt() = 1 and e.stripParens() instanceof ObjectExpr
+    )
+  )
+  or
+  exists(SeqExpr seq, int i, int n |
+    e = seq.getOperand(i) and
+    n = seq.getNumOperands()
+  |
+    i < n - 1 or inVoidContext(seq)
+  )
+  or
+  exists(ForStmt stmt | e = stmt.getUpdate())
+  or
+  exists(ForStmt stmt | e = stmt.getInit() |
+    // Allow the pattern `for(i; i < 10; i++)`
+    not e instanceof VarAccess
+  )
+  or
+  exists(LogicalBinaryExpr logical | e = logical.getRightOperand() and inVoidContext(logical))
+}
diff --git a/javascript/ql/test/query-tests/Declarations/UnusedProperty/UnusedProperty.expected b/javascript/ql/test/query-tests/Declarations/UnusedProperty/UnusedProperty.expected
new file mode 100644
index 000000000000..f976725b5592
--- /dev/null
+++ b/javascript/ql/test/query-tests/Declarations/UnusedProperty/UnusedProperty.expected
@@ -0,0 +1,9 @@
+| tst.js:4:9:4:19 | unused1: 42 | Unused property unused1. |
+| tst.js:19:5:19:15 | unused9: 42 | Unused property unused9. |
+| tst.js:26:13:26:24 | unused11: 42 | Unused property unused11. |
+| tst.js:31:13:31:35 | used12_ ... lly: 42 | Unused property used12_butNotReally. |
+| tst.js:32:13:32:24 | unused12: 42 | Unused property unused12. |
+| tst.js:52:3:52:14 | unused14: 42 | Unused property unused14. |
+| tst.js:54:2:54:20 | captured14.unused14 | Unused property unused14. |
+| tst.js:55:2:55:20 | captured14.unused14 | Unused property unused14. |
+| tst.ts:24:21:24:25 | p: 42 | Unused property p. |
diff --git a/javascript/ql/test/query-tests/Declarations/UnusedProperty/UnusedProperty.qlref b/javascript/ql/test/query-tests/Declarations/UnusedProperty/UnusedProperty.qlref
new file mode 100644
index 000000000000..9583241c2f0d
--- /dev/null
+++ b/javascript/ql/test/query-tests/Declarations/UnusedProperty/UnusedProperty.qlref
@@ -0,0 +1 @@
+Declarations/UnusedProperty.ql
diff --git a/javascript/ql/test/query-tests/Declarations/UnusedProperty/tst.js b/javascript/ql/test/query-tests/Declarations/UnusedProperty/tst.js
new file mode 100644
index 000000000000..26cd1a110bba
--- /dev/null
+++ b/javascript/ql/test/query-tests/Declarations/UnusedProperty/tst.js
@@ -0,0 +1,83 @@
+(function(){
+    var captured1 = {
+        used1: 42,
+        unused1: 42
+    };
+    captured1.used1;
+
+    var unused2 = {
+        unused2a: 42,
+        unused2b: 42
+    };
+
+    for (x.p in { used3: 42 });
+    for (x.p of { used4: 42 });
+    42 in { used5: 42 };
+    f(...{used6: 42});
+    [...{used7: 42}];
+	({...{used8: 42}});
+	({ unused9: 42 }) + "";
+    ({ used10: 42 }).hasOwnProperty;
+    ({ used10: 42 }).propertyIsEnumerable;
+
+    (function(){
+        var captured11 = {
+            used11: 42,
+            unused11: 42
+        };
+        captured11.used11;
+
+        var captured12 = {
+            used12_butNotReally: 42,
+            unused12: 42
+        };
+
+        throw x;
+
+        captured12.used12_butNotReally;
+
+        var captured13 = {
+            used13: 42,
+            unused13: 42
+        };
+        captured13.used13;
+    });
+    (function(options){
+        if(unknown)
+            options = {};
+        options.output = 42;
+    });
+
+	var captured14 = {
+		unused14: 42
+	};
+	captured14.unused14 = 42;
+	captured14.unused14 = 42;
+
+
+	var captured15 = {
+		semiUnused15: 42
+	};
+	captured15.semiUnused15 = 42;
+	captured15.semiUnused15;
+});
+(function(unusedParam = {unusedProp: 42}){
+
+});
+(function(){
+	var unusedObj = {
+		unusedProp: 42
+	};
+});
+(function(){
+	var unusedSpecials = {
+		toString: function(){},
+		valueOf: function(){},
+		'@@iterator': function(){}
+	};
+	unusedSpecials.foo;
+});
+
+(function(){
+	({ unusedProp: 42 }, 42);
+});
diff --git a/javascript/ql/test/query-tests/Declarations/UnusedProperty/tst.ts b/javascript/ql/test/query-tests/Declarations/UnusedProperty/tst.ts
new file mode 100644
index 000000000000..7ad7c508df8d
--- /dev/null
+++ b/javascript/ql/test/query-tests/Declarations/UnusedProperty/tst.ts
@@ -0,0 +1,28 @@
+(function(){
+    var o1: { p: int, q: int } = { p: 42, q: 42 };
+    o1.q;
+
+    var o2 = <{ p: int, q: int }>{ p: 42, q: 42 };
+    o2.q;
+
+    var o3: { p: int, q: int } = f();
+    o3 = o3 || { p: 42, q: 42 };
+    o3.q;
+
+});
+
+class C {
+    private o: { p: int, q: int };
+
+    constructor() {
+        this.o = { p: 42, q: 42 };
+        this.o.q;
+    }
+}
+
+(function(){
+    var o1: any = { p: 42, q: 42 };
+    o1.q;
+    var o2: any = { p: 42, q: 42 };
+    var o3: { p: int, q: int } = o2;
+})

From 8af501d4d53771c12195ce89e39068226d1199eb Mon Sep 17 00:00:00 2001
From: Esben Sparre Andreasen <esben@semmle.com>
Date: Tue, 12 Feb 2019 15:07:28 +0100
Subject: [PATCH 04/16] JS: avoid double reporting dead code with
 js/unused-variable

---
 javascript/ql/src/Declarations/UnusedVariable.ql          | 8 ++++++--
 .../test/query-tests/Declarations/UnusedVariable/dead.js  | 4 ++++
 2 files changed, 10 insertions(+), 2 deletions(-)
 create mode 100644 javascript/ql/test/query-tests/Declarations/UnusedVariable/dead.js

diff --git a/javascript/ql/src/Declarations/UnusedVariable.ql b/javascript/ql/src/Declarations/UnusedVariable.ql
index 10eb914701eb..2c86549c4ec4 100644
--- a/javascript/ql/src/Declarations/UnusedVariable.ql
+++ b/javascript/ql/src/Declarations/UnusedVariable.ql
@@ -190,6 +190,10 @@ predicate unusedImports(ImportVarDeclProvider provider, string msg) {
 
 from ASTNode sel, string msg
 where
-  unusedNonImports(sel, msg) or
-  unusedImports(sel, msg)
+  (
+    unusedNonImports(sel, msg) or
+    unusedImports(sel, msg)
+  ) and
+  // avoid reporting if the definition is unreachable
+  sel.getFirstControlFlowNode().getBasicBlock() instanceof ReachableBasicBlock
 select sel, msg
diff --git a/javascript/ql/test/query-tests/Declarations/UnusedVariable/dead.js b/javascript/ql/test/query-tests/Declarations/UnusedVariable/dead.js
new file mode 100644
index 000000000000..10fcd168991e
--- /dev/null
+++ b/javascript/ql/test/query-tests/Declarations/UnusedVariable/dead.js
@@ -0,0 +1,4 @@
+(function(){
+	throw 42;
+	var x = 42;
+});

From c84d89872710441d07862cf6a20c30a12cb54522 Mon Sep 17 00:00:00 2001
From: Esben Sparre Andreasen <esben@semmle.com>
Date: Tue, 12 Feb 2019 15:15:27 +0100
Subject: [PATCH 05/16] JS: change notes for js/unused-property and
 js/unused-variable

---
 change-notes/1.20/analysis-javascript.md | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/change-notes/1.20/analysis-javascript.md b/change-notes/1.20/analysis-javascript.md
index f0d5435e98bd..20c5ed006a3b 100644
--- a/change-notes/1.20/analysis-javascript.md
+++ b/change-notes/1.20/analysis-javascript.md
@@ -26,6 +26,7 @@
 | Incomplete URL substring sanitization | correctness, security, external/cwe/cwe-020 | Highlights URL sanitizers that are likely to be incomplete, indicating a violation of [CWE-020](https://cwe.mitre.org/data/definitions/20.html). Results shown on LGTM by default. |
 | Incorrect suffix check (`js/incorrect-suffix-check`) | correctness, security, external/cwe/cwe-020 | Highlights error-prone suffix checks based on `indexOf`, indicating a potential violation of [CWE-20](https://cwe.mitre.org/data/definitions/20.html). Results are shown on LGTM by default. |
 | Loop iteration skipped due to shifting (`js/loop-iteration-skipped-due-to-shifting`) | correctness | Highlights code that removes an element from an array while iterating over it, causing the loop to skip over some elements. Results are shown on LGTM by default. |
+| Unused property (`js/unused-property`) | maintainability | Highlights properties that are unused. Results are shown on LGTM by default. |
 | Useless comparison test (`js/useless-comparison-test`) | correctness | Highlights code that is unreachable due to a numeric comparison that is always true or always false. Results are shown on LGTM by default. |
 
 ## Changes to existing queries
@@ -41,7 +42,7 @@
 | Unbound event handler receiver (`js/unbound-event-handler-receiver`) | Fewer false positive results | Additional ways that class methods can be bound are recognized. |
 | Uncontrolled data used in network request  | More results                 | This rule now recognizes host values that are vulnerable to injection. |
 | Unused parameter                           | Fewer false-positive results | This rule no longer flags parameters with leading underscore. |
-| Unused variable, import, function or class | Fewer false-positive results | This rule now flags fewer variables that are implictly used by JSX elements, and no longer flags variables with leading underscore. |
+| Unused variable, import, function or class | Fewer false-positive results | This rule now flags fewer variables that are implictly used by JSX elements, no longer flags variables with leading underscore, and no longer flags variables in dead code. |
 | Uncontrolled data used in path expression | Fewer false-positive results | This rule now recognizes the Express `root` option, which prevents path traversal. |
 | Unneeded defensive code | More true-positive results, fewer false-positive results. | This rule now recognizes additional defensive code patterns. |
 | Useless conditional | Fewer results | Additional defensive coding patterns are now ignored. |

From bdd8691e65d236c0657455eae8ac8c055b3e868a Mon Sep 17 00:00:00 2001
From: Esben Sparre Andreasen <esben@semmle.com>
Date: Tue, 12 Feb 2019 13:06:24 +0100
Subject: [PATCH 06/16] JS: add type inference for the return value of captured
 method calls

---
 .../internal/InterProceduralTypeInference.qll | 46 +++++++++++++++++++
 .../MethodCallTypeInference.expected          | 10 ++--
 .../MethodCallTypeInferenceUsage.expected     |  6 +--
 .../CallWithAnalyzedReturnFlow.expected       |  1 +
 .../InvokeNodeValue.expected                  |  2 +-
 5 files changed, 56 insertions(+), 9 deletions(-)

diff --git a/javascript/ql/src/semmle/javascript/dataflow/internal/InterProceduralTypeInference.qll b/javascript/ql/src/semmle/javascript/dataflow/internal/InterProceduralTypeInference.qll
index 459cfe7c2bae..fcb3e00fb255 100644
--- a/javascript/ql/src/semmle/javascript/dataflow/internal/InterProceduralTypeInference.qll
+++ b/javascript/ql/src/semmle/javascript/dataflow/internal/InterProceduralTypeInference.qll
@@ -7,6 +7,8 @@
 import javascript
 import AbstractValuesImpl
 
+import semmle.javascript.dataflow.CapturedNodes
+
 /**
  * Flow analysis for `this` expressions inside functions.
  */
@@ -230,3 +232,47 @@ private class TypeInferredCalleeWithAnalyzedReturnFlow extends CallWithNonLocalA
 
   override AnalyzedFunction getACallee() { result = fun }
 }
+
+/**
+ * Holds if `call` uses `receiver` as its only receiver value.
+ */
+pragma[noinline]
+private predicate hasDefiniteReceiver(
+  DataFlow::MethodCallNode call, CapturedSource receiver
+) {
+  call = receiver.getAMethodCall() and
+  exists (DataFlow::AnalyzedNode receiverNode, AbstractValue abstractCapturedReceiver |
+    receiverNode = call.getReceiver() and
+    not receiverNode.getALocalValue().isIndefinite(_) and
+    abstractCapturedReceiver = receiver.analyze().getALocalValue() and
+    forall(DataFlow::AbstractValue v |
+      receiverNode.getALocalValue() = v |
+      v = abstractCapturedReceiver
+    )
+  )
+}
+
+/**
+ * Enables inter-procedural type inference for the return value of a
+ * method call to a flow-insensitively type-inferred callee.
+ */
+class TypeInferredMethodWithAnalyzedReturnFlow extends CallWithNonLocalAnalyzedReturnFlow {
+  DataFlow::FunctionNode fun;
+
+  TypeInferredMethodWithAnalyzedReturnFlow() {
+    exists(CapturedSource s, DataFlow::PropWrite w, string name |
+      this.(DataFlow::MethodCallNode).getMethodName() = name and
+      s.hasOwnProperty(name) and
+      hasDefiniteReceiver(this, s) and
+      w = s.getAPropertyWrite() and
+      fun.flowsTo(w.getRhs()) and
+      (
+        not exists(w.getPropertyName())
+        or
+        w.getPropertyName() = name
+      )
+    )
+  }
+
+  override AnalyzedFunction getACallee() { result = fun }
+}
\ No newline at end of file
diff --git a/javascript/ql/test/library-tests/CapturedNodes/MethodCallTypeInference.expected b/javascript/ql/test/library-tests/CapturedNodes/MethodCallTypeInference.expected
index 4c11147ddead..1192bcdb1275 100644
--- a/javascript/ql/test/library-tests/CapturedNodes/MethodCallTypeInference.expected
+++ b/javascript/ql/test/library-tests/CapturedNodes/MethodCallTypeInference.expected
@@ -1,10 +1,10 @@
-| method-calls.js:8:2:8:8 | o1.m1() | boolean, class, date, function, null, number, object, regular expression,string or undefined |
-| method-calls.js:9:2:9:8 | o1.m2() | boolean, class, date, function, null, number, object, regular expression,string or undefined |
+| method-calls.js:8:2:8:8 | o1.m1() | object |
+| method-calls.js:9:2:9:8 | o1.m2() | object |
 | method-calls.js:12:2:12:7 | o.m3() | boolean, class, date, function, null, number, object, regular expression,string or undefined |
 | method-calls.js:19:2:19:7 | o2.m() | boolean, class, date, function, null, number, object, regular expression,string or undefined |
-| method-calls.js:23:11:23:21 | {m: f1}.m() | boolean, class, date, function, null, number, object, regular expression,string or undefined |
-| method-calls.js:28:11:28:16 | o2.m() | boolean, class, date, function, null, number, object, regular expression,string or undefined |
-| method-calls.js:32:11:32:23 | ({m: f3}).m() | boolean, class, date, function, null, number, object, regular expression,string or undefined |
+| method-calls.js:23:11:23:21 | {m: f1}.m() | object |
+| method-calls.js:28:11:28:16 | o2.m() | object |
+| method-calls.js:32:11:32:23 | ({m: f3}).m() | object |
 | method-calls.js:37:11:37:16 | o4.m() | boolean, class, date, function, null, number, object, regular expression,string or undefined |
 | method-calls.js:43:12:43:16 | o.m() | boolean, class, date, function, null, number, object, regular expression,string or undefined |
 | method-calls.js:47:12:47:16 | o.m() | boolean, class, date, function, null, number, object, regular expression,string or undefined |
diff --git a/javascript/ql/test/library-tests/CapturedNodes/MethodCallTypeInferenceUsage.expected b/javascript/ql/test/library-tests/CapturedNodes/MethodCallTypeInferenceUsage.expected
index 8d3ed2834a6f..7c393230c8bc 100644
--- a/javascript/ql/test/library-tests/CapturedNodes/MethodCallTypeInferenceUsage.expected
+++ b/javascript/ql/test/library-tests/CapturedNodes/MethodCallTypeInferenceUsage.expected
@@ -1,4 +1,4 @@
-| method-calls.js:23:11:23:21 | {m: f1}.m() | method-calls.js:24:2:24:3 | v1 | file://:0:0:0:0 | indefinite value (call) |
-| method-calls.js:28:11:28:16 | o2.m() | method-calls.js:29:2:29:3 | v2 | file://:0:0:0:0 | indefinite value (call) |
-| method-calls.js:32:11:32:23 | ({m: f3}).m() | method-calls.js:33:2:33:3 | v3 | file://:0:0:0:0 | indefinite value (call) |
+| method-calls.js:23:11:23:21 | {m: f1}.m() | method-calls.js:24:2:24:3 | v1 | method-calls.js:22:23:22:24 | object literal |
+| method-calls.js:28:11:28:16 | o2.m() | method-calls.js:29:2:29:3 | v2 | method-calls.js:26:23:26:24 | object literal |
+| method-calls.js:32:11:32:23 | ({m: f3}).m() | method-calls.js:33:2:33:3 | v3 | method-calls.js:31:23:31:24 | object literal |
 | method-calls.js:37:11:37:16 | o4.m() | method-calls.js:38:2:38:3 | v4 | file://:0:0:0:0 | indefinite value (call) |
diff --git a/javascript/ql/test/library-tests/TypeInference/CallWithAnalyzedReturnFlow/CallWithAnalyzedReturnFlow.expected b/javascript/ql/test/library-tests/TypeInference/CallWithAnalyzedReturnFlow/CallWithAnalyzedReturnFlow.expected
index ab6686a25fff..650ca382a063 100644
--- a/javascript/ql/test/library-tests/TypeInference/CallWithAnalyzedReturnFlow/CallWithAnalyzedReturnFlow.expected
+++ b/javascript/ql/test/library-tests/TypeInference/CallWithAnalyzedReturnFlow/CallWithAnalyzedReturnFlow.expected
@@ -7,6 +7,7 @@
 | tst.js:11:5:11:8 | f1() | file://:0:0:0:0 | undefined |
 | tst.js:14:5:14:8 | f2() | file://:0:0:0:0 | undefined |
 | tst.js:15:5:15:8 | f2() | file://:0:0:0:0 | undefined |
+| tst.js:18:5:18:11 | o1.f3() | file://:0:0:0:0 | undefined |
 | tst.js:23:5:23:8 | f4() | tst.js:21:16:21:30 | function f5 |
 | tst.js:23:5:23:10 | f4()() | file://:0:0:0:0 | undefined |
 | tst.js:30:5:30:8 | f6() | tst.js:26:16:28:9 | function f7 |
diff --git a/javascript/ql/test/library-tests/TypeInference/CallWithAnalyzedReturnFlow/InvokeNodeValue.expected b/javascript/ql/test/library-tests/TypeInference/CallWithAnalyzedReturnFlow/InvokeNodeValue.expected
index 85b74fa0d7ed..a3a83471adab 100644
--- a/javascript/ql/test/library-tests/TypeInference/CallWithAnalyzedReturnFlow/InvokeNodeValue.expected
+++ b/javascript/ql/test/library-tests/TypeInference/CallWithAnalyzedReturnFlow/InvokeNodeValue.expected
@@ -11,7 +11,7 @@
 | tst.js:11:5:11:8 | f1() | file://:0:0:0:0 | undefined |
 | tst.js:14:5:14:8 | f2() | file://:0:0:0:0 | undefined |
 | tst.js:15:5:15:8 | f2() | file://:0:0:0:0 | undefined |
-| tst.js:18:5:18:11 | o1.f3() | file://:0:0:0:0 | indefinite value (call) |
+| tst.js:18:5:18:11 | o1.f3() | file://:0:0:0:0 | undefined |
 | tst.js:23:5:23:8 | f4() | tst.js:21:16:21:30 | function f5 |
 | tst.js:23:5:23:10 | f4()() | file://:0:0:0:0 | undefined |
 | tst.js:30:5:30:8 | f6() | tst.js:26:16:28:9 | function f7 |

From 676671686785114bf25b72ee925796d4c04ff00b Mon Sep 17 00:00:00 2001
From: Esben Sparre Andreasen <esben@semmle.com>
Date: Mon, 18 Feb 2019 15:00:45 +0100
Subject: [PATCH 07/16] JS: add PropWrite tests for parameter field
 initializers

---
 .../ql/test/library-tests/PropWrite/PropWrite.expected    | 4 ++++
 .../test/library-tests/PropWrite/PropWriteBase.expected   | 2 ++
 .../library-tests/PropWrite/PropWritePropName.expected    | 4 ++++
 .../ql/test/library-tests/PropWrite/PropWriteRhs.expected | 4 ++++
 javascript/ql/test/library-tests/PropWrite/classes.ts     | 8 ++++++++
 .../PropWrite/getAPropertyReference.expected              | 2 ++
 .../PropWrite/getAPropertyReference2.expected             | 2 ++
 .../library-tests/PropWrite/getAPropertySource.expected   | 3 +++
 .../library-tests/PropWrite/getAPropertyWrite.expected    | 2 ++
 .../library-tests/PropWrite/getAPropertyWrite2.expected   | 2 ++
 .../library-tests/PropWrite/hasPropertyWrite.expected     | 2 ++
 11 files changed, 35 insertions(+)

diff --git a/javascript/ql/test/library-tests/PropWrite/PropWrite.expected b/javascript/ql/test/library-tests/PropWrite/PropWrite.expected
index 091d4f792b65..fe5fd6f52968 100644
--- a/javascript/ql/test/library-tests/PropWrite/PropWrite.expected
+++ b/javascript/ql/test/library-tests/PropWrite/PropWrite.expected
@@ -2,6 +2,10 @@
 | classes.ts:4:3:4:24 | instanc ...  foo(); |
 | classes.ts:8:3:8:39 | constru ... eld) {} |
 | classes.ts:8:15:8:35 | public  ... erField |
+| classes.ts:12:5:12:68 | constru ... + 42; } |
+| classes.ts:12:17:12:37 | public  ... erField |
+| classes.ts:16:5:16:46 | constru ...  {}) {} |
+| classes.ts:16:17:16:37 | public  ... erField |
 | tst.js:3:5:3:8 | x: 4 |
 | tst.js:4:5:6:5 | func: f ... ;\\n    } |
 | tst.js:7:5:9:5 | f() {\\n  ... ;\\n    } |
diff --git a/javascript/ql/test/library-tests/PropWrite/PropWriteBase.expected b/javascript/ql/test/library-tests/PropWrite/PropWriteBase.expected
index 241e2c7d583f..1d8e78556df6 100644
--- a/javascript/ql/test/library-tests/PropWrite/PropWriteBase.expected
+++ b/javascript/ql/test/library-tests/PropWrite/PropWriteBase.expected
@@ -1,5 +1,7 @@
 | classes.ts:4:3:4:24 | instanc ...  foo(); | classes.ts:3:21:3:20 | this |
 | classes.ts:8:15:8:35 | public  ... erField | classes.ts:8:3:8:2 | this |
+| classes.ts:12:17:12:37 | public  ... erField | classes.ts:12:5:12:4 | this |
+| classes.ts:16:17:16:37 | public  ... erField | classes.ts:16:5:16:4 | this |
 | tst.js:3:5:3:8 | x: 4 | tst.js:2:11:10:1 | {\\n    x ...     }\\n} |
 | tst.js:4:5:6:5 | func: f ... ;\\n    } | tst.js:2:11:10:1 | {\\n    x ...     }\\n} |
 | tst.js:7:5:9:5 | f() {\\n  ... ;\\n    } | tst.js:2:11:10:1 | {\\n    x ...     }\\n} |
diff --git a/javascript/ql/test/library-tests/PropWrite/PropWritePropName.expected b/javascript/ql/test/library-tests/PropWrite/PropWritePropName.expected
index d2ac2679d25e..f00531e92e6d 100644
--- a/javascript/ql/test/library-tests/PropWrite/PropWritePropName.expected
+++ b/javascript/ql/test/library-tests/PropWrite/PropWritePropName.expected
@@ -2,6 +2,10 @@
 | classes.ts:4:3:4:24 | instanc ...  foo(); | instanceField |
 | classes.ts:8:3:8:39 | constru ... eld) {} | constructor |
 | classes.ts:8:15:8:35 | public  ... erField | parameterField |
+| classes.ts:12:5:12:68 | constru ... + 42; } | constructor |
+| classes.ts:12:17:12:37 | public  ... erField | parameterField |
+| classes.ts:16:5:16:46 | constru ...  {}) {} | constructor |
+| classes.ts:16:17:16:37 | public  ... erField | parameterField |
 | tst.js:3:5:3:8 | x: 4 | x |
 | tst.js:4:5:6:5 | func: f ... ;\\n    } | func |
 | tst.js:7:5:9:5 | f() {\\n  ... ;\\n    } | f |
diff --git a/javascript/ql/test/library-tests/PropWrite/PropWriteRhs.expected b/javascript/ql/test/library-tests/PropWrite/PropWriteRhs.expected
index 68c111317538..48eacf21bc54 100644
--- a/javascript/ql/test/library-tests/PropWrite/PropWriteRhs.expected
+++ b/javascript/ql/test/library-tests/PropWrite/PropWriteRhs.expected
@@ -2,6 +2,10 @@
 | classes.ts:4:3:4:24 | instanc ...  foo(); | classes.ts:4:19:4:23 | foo() |
 | classes.ts:8:3:8:39 | constru ... eld) {} | classes.ts:8:3:8:39 | constru ... eld) {} |
 | classes.ts:8:15:8:35 | public  ... erField | classes.ts:8:22:8:35 | parameterField |
+| classes.ts:12:5:12:68 | constru ... + 42; } | classes.ts:12:5:12:68 | constru ... + 42; } |
+| classes.ts:12:17:12:37 | public  ... erField | classes.ts:12:24:12:37 | parameterField |
+| classes.ts:16:5:16:46 | constru ...  {}) {} | classes.ts:16:5:16:46 | constru ...  {}) {} |
+| classes.ts:16:17:16:37 | public  ... erField | classes.ts:16:24:16:37 | parameterField |
 | tst.js:3:5:3:8 | x: 4 | tst.js:3:8:3:8 | 4 |
 | tst.js:4:5:6:5 | func: f ... ;\\n    } | tst.js:4:11:6:5 | functio ... ;\\n    } |
 | tst.js:7:5:9:5 | f() {\\n  ... ;\\n    } | tst.js:7:6:9:5 | () {\\n   ... ;\\n    } |
diff --git a/javascript/ql/test/library-tests/PropWrite/classes.ts b/javascript/ql/test/library-tests/PropWrite/classes.ts
index 7fff3a522c16..fed5f11c43e6 100644
--- a/javascript/ql/test/library-tests/PropWrite/classes.ts
+++ b/javascript/ql/test/library-tests/PropWrite/classes.ts
@@ -7,3 +7,11 @@ class InstanceField {
 class ParameterField {
   constructor(public parameterField) {}
 }
+
+class ParameterFieldInit  {
+    constructor(public parameterField = {}) { parameterField + 42; }
+}
+
+class ParameterFieldInitUnused  {
+    constructor(public parameterField = {}) {}
+}
diff --git a/javascript/ql/test/library-tests/PropWrite/getAPropertyReference.expected b/javascript/ql/test/library-tests/PropWrite/getAPropertyReference.expected
index bf832301b9b4..a6692956438c 100644
--- a/javascript/ql/test/library-tests/PropWrite/getAPropertyReference.expected
+++ b/javascript/ql/test/library-tests/PropWrite/getAPropertyReference.expected
@@ -1,5 +1,7 @@
 | classes.ts:3:21:3:20 | this | classes.ts:4:3:4:24 | instanc ...  foo(); |
 | classes.ts:8:3:8:2 | this | classes.ts:8:15:8:35 | public  ... erField |
+| classes.ts:12:5:12:4 | this | classes.ts:12:17:12:37 | public  ... erField |
+| classes.ts:16:5:16:4 | this | classes.ts:16:17:16:37 | public  ... erField |
 | tst.js:1:1:1:0 | this | tst.js:23:15:23:29 | this.someMethod |
 | tst.js:1:1:1:0 | this | tst.js:24:36:24:45 | this.state |
 | tst.js:2:11:10:1 | {\\n    x ...     }\\n} | tst.js:3:5:3:8 | x: 4 |
diff --git a/javascript/ql/test/library-tests/PropWrite/getAPropertyReference2.expected b/javascript/ql/test/library-tests/PropWrite/getAPropertyReference2.expected
index 55c3bb6ea681..8bf36ce9ae77 100644
--- a/javascript/ql/test/library-tests/PropWrite/getAPropertyReference2.expected
+++ b/javascript/ql/test/library-tests/PropWrite/getAPropertyReference2.expected
@@ -1,5 +1,7 @@
 | classes.ts:3:21:3:20 | this | instanceField | classes.ts:4:3:4:24 | instanc ...  foo(); |
 | classes.ts:8:3:8:2 | this | parameterField | classes.ts:8:15:8:35 | public  ... erField |
+| classes.ts:12:5:12:4 | this | parameterField | classes.ts:12:17:12:37 | public  ... erField |
+| classes.ts:16:5:16:4 | this | parameterField | classes.ts:16:17:16:37 | public  ... erField |
 | tst.js:1:1:1:0 | this | someMethod | tst.js:23:15:23:29 | this.someMethod |
 | tst.js:1:1:1:0 | this | state | tst.js:24:36:24:45 | this.state |
 | tst.js:2:11:10:1 | {\\n    x ...     }\\n} | f | tst.js:7:5:9:5 | f() {\\n  ... ;\\n    } |
diff --git a/javascript/ql/test/library-tests/PropWrite/getAPropertySource.expected b/javascript/ql/test/library-tests/PropWrite/getAPropertySource.expected
index 6cdb0154c11d..54f645ee4ccd 100644
--- a/javascript/ql/test/library-tests/PropWrite/getAPropertySource.expected
+++ b/javascript/ql/test/library-tests/PropWrite/getAPropertySource.expected
@@ -1,5 +1,8 @@
 | classes.ts:3:21:3:20 | this | instanceField | classes.ts:4:19:4:23 | foo() |
 | classes.ts:8:3:8:2 | this | parameterField | classes.ts:8:22:8:35 | parameterField |
+| classes.ts:12:5:12:4 | this | parameterField | classes.ts:12:24:12:37 | parameterField |
+| classes.ts:12:5:12:4 | this | parameterField | classes.ts:12:41:12:42 | {} |
+| classes.ts:16:5:16:4 | this | parameterField | classes.ts:16:24:16:37 | parameterField |
 | tst.js:2:11:10:1 | {\\n    x ...     }\\n} | f | tst.js:7:6:9:5 | () {\\n   ... ;\\n    } |
 | tst.js:2:11:10:1 | {\\n    x ...     }\\n} | func | tst.js:4:11:6:5 | functio ... ;\\n    } |
 | tst.js:12:1:19:1 | class C ... ;\\n  }\\n} | func | tst.js:13:14:15:3 | (x) {\\n  ... x);\\n  } |
diff --git a/javascript/ql/test/library-tests/PropWrite/getAPropertyWrite.expected b/javascript/ql/test/library-tests/PropWrite/getAPropertyWrite.expected
index 39344ef11d7b..485beb857984 100644
--- a/javascript/ql/test/library-tests/PropWrite/getAPropertyWrite.expected
+++ b/javascript/ql/test/library-tests/PropWrite/getAPropertyWrite.expected
@@ -1,5 +1,7 @@
 | classes.ts:3:21:3:20 | this | classes.ts:4:3:4:24 | instanc ...  foo(); |
 | classes.ts:8:3:8:2 | this | classes.ts:8:15:8:35 | public  ... erField |
+| classes.ts:12:5:12:4 | this | classes.ts:12:17:12:37 | public  ... erField |
+| classes.ts:16:5:16:4 | this | classes.ts:16:17:16:37 | public  ... erField |
 | tst.js:2:11:10:1 | {\\n    x ...     }\\n} | tst.js:3:5:3:8 | x: 4 |
 | tst.js:2:11:10:1 | {\\n    x ...     }\\n} | tst.js:4:5:6:5 | func: f ... ;\\n    } |
 | tst.js:2:11:10:1 | {\\n    x ...     }\\n} | tst.js:7:5:9:5 | f() {\\n  ... ;\\n    } |
diff --git a/javascript/ql/test/library-tests/PropWrite/getAPropertyWrite2.expected b/javascript/ql/test/library-tests/PropWrite/getAPropertyWrite2.expected
index 4e38837a447b..e276e476ec83 100644
--- a/javascript/ql/test/library-tests/PropWrite/getAPropertyWrite2.expected
+++ b/javascript/ql/test/library-tests/PropWrite/getAPropertyWrite2.expected
@@ -1,5 +1,7 @@
 | classes.ts:3:21:3:20 | this | instanceField | classes.ts:4:3:4:24 | instanc ...  foo(); |
 | classes.ts:8:3:8:2 | this | parameterField | classes.ts:8:15:8:35 | public  ... erField |
+| classes.ts:12:5:12:4 | this | parameterField | classes.ts:12:17:12:37 | public  ... erField |
+| classes.ts:16:5:16:4 | this | parameterField | classes.ts:16:17:16:37 | public  ... erField |
 | tst.js:2:11:10:1 | {\\n    x ...     }\\n} | f | tst.js:7:5:9:5 | f() {\\n  ... ;\\n    } |
 | tst.js:2:11:10:1 | {\\n    x ...     }\\n} | func | tst.js:4:5:6:5 | func: f ... ;\\n    } |
 | tst.js:2:11:10:1 | {\\n    x ...     }\\n} | x | tst.js:3:5:3:8 | x: 4 |
diff --git a/javascript/ql/test/library-tests/PropWrite/hasPropertyWrite.expected b/javascript/ql/test/library-tests/PropWrite/hasPropertyWrite.expected
index b4d999ab9a29..4a8c9979f558 100644
--- a/javascript/ql/test/library-tests/PropWrite/hasPropertyWrite.expected
+++ b/javascript/ql/test/library-tests/PropWrite/hasPropertyWrite.expected
@@ -1,5 +1,7 @@
 | classes.ts:3:21:3:20 | this | instanceField | classes.ts:4:19:4:23 | foo() |
 | classes.ts:8:3:8:2 | this | parameterField | classes.ts:8:22:8:35 | parameterField |
+| classes.ts:12:5:12:4 | this | parameterField | classes.ts:12:24:12:37 | parameterField |
+| classes.ts:16:5:16:4 | this | parameterField | classes.ts:16:24:16:37 | parameterField |
 | tst.js:2:11:10:1 | {\\n    x ...     }\\n} | f | tst.js:7:6:9:5 | () {\\n   ... ;\\n    } |
 | tst.js:2:11:10:1 | {\\n    x ...     }\\n} | func | tst.js:4:11:6:5 | functio ... ;\\n    } |
 | tst.js:2:11:10:1 | {\\n    x ...     }\\n} | x | tst.js:3:8:3:8 | 4 |

From 6c1b29e4b6f467b78f3e9fa8a800be24a5eac4f8 Mon Sep 17 00:00:00 2001
From: Esben Sparre Andreasen <esben@semmle.com>
Date: Mon, 18 Feb 2019 15:05:03 +0100
Subject: [PATCH 08/16] JS: add missing flowstep for unused parameter field
 initializers

---
 .../ql/src/semmle/javascript/dataflow/DataFlow.qll  | 13 ++++++++++++-
 .../library-tests/PropWrite/PropWriteRhs.expected   |  1 +
 .../PropWrite/getAPropertySource.expected           |  1 +
 .../PropWrite/hasPropertyWrite.expected             |  1 +
 4 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/javascript/ql/src/semmle/javascript/dataflow/DataFlow.qll b/javascript/ql/src/semmle/javascript/dataflow/DataFlow.qll
index 647f4c7a2378..5c0482ab9df9 100644
--- a/javascript/ql/src/semmle/javascript/dataflow/DataFlow.qll
+++ b/javascript/ql/src/semmle/javascript/dataflow/DataFlow.qll
@@ -558,7 +558,18 @@ module DataFlow {
 
     override string getPropertyName() { result = prop.getName() }
 
-    override Node getRhs() { result = parameterNode(prop.getParameter()) }
+    override Node getRhs() {
+      exists(Parameter param, Node paramNode |
+        param = prop.getParameter() and
+        parameterNode(paramNode, param)
+        |
+        result = paramNode
+        or
+        // special case: there is no SSA flow step for unused parameters
+        paramNode instanceof UnusedParameterNode and
+        result = param.getDefault().flow()
+      )
+    }
 
     override ControlFlowNode getWriteNode() { result = prop.getParameter() }
   }
diff --git a/javascript/ql/test/library-tests/PropWrite/PropWriteRhs.expected b/javascript/ql/test/library-tests/PropWrite/PropWriteRhs.expected
index 48eacf21bc54..42087edc9c1e 100644
--- a/javascript/ql/test/library-tests/PropWrite/PropWriteRhs.expected
+++ b/javascript/ql/test/library-tests/PropWrite/PropWriteRhs.expected
@@ -6,6 +6,7 @@
 | classes.ts:12:17:12:37 | public  ... erField | classes.ts:12:24:12:37 | parameterField |
 | classes.ts:16:5:16:46 | constru ...  {}) {} | classes.ts:16:5:16:46 | constru ...  {}) {} |
 | classes.ts:16:17:16:37 | public  ... erField | classes.ts:16:24:16:37 | parameterField |
+| classes.ts:16:17:16:37 | public  ... erField | classes.ts:16:41:16:42 | {} |
 | tst.js:3:5:3:8 | x: 4 | tst.js:3:8:3:8 | 4 |
 | tst.js:4:5:6:5 | func: f ... ;\\n    } | tst.js:4:11:6:5 | functio ... ;\\n    } |
 | tst.js:7:5:9:5 | f() {\\n  ... ;\\n    } | tst.js:7:6:9:5 | () {\\n   ... ;\\n    } |
diff --git a/javascript/ql/test/library-tests/PropWrite/getAPropertySource.expected b/javascript/ql/test/library-tests/PropWrite/getAPropertySource.expected
index 54f645ee4ccd..101dba0e8a49 100644
--- a/javascript/ql/test/library-tests/PropWrite/getAPropertySource.expected
+++ b/javascript/ql/test/library-tests/PropWrite/getAPropertySource.expected
@@ -3,6 +3,7 @@
 | classes.ts:12:5:12:4 | this | parameterField | classes.ts:12:24:12:37 | parameterField |
 | classes.ts:12:5:12:4 | this | parameterField | classes.ts:12:41:12:42 | {} |
 | classes.ts:16:5:16:4 | this | parameterField | classes.ts:16:24:16:37 | parameterField |
+| classes.ts:16:5:16:4 | this | parameterField | classes.ts:16:41:16:42 | {} |
 | tst.js:2:11:10:1 | {\\n    x ...     }\\n} | f | tst.js:7:6:9:5 | () {\\n   ... ;\\n    } |
 | tst.js:2:11:10:1 | {\\n    x ...     }\\n} | func | tst.js:4:11:6:5 | functio ... ;\\n    } |
 | tst.js:12:1:19:1 | class C ... ;\\n  }\\n} | func | tst.js:13:14:15:3 | (x) {\\n  ... x);\\n  } |
diff --git a/javascript/ql/test/library-tests/PropWrite/hasPropertyWrite.expected b/javascript/ql/test/library-tests/PropWrite/hasPropertyWrite.expected
index 4a8c9979f558..e6138ec0a33d 100644
--- a/javascript/ql/test/library-tests/PropWrite/hasPropertyWrite.expected
+++ b/javascript/ql/test/library-tests/PropWrite/hasPropertyWrite.expected
@@ -2,6 +2,7 @@
 | classes.ts:8:3:8:2 | this | parameterField | classes.ts:8:22:8:35 | parameterField |
 | classes.ts:12:5:12:4 | this | parameterField | classes.ts:12:24:12:37 | parameterField |
 | classes.ts:16:5:16:4 | this | parameterField | classes.ts:16:24:16:37 | parameterField |
+| classes.ts:16:5:16:4 | this | parameterField | classes.ts:16:41:16:42 | {} |
 | tst.js:2:11:10:1 | {\\n    x ...     }\\n} | f | tst.js:7:6:9:5 | () {\\n   ... ;\\n    } |
 | tst.js:2:11:10:1 | {\\n    x ...     }\\n} | func | tst.js:4:11:6:5 | functio ... ;\\n    } |
 | tst.js:2:11:10:1 | {\\n    x ...     }\\n} | x | tst.js:3:8:3:8 | 4 |

From 047b69a4c2fd73de2fc737802ad142cd40121e66 Mon Sep 17 00:00:00 2001
From: Esben Sparre Andreasen <esben@semmle.com>
Date: Mon, 25 Feb 2019 15:08:56 +0100
Subject: [PATCH 09/16] JS: address review comments

---
 .../ql/src/Declarations/UnusedParameter.qll   |  2 +-
 .../ql/src/Declarations/UnusedProperty.ql     | 30 +++++++++++--------
 .../ql/src/Declarations/UnusedVariable.qll    |  2 +-
 .../ql/src/Expressions/ExprHasNoEffect.qll    |  2 +-
 .../javascript/dataflow/CapturedNodes.qll     |  4 +--
 .../internal/InterProceduralTypeInference.qll | 18 ++++++-----
 6 files changed, 32 insertions(+), 26 deletions(-)

diff --git a/javascript/ql/src/Declarations/UnusedParameter.qll b/javascript/ql/src/Declarations/UnusedParameter.qll
index b4fb5cb578a7..b8ff6b1d38e5 100644
--- a/javascript/ql/src/Declarations/UnusedParameter.qll
+++ b/javascript/ql/src/Declarations/UnusedParameter.qll
@@ -1,5 +1,5 @@
 /**
- * This library contains the majority of the 'js/unused-parameter' query implementation.
+ * Provides classes and predicates for the 'js/unused-parameter' query.
  *
  * In order to suppress alerts that are similar to the 'js/unused-parameter' alerts,
  * `isAnAccidentallyUnusedParameter` should be used since it holds iff that alert is active.
diff --git a/javascript/ql/src/Declarations/UnusedProperty.ql b/javascript/ql/src/Declarations/UnusedProperty.ql
index b364a2b21483..117d402e0dff 100644
--- a/javascript/ql/src/Declarations/UnusedProperty.ql
+++ b/javascript/ql/src/Declarations/UnusedProperty.ql
@@ -30,9 +30,12 @@ predicate hasUnknownPropertyRead(CapturedSource obj) {
   exists(obj.getAPropertyRead("propertyIsEnumerable"))
 }
 
-predicate flowsToTypeRestrictedExpression(CapturedSource n) {
+/**
+ * Holds if `obj` flows to an expression that must have a specific type.
+ */
+predicate flowsToTypeRestrictedExpression(CapturedSource obj) {
   exists (Expr restricted, TypeExpr type |
-    n.flowsToExpr(restricted) and
+    obj.flowsToExpr(restricted) and
     not type.isAny() |
     exists (TypeAssertion assertion |
       type = assertion.getTypeAnnotation() and
@@ -47,14 +50,15 @@ predicate flowsToTypeRestrictedExpression(CapturedSource n) {
   )
 }
 
-from DataFlow::PropWrite w, CapturedSource n, string name
+from DataFlow::PropWrite write, CapturedSource obj, string name
 where
-  w = n.getAPropertyWrite(name) and
-  not exists(n.getAPropertyRead(name)) and
-  not w.getBase().analyze().getAValue() != n.analyze().getAValue() and
-  not hasUnknownPropertyRead(n) and
+  write = obj.getAPropertyWrite(name) and
+  not exists(obj.getAPropertyRead(name)) and
+  // `obj` is the only base object for the write: it is not spurious
+  not write.getBase().analyze().getAValue() != obj.analyze().getAValue() and
+  not hasUnknownPropertyRead(obj) and
   // avoid reporting if the definition is unreachable
-  w.getAstNode().getFirstControlFlowNode().getBasicBlock() instanceof ReachableBasicBlock and
+  write.getAstNode().getFirstControlFlowNode().getBasicBlock() instanceof ReachableBasicBlock and
   // avoid implicitly read properties
   not (
     name = "toString" or
@@ -62,13 +66,13 @@ where
     name.matches("@@%") // @@iterator, for example
   ) and
   // avoid flagging properties that a type system requires
-  not flowsToTypeRestrictedExpression(n) and
+  not flowsToTypeRestrictedExpression(obj) and
   // flagged by js/unused-local-variable
-  not exists(UnusedLocal l | l.getAnAssignedExpr().getUnderlyingValue().flow() = n) and
+  not exists(UnusedLocal l | l.getAnAssignedExpr().getUnderlyingValue().flow() = obj) and
   // flagged by js/unused-parameter
   not exists(Parameter p | isAnAccidentallyUnusedParameter(p) |
-    p.getDefault().getUnderlyingValue().flow() = n
+    p.getDefault().getUnderlyingValue().flow() = obj
   ) and
   // flagged by js/useless-expression
-  not inVoidContext(n.asExpr())
-select w, "Unused property " + name + "."
+  not inVoidContext(obj.asExpr())
+select write, "Unused property " + name + "."
diff --git a/javascript/ql/src/Declarations/UnusedVariable.qll b/javascript/ql/src/Declarations/UnusedVariable.qll
index 98d69b80938c..143a8c5e0746 100644
--- a/javascript/ql/src/Declarations/UnusedVariable.qll
+++ b/javascript/ql/src/Declarations/UnusedVariable.qll
@@ -1,5 +1,5 @@
 /**
- * This library contains parts of the 'js/unused-local-variable' query implementation.
+ * Provides classes and predicates for the 'js/unused-local-variable' query.
  */
 
 import javascript
diff --git a/javascript/ql/src/Expressions/ExprHasNoEffect.qll b/javascript/ql/src/Expressions/ExprHasNoEffect.qll
index 3b6f4703ad6e..858f719ba0a0 100644
--- a/javascript/ql/src/Expressions/ExprHasNoEffect.qll
+++ b/javascript/ql/src/Expressions/ExprHasNoEffect.qll
@@ -1,5 +1,5 @@
 /**
- * This library contains parts of the 'js/useless-expression' query implementation.
+ * Provides classes and predicates for the 'js/useless-expression' query.
  */
 
 import javascript
diff --git a/javascript/ql/src/semmle/javascript/dataflow/CapturedNodes.qll b/javascript/ql/src/semmle/javascript/dataflow/CapturedNodes.qll
index bcb9c44abe5a..db4a0c309399 100644
--- a/javascript/ql/src/semmle/javascript/dataflow/CapturedNodes.qll
+++ b/javascript/ql/src/semmle/javascript/dataflow/CapturedNodes.qll
@@ -42,7 +42,7 @@ private predicate exposedAsReceiver(DataFlow::SourceNode n) {
   n.flowsToExpr(any(FunctionBindExpr bind).getObject())
   or
   // technically, the builtin prototypes could have a `this`-using function through which this node escapes, but we ignore that here
-  // (we also ignore `o['__' + 'proto__"] = ...`)
+  // (we also ignore `o['__' + 'proto__'] = ...`)
   exists(n.getAPropertyWrite("__proto__"))
   or
   // could check the assigned value of all affected variables, but it is unlikely to matter in practice
@@ -51,7 +51,7 @@ private predicate exposedAsReceiver(DataFlow::SourceNode n) {
 
 /**
  * A source for which the flow is entirely captured by the dataflow library.
- * All uses of the node is represented by `this.flowsTo(_)` and friends.
+ * All uses of the node are modeled by `this.flowsTo(_)` and related predicates.
  */
 class CapturedSource extends DataFlow::SourceNode {
   CapturedSource() {
diff --git a/javascript/ql/src/semmle/javascript/dataflow/internal/InterProceduralTypeInference.qll b/javascript/ql/src/semmle/javascript/dataflow/internal/InterProceduralTypeInference.qll
index fcb3e00fb255..9574f7caad2e 100644
--- a/javascript/ql/src/semmle/javascript/dataflow/internal/InterProceduralTypeInference.qll
+++ b/javascript/ql/src/semmle/javascript/dataflow/internal/InterProceduralTypeInference.qll
@@ -256,20 +256,22 @@ private predicate hasDefiniteReceiver(
  * Enables inter-procedural type inference for the return value of a
  * method call to a flow-insensitively type-inferred callee.
  */
-class TypeInferredMethodWithAnalyzedReturnFlow extends CallWithNonLocalAnalyzedReturnFlow {
+private class TypeInferredMethodWithAnalyzedReturnFlow extends CallWithNonLocalAnalyzedReturnFlow {
   DataFlow::FunctionNode fun;
 
   TypeInferredMethodWithAnalyzedReturnFlow() {
-    exists(CapturedSource s, DataFlow::PropWrite w, string name |
+    exists(CapturedSource obj, DataFlow::PropWrite write, string name |
       this.(DataFlow::MethodCallNode).getMethodName() = name and
-      s.hasOwnProperty(name) and
-      hasDefiniteReceiver(this, s) and
-      w = s.getAPropertyWrite() and
-      fun.flowsTo(w.getRhs()) and
+      obj.hasOwnProperty(name) and
+      hasDefiniteReceiver(this, obj) and
+      // include all potential callees
+      // by construction, there are no unknown methods on `obj`
+      write = obj.getAPropertyWrite() and
+      fun.flowsTo(write.getRhs()) and
       (
-        not exists(w.getPropertyName())
+        not exists(write.getPropertyName())
         or
-        w.getPropertyName() = name
+        write.getPropertyName() = name
       )
     )
   }

From 0d94fe3f540e9e18bdcc5659a14bf709f8e46718 Mon Sep 17 00:00:00 2001
From: Esben Sparre Andreasen <esben@semmle.com>
Date: Mon, 25 Feb 2019 15:31:53 +0100
Subject: [PATCH 10/16] JS: analyze assignments in `with` correctly

---
 .../ql/src/semmle/javascript/dataflow/CapturedNodes.qll     | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/javascript/ql/src/semmle/javascript/dataflow/CapturedNodes.qll b/javascript/ql/src/semmle/javascript/dataflow/CapturedNodes.qll
index db4a0c309399..85cbd3ae5fb0 100644
--- a/javascript/ql/src/semmle/javascript/dataflow/CapturedNodes.qll
+++ b/javascript/ql/src/semmle/javascript/dataflow/CapturedNodes.qll
@@ -20,7 +20,11 @@ private predicate isEscape(DataFlow::Node escape, string cause) {
   or
   escape = any(ExportDeclaration e).getSourceNode(_) and cause = "export"
   or
-  any(WithStmt with).mayAffect(escape.asExpr()) and cause = "heap"
+  exists (WithStmt with, Assignment assign |
+    with.mayAffect(assign.getLhs()) and
+    assign.getRhs().flow() = escape and
+    cause = "heap"
+  )
 }
 
 private DataFlow::Node getAnEscape() {

From 1150f4c02b17e1bb4124142a60b33a85782ec11e Mon Sep 17 00:00:00 2001
From: Esben Sparre Andreasen <esben@semmle.com>
Date: Mon, 25 Feb 2019 15:52:23 +0100
Subject: [PATCH 11/16] JS: add documentation to test case

---
 .../CapturedNodes/CapturedSource.expected     |  2 +-
 .../CapturedSource_hasOwnProperty.expected    |  2 +-
 .../CapturedNodes/method-calls.js             | 28 +++++++++----------
 3 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/javascript/ql/test/library-tests/CapturedNodes/CapturedSource.expected b/javascript/ql/test/library-tests/CapturedNodes/CapturedSource.expected
index d78dd6e228ff..c0e0331413f5 100644
--- a/javascript/ql/test/library-tests/CapturedNodes/CapturedSource.expected
+++ b/javascript/ql/test/library-tests/CapturedNodes/CapturedSource.expected
@@ -4,7 +4,7 @@
 | method-calls.js:23:11:23:17 | {m: f1} |
 | method-calls.js:27:11:27:17 | {m: f2} |
 | method-calls.js:32:12:32:18 | {m: f3} |
-| method-calls.js:36:15:36:21 | {m: f2} |
+| method-calls.js:36:15:36:21 | {m: f4} |
 | method-calls.js:42:16:42:28 | {m: () => 42} |
 | method-calls.js:46:17:46:29 | {m: () => 42} |
 | method-calls.js:50:16:50:28 | {m: () => 42} |
diff --git a/javascript/ql/test/library-tests/CapturedNodes/CapturedSource_hasOwnProperty.expected b/javascript/ql/test/library-tests/CapturedNodes/CapturedSource_hasOwnProperty.expected
index 76e494cf1ad0..3fb32b5407ed 100644
--- a/javascript/ql/test/library-tests/CapturedNodes/CapturedSource_hasOwnProperty.expected
+++ b/javascript/ql/test/library-tests/CapturedNodes/CapturedSource_hasOwnProperty.expected
@@ -5,7 +5,7 @@
 | method-calls.js:23:11:23:17 | {m: f1} | m |
 | method-calls.js:27:11:27:17 | {m: f2} | m |
 | method-calls.js:32:12:32:18 | {m: f3} | m |
-| method-calls.js:36:15:36:21 | {m: f2} | m |
+| method-calls.js:36:15:36:21 | {m: f4} | m |
 | method-calls.js:42:16:42:28 | {m: () => 42} | m |
 | method-calls.js:46:17:46:29 | {m: () => 42} | m |
 | method-calls.js:50:16:50:28 | {m: () => 42} | m |
diff --git a/javascript/ql/test/library-tests/CapturedNodes/method-calls.js b/javascript/ql/test/library-tests/CapturedNodes/method-calls.js
index f17f72e58962..bee0b6a58030 100644
--- a/javascript/ql/test/library-tests/CapturedNodes/method-calls.js
+++ b/javascript/ql/test/library-tests/CapturedNodes/method-calls.js
@@ -5,53 +5,53 @@
 		m3: function(){ return {}; },
 		m4: function(){ return {}; }
 	};
-	o1.m1();
-	o1.m2();
+	o1.m1(); // analyzed precisely
+	o1.m2(); // analyzed precisely
 	unknown(o1.m2);
 	var o = unknown? o1: {};
-	o.m3(); // NOT supported
+	o.m3(); // not analyzed precisely: `o1` is not the only receiver.
 	var m4 = o.m4;
-	m4();
+	m4(); // (not a method call)
 
 	var o2 = {};
 	o2.m = function() { return {}; };
-	o2[unknown] = function() { return true; }; // could be __proto__
+	// not analyzed precisely: `m` may be in the prototype since `m` is not in the initializer, and we do not attempt to reason flow sensitively beyond that at the moment
 	o2.m();
 });
 (function(){
 	function f1(){return {};}
-	var v1 = {m: f1}.m();
+	var v1 = {m: f1}.m(); // analyzed precisely
 	v1 === true;
 
 	function f2(){return {};}
 	var o2 = {m: f2};
-	var v2 = o2.m();
+	var v2 = o2.m(); // analyzed precisely
 	v2 === true;
 
 	function f3(){return {};}
-	var v3 = ({m: f3}).m();
+	var v3 = ({m: f3}).m(); // analyzed precisely
 	v3 === true;
 
 	function f4(){return {};}
-	var { o4 } = {m: f2};
-	var v4 = o4.m();
+	var { o4 } = {m: f4};
+	var v4 = o4.m(); // not analyzed precisely: o4 is from a destructuring assignment (and is even `undefined` in this case
 	v4 === true;
 });
 
 (function(){
 	(function(o = {m: () => 42}){
-		var v1 = o.m();
+		var v1 = o.m(); // not analyzed precisely: `o` may be `unknown`
 	})(unknown);
 
 	function f(o = {m: () => 42}){
-		var v2 = o.m();
+		var v2 = o.m(); // not analyzed precisely: `o` may be `unknown`
 	};
 	f(unknown);
 	(function(o = {m: () => 42}){
-		var v3 = o.m();
+		var v3 = o.m(); // not analyzed precisely: `o.m` may be `unknown`
 	})({m: unknown});
 	(function(o = {m: () => 42}){
-		var v4 = o.m();
+		var v4 = o.m(); // not analyzed precisely: we only support unique receivers at the moment
 	})({m: () => true});
 
 });

From 65fb1423b72994279391e2883241c2259504de80 Mon Sep 17 00:00:00 2001
From: Esben Sparre Andreasen <esben@semmle.com>
Date: Mon, 25 Feb 2019 15:55:44 +0100
Subject: [PATCH 12/16] JS: format test case (update expected output)

---
 .../CapturedNodes/CapturedSource.expected        | 16 ++++++++--------
 .../CapturedSource_hasOwnProperty.expected       | 16 ++++++++--------
 .../MethodCallTypeInference.expected             | 14 +++++++-------
 .../MethodCallTypeInferenceUsage.expected        |  8 ++++----
 .../library-tests/CapturedNodes/method-calls.js  | 12 +++++++++---
 5 files changed, 36 insertions(+), 30 deletions(-)

diff --git a/javascript/ql/test/library-tests/CapturedNodes/CapturedSource.expected b/javascript/ql/test/library-tests/CapturedNodes/CapturedSource.expected
index c0e0331413f5..173282ed749b 100644
--- a/javascript/ql/test/library-tests/CapturedNodes/CapturedSource.expected
+++ b/javascript/ql/test/library-tests/CapturedNodes/CapturedSource.expected
@@ -1,14 +1,14 @@
 | method-calls.js:2:11:7:2 | {\\n\\t\\tm1: ... }; }\\n\\t} |
 | method-calls.js:11:23:11:24 | {} |
 | method-calls.js:16:11:16:12 | {} |
-| method-calls.js:23:11:23:17 | {m: f1} |
-| method-calls.js:27:11:27:17 | {m: f2} |
-| method-calls.js:32:12:32:18 | {m: f3} |
-| method-calls.js:36:15:36:21 | {m: f4} |
-| method-calls.js:42:16:42:28 | {m: () => 42} |
-| method-calls.js:46:17:46:29 | {m: () => 42} |
-| method-calls.js:50:16:50:28 | {m: () => 42} |
-| method-calls.js:53:16:53:28 | {m: () => 42} |
+| method-calls.js:25:11:25:17 | {m: f1} |
+| method-calls.js:29:11:29:17 | {m: f2} |
+| method-calls.js:34:12:34:18 | {m: f3} |
+| method-calls.js:38:15:38:21 | {m: f4} |
+| method-calls.js:46:16:46:28 | {m: () => 42} |
+| method-calls.js:50:17:50:29 | {m: () => 42} |
+| method-calls.js:54:16:54:28 | {m: () => 42} |
+| method-calls.js:57:16:57:28 | {m: () => 42} |
 | tst.js:3:18:3:19 | {} |
 | tst.js:4:18:4:36 | { f: function(){} } |
 | tst.js:5:18:5:34 | { [unknown]: 42 } |
diff --git a/javascript/ql/test/library-tests/CapturedNodes/CapturedSource_hasOwnProperty.expected b/javascript/ql/test/library-tests/CapturedNodes/CapturedSource_hasOwnProperty.expected
index 3fb32b5407ed..ef9d64c3c70a 100644
--- a/javascript/ql/test/library-tests/CapturedNodes/CapturedSource_hasOwnProperty.expected
+++ b/javascript/ql/test/library-tests/CapturedNodes/CapturedSource_hasOwnProperty.expected
@@ -2,14 +2,14 @@
 | method-calls.js:2:11:7:2 | {\\n\\t\\tm1: ... }; }\\n\\t} | m2 |
 | method-calls.js:2:11:7:2 | {\\n\\t\\tm1: ... }; }\\n\\t} | m3 |
 | method-calls.js:2:11:7:2 | {\\n\\t\\tm1: ... }; }\\n\\t} | m4 |
-| method-calls.js:23:11:23:17 | {m: f1} | m |
-| method-calls.js:27:11:27:17 | {m: f2} | m |
-| method-calls.js:32:12:32:18 | {m: f3} | m |
-| method-calls.js:36:15:36:21 | {m: f4} | m |
-| method-calls.js:42:16:42:28 | {m: () => 42} | m |
-| method-calls.js:46:17:46:29 | {m: () => 42} | m |
-| method-calls.js:50:16:50:28 | {m: () => 42} | m |
-| method-calls.js:53:16:53:28 | {m: () => 42} | m |
+| method-calls.js:25:11:25:17 | {m: f1} | m |
+| method-calls.js:29:11:29:17 | {m: f2} | m |
+| method-calls.js:34:12:34:18 | {m: f3} | m |
+| method-calls.js:38:15:38:21 | {m: f4} | m |
+| method-calls.js:46:16:46:28 | {m: () => 42} | m |
+| method-calls.js:50:17:50:29 | {m: () => 42} | m |
+| method-calls.js:54:16:54:28 | {m: () => 42} | m |
+| method-calls.js:57:16:57:28 | {m: () => 42} | m |
 | tst.js:4:18:4:36 | { f: function(){} } | f |
 | tst.js:38:18:38:26 | { p: 42 } | p |
 | tst.js:42:18:42:33 | { p: 42, q: 42 } | p |
diff --git a/javascript/ql/test/library-tests/CapturedNodes/MethodCallTypeInference.expected b/javascript/ql/test/library-tests/CapturedNodes/MethodCallTypeInference.expected
index 1192bcdb1275..afab24f56bca 100644
--- a/javascript/ql/test/library-tests/CapturedNodes/MethodCallTypeInference.expected
+++ b/javascript/ql/test/library-tests/CapturedNodes/MethodCallTypeInference.expected
@@ -1,12 +1,12 @@
 | method-calls.js:8:2:8:8 | o1.m1() | object |
 | method-calls.js:9:2:9:8 | o1.m2() | object |
 | method-calls.js:12:2:12:7 | o.m3() | boolean, class, date, function, null, number, object, regular expression,string or undefined |
-| method-calls.js:19:2:19:7 | o2.m() | boolean, class, date, function, null, number, object, regular expression,string or undefined |
-| method-calls.js:23:11:23:21 | {m: f1}.m() | object |
-| method-calls.js:28:11:28:16 | o2.m() | object |
-| method-calls.js:32:11:32:23 | ({m: f3}).m() | object |
-| method-calls.js:37:11:37:16 | o4.m() | boolean, class, date, function, null, number, object, regular expression,string or undefined |
-| method-calls.js:43:12:43:16 | o.m() | boolean, class, date, function, null, number, object, regular expression,string or undefined |
+| method-calls.js:21:2:21:7 | o2.m() | boolean, class, date, function, null, number, object, regular expression,string or undefined |
+| method-calls.js:25:11:25:21 | {m: f1}.m() | object |
+| method-calls.js:30:11:30:16 | o2.m() | object |
+| method-calls.js:34:11:34:23 | ({m: f3}).m() | object |
+| method-calls.js:41:11:41:16 | o4.m() | boolean, class, date, function, null, number, object, regular expression,string or undefined |
 | method-calls.js:47:12:47:16 | o.m() | boolean, class, date, function, null, number, object, regular expression,string or undefined |
 | method-calls.js:51:12:51:16 | o.m() | boolean, class, date, function, null, number, object, regular expression,string or undefined |
-| method-calls.js:54:12:54:16 | o.m() | boolean, class, date, function, null, number, object, regular expression,string or undefined |
+| method-calls.js:55:12:55:16 | o.m() | boolean, class, date, function, null, number, object, regular expression,string or undefined |
+| method-calls.js:60:12:60:16 | o.m() | boolean, class, date, function, null, number, object, regular expression,string or undefined |
diff --git a/javascript/ql/test/library-tests/CapturedNodes/MethodCallTypeInferenceUsage.expected b/javascript/ql/test/library-tests/CapturedNodes/MethodCallTypeInferenceUsage.expected
index 7c393230c8bc..257fd863f811 100644
--- a/javascript/ql/test/library-tests/CapturedNodes/MethodCallTypeInferenceUsage.expected
+++ b/javascript/ql/test/library-tests/CapturedNodes/MethodCallTypeInferenceUsage.expected
@@ -1,4 +1,4 @@
-| method-calls.js:23:11:23:21 | {m: f1}.m() | method-calls.js:24:2:24:3 | v1 | method-calls.js:22:23:22:24 | object literal |
-| method-calls.js:28:11:28:16 | o2.m() | method-calls.js:29:2:29:3 | v2 | method-calls.js:26:23:26:24 | object literal |
-| method-calls.js:32:11:32:23 | ({m: f3}).m() | method-calls.js:33:2:33:3 | v3 | method-calls.js:31:23:31:24 | object literal |
-| method-calls.js:37:11:37:16 | o4.m() | method-calls.js:38:2:38:3 | v4 | file://:0:0:0:0 | indefinite value (call) |
+| method-calls.js:25:11:25:21 | {m: f1}.m() | method-calls.js:26:2:26:3 | v1 | method-calls.js:24:23:24:24 | object literal |
+| method-calls.js:30:11:30:16 | o2.m() | method-calls.js:31:2:31:3 | v2 | method-calls.js:28:23:28:24 | object literal |
+| method-calls.js:34:11:34:23 | ({m: f3}).m() | method-calls.js:35:2:35:3 | v3 | method-calls.js:33:23:33:24 | object literal |
+| method-calls.js:41:11:41:16 | o4.m() | method-calls.js:42:2:42:3 | v4 | file://:0:0:0:0 | indefinite value (call) |
diff --git a/javascript/ql/test/library-tests/CapturedNodes/method-calls.js b/javascript/ql/test/library-tests/CapturedNodes/method-calls.js
index bee0b6a58030..79947632eea3 100644
--- a/javascript/ql/test/library-tests/CapturedNodes/method-calls.js
+++ b/javascript/ql/test/library-tests/CapturedNodes/method-calls.js
@@ -15,7 +15,9 @@
 
 	var o2 = {};
 	o2.m = function() { return {}; };
-	// not analyzed precisely: `m` may be in the prototype since `m` is not in the initializer, and we do not attempt to reason flow sensitively beyond that at the moment
+	// not analyzed precisely: `m` may be in the prototype since `m`
+	// is not in the initializer, and we do not attempt to reason flow
+	// sensitively beyond that at the moment
 	o2.m();
 });
 (function(){
@@ -34,7 +36,9 @@
 
 	function f4(){return {};}
 	var { o4 } = {m: f4};
-	var v4 = o4.m(); // not analyzed precisely: o4 is from a destructuring assignment (and is even `undefined` in this case
+	// not analyzed precisely: o4 is from a destructuring assignment
+	// (and it is even `undefined` in this case)
+	var v4 = o4.m();
 	v4 === true;
 });
 
@@ -51,7 +55,9 @@
 		var v3 = o.m(); // not analyzed precisely: `o.m` may be `unknown`
 	})({m: unknown});
 	(function(o = {m: () => 42}){
-		var v4 = o.m(); // not analyzed precisely: we only support unique receivers at the moment
+		// not analyzed precisely: we only support unique receivers at
+		// the moment
+		var v4 = o.m();
 	})({m: () => true});
 
 });

From 66367987af24cd5c7fcbfd49383f70bfc1a3c000 Mon Sep 17 00:00:00 2001
From: Esben Sparre Andreasen <esben@semmle.com>
Date: Mon, 25 Feb 2019 16:04:37 +0100
Subject: [PATCH 13/16] JS: rename CapturedSource -> LocalObject

---
 javascript/ql/src/Declarations/UnusedProperty.ql            | 6 +++---
 .../ql/src/semmle/javascript/dataflow/CapturedNodes.qll     | 6 +++---
 .../dataflow/internal/InterProceduralTypeInference.qll      | 4 ++--
 .../ql/test/library-tests/CapturedNodes/CapturedSource.ql   | 2 +-
 .../CapturedNodes/CapturedSource_hasOwnProperty.ql          | 2 +-
 5 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/javascript/ql/src/Declarations/UnusedProperty.ql b/javascript/ql/src/Declarations/UnusedProperty.ql
index 117d402e0dff..d7208c80c942 100644
--- a/javascript/ql/src/Declarations/UnusedProperty.ql
+++ b/javascript/ql/src/Declarations/UnusedProperty.ql
@@ -14,7 +14,7 @@ import UnusedVariable
 import UnusedParameter
 import Expressions.ExprHasNoEffect
 
-predicate hasUnknownPropertyRead(CapturedSource obj) {
+predicate hasUnknownPropertyRead(LocalObject obj) {
   // dynamic reads
   exists(DataFlow::PropRead r | obj.getAPropertyRead() = r | not exists(r.getPropertyName()))
   or
@@ -33,7 +33,7 @@ predicate hasUnknownPropertyRead(CapturedSource obj) {
 /**
  * Holds if `obj` flows to an expression that must have a specific type.
  */
-predicate flowsToTypeRestrictedExpression(CapturedSource obj) {
+predicate flowsToTypeRestrictedExpression(LocalObject obj) {
   exists (Expr restricted, TypeExpr type |
     obj.flowsToExpr(restricted) and
     not type.isAny() |
@@ -50,7 +50,7 @@ predicate flowsToTypeRestrictedExpression(CapturedSource obj) {
   )
 }
 
-from DataFlow::PropWrite write, CapturedSource obj, string name
+from DataFlow::PropWrite write, LocalObject obj, string name
 where
   write = obj.getAPropertyWrite(name) and
   not exists(obj.getAPropertyRead(name)) and
diff --git a/javascript/ql/src/semmle/javascript/dataflow/CapturedNodes.qll b/javascript/ql/src/semmle/javascript/dataflow/CapturedNodes.qll
index 85cbd3ae5fb0..cf7575fcbd45 100644
--- a/javascript/ql/src/semmle/javascript/dataflow/CapturedNodes.qll
+++ b/javascript/ql/src/semmle/javascript/dataflow/CapturedNodes.qll
@@ -54,11 +54,11 @@ private predicate exposedAsReceiver(DataFlow::SourceNode n) {
 }
 
 /**
- * A source for which the flow is entirely captured by the dataflow library.
+ * A source that is entirely local, i.e. the dataflow library models all of its flow.
  * All uses of the node are modeled by `this.flowsTo(_)` and related predicates.
  */
-class CapturedSource extends DataFlow::SourceNode {
-  CapturedSource() {
+class LocalObject extends DataFlow::SourceNode {
+  LocalObject() {
     // pragmatic limitation: object literals only
     this instanceof DataFlow::ObjectLiteralNode and
     not flowsTo(getAnEscape()) and
diff --git a/javascript/ql/src/semmle/javascript/dataflow/internal/InterProceduralTypeInference.qll b/javascript/ql/src/semmle/javascript/dataflow/internal/InterProceduralTypeInference.qll
index 9574f7caad2e..73d43c4a1a64 100644
--- a/javascript/ql/src/semmle/javascript/dataflow/internal/InterProceduralTypeInference.qll
+++ b/javascript/ql/src/semmle/javascript/dataflow/internal/InterProceduralTypeInference.qll
@@ -238,7 +238,7 @@ private class TypeInferredCalleeWithAnalyzedReturnFlow extends CallWithNonLocalA
  */
 pragma[noinline]
 private predicate hasDefiniteReceiver(
-  DataFlow::MethodCallNode call, CapturedSource receiver
+  DataFlow::MethodCallNode call, LocalObject receiver
 ) {
   call = receiver.getAMethodCall() and
   exists (DataFlow::AnalyzedNode receiverNode, AbstractValue abstractCapturedReceiver |
@@ -260,7 +260,7 @@ private class TypeInferredMethodWithAnalyzedReturnFlow extends CallWithNonLocalA
   DataFlow::FunctionNode fun;
 
   TypeInferredMethodWithAnalyzedReturnFlow() {
-    exists(CapturedSource obj, DataFlow::PropWrite write, string name |
+    exists(LocalObject obj, DataFlow::PropWrite write, string name |
       this.(DataFlow::MethodCallNode).getMethodName() = name and
       obj.hasOwnProperty(name) and
       hasDefiniteReceiver(this, obj) and
diff --git a/javascript/ql/test/library-tests/CapturedNodes/CapturedSource.ql b/javascript/ql/test/library-tests/CapturedNodes/CapturedSource.ql
index 0731e04afee1..0a9997259781 100644
--- a/javascript/ql/test/library-tests/CapturedNodes/CapturedSource.ql
+++ b/javascript/ql/test/library-tests/CapturedNodes/CapturedSource.ql
@@ -1,4 +1,4 @@
 import javascript
 import semmle.javascript.dataflow.CapturedNodes
 
-select any(CapturedSource n)
+select any(LocalObject n)
diff --git a/javascript/ql/test/library-tests/CapturedNodes/CapturedSource_hasOwnProperty.ql b/javascript/ql/test/library-tests/CapturedNodes/CapturedSource_hasOwnProperty.ql
index 14911b329daa..7c9fe8d469a3 100644
--- a/javascript/ql/test/library-tests/CapturedNodes/CapturedSource_hasOwnProperty.ql
+++ b/javascript/ql/test/library-tests/CapturedNodes/CapturedSource_hasOwnProperty.ql
@@ -1,6 +1,6 @@
 import javascript
 import semmle.javascript.dataflow.CapturedNodes
 
-from CapturedSource src, string name
+from LocalObject src, string name
 where src.hasOwnProperty(name)
 select src, name

From 4dc147d506e2ed3da0e2576c405baf0b96846e3c Mon Sep 17 00:00:00 2001
From: Esben Sparre Andreasen <esben@semmle.com>
Date: Mon, 25 Feb 2019 16:09:07 +0100
Subject: [PATCH 14/16] JS: rename CapturedSource -> LocalObject (files)

---
 javascript/ql/src/Declarations/UnusedProperty.ql              | 2 +-
 .../dataflow/{CapturedNodes.qll => LocalObjects.qll}          | 0
 .../dataflow/internal/InterProceduralTypeInference.qll        | 3 +--
 .../ql/test/library-tests/CapturedNodes/CapturedSource.ql     | 4 ----
 .../LocalObject.expected}                                     | 0
 javascript/ql/test/library-tests/LocalObjects/LocalObject.ql  | 4 ++++
 .../LocalObject_hasOwnProperty.expected}                      | 0
 .../LocalObject_hasOwnProperty.ql}                            | 2 +-
 .../MethodCallTypeInference.expected                          | 0
 .../MethodCallTypeInference.ql                                | 0
 .../MethodCallTypeInferenceUsage.expected                     | 0
 .../MethodCallTypeInferenceUsage.ql                           | 0
 .../{CapturedNodes => LocalObjects}/method-calls.js           | 0
 .../test/library-tests/{CapturedNodes => LocalObjects}/tst.js | 0
 .../test/library-tests/{CapturedNodes => LocalObjects}/tst.ts | 0
 15 files changed, 7 insertions(+), 8 deletions(-)
 rename javascript/ql/src/semmle/javascript/dataflow/{CapturedNodes.qll => LocalObjects.qll} (100%)
 delete mode 100644 javascript/ql/test/library-tests/CapturedNodes/CapturedSource.ql
 rename javascript/ql/test/library-tests/{CapturedNodes/CapturedSource.expected => LocalObjects/LocalObject.expected} (100%)
 create mode 100644 javascript/ql/test/library-tests/LocalObjects/LocalObject.ql
 rename javascript/ql/test/library-tests/{CapturedNodes/CapturedSource_hasOwnProperty.expected => LocalObjects/LocalObject_hasOwnProperty.expected} (100%)
 rename javascript/ql/test/library-tests/{CapturedNodes/CapturedSource_hasOwnProperty.ql => LocalObjects/LocalObject_hasOwnProperty.ql} (67%)
 rename javascript/ql/test/library-tests/{CapturedNodes => LocalObjects}/MethodCallTypeInference.expected (100%)
 rename javascript/ql/test/library-tests/{CapturedNodes => LocalObjects}/MethodCallTypeInference.ql (100%)
 rename javascript/ql/test/library-tests/{CapturedNodes => LocalObjects}/MethodCallTypeInferenceUsage.expected (100%)
 rename javascript/ql/test/library-tests/{CapturedNodes => LocalObjects}/MethodCallTypeInferenceUsage.ql (100%)
 rename javascript/ql/test/library-tests/{CapturedNodes => LocalObjects}/method-calls.js (100%)
 rename javascript/ql/test/library-tests/{CapturedNodes => LocalObjects}/tst.js (100%)
 rename javascript/ql/test/library-tests/{CapturedNodes => LocalObjects}/tst.ts (100%)

diff --git a/javascript/ql/src/Declarations/UnusedProperty.ql b/javascript/ql/src/Declarations/UnusedProperty.ql
index d7208c80c942..8edc34936666 100644
--- a/javascript/ql/src/Declarations/UnusedProperty.ql
+++ b/javascript/ql/src/Declarations/UnusedProperty.ql
@@ -9,7 +9,7 @@
  */
 
 import javascript
-import semmle.javascript.dataflow.CapturedNodes
+import semmle.javascript.dataflow.LocalObjects
 import UnusedVariable
 import UnusedParameter
 import Expressions.ExprHasNoEffect
diff --git a/javascript/ql/src/semmle/javascript/dataflow/CapturedNodes.qll b/javascript/ql/src/semmle/javascript/dataflow/LocalObjects.qll
similarity index 100%
rename from javascript/ql/src/semmle/javascript/dataflow/CapturedNodes.qll
rename to javascript/ql/src/semmle/javascript/dataflow/LocalObjects.qll
diff --git a/javascript/ql/src/semmle/javascript/dataflow/internal/InterProceduralTypeInference.qll b/javascript/ql/src/semmle/javascript/dataflow/internal/InterProceduralTypeInference.qll
index 73d43c4a1a64..436b4966b57c 100644
--- a/javascript/ql/src/semmle/javascript/dataflow/internal/InterProceduralTypeInference.qll
+++ b/javascript/ql/src/semmle/javascript/dataflow/internal/InterProceduralTypeInference.qll
@@ -6,8 +6,7 @@
 
 import javascript
 import AbstractValuesImpl
-
-import semmle.javascript.dataflow.CapturedNodes
+import semmle.javascript.dataflow.LocalObjects
 
 /**
  * Flow analysis for `this` expressions inside functions.
diff --git a/javascript/ql/test/library-tests/CapturedNodes/CapturedSource.ql b/javascript/ql/test/library-tests/CapturedNodes/CapturedSource.ql
deleted file mode 100644
index 0a9997259781..000000000000
--- a/javascript/ql/test/library-tests/CapturedNodes/CapturedSource.ql
+++ /dev/null
@@ -1,4 +0,0 @@
-import javascript
-import semmle.javascript.dataflow.CapturedNodes
-
-select any(LocalObject n)
diff --git a/javascript/ql/test/library-tests/CapturedNodes/CapturedSource.expected b/javascript/ql/test/library-tests/LocalObjects/LocalObject.expected
similarity index 100%
rename from javascript/ql/test/library-tests/CapturedNodes/CapturedSource.expected
rename to javascript/ql/test/library-tests/LocalObjects/LocalObject.expected
diff --git a/javascript/ql/test/library-tests/LocalObjects/LocalObject.ql b/javascript/ql/test/library-tests/LocalObjects/LocalObject.ql
new file mode 100644
index 000000000000..d2bfd93b5368
--- /dev/null
+++ b/javascript/ql/test/library-tests/LocalObjects/LocalObject.ql
@@ -0,0 +1,4 @@
+import javascript
+import semmle.javascript.dataflow.LocalObjects
+
+select any(LocalObject n)
diff --git a/javascript/ql/test/library-tests/CapturedNodes/CapturedSource_hasOwnProperty.expected b/javascript/ql/test/library-tests/LocalObjects/LocalObject_hasOwnProperty.expected
similarity index 100%
rename from javascript/ql/test/library-tests/CapturedNodes/CapturedSource_hasOwnProperty.expected
rename to javascript/ql/test/library-tests/LocalObjects/LocalObject_hasOwnProperty.expected
diff --git a/javascript/ql/test/library-tests/CapturedNodes/CapturedSource_hasOwnProperty.ql b/javascript/ql/test/library-tests/LocalObjects/LocalObject_hasOwnProperty.ql
similarity index 67%
rename from javascript/ql/test/library-tests/CapturedNodes/CapturedSource_hasOwnProperty.ql
rename to javascript/ql/test/library-tests/LocalObjects/LocalObject_hasOwnProperty.ql
index 7c9fe8d469a3..28dfabddb38e 100644
--- a/javascript/ql/test/library-tests/CapturedNodes/CapturedSource_hasOwnProperty.ql
+++ b/javascript/ql/test/library-tests/LocalObjects/LocalObject_hasOwnProperty.ql
@@ -1,5 +1,5 @@
 import javascript
-import semmle.javascript.dataflow.CapturedNodes
+import semmle.javascript.dataflow.LocalObjects
 
 from LocalObject src, string name
 where src.hasOwnProperty(name)
diff --git a/javascript/ql/test/library-tests/CapturedNodes/MethodCallTypeInference.expected b/javascript/ql/test/library-tests/LocalObjects/MethodCallTypeInference.expected
similarity index 100%
rename from javascript/ql/test/library-tests/CapturedNodes/MethodCallTypeInference.expected
rename to javascript/ql/test/library-tests/LocalObjects/MethodCallTypeInference.expected
diff --git a/javascript/ql/test/library-tests/CapturedNodes/MethodCallTypeInference.ql b/javascript/ql/test/library-tests/LocalObjects/MethodCallTypeInference.ql
similarity index 100%
rename from javascript/ql/test/library-tests/CapturedNodes/MethodCallTypeInference.ql
rename to javascript/ql/test/library-tests/LocalObjects/MethodCallTypeInference.ql
diff --git a/javascript/ql/test/library-tests/CapturedNodes/MethodCallTypeInferenceUsage.expected b/javascript/ql/test/library-tests/LocalObjects/MethodCallTypeInferenceUsage.expected
similarity index 100%
rename from javascript/ql/test/library-tests/CapturedNodes/MethodCallTypeInferenceUsage.expected
rename to javascript/ql/test/library-tests/LocalObjects/MethodCallTypeInferenceUsage.expected
diff --git a/javascript/ql/test/library-tests/CapturedNodes/MethodCallTypeInferenceUsage.ql b/javascript/ql/test/library-tests/LocalObjects/MethodCallTypeInferenceUsage.ql
similarity index 100%
rename from javascript/ql/test/library-tests/CapturedNodes/MethodCallTypeInferenceUsage.ql
rename to javascript/ql/test/library-tests/LocalObjects/MethodCallTypeInferenceUsage.ql
diff --git a/javascript/ql/test/library-tests/CapturedNodes/method-calls.js b/javascript/ql/test/library-tests/LocalObjects/method-calls.js
similarity index 100%
rename from javascript/ql/test/library-tests/CapturedNodes/method-calls.js
rename to javascript/ql/test/library-tests/LocalObjects/method-calls.js
diff --git a/javascript/ql/test/library-tests/CapturedNodes/tst.js b/javascript/ql/test/library-tests/LocalObjects/tst.js
similarity index 100%
rename from javascript/ql/test/library-tests/CapturedNodes/tst.js
rename to javascript/ql/test/library-tests/LocalObjects/tst.js
diff --git a/javascript/ql/test/library-tests/CapturedNodes/tst.ts b/javascript/ql/test/library-tests/LocalObjects/tst.ts
similarity index 100%
rename from javascript/ql/test/library-tests/CapturedNodes/tst.ts
rename to javascript/ql/test/library-tests/LocalObjects/tst.ts

From ab1b1c1431058c0c8dac45814704b1110c8fdafe Mon Sep 17 00:00:00 2001
From: Esben Sparre Andreasen <esben@semmle.com>
Date: Mon, 25 Feb 2019 16:11:35 +0100
Subject: [PATCH 15/16] JS: update docstring

---
 javascript/ql/src/semmle/javascript/dataflow/LocalObjects.qll | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/javascript/ql/src/semmle/javascript/dataflow/LocalObjects.qll b/javascript/ql/src/semmle/javascript/dataflow/LocalObjects.qll
index cf7575fcbd45..21e24c84c1c4 100644
--- a/javascript/ql/src/semmle/javascript/dataflow/LocalObjects.qll
+++ b/javascript/ql/src/semmle/javascript/dataflow/LocalObjects.qll
@@ -1,5 +1,5 @@
 /**
- * Provides classes for the nodes that the dataflow library can reason about soundly.
+ * Provides classes for the local objects that the dataflow library can reason about soundly.
  */
 
 import javascript

From 9511bdf6ae766416e808b293fb850246ee2bd9d2 Mon Sep 17 00:00:00 2001
From: Esben Sparre Andreasen <esben@semmle.com>
Date: Tue, 26 Feb 2019 10:07:00 +0100
Subject: [PATCH 16/16] JS: address review comment

---
 .../ql/src/semmle/javascript/dataflow/LocalObjects.qll      | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/javascript/ql/src/semmle/javascript/dataflow/LocalObjects.qll b/javascript/ql/src/semmle/javascript/dataflow/LocalObjects.qll
index 21e24c84c1c4..c8f1bcf0f0df 100644
--- a/javascript/ql/src/semmle/javascript/dataflow/LocalObjects.qll
+++ b/javascript/ql/src/semmle/javascript/dataflow/LocalObjects.qll
@@ -54,8 +54,10 @@ private predicate exposedAsReceiver(DataFlow::SourceNode n) {
 }
 
 /**
- * A source that is entirely local, i.e. the dataflow library models all of its flow.
- * All uses of the node are modeled by `this.flowsTo(_)` and related predicates.
+ * An object that is entirely local, in the sense that the dataflow
+ * library models all of its flow.
+ *
+ * All uses of this node are modeled by `this.flowsTo(_)` and related predicates.
  */
 class LocalObject extends DataFlow::SourceNode {
   LocalObject() {