Merge pull request #1017 from Pauan/tree-shake-pure-es5-classes

Removes `new Foo(...)` when Foo is a pure function

src/ast/nodes/CallExpression.js

@@ -27,7 +27,7 @@ export default class CallExpression extends Node {
 	}
 
 	hasEffects ( scope ) {
-		return callHasEffects( scope, this.callee );
+		return callHasEffects( scope, this.callee, false );
 	}
 
 	initialise ( scope ) {

src/ast/nodes/NewExpression.js

@@ -3,6 +3,6 @@ import callHasEffects from './shared/callHasEffects.js';
 
 export default class NewExpression extends Node {
 	hasEffects ( scope ) {
-		return callHasEffects( scope, this.callee );
+		return callHasEffects( scope, this.callee, true );
 	}
 }

src/ast/nodes/shared/callHasEffects.js

@@ -5,7 +5,43 @@ import { UNKNOWN } from '../../values.js';
 
 const currentlyCalling = new Set();
 
-function fnHasEffects ( fn ) {
+function isES5Function ( node ) {
+	return node.type === 'FunctionExpression' || node.type === 'FunctionDeclaration';
+}
+
+function hasEffectsNew ( node, scope ) {
+	let inner = node;
+
+	if ( inner.type === 'ExpressionStatement' ) {
+		inner = inner.expression;
+
+		if ( inner.type === 'AssignmentExpression' ) {
+			if ( inner.right.hasEffects( scope ) ) {
+				return true;
+
+			} else {
+				inner = inner.left;
+
+				if ( inner.type === 'MemberExpression' ) {
+					if ( inner.computed && inner.property.hasEffects( scope ) ) {
+						return true;
+
+					} else {
+						inner = inner.object;
+
+						if ( inner.type === 'ThisExpression' ) {
+							return false;
+						}
+					}
+				}
+			}
+		}
+	}
+
+	return node.hasEffects( scope );
+}
+
+function fnHasEffects ( fn, isNew ) {
 	if ( currentlyCalling.has( fn ) ) return false; // prevent infinite loops... TODO there must be a better way
 	currentlyCalling.add( fn );
 
@@ -14,7 +50,7 @@ function fnHasEffects ( fn ) {
 	const body = fn.body.type === 'BlockStatement' ? fn.body.body : [ fn.body ];
 
 	for ( const node of body ) {
-		if ( node.hasEffects( scope ) ) {
+		if ( isNew ? hasEffectsNew( node, scope ) : node.hasEffects( scope ) ) {
 			currentlyCalling.delete( fn );
 			return true;
 		}
@@ -24,14 +60,14 @@ function fnHasEffects ( fn ) {
 	return false;
 }
 
-export default function callHasEffects ( scope, callee ) {
+export default function callHasEffects ( scope, callee, isNew ) {
 	const values = new Set([ callee ]);
 
 	for ( const node of values ) {
 		if ( node === UNKNOWN ) return true; // err on side of caution
 
 		if ( /Function/.test( node.type ) ) {
-			if ( fnHasEffects( node ) ) return true;
+			if ( fnHasEffects( node, isNew && isES5Function( node ) ) ) return true;
 		}
 
 		else if ( isReference( node ) ) {

test/form/side-effect-es5-classes/_config.js

@@ -0,0 +1,3 @@
+module.exports = {
+	description: 'omits ES5 classes which are pure (e.g. they only assign to `this`)'
+};

test/form/side-effect-es5-classes/_expected/amd.js

@@ -0,0 +1,34 @@
+define(function () { 'use strict';
+
+	function Bar ( x ) {
+		console.log( 'side effect' );
+		this.value = x;
+	}
+
+	function Baz ( x ) {
+		this[ console.log( 'side effect' ) ] = x;
+	}
+
+	function Qux ( x ) {
+		this.value = console.log( 'side effect' );
+	}
+
+	function Corge ( x ) {
+		this.value = x;
+	}
+
+	var Arrow = ( x ) => {
+		undefined.value = x;
+	};
+
+	console.log( 'before' );
+
+	var bar = new Bar(5);
+	var baz = new Baz(5);
+	var qux = new Qux(5);
+	var corge = Corge(5);
+	var arrow = new Arrow(5);
+
+	console.log( 'after' );
+
+});

test/form/side-effect-es5-classes/_expected/cjs.js

@@ -0,0 +1,32 @@
+'use strict';
+
+function Bar ( x ) {
+	console.log( 'side effect' );
+	this.value = x;
+}
+
+function Baz ( x ) {
+	this[ console.log( 'side effect' ) ] = x;
+}
+
+function Qux ( x ) {
+	this.value = console.log( 'side effect' );
+}
+
+function Corge ( x ) {
+	this.value = x;
+}
+
+var Arrow = ( x ) => {
+	undefined.value = x;
+};
+
+console.log( 'before' );
+
+var bar = new Bar(5);
+var baz = new Baz(5);
+var qux = new Qux(5);
+var corge = Corge(5);
+var arrow = new Arrow(5);
+
+console.log( 'after' );

test/form/side-effect-es5-classes/_expected/es.js

@@ -0,0 +1,30 @@
+function Bar ( x ) {
+	console.log( 'side effect' );
+	this.value = x;
+}
+
+function Baz ( x ) {
+	this[ console.log( 'side effect' ) ] = x;
+}
+
+function Qux ( x ) {
+	this.value = console.log( 'side effect' );
+}
+
+function Corge ( x ) {
+	this.value = x;
+}
+
+var Arrow = ( x ) => {
+	undefined.value = x;
+};
+
+console.log( 'before' );
+
+var bar = new Bar(5);
+var baz = new Baz(5);
+var qux = new Qux(5);
+var corge = Corge(5);
+var arrow = new Arrow(5);
+
+console.log( 'after' );

test/form/side-effect-es5-classes/_expected/iife.js

@@ -0,0 +1,35 @@
+(function () {
+	'use strict';
+
+	function Bar ( x ) {
+		console.log( 'side effect' );
+		this.value = x;
+	}
+
+	function Baz ( x ) {
+		this[ console.log( 'side effect' ) ] = x;
+	}
+
+	function Qux ( x ) {
+		this.value = console.log( 'side effect' );
+	}
+
+	function Corge ( x ) {
+		this.value = x;
+	}
+
+	var Arrow = ( x ) => {
+		undefined.value = x;
+	};
+
+	console.log( 'before' );
+
+	var bar = new Bar(5);
+	var baz = new Baz(5);
+	var qux = new Qux(5);
+	var corge = Corge(5);
+	var arrow = new Arrow(5);
+
+	console.log( 'after' );
+
+}());

test/form/side-effect-es5-classes/_expected/umd.js

@@ -0,0 +1,38 @@
+(function (global, factory) {
+	typeof exports === 'object' && typeof module !== 'undefined' ? factory() :
+	typeof define === 'function' && define.amd ? define(factory) :
+	(factory());
+}(this, (function () { 'use strict';
+
+	function Bar ( x ) {
+		console.log( 'side effect' );
+		this.value = x;
+	}
+
+	function Baz ( x ) {
+		this[ console.log( 'side effect' ) ] = x;
+	}
+
+	function Qux ( x ) {
+		this.value = console.log( 'side effect' );
+	}
+
+	function Corge ( x ) {
+		this.value = x;
+	}
+
+	var Arrow = ( x ) => {
+		undefined.value = x;
+	};
+
+	console.log( 'before' );
+
+	var bar = new Bar(5);
+	var baz = new Baz(5);
+	var qux = new Qux(5);
+	var corge = Corge(5);
+	var arrow = new Arrow(5);
+
+	console.log( 'after' );
+
+})));

test/form/side-effect-es5-classes/arrow.js

@@ -0,0 +1,3 @@
+export var Arrow = ( x ) => {
+	this.value = x;
+};

test/form/side-effect-es5-classes/bar.js

@@ -0,0 +1,4 @@
+export function Bar ( x ) {
+	console.log( 'side effect' );
+	this.value = x;
+}

test/form/side-effect-es5-classes/baz.js

@@ -0,0 +1,3 @@
+export function Baz ( x ) {
+	this[ console.log( 'side effect' ) ] = x;
+}

test/form/side-effect-es5-classes/corge.js

@@ -0,0 +1,3 @@
+export function Corge ( x ) {
+	this.value = x;
+}

test/form/side-effect-es5-classes/foo.js

@@ -0,0 +1,4 @@
+export function Foo ( x ) {
+	this.value = x;
+	this["string property"] = 20;
+}

test/form/side-effect-es5-classes/main.js

@@ -0,0 +1,17 @@
+import { Foo } from './foo';
+import { Bar } from './bar';
+import { Baz } from './baz';
+import { Qux } from './qux';
+import { Corge } from './corge';
+import { Arrow } from './arrow';
+
+console.log( 'before' );
+
+var foo = new Foo(5);
+var bar = new Bar(5);
+var baz = new Baz(5);
+var qux = new Qux(5);
+var corge = Corge(5);
+var arrow = new Arrow(5);
+
+console.log( 'after' );

test/form/side-effect-es5-classes/qux.js

@@ -0,0 +1,3 @@
+export function Qux ( x ) {
+	this.value = console.log( 'side effect' );
+}

test/form/unmodified-default-exports/_expected/amd.js

@@ -1,6 +1,7 @@
 define(function () { 'use strict';
 
 	var Foo = function () {
+		console.log( 'side effect' );
 		this.isFoo = true;
 	};
 

test/form/unmodified-default-exports/_expected/cjs.js

@@ -1,6 +1,7 @@
 'use strict';
 
 var Foo = function () {
+	console.log( 'side effect' );
 	this.isFoo = true;
 };
 

test/form/unmodified-default-exports/_expected/es.js

@@ -1,4 +1,5 @@
 var Foo = function () {
+	console.log( 'side effect' );
 	this.isFoo = true;
 };
 

test/form/unmodified-default-exports/_expected/iife.js

@@ -2,6 +2,7 @@
 	'use strict';
 
 	var Foo = function () {
+		console.log( 'side effect' );
 		this.isFoo = true;
 	};
 

test/form/unmodified-default-exports/_expected/umd.js

@@ -5,6 +5,7 @@
 }(this, (function () { 'use strict';
 
 	var Foo = function () {
+		console.log( 'side effect' );
 		this.isFoo = true;
 	};
 
@@ -16,4 +17,4 @@
 
 	var foo = new Foo();
 
-})));
+})));

test/form/unmodified-default-exports/foo.js

@@ -1,4 +1,5 @@
 var Foo = function () {
+	console.log( 'side effect' );
 	this.isFoo = true;
 };
 

test/function/es5-class-called-without-new/_config.js

@@ -0,0 +1,2 @@
+module.exports = {
+};

test/function/es5-class-called-without-new/foo.js

@@ -0,0 +1,7 @@
+export function Foo ( x ) {
+	this.value = x;
+}
+
+export function Bar ( x ) {
+	this.value = x;
+}

test/function/es5-class-called-without-new/main.js

@@ -0,0 +1,7 @@
+import { Foo, Bar } from './foo';
+
+assert.equal( new Foo(5).value, 5 );
+
+assert.throws( function () {
+	Bar(5);
+}, /^TypeError: Cannot set property 'value' of undefined$/ );