Merge pull request #1037 from rollup/gh-1004

Automatic semicolon insertion

src/Bundle.js

@@ -17,7 +17,7 @@ import transformBundle from './utils/transformBundle.js';
 import collapseSourcemaps from './utils/collapseSourcemaps.js';
 import SOURCEMAPPING_URL from './utils/sourceMappingURL.js';
 import callIfFunction from './utils/callIfFunction.js';
-import { dirname, isRelative, isAbsolute, normalize, relative, resolve, join } from './utils/path.js';
+import { dirname, isRelative, isAbsolute, normalize, relative, resolve } from './utils/path.js';
 import BundleScope from './ast/scopes/BundleScope.js';
 
 export default class Bundle {

src/ast/Node.js

@@ -66,6 +66,12 @@ export default class Node {
 		this.eachChild( child => child.initialise( this.scope || scope ) );
 	}
 
+	insertSemicolon ( code ) {
+		if ( code.original[ this.end - 1 ] !== ';' ) {
+			code.insertLeft( this.end, ';' );
+		}
+	}
+
 	locate () {
 		// useful for debugging
 		const location = getLocation( this.module.code, this.start );

src/ast/nodes/ExportDefaultDeclaration.js

@@ -67,6 +67,8 @@ export default class ExportDefaultDeclaration extends Node {
 					} else {
 						code.overwrite( this.start, this.declaration.start, `${this.module.bundle.varOrConst} ${name} = ` );
 					}
+
+					this.insertSemicolon( code );
 				}
 			} else {
 				// remove `var foo` from `var foo = bar()`, if `foo` is unused

src/ast/nodes/ExpressionStatement.js

@@ -1,5 +1,8 @@
 import Statement from './shared/Statement.js';
 
 export default class ExpressionStatement extends Statement {
-
+	render ( code, es ) {
+		super.render( code, es );
+		if ( this.shouldInclude ) this.insertSemicolon( code );
+	}
 }

src/ast/nodes/ThisExpression.js

@@ -1,5 +1,4 @@
 import Node from '../Node.js';
-import {normalize} from '../../utils/path';
 
 export default class ThisExpression extends Node {
 	initialise ( scope ) {

src/ast/nodes/VariableDeclaration.js

@@ -14,7 +14,7 @@ function getSeparator ( code, start ) {
 	return `;\n${lineStart}`;
 }
 
-const forStatement = /^For(?:Of|In)Statement/;
+const forStatement = /^For(?:Of|In)?Statement/;
 
 export default class VariableDeclaration extends Node {
 	initialise ( scope ) {
@@ -92,9 +92,16 @@ export default class VariableDeclaration extends Node {
 
 		if ( treeshake && empty ) {
 			code.remove( this.leadingCommentStart || this.start, this.next || this.end );
-		} else if ( this.end > c ) {
-			const hasSemicolon = code.original[ this.end - 1 ] === ';';
-			code.overwrite( c, this.end, hasSemicolon ? ';' : '' );
+		} else {
+			// always include a semi-colon (https://github.com/rollup/rollup/pull/1013),
+			// unless it's a var declaration in a loop head
+			const needsSemicolon = !forStatement.test( this.parent.type );
+
+			if ( this.end > c ) {
+				code.overwrite( c, this.end, needsSemicolon ? ';' : '' );
+			} else if ( needsSemicolon  ) {
+				this.insertSemicolon( code );
+			}
 		}
 	}
 }

test/form/assignment-to-exports-class-declaration/_expected/amd.js

@@ -1,6 +1,6 @@
 define(['exports'], function (exports) { 'use strict';
 
-	exports.Foo = class Foo {}
+	exports.Foo = class Foo {};
 	exports.Foo = lol( exports.Foo );
 
 	Object.defineProperty(exports, '__esModule', { value: true });

test/form/assignment-to-exports-class-declaration/_expected/cjs.js

@@ -2,5 +2,5 @@
 
 Object.defineProperty(exports, '__esModule', { value: true });
 
-exports.Foo = class Foo {}
+exports.Foo = class Foo {};
 exports.Foo = lol( exports.Foo );

test/form/assignment-to-exports-class-declaration/_expected/es.js

@@ -1,4 +1,4 @@
-let Foo = class Foo {}
+let Foo = class Foo {};
 Foo = lol( Foo );
 
 export { Foo };

test/form/assignment-to-exports-class-declaration/_expected/iife.js

@@ -1,7 +1,7 @@
 (function (exports) {
 	'use strict';
 
-	exports.Foo = class Foo {}
+	exports.Foo = class Foo {};
 	exports.Foo = lol( exports.Foo );
 
 }((this.myModule = this.myModule || {})));

test/form/assignment-to-exports-class-declaration/_expected/umd.js

@@ -4,7 +4,7 @@
 	(factory((global.myModule = global.myModule || {})));
 }(this, (function (exports) { 'use strict';
 
-	exports.Foo = class Foo {}
+	exports.Foo = class Foo {};
 	exports.Foo = lol( exports.Foo );
 
 	Object.defineProperty(exports, '__esModule', { value: true });

test/form/removes-existing-sourcemap-comments/_expected/amd.js

@@ -2,7 +2,7 @@ define(function () { 'use strict';
 
 	var foo = function () {
 		return 42;
-	}
+	};
 
 	console.log( foo() );
 

test/form/removes-existing-sourcemap-comments/_expected/cjs.js

@@ -2,6 +2,6 @@
 
 var foo = function () {
 	return 42;
-}
+};
 
 console.log( foo() );

test/form/removes-existing-sourcemap-comments/_expected/es.js

@@ -1,5 +1,5 @@
 var foo = function () {
 	return 42;
-}
+};
 
 console.log( foo() );

test/form/removes-existing-sourcemap-comments/_expected/iife.js

@@ -3,7 +3,7 @@
 
 	var foo = function () {
 		return 42;
-	}
+	};
 
 	console.log( foo() );
 

test/form/removes-existing-sourcemap-comments/_expected/umd.js

@@ -6,7 +6,7 @@
 
 	var foo = function () {
 		return 42;
-	}
+	};
 
 	console.log( foo() );
 

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

@@ -10,7 +10,7 @@ define(function () { 'use strict';
 	var foo = odd( 12 );
 
 	function even ( n ) {
-		alert( counter++ )
+		alert( counter++ );
 		return n === 0 || odd( n - 1 );
 	}
 

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

@@ -10,7 +10,7 @@ var counter = 0;
 var foo = odd( 12 );
 
 function even ( n ) {
-	alert( counter++ )
+	alert( counter++ );
 	return n === 0 || odd( n - 1 );
 }
 

test/form/side-effect-m/_expected/es.js

@@ -8,7 +8,7 @@ var counter = 0;
 var foo = odd( 12 );
 
 function even ( n ) {
-	alert( counter++ )
+	alert( counter++ );
 	return n === 0 || odd( n - 1 );
 }
 

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

@@ -11,7 +11,7 @@
 	var foo = odd( 12 );
 
 	function even ( n ) {
-		alert( counter++ )
+		alert( counter++ );
 		return n === 0 || odd( n - 1 );
 	}
 

test/form/side-effect-m/_expected/umd.js

@@ -14,7 +14,7 @@
 	var foo = odd( 12 );
 
 	function even ( n ) {
-		alert( counter++ )
+		alert( counter++ );
 		return n === 0 || odd( n - 1 );
 	}
 

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

@@ -5,7 +5,7 @@ define(function () { 'use strict';
 	const hs = document.documentElement.style;
 
 	if ( bool ) {
-		hs.color = "#222"
+		hs.color = "#222";
 	}
 
-});
+});

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

@@ -5,5 +5,5 @@ var bool = true;
 const hs = document.documentElement.style;
 
 if ( bool ) {
-	hs.color = "#222"
-}
+	hs.color = "#222";
+}

test/form/side-effect-p/_expected/es.js

@@ -3,5 +3,5 @@ var bool = true;
 const hs = document.documentElement.style;
 
 if ( bool ) {
-	hs.color = "#222"
-}
+	hs.color = "#222";
+}

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

@@ -6,7 +6,7 @@
 	const hs = document.documentElement.style;
 
 	if ( bool ) {
-		hs.color = "#222"
+		hs.color = "#222";
 	}
 
-}());
+}());

test/form/side-effect-p/_expected/umd.js

@@ -9,7 +9,7 @@
 	const hs = document.documentElement.style;
 
 	if ( bool ) {
-		hs.color = "#222"
+		hs.color = "#222";
 	}
 
-})));
+})));

test/form/this-is-undefined/_expected/amd.js

@@ -10,7 +10,7 @@ define(function () { 'use strict';
 	const bar = () => {
 		// ...unless it's an arrow function
 		assert.strictEqual( undefined, undefined );
-	}
+	};
 
 	foo.call( fooContext );
 	bar.call( {} );

test/form/this-is-undefined/_expected/cjs.js

@@ -10,7 +10,7 @@ function foo () {
 const bar = () => {
 	// ...unless it's an arrow function
 	assert.strictEqual( undefined, undefined );
-}
+};
 
 foo.call( fooContext );
 bar.call( {} );

test/form/this-is-undefined/_expected/es.js

@@ -8,7 +8,7 @@ function foo () {
 const bar = () => {
 	// ...unless it's an arrow function
 	assert.strictEqual( undefined, undefined );
-}
+};
 
 foo.call( fooContext );
 bar.call( {} );

test/form/this-is-undefined/_expected/iife.js

@@ -11,7 +11,7 @@
 	const bar = () => {
 		// ...unless it's an arrow function
 		assert.strictEqual( undefined, undefined );
-	}
+	};
 
 	foo.call( fooContext );
 	bar.call( {} );

test/form/this-is-undefined/_expected/umd.js

@@ -14,7 +14,7 @@
 	const bar = () => {
 		// ...unless it's an arrow function
 		assert.strictEqual( undefined, undefined );
-	}
+	};
 
 	foo.call( fooContext );
 	bar.call( {} );

test/function/adds-semicolons-if-necessary-b/_config.js

@@ -0,0 +1,3 @@
+module.exports = {
+	description: 'adds semi-colons if necessary'
+};

test/function/adds-semicolons-if-necessary-b/foo.js

@@ -0,0 +1 @@
+assert.ok( true )

test/function/adds-semicolons-if-necessary-b/main.js

@@ -0,0 +1,3 @@
+import './foo.js';
+
+(function bar() {})();

test/function/adds-semicolons-if-necessary-c/_config.js

@@ -0,0 +1,3 @@
+module.exports = {
+	description: 'adds semi-colons if necessary'
+};

test/function/adds-semicolons-if-necessary-c/foo.js

@@ -0,0 +1,3 @@
+export default function () {
+	return 42;
+}

test/function/adds-semicolons-if-necessary-c/main.js

@@ -0,0 +1,7 @@
+import foo from './foo.js';
+
+(function bar() {
+	assert.ok( true );
+})();
+
+assert.equal( foo(), 42 );

test/function/adds-semicolons-if-necessary/_config.js

@@ -0,0 +1,3 @@
+module.exports = {
+	description: 'adds semi-colons if necessary'
+};

test/function/adds-semicolons-if-necessary/foo.js

@@ -0,0 +1,3 @@
+export var foo = function () {
+	return 42;
+}

test/function/adds-semicolons-if-necessary/main.js

@@ -0,0 +1,7 @@
+import { foo } from './foo.js';
+
+(function bar() {
+	assert.ok( true );
+})();
+
+assert.equal( foo(), 42 );