preserve effects in for-of and for-in statements. fixes #870

src/ast/nodes/ExpressionStatement.js

@@ -1,16 +1,5 @@
-import Node from '../Node.js';
+import Statement from './shared/Statement.js';
 
-export default class ExpressionStatement extends Node {
-	render ( code, es ) {
-		if ( !this.module.bundle.treeshake || this.shouldInclude ) {
-			super.render( code, es );
-		} else {
-			code.remove( this.leadingCommentStart || this.start, this.next || this.end );
-		}
-	}
+export default class ExpressionStatement extends Statement {
 
-	run ( scope ) {
-		this.shouldInclude = true;
-		super.run( scope );
-	}
 }

src/ast/nodes/ForInStatement.js

@@ -1,15 +1,10 @@
-import Node from '../Node.js';
+import Statement from './shared/Statement.js';
+import assignTo from './shared/assignTo.js';
 import { STRING } from '../values.js';
 
-export default class ForInStatement extends Node {
+export default class ForInStatement extends Statement {
 	initialise ( scope ) {
 		super.initialise( scope );
-
-		// special case
-		if ( this.left.type === 'VariableDeclaration' ) {
-			for ( const proxy of this.left.declarations[0].proxies.values() ) {
-				proxy.possibleValues.add( STRING );
-			}
-		}
+		assignTo( this.left, scope, STRING );
 	}
 }

src/ast/nodes/ForOfStatement.js

@@ -1,15 +1,10 @@
-import Node from '../Node.js';
+import Statement from './shared/Statement.js';
+import assignTo from './shared/assignTo.js';
 import { UNKNOWN } from '../values.js';
 
-export default class ForOfStatement extends Node {
+export default class ForOfStatement extends Statement {
 	initialise ( scope ) {
 		super.initialise( scope );
-
-		// special case
-		if ( this.left.type === 'VariableDeclaration' ) {
-			for ( const proxy of this.left.declarations[0].proxies.values() ) {
-				proxy.possibleValues.add( UNKNOWN );
-			}
-		}
+		assignTo( this.left, scope, UNKNOWN );
 	}
 }

src/ast/nodes/IfStatement.js

@@ -1,8 +1,8 @@
-import Node from '../Node.js';
+import Statement from './shared/Statement.js';
 import { UNKNOWN } from '../values.js';
 
 // TODO DRY this out
-export default class IfStatement extends Node {
+export default class IfStatement extends Statement {
 	initialise ( scope ) {
 		this.testValue = this.test.getValue();
 

src/ast/nodes/ThrowStatement.js

@@ -0,0 +1,7 @@
+import Node from '../Node.js';
+
+export default class ThrowStatement extends Node {
+	hasEffects ( scope ) {
+		return scope.findLexicalBoundary().isModuleScope; // TODO should this just be `true`? probably...
+	}
+}

src/ast/nodes/index.js

@@ -26,6 +26,7 @@ import ParenthesizedExpression from './ParenthesizedExpression.js';
 import ReturnStatement from './ReturnStatement.js';
 import TemplateLiteral from './TemplateLiteral.js';
 import ThisExpression from './ThisExpression.js';
+import ThrowStatement from './ThrowStatement.js';
 import UnaryExpression from './UnaryExpression.js';
 import UpdateExpression from './UpdateExpression.js';
 import VariableDeclarator from './VariableDeclarator.js';
@@ -60,6 +61,7 @@ export default {
 	ReturnStatement,
 	TemplateLiteral,
 	ThisExpression,
+	ThrowStatement,
 	UnaryExpression,
 	UpdateExpression,
 	VariableDeclarator,

src/ast/nodes/shared/Statement.js

@@ -0,0 +1,16 @@
+import Node from '../../Node.js';
+
+export default class Statement extends Node {
+	render ( code, es ) {
+		if ( !this.module.bundle.treeshake || this.shouldInclude ) {
+			super.render( code, es );
+		} else {
+			code.remove( this.leadingCommentStart || this.start, this.next || this.end );
+		}
+	}
+
+	run ( scope ) {
+		this.shouldInclude = true;
+		super.run( scope );
+	}
+}

src/ast/nodes/shared/assignTo.js

@@ -0,0 +1,29 @@
+import extractNames from '../../utils/extractNames.js';
+
+export default function assignToForLoopLeft ( node, scope, value ) {
+	if ( node.type === 'VariableDeclaration' ) {
+		for ( const proxy of node.declarations[0].proxies.values() ) {
+			proxy.possibleValues.add( value );
+		}
+	}
+
+	else {
+		while ( node.type === 'ParenthesizedExpression' ) node = node.expression;
+
+		if ( node.type === 'MemberExpression' ) {
+			// apparently this is legal JavaScript? Though I don't know what
+			// kind of monster would write `for ( foo.bar of thing ) {...}`
+
+			// for now, do nothing, as I'm not sure anything needs to happen...
+		}
+
+		else {
+			for ( const name of extractNames( node ) ) {
+				const declaration = scope.findDeclaration( name );
+				if ( declaration.possibleValues ) {
+					declaration.possibleValues.add( value );
+				}
+			}
+		}
+	}
+}

test/form/effect-in-for-of-loop-in-functions/_config.js

@@ -0,0 +1,3 @@
+module.exports = {
+	description: 'includes effects in for-of loop (#870)'
+}

test/form/effect-in-for-of-loop-in-functions/_expected/amd.js

@@ -0,0 +1,28 @@
+define(function () { 'use strict';
+
+	const items = [{}, {}, {}];
+
+	function a () {
+		for ( const item of items.children ) {
+			item.foo = 'a';
+		}
+	}
+
+	a();
+
+	function c () {
+		let item;
+		for ( item of items.children ) {
+			item.bar = 'c';
+		}
+	}
+
+	c();
+
+	assert.deepEqual( items, [
+		{ foo: 'a', bar: 'c' },
+		{ foo: 'a', bar: 'c' },
+		{ foo: 'a', bar: 'c' }
+	]);
+
+});

test/form/effect-in-for-of-loop-in-functions/_expected/cjs.js

@@ -0,0 +1,26 @@
+'use strict';
+
+const items = [{}, {}, {}];
+
+function a () {
+	for ( const item of items.children ) {
+		item.foo = 'a';
+	}
+}
+
+a();
+
+function c () {
+	let item;
+	for ( item of items.children ) {
+		item.bar = 'c';
+	}
+}
+
+c();
+
+assert.deepEqual( items, [
+	{ foo: 'a', bar: 'c' },
+	{ foo: 'a', bar: 'c' },
+	{ foo: 'a', bar: 'c' }
+]);

test/form/effect-in-for-of-loop-in-functions/_expected/es.js

@@ -0,0 +1,24 @@
+const items = [{}, {}, {}];
+
+function a () {
+	for ( const item of items.children ) {
+		item.foo = 'a';
+	}
+}
+
+a();
+
+function c () {
+	let item;
+	for ( item of items.children ) {
+		item.bar = 'c';
+	}
+}
+
+c();
+
+assert.deepEqual( items, [
+	{ foo: 'a', bar: 'c' },
+	{ foo: 'a', bar: 'c' },
+	{ foo: 'a', bar: 'c' }
+]);

test/form/effect-in-for-of-loop-in-functions/_expected/iife.js

@@ -0,0 +1,29 @@
+(function () {
+	'use strict';
+
+	const items = [{}, {}, {}];
+
+	function a () {
+		for ( const item of items.children ) {
+			item.foo = 'a';
+		}
+	}
+
+	a();
+
+	function c () {
+		let item;
+		for ( item of items.children ) {
+			item.bar = 'c';
+		}
+	}
+
+	c();
+
+	assert.deepEqual( items, [
+		{ foo: 'a', bar: 'c' },
+		{ foo: 'a', bar: 'c' },
+		{ foo: 'a', bar: 'c' }
+	]);
+
+}());

test/form/effect-in-for-of-loop-in-functions/_expected/umd.js

@@ -0,0 +1,32 @@
+(function (global, factory) {
+	typeof exports === 'object' && typeof module !== 'undefined' ? factory() :
+	typeof define === 'function' && define.amd ? define(factory) :
+	(factory());
+}(this, (function () { 'use strict';
+
+	const items = [{}, {}, {}];
+
+	function a () {
+		for ( const item of items.children ) {
+			item.foo = 'a';
+		}
+	}
+
+	a();
+
+	function c () {
+		let item;
+		for ( item of items.children ) {
+			item.bar = 'c';
+		}
+	}
+
+	c();
+
+	assert.deepEqual( items, [
+		{ foo: 'a', bar: 'c' },
+		{ foo: 'a', bar: 'c' },
+		{ foo: 'a', bar: 'c' }
+	]);
+
+})));

test/form/effect-in-for-of-loop-in-functions/main.js

@@ -0,0 +1,41 @@
+const items = [{}, {}, {}];
+
+function a () {
+	for ( const item of items.children ) {
+		item.foo = 'a';
+	}
+}
+
+a();
+
+function b () {
+	for ( const item of items.children ) {
+		// do nothing
+	}
+}
+
+b();
+
+function c () {
+	let item;
+	for ( item of items.children ) {
+		item.bar = 'c';
+	}
+}
+
+c();
+
+function d () {
+	let item;
+	for ( item of items.children ) {
+		// do nothing
+	}
+}
+
+d();
+
+assert.deepEqual( items, [
+	{ foo: 'a', bar: 'c' },
+	{ foo: 'a', bar: 'c' },
+	{ foo: 'a', bar: 'c' }
+]);

test/form/effect-in-for-of-loop/_expected/amd.js

@@ -2,18 +2,19 @@ define(function () { 'use strict';
 
 	const items = [{}, {}, {}];
 
-	function x () {
-		for ( const item of items.children ) {
-			item.foo = 'bar';
-		}
+	for ( const a of items.children ) {
+		a.foo = 'a';
 	}
 
-	x();
+	let c;
+	for ( c of items.children ) {
+		c.bar = 'c';
+	}
 
 	assert.deepEqual( items, [
-		{ foo: 'bar' },
-		{ foo: 'bar' },
-		{ foo: 'bar' }
+		{ foo: 'a', bar: 'c' },
+		{ foo: 'a', bar: 'c' },
+		{ foo: 'a', bar: 'c' }
 	]);
 
 });

test/form/effect-in-for-of-loop/_expected/cjs.js

@@ -2,16 +2,17 @@
 
 const items = [{}, {}, {}];
 
-function x () {
-	for ( const item of items.children ) {
-		item.foo = 'bar';
-	}
+for ( const a of items.children ) {
+	a.foo = 'a';
 }
 
-x();
+let c;
+for ( c of items.children ) {
+	c.bar = 'c';
+}
 
 assert.deepEqual( items, [
-	{ foo: 'bar' },
-	{ foo: 'bar' },
-	{ foo: 'bar' }
+	{ foo: 'a', bar: 'c' },
+	{ foo: 'a', bar: 'c' },
+	{ foo: 'a', bar: 'c' }
 ]);

test/form/effect-in-for-of-loop/_expected/es.js

@@ -1,15 +1,16 @@
 const items = [{}, {}, {}];
 
-function x () {
-	for ( const item of items.children ) {
-		item.foo = 'bar';
-	}
+for ( const a of items.children ) {
+	a.foo = 'a';
 }
 
-x();
+let c;
+for ( c of items.children ) {
+	c.bar = 'c';
+}
 
 assert.deepEqual( items, [
-	{ foo: 'bar' },
-	{ foo: 'bar' },
-	{ foo: 'bar' }
+	{ foo: 'a', bar: 'c' },
+	{ foo: 'a', bar: 'c' },
+	{ foo: 'a', bar: 'c' }
 ]);

test/form/effect-in-for-of-loop/_expected/iife.js

@@ -3,18 +3,19 @@
 
 	const items = [{}, {}, {}];
 
-	function x () {
-		for ( const item of items.children ) {
-			item.foo = 'bar';
-		}
+	for ( const a of items.children ) {
+		a.foo = 'a';
 	}
 
-	x();
+	let c;
+	for ( c of items.children ) {
+		c.bar = 'c';
+	}
 
 	assert.deepEqual( items, [
-		{ foo: 'bar' },
-		{ foo: 'bar' },
-		{ foo: 'bar' }
+		{ foo: 'a', bar: 'c' },
+		{ foo: 'a', bar: 'c' },
+		{ foo: 'a', bar: 'c' }
 	]);
 
 }());

test/form/effect-in-for-of-loop/_expected/umd.js

@@ -6,18 +6,19 @@
 
 	const items = [{}, {}, {}];
 
-	function x () {
-		for ( const item of items.children ) {
-			item.foo = 'bar';
-		}
+	for ( const a of items.children ) {
+		a.foo = 'a';
 	}
 
-	x();
+	let c;
+	for ( c of items.children ) {
+		c.bar = 'c';
+	}
 
 	assert.deepEqual( items, [
-		{ foo: 'bar' },
-		{ foo: 'bar' },
-		{ foo: 'bar' }
+		{ foo: 'a', bar: 'c' },
+		{ foo: 'a', bar: 'c' },
+		{ foo: 'a', bar: 'c' }
 	]);
 
 })));

test/form/effect-in-for-of-loop/main.js

@@ -1,23 +1,25 @@
 const items = [{}, {}, {}];
 
-function x () {
-	for ( const item of items.children ) {
-		item.foo = 'bar';
-	}
+for ( const a of items.children ) {
+	a.foo = 'a';
 }
 
-x();
+for ( const b of items.children ) {
+	// do nothing
+}
 
-function y () {
-	for ( const item of items.children ) {
-		// do nothing
-	}
+let c;
+for ( c of items.children ) {
+	c.bar = 'c';
 }
 
-y();
+let d;
+for ( d of items.children ) {
+	// do nothing
+}
 
 assert.deepEqual( items, [
-	{ foo: 'bar' },
-	{ foo: 'bar' },
-	{ foo: 'bar' }
+	{ foo: 'a', bar: 'c' },
+	{ foo: 'a', bar: 'c' },
+	{ foo: 'a', bar: 'c' }
 ]);