Merge branch 'master' into gh-92

src/Statement.js

@@ -8,6 +8,14 @@ function isIife ( node, parent ) {
 	return parent && parent.type === 'CallExpression' && node === parent.callee;
 }
 
+function isFunctionDeclaration ( node, parent ) {
+	// `function foo () {}`
+	if ( node.type === 'FunctionDeclaration' ) return true;
+
+	// `var foo = function () {}` - same thing for present purposes
+	if ( node.type === 'FunctionExpression' && parent.type === 'VariableDeclarator' ) return true;
+}
+
 export default class Statement {
 	constructor ( node, module, start, end ) {
 		this.node = node;
@@ -117,22 +125,34 @@ export default class Statement {
 		//    calledImmediately();
 		//
 		// ...but it's better than nothing
-		let depth = 0;
+		let readDepth = 0;
+
+		// This allows us to track whether a modifying statement (i.e. assignment
+		// /update expressions) need to be captured
+		let writeDepth = 0;
 
 		if ( !this.isImportDeclaration ) {
 			walk( this.node, {
 				enter: ( node, parent ) => {
 					if ( node._scope ) {
-						if ( !scope.isBlockScope && !isIife( node, parent ) ) depth += 1;
+						if ( !scope.isBlockScope ) {
+							if ( !isIife( node, parent ) ) readDepth += 1;
+							if ( isFunctionDeclaration( node, parent ) ) writeDepth += 1;
+						}
+
 						scope = node._scope;
 					}
 
-					this.checkForReads( scope, node, parent, !depth );
-					this.checkForWrites( scope, node );
+					this.checkForReads( scope, node, parent, !readDepth );
+					this.checkForWrites( scope, node, writeDepth );
 				},
 				leave: ( node, parent ) => {
 					if ( node._scope ) {
-						if ( !scope.isBlockScope && !isIife( node, parent ) ) depth -= 1;
+						if ( !scope.isBlockScope ) {
+							if ( !isIife( node, parent ) ) readDepth -= 1;
+							if ( isFunctionDeclaration( node, parent ) ) writeDepth -= 1;
+						}
+
 						scope = scope.parent;
 					}
 				}
@@ -171,7 +191,7 @@ export default class Statement {
 		}
 	}
 
-	checkForWrites ( scope, node ) {
+	checkForWrites ( scope, node, writeDepth ) {
 		const addNode = ( node, isAssignment ) => {
 			let depth = 0; // determine whether we're illegally modifying a binding or namespace
 
@@ -209,7 +229,13 @@ export default class Statement {
 				}
 			}
 
-			if ( node.type === 'Identifier' ) {
+			// we only care about writes that happen a) at the top level,
+			// or b) inside a function that could be immediately invoked.
+			// Writes inside named functions are only relevant if the
+			// function is called, in which case we don't need to do
+			// anything (but we still need to call checkForWrites to
+			// catch illegal reassignments to imported bindings)
+			if ( writeDepth === 0 && node.type === 'Identifier' ) {
 				this.modifies[ node.name ] = true;
 			}
 		};

test/form/exclude-unnecessary-modifications/_config.js

@@ -0,0 +1,3 @@
+module.exports = {
+	description: 'statements that modify definitions within unused functions are excluded'
+};

test/form/exclude-unnecessary-modifications/_expected/amd.js

@@ -0,0 +1,28 @@
+define(function () { 'use strict';
+
+	var foo = {};
+
+	mutate1( foo );
+
+	// should be included
+	[ 'a', 'b', 'c' ].forEach( function ( letter, i ) {
+		foo[ letter ] = i;
+	});
+
+	[ 'd', 'e', 'f' ].forEach( ( letter, i ) => {
+		foo[ letter ] = i;
+	});
+
+	function mutate1 () {
+		foo.mutated = 1;
+	}
+
+	({
+		mutate2: function () {
+			foo.mutated = 2;
+		}
+	}).mutate2();
+
+	console.log( foo );
+
+});

test/form/exclude-unnecessary-modifications/_expected/cjs.js

@@ -0,0 +1,26 @@
+'use strict';
+
+var foo = {};
+
+mutate1( foo );
+
+// should be included
+[ 'a', 'b', 'c' ].forEach( function ( letter, i ) {
+	foo[ letter ] = i;
+});
+
+[ 'd', 'e', 'f' ].forEach( ( letter, i ) => {
+	foo[ letter ] = i;
+});
+
+function mutate1 () {
+	foo.mutated = 1;
+}
+
+({
+	mutate2: function () {
+		foo.mutated = 2;
+	}
+}).mutate2();
+
+console.log( foo );

test/form/exclude-unnecessary-modifications/_expected/es6.js

@@ -0,0 +1,24 @@
+var foo = {};
+
+mutate1( foo );
+
+// should be included
+[ 'a', 'b', 'c' ].forEach( function ( letter, i ) {
+	foo[ letter ] = i;
+});
+
+[ 'd', 'e', 'f' ].forEach( ( letter, i ) => {
+	foo[ letter ] = i;
+});
+
+function mutate1 () {
+	foo.mutated = 1;
+}
+
+({
+	mutate2: function () {
+		foo.mutated = 2;
+	}
+}).mutate2();
+
+console.log( foo );

test/form/exclude-unnecessary-modifications/_expected/iife.js

@@ -0,0 +1,28 @@
+(function () { 'use strict';
+
+	var foo = {};
+
+	mutate1( foo );
+
+	// should be included
+	[ 'a', 'b', 'c' ].forEach( function ( letter, i ) {
+		foo[ letter ] = i;
+	});
+
+	[ 'd', 'e', 'f' ].forEach( ( letter, i ) => {
+		foo[ letter ] = i;
+	});
+
+	function mutate1 () {
+		foo.mutated = 1;
+	}
+
+	({
+		mutate2: function () {
+			foo.mutated = 2;
+		}
+	}).mutate2();
+
+	console.log( foo );
+
+})();

test/form/exclude-unnecessary-modifications/_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';
+
+	var foo = {};
+
+	mutate1( foo );
+
+	// should be included
+	[ 'a', 'b', 'c' ].forEach( function ( letter, i ) {
+		foo[ letter ] = i;
+	});
+
+	[ 'd', 'e', 'f' ].forEach( ( letter, i ) => {
+		foo[ letter ] = i;
+	});
+
+	function mutate1 () {
+		foo.mutated = 1;
+	}
+
+	({
+		mutate2: function () {
+			foo.mutated = 2;
+		}
+	}).mutate2();
+
+	console.log( foo );
+
+}));

test/form/exclude-unnecessary-modifications/foo.js

@@ -0,0 +1,37 @@
+var foo = {};
+
+mutate1( foo );
+
+// should be included
+[ 'a', 'b', 'c' ].forEach( function ( letter, i ) {
+	foo[ letter ] = i;
+});
+
+[ 'd', 'e', 'f' ].forEach( ( letter, i ) => {
+	foo[ letter ] = i;
+});
+
+function mutate1 () {
+	foo.mutated = 1;
+}
+
+({
+	mutate2: function () {
+		foo.mutated = 2;
+	}
+}).mutate2();
+
+// should be excluded
+var mutate2 = function () {
+	foo.mutated = 2;
+}
+
+function unused1 () {
+	foo.wat = 'nope';
+}
+
+function unused2 () {
+	mutate1( foo );
+}
+
+export default foo;

test/form/exclude-unnecessary-modifications/main.js

@@ -0,0 +1,2 @@
+import foo from './foo';
+console.log( foo );