first (failing) stab at better side-effect detection

src/Declaration.js

@@ -0,0 +1,87 @@
+import { walk } from 'estree-walker';
+import { keys } from './utils/object';
+
+const modifierNodes = {
+	AssignmentExpression: 'left',
+	UpdateExpression: 'argument'
+};
+
+export default class Declaration {
+	constructor ( node ) {
+		if ( node ) {
+			if ( node.type === 'FunctionDeclaration' ) {
+				this.isFunctionDeclaration = true;
+				this.functionBody = node.body;
+			} else if ( node.type === 'VariableDeclarator' && node.init && /FunctionExpression/.test( node.init.type ) ) {
+				this.isFunctionDeclaration = true;
+				this.functionBody = node.init.body;
+			}
+		}
+
+		this.statement = null;
+		this.name = null;
+
+		this.isReassigned = false;
+		this.aliases = [];
+	}
+
+	addAlias ( declaration ) {
+		this.aliases.push( declaration );
+	}
+
+	addReference ( reference ) {
+		reference.declaration = this;
+		this.name = reference.name; // TODO handle differences of opinion
+
+		if ( reference.isReassignment ) this.isReassigned = true;
+	}
+
+	mutates () {
+		// returns a list of things this function mutates when it gets called
+		if ( !this._mutates ) {
+			let mutatedNames = {};
+
+			const statement = this.statement;
+			let scope = statement.scope;
+
+			const addNode = node => {
+				while ( node.type === 'MemberExpression' ) node = node.object;
+				if ( node.type === 'Identifier' ) mutatedNames[ node.name ] = true;
+			};
+
+			walk( this.functionBody, {
+				enter ( node ) {
+					if ( node._scope ) scope = node._scope;
+
+					if ( node.type in modifierNodes ) {
+						addNode( node[ modifierNodes[ node.type ] ] );
+					} else if ( node.type === 'CallExpression' ) {
+						addNode( node.callee );
+					}
+				},
+
+				leave ( node ) {
+					if ( node._scope ) scope = scope.parent;
+				}
+			});
+
+			this._mutates = keys( mutatedNames );
+		}
+
+		return this._mutates;
+	}
+
+	render ( es6 ) {
+		if ( es6 ) return this.name;
+		if ( !this.isReassigned || !this.isExported ) return this.name;
+
+		return `exports.${this.name}`;
+	}
+
+	use () {
+		this.isUsed = true;
+		if ( this.statement ) this.statement.mark();
+
+		this.aliases.forEach( alias => alias.use() );
+	}
+}

src/Module.js

@@ -35,6 +35,10 @@ class SyntheticDefaultDeclaration {
 		this.original = declaration;
 	}
 
+	mutates () {
+		return this.original.mutates();
+	}
+
 	render () {
 		return !this.original || this.original.isReassigned ?
 			this.name :

src/Statement.js

@@ -185,15 +185,61 @@ export default class Statement {
 		let hasSideEffect = false;
 
 		walk( this.node, {
-			enter ( node, parent ) {
-				if ( /Function/.test( node.type ) && !isIife( node, parent ) ) return this.skip();
+			enter ( node ) {
+				// Don't descend into (quasi) function declarations – we'll worry about those
+				// if they get called
+				if ( node.type === 'FunctionDeclaration' || node.type === 'VariableDeclarator' && node.init && /FunctionExpression/.test( node.init.type ) ) {
+					return this.skip();
+				}
 
 				// If this is a top-level call expression, or an assignment to a global,
 				// this statement will need to be marked
-				if ( node.type === 'CallExpression' || node.type === 'NewExpression' ) {
+				if ( node.type === 'NewExpression' ) {
 					hasSideEffect = true;
 				}
 
+				else if ( node.type === 'CallExpression' ) {
+					if ( node.callee.type === 'Identifier' ) {
+						const declaration = statement.module.trace( node.callee.name );
+
+						if ( !declaration || !declaration.isFunctionDeclaration ) {
+							hasSideEffect = true;
+						}
+
+						else {
+							const mutatedByFunction = declaration.mutates();
+							let i = mutatedByFunction.length;
+							while ( i-- ) {
+								const mutatedDeclaration = statement.module.trace( mutatedByFunction[i] );
+
+								if ( !mutatedDeclaration || mutatedDeclaration.isUsed ) {
+									hasSideEffect = true;
+									break;
+								}
+							}
+
+							// if ( declaration.hasSideEffect() ) {
+							// 	// if calling this function creates side-effects...
+							// 	hasSideEffect = true;
+							// }
+							//
+							// else {
+							// 	// ...or mutates inputs that are included...
+							// 	hasSideEffect = true;
+							// }
+
+							// TODO does function mutate inputs that are needed?
+						}
+					}
+
+					else if ( node.callee.type === 'MemberExpression' ) {
+						// if we're calling e.g. Object.keys(thing), there are no side-effects
+						// TODO
+
+						hasSideEffect = true;
+					}
+				}
+
 				else if ( node.type in modifierNodes ) {
 					let subject = node[ modifierNodes[ node.type ] ];
 					while ( subject.type === 'MemberExpression' ) subject = subject.object;

src/ast/Scope.js

@@ -1,4 +1,5 @@
 import { blank, keys } from '../utils/object.js';
+import Declaration from '../Declaration.js';
 
 const extractors = {
 	Identifier ( names, param ) {
@@ -33,41 +34,6 @@ function extractNames ( param ) {
 	return names;
 }
 
-class Declaration {
-	constructor () {
-		this.statement = null;
-		this.name = null;
-
-		this.isReassigned = false;
-		this.aliases = [];
-	}
-
-	addAlias ( declaration ) {
-		this.aliases.push( declaration );
-	}
-
-	addReference ( reference ) {
-		reference.declaration = this;
-		this.name = reference.name; // TODO handle differences of opinion
-
-		if ( reference.isReassignment ) this.isReassigned = true;
-	}
-
-	render ( es6 ) {
-		if ( es6 ) return this.name;
-		if ( !this.isReassigned || !this.isExported ) return this.name;
-
-		return `exports.${this.name}`;
-	}
-
-	use () {
-		this.isUsed = true;
-		if ( this.statement ) this.statement.mark();
-
-		this.aliases.forEach( alias => alias.use() );
-	}
-}
-
 export default class Scope {
 	constructor ( options ) {
 		options = options || {};
@@ -80,7 +46,7 @@ export default class Scope {
 		if ( options.params ) {
 			options.params.forEach( param => {
 				extractNames( param ).forEach( name => {
-					this.declarations[ name ] = new Declaration( name );
+					this.declarations[ name ] = new Declaration();
 				});
 			});
 		}
@@ -93,7 +59,7 @@ export default class Scope {
 			this.parent.addDeclaration( node, isBlockDeclaration, isVar );
 		} else {
 			extractNames( node.id ).forEach( name => {
-				this.declarations[ name ] = new Declaration( name );
+				this.declarations[ name ] = new Declaration( node );
 			});
 		}
 	}

test/form/namespace-optimization-b/foo.js

@@ -1,2 +1,3 @@
-export function foo() {
-};
+export function foo () {
+	console.log( 'foo' );
+}

test/form/namespace-optimization-b/main.js

@@ -1,10 +1,13 @@
 import * as foo from './foo';
 
-function a() {
-  foo.foo();
-  foo.foo();
-  var a;
-  if (a.b) {
-  }
+function a () {
+	foo.foo();
+	foo.foo();
+
+	var a;
+	if ( a.b ) {
+		// empty
+	}
 }
+
 a();