Merge pull request #309 from rollup/no-aggressive

Remove aggressive mode

src/Bundle.js

@@ -49,7 +49,6 @@ export default class Bundle {
 
 		this.external = options.external || [];
 		this.onwarn = options.onwarn || onwarn;
-		this.aggressive = options.aggressive;
 
 		// TODO strictly speaking, this only applies with non-ES6, non-default-only bundles
 		[ 'module', 'exports' ].forEach( global => this.assumedGlobals[ global ] = true );
@@ -86,13 +85,9 @@ export default class Bundle {
 				while ( !settled ) {
 					settled = true;
 
-					if ( this.aggressive ) {
-						settled = !entryModule.run();
-					} else {
-						this.modules.forEach( module => {
-							if ( module.run() ) settled = false;
-						});
-					}
+					this.modules.forEach( module => {
+						if ( module.run() ) settled = false;
+					});
 				}
 
 				// Phase 4 – final preparation. We order the modules with an

src/Declaration.js

@@ -2,7 +2,7 @@ import { blank, keys } from './utils/object.js';
 import run from './utils/run.js';
 
 export default class Declaration {
-	constructor ( node, isParam ) {
+	constructor ( node, isParam, statement ) {
 		if ( node ) {
 			if ( node.type === 'FunctionDeclaration' ) {
 				this.isFunctionDeclaration = true;
@@ -13,7 +13,7 @@ export default class Declaration {
 			}
 		}
 
-		this.statement = null;
+		this.statement = statement;
 		this.name = null;
 		this.isParam = isParam;
 
@@ -43,10 +43,10 @@ export default class Declaration {
 		if ( this.tested ) return this.hasSideEffects;
 		this.tested = true;
 
-		if ( !this.statement || !this.functionNode ) {
+		if ( !this.functionNode ) {
 			this.hasSideEffects = true; // err on the side of caution. TODO handle unambiguous `var x; x = y => z` cases
 		} else {
-			this.hasSideEffects = run( this.functionNode.body, this.functionNode._scope, this.statement, strongDependencies );
+			this.hasSideEffects = run( this.functionNode.body, this.functionNode._scope, this.statement, strongDependencies, false );
 		}
 
 		return this.hasSideEffects;
@@ -101,7 +101,7 @@ export class SyntheticDefaultDeclaration {
 		}
 
 		if ( /FunctionExpression/.test( this.node.declaration.type ) ) {
-			return run( this.node.declaration.body, this.statement.scope, this.statement, strongDependencies );
+			return run( this.node.declaration.body, this.statement.scope, this.statement, strongDependencies, false );
 		}
 	}
 
@@ -226,6 +226,10 @@ export class ExternalDeclaration {
 		return es6 ? this.name : `${this.module.name}.${this.name}`;
 	}
 
+	run () {
+		return true;
+	}
+
 	use () {
 		// noop?
 	}

src/Module.js

@@ -559,11 +559,11 @@ export default class Module {
 		return magicString.trim();
 	}
 
-	run () {
+	run ( safe ) {
 		let marked = false;
 
 		this.statements.forEach( statement => {
-			marked = marked || statement.run( this.strongDependencies );
+			marked = marked || statement.run( this.strongDependencies, safe );
 		});
 
 		return marked;

src/Statement.js

@@ -39,7 +39,7 @@ export default class Statement {
 		this.end = end;
 		this.next = null; // filled in later
 
-		this.scope = new Scope();
+		this.scope = new Scope({ statement: this });
 
 		this.references = [];
 		this.stringLiteralRanges = [];
@@ -61,12 +61,6 @@ export default class Statement {
 		// attach scopes
 		attachScopes( this );
 
-		// attach statement to each top-level declaration,
-		// so we can mark statements easily
-		this.scope.eachDeclaration( ( name, declaration ) => {
-			declaration.statement = this;
-		});
-
 		// find references
 		const statement = this;
 		let { module, references, scope, stringLiteralRanges } = this;
@@ -159,11 +153,11 @@ export default class Statement {
 		});
 	}
 
-	run ( strongDependencies ) {
+	run ( strongDependencies, safe ) {
 		if ( ( this.ran && this.isIncluded ) || this.isImportDeclaration || this.isFunctionDeclaration ) return;
 		this.ran = true;
 
-		if ( run( this.node, this.scope, this, strongDependencies ) ) {
+		if ( run( this.node, this.scope, this, strongDependencies, false, safe ) ) {
 			this.mark();
 			return true;
 		}

src/ast/Scope.js

@@ -39,6 +39,7 @@ export default class Scope {
 		options = options || {};
 
 		this.parent = options.parent;
+		this.statement = options.statement || this.parent.statement;
 		this.isBlockScope = !!options.block;
 		this.isTopLevel = !this.parent || ( this.parent.isTopLevel && this.isBlockScope );
 
@@ -47,7 +48,7 @@ export default class Scope {
 		if ( options.params ) {
 			options.params.forEach( param => {
 				extractNames( param ).forEach( name => {
-					this.declarations[ name ] = new Declaration( param, true );
+					this.declarations[ name ] = new Declaration( param, true, this.statement );
 				});
 			});
 		}
@@ -60,7 +61,7 @@ export default class Scope {
 			this.parent.addDeclaration( node, isBlockDeclaration, isVar );
 		} else {
 			extractNames( node.id ).forEach( name => {
-				this.declarations[ name ] = new Declaration( node );
+				this.declarations[ name ] = new Declaration( node, false, this.statement );
 			});
 		}
 	}

src/utils/run.js

@@ -60,7 +60,9 @@ export default function run ( node, scope, statement, strongDependencies, force
 
 				if ( flattened.name === 'arguments' ) {
 					hasSideEffect = true;
-				} if ( !scope.contains( flattened.name ) ) {
+				}
+
+				else if ( !scope.contains( flattened.name ) ) {
 					const declaration = statement.module.trace( flattened.name );
 					if ( declaration && !declaration.isExternal ) {
 						const module = declaration.module || declaration.statement.module; // TODO is this right?
@@ -81,7 +83,7 @@ export default function run ( node, scope, statement, strongDependencies, force
 					                    statement.module.trace( node.callee.name );
 
 					if ( declaration ) {
-						if ( declaration.isExternal || declaration.run( strongDependencies ) ) {
+						if ( declaration.run( strongDependencies ) ) {
 							hasSideEffect = true;
 						}
 					} else if ( !pureFunctions[ node.callee.name ] ) {
@@ -102,7 +104,7 @@ export default function run ( node, scope, statement, strongDependencies, force
 						}
 					} else {
 						// is not a keypath like `foo.bar.baz` – could be e.g.
-						// `(a || b).foo()`. Err on the side of caution
+						// `foo[bar].baz()`. Err on the side of caution
 						hasSideEffect = true;
 					}
 				}

test/form/aggressive/_config.js

@@ -1,6 +0,0 @@
-module.exports = {
-	description: 'ignores side-effects outside entry module in aggressive mode',
-	options: {
-		aggressive: true
-	}
-}

test/form/aggressive/_expected/amd.js

@@ -1,9 +0,0 @@
-define(function () { 'use strict';
-
-	function foo () {
-		return 42;
-	}
-
-	assert.equal( foo(), 42 );
-
-});

test/form/aggressive/_expected/cjs.js

@@ -1,7 +0,0 @@
-'use strict';
-
-function foo () {
-	return 42;
-}
-
-assert.equal( foo(), 42 );

test/form/aggressive/_expected/es6.js

@@ -1,5 +0,0 @@
-function foo () {
-	return 42;
-}
-
-assert.equal( foo(), 42 );

test/form/aggressive/_expected/iife.js

@@ -1,9 +0,0 @@
-(function () { 'use strict';
-
-	function foo () {
-		return 42;
-	}
-
-	assert.equal( foo(), 42 );
-
-})();

test/form/aggressive/foo.js

@@ -1,9 +0,0 @@
-function x () {
-	console.log( 'side-effect' );
-}
-
-x();
-
-export function foo () {
-	return 42;
-}

test/form/aggressive/main.js

@@ -1,3 +0,0 @@
-import { foo } from './foo';
-
-assert.equal( foo(), 42 );

test/form/side-effect-l/_config.js

@@ -0,0 +1,3 @@
+module.exports = {
+	description: 'discards function with no side-effects in imported module'
+};

test/form/side-effect-l/_expected/amd.js

@@ -0,0 +1,5 @@
+define(function () { 'use strict';
+
+
+
+});

test/form/side-effect-l/_expected/cjs.js

@@ -0,0 +1 @@
+'use strict';

test/form/side-effect-l/_expected/iife.js

@@ -0,0 +1,5 @@
+(function () { 'use strict';
+
+
+
+})();

test/form/aggressive/_expected/umd.js → test/form/side-effect-l/_expected/umd.js

@@ -4,10 +4,6 @@
 	factory();
 }(this, function () { 'use strict';
 
-	function foo () {
-		return 42;
-	}
 
-	assert.equal( foo(), 42 );
 
-}));
+}));

test/form/side-effect-l/foo.js

@@ -0,0 +1,6 @@
+export default function foo () {
+	bar();
+	function bar () {}
+}
+
+var x = foo();

test/form/side-effect-l/main.js

@@ -0,0 +1 @@
+import './foo.js';