From be952fef124613ae484d654f7ac41c3ed22510b5 Mon Sep 17 00:00:00 2001 From: Rich-Harris Date: Sun, 9 Oct 2016 14:12:05 -0400 Subject: [PATCH 1/4] automatic semicolon insertion (fixes #1004, #1009). fight me --- src/ast/Node.js | 6 ++++++ src/ast/nodes/ExpressionStatement.js | 5 ++++- src/ast/nodes/VariableDeclaration.js | 15 +++++++++++---- .../adds-semicolons-if-necessary-b/_config.js | 3 +++ .../adds-semicolons-if-necessary-b/foo.js | 1 + .../adds-semicolons-if-necessary-b/main.js | 3 +++ .../adds-semicolons-if-necessary/_config.js | 3 +++ test/function/adds-semicolons-if-necessary/foo.js | 3 +++ .../function/adds-semicolons-if-necessary/main.js | 7 +++++++ 9 files changed, 41 insertions(+), 5 deletions(-) create mode 100644 test/function/adds-semicolons-if-necessary-b/_config.js create mode 100644 test/function/adds-semicolons-if-necessary-b/foo.js create mode 100644 test/function/adds-semicolons-if-necessary-b/main.js create mode 100644 test/function/adds-semicolons-if-necessary/_config.js create mode 100644 test/function/adds-semicolons-if-necessary/foo.js create mode 100644 test/function/adds-semicolons-if-necessary/main.js diff --git a/src/ast/Node.js b/src/ast/Node.js index 4f65871..6a80ede 100644 --- a/src/ast/Node.js +++ b/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 ); diff --git a/src/ast/nodes/ExpressionStatement.js b/src/ast/nodes/ExpressionStatement.js index 237441a..7198541 100644 --- a/src/ast/nodes/ExpressionStatement.js +++ b/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 ); + } } diff --git a/src/ast/nodes/VariableDeclaration.js b/src/ast/nodes/VariableDeclaration.js index e1be1a0..77f4433 100644 --- a/src/ast/nodes/VariableDeclaration.js +++ b/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 ); + } } } } diff --git a/test/function/adds-semicolons-if-necessary-b/_config.js b/test/function/adds-semicolons-if-necessary-b/_config.js new file mode 100644 index 0000000..bb17123 --- /dev/null +++ b/test/function/adds-semicolons-if-necessary-b/_config.js @@ -0,0 +1,3 @@ +module.exports = { + description: 'adds semi-colons if necessary' +}; diff --git a/test/function/adds-semicolons-if-necessary-b/foo.js b/test/function/adds-semicolons-if-necessary-b/foo.js new file mode 100644 index 0000000..b2602d2 --- /dev/null +++ b/test/function/adds-semicolons-if-necessary-b/foo.js @@ -0,0 +1 @@ +assert.ok( true ) diff --git a/test/function/adds-semicolons-if-necessary-b/main.js b/test/function/adds-semicolons-if-necessary-b/main.js new file mode 100644 index 0000000..c6ddba4 --- /dev/null +++ b/test/function/adds-semicolons-if-necessary-b/main.js @@ -0,0 +1,3 @@ +import './foo.js'; + +(function bar() {})(); diff --git a/test/function/adds-semicolons-if-necessary/_config.js b/test/function/adds-semicolons-if-necessary/_config.js new file mode 100644 index 0000000..bb17123 --- /dev/null +++ b/test/function/adds-semicolons-if-necessary/_config.js @@ -0,0 +1,3 @@ +module.exports = { + description: 'adds semi-colons if necessary' +}; diff --git a/test/function/adds-semicolons-if-necessary/foo.js b/test/function/adds-semicolons-if-necessary/foo.js new file mode 100644 index 0000000..14d0105 --- /dev/null +++ b/test/function/adds-semicolons-if-necessary/foo.js @@ -0,0 +1,3 @@ +export var foo = function () { + return 42; +} diff --git a/test/function/adds-semicolons-if-necessary/main.js b/test/function/adds-semicolons-if-necessary/main.js new file mode 100644 index 0000000..2a031e5 --- /dev/null +++ b/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 ); From e8858ef1371c6bf9e7a50d7dfb9ed090d751c9a5 Mon Sep 17 00:00:00 2001 From: Rich-Harris Date: Sun, 9 Oct 2016 14:15:06 -0400 Subject: [PATCH 2/4] update tests --- .../assignment-to-exports-class-declaration/_expected/amd.js | 2 +- .../assignment-to-exports-class-declaration/_expected/cjs.js | 2 +- .../assignment-to-exports-class-declaration/_expected/es.js | 2 +- .../assignment-to-exports-class-declaration/_expected/iife.js | 2 +- .../assignment-to-exports-class-declaration/_expected/umd.js | 2 +- test/form/side-effect-m/_expected/amd.js | 2 +- test/form/side-effect-m/_expected/cjs.js | 2 +- test/form/side-effect-m/_expected/es.js | 2 +- test/form/side-effect-m/_expected/iife.js | 2 +- test/form/side-effect-m/_expected/umd.js | 2 +- test/form/side-effect-p/_expected/amd.js | 4 ++-- test/form/side-effect-p/_expected/cjs.js | 4 ++-- test/form/side-effect-p/_expected/es.js | 4 ++-- test/form/side-effect-p/_expected/iife.js | 4 ++-- test/form/side-effect-p/_expected/umd.js | 4 ++-- test/form/this-is-undefined/_expected/amd.js | 2 +- test/form/this-is-undefined/_expected/cjs.js | 2 +- test/form/this-is-undefined/_expected/es.js | 2 +- test/form/this-is-undefined/_expected/iife.js | 2 +- test/form/this-is-undefined/_expected/umd.js | 2 +- 20 files changed, 25 insertions(+), 25 deletions(-) diff --git a/test/form/assignment-to-exports-class-declaration/_expected/amd.js b/test/form/assignment-to-exports-class-declaration/_expected/amd.js index bdbdbe5..b6e6ac9 100644 --- a/test/form/assignment-to-exports-class-declaration/_expected/amd.js +++ b/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 }); diff --git a/test/form/assignment-to-exports-class-declaration/_expected/cjs.js b/test/form/assignment-to-exports-class-declaration/_expected/cjs.js index 03bdfb8..d291984 100644 --- a/test/form/assignment-to-exports-class-declaration/_expected/cjs.js +++ b/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 ); diff --git a/test/form/assignment-to-exports-class-declaration/_expected/es.js b/test/form/assignment-to-exports-class-declaration/_expected/es.js index 2631630..d4297e6 100644 --- a/test/form/assignment-to-exports-class-declaration/_expected/es.js +++ b/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 }; diff --git a/test/form/assignment-to-exports-class-declaration/_expected/iife.js b/test/form/assignment-to-exports-class-declaration/_expected/iife.js index fa37538..09261c7 100644 --- a/test/form/assignment-to-exports-class-declaration/_expected/iife.js +++ b/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 || {}))); diff --git a/test/form/assignment-to-exports-class-declaration/_expected/umd.js b/test/form/assignment-to-exports-class-declaration/_expected/umd.js index 6de079e..33b2140 100644 --- a/test/form/assignment-to-exports-class-declaration/_expected/umd.js +++ b/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 }); diff --git a/test/form/side-effect-m/_expected/amd.js b/test/form/side-effect-m/_expected/amd.js index 8c7787a..af4a5b5 100644 --- a/test/form/side-effect-m/_expected/amd.js +++ b/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 ); } diff --git a/test/form/side-effect-m/_expected/cjs.js b/test/form/side-effect-m/_expected/cjs.js index 43eacf4..1e7377f 100644 --- a/test/form/side-effect-m/_expected/cjs.js +++ b/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 ); } diff --git a/test/form/side-effect-m/_expected/es.js b/test/form/side-effect-m/_expected/es.js index 61bbbbf..d4df0da 100644 --- a/test/form/side-effect-m/_expected/es.js +++ b/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 ); } diff --git a/test/form/side-effect-m/_expected/iife.js b/test/form/side-effect-m/_expected/iife.js index d4be68e..0d378a8 100644 --- a/test/form/side-effect-m/_expected/iife.js +++ b/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 ); } diff --git a/test/form/side-effect-m/_expected/umd.js b/test/form/side-effect-m/_expected/umd.js index b0419fe..38f9417 100644 --- a/test/form/side-effect-m/_expected/umd.js +++ b/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 ); } diff --git a/test/form/side-effect-p/_expected/amd.js b/test/form/side-effect-p/_expected/amd.js index 35e8591..7a76cb7 100644 --- a/test/form/side-effect-p/_expected/amd.js +++ b/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"; } -}); \ No newline at end of file +}); diff --git a/test/form/side-effect-p/_expected/cjs.js b/test/form/side-effect-p/_expected/cjs.js index 08dc383..9553e7d 100644 --- a/test/form/side-effect-p/_expected/cjs.js +++ b/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" -} \ No newline at end of file + hs.color = "#222"; +} diff --git a/test/form/side-effect-p/_expected/es.js b/test/form/side-effect-p/_expected/es.js index 8e80199..c6eddb2 100644 --- a/test/form/side-effect-p/_expected/es.js +++ b/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" -} \ No newline at end of file + hs.color = "#222"; +} diff --git a/test/form/side-effect-p/_expected/iife.js b/test/form/side-effect-p/_expected/iife.js index 4ea2e35..7ab23ea 100644 --- a/test/form/side-effect-p/_expected/iife.js +++ b/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"; } -}()); \ No newline at end of file +}()); diff --git a/test/form/side-effect-p/_expected/umd.js b/test/form/side-effect-p/_expected/umd.js index f99ce83..57553d3 100644 --- a/test/form/side-effect-p/_expected/umd.js +++ b/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"; } -}))); \ No newline at end of file +}))); diff --git a/test/form/this-is-undefined/_expected/amd.js b/test/form/this-is-undefined/_expected/amd.js index 4452e03..c90e975 100644 --- a/test/form/this-is-undefined/_expected/amd.js +++ b/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( {} ); diff --git a/test/form/this-is-undefined/_expected/cjs.js b/test/form/this-is-undefined/_expected/cjs.js index bfa1110..c99ffc5 100644 --- a/test/form/this-is-undefined/_expected/cjs.js +++ b/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( {} ); diff --git a/test/form/this-is-undefined/_expected/es.js b/test/form/this-is-undefined/_expected/es.js index f5bd3ba..686db33 100644 --- a/test/form/this-is-undefined/_expected/es.js +++ b/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( {} ); diff --git a/test/form/this-is-undefined/_expected/iife.js b/test/form/this-is-undefined/_expected/iife.js index 890862c..f82edce 100644 --- a/test/form/this-is-undefined/_expected/iife.js +++ b/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( {} ); diff --git a/test/form/this-is-undefined/_expected/umd.js b/test/form/this-is-undefined/_expected/umd.js index b7f3efa..996e798 100644 --- a/test/form/this-is-undefined/_expected/umd.js +++ b/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( {} ); From 70c38bfaffabbcde550585fa6b35972beec3fcc3 Mon Sep 17 00:00:00 2001 From: Rich-Harris Date: Sun, 9 Oct 2016 14:20:54 -0400 Subject: [PATCH 3/4] linting --- src/Bundle.js | 2 +- src/ast/nodes/ThisExpression.js | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Bundle.js b/src/Bundle.js index 66deba6..7aeeec8 100644 --- a/src/Bundle.js +++ b/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 { diff --git a/src/ast/nodes/ThisExpression.js b/src/ast/nodes/ThisExpression.js index 5d295fe..7cb1de6 100644 --- a/src/ast/nodes/ThisExpression.js +++ b/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 ) { From 40643bf543f3df399619c3b81c3aff0c7be3e657 Mon Sep 17 00:00:00 2001 From: Rich-Harris Date: Sun, 9 Oct 2016 14:26:58 -0400 Subject: [PATCH 4/4] include default exports in ASI --- src/ast/nodes/ExportDefaultDeclaration.js | 2 ++ .../removes-existing-sourcemap-comments/_expected/amd.js | 2 +- .../removes-existing-sourcemap-comments/_expected/cjs.js | 2 +- .../removes-existing-sourcemap-comments/_expected/es.js | 2 +- .../removes-existing-sourcemap-comments/_expected/iife.js | 2 +- .../removes-existing-sourcemap-comments/_expected/umd.js | 2 +- test/function/adds-semicolons-if-necessary-c/_config.js | 3 +++ test/function/adds-semicolons-if-necessary-c/foo.js | 3 +++ test/function/adds-semicolons-if-necessary-c/main.js | 7 +++++++ 9 files changed, 20 insertions(+), 5 deletions(-) create mode 100644 test/function/adds-semicolons-if-necessary-c/_config.js create mode 100644 test/function/adds-semicolons-if-necessary-c/foo.js create mode 100644 test/function/adds-semicolons-if-necessary-c/main.js diff --git a/src/ast/nodes/ExportDefaultDeclaration.js b/src/ast/nodes/ExportDefaultDeclaration.js index ab4a776..81ae1c9 100644 --- a/src/ast/nodes/ExportDefaultDeclaration.js +++ b/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 diff --git a/test/form/removes-existing-sourcemap-comments/_expected/amd.js b/test/form/removes-existing-sourcemap-comments/_expected/amd.js index 99bdf35..b89e399 100644 --- a/test/form/removes-existing-sourcemap-comments/_expected/amd.js +++ b/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() ); diff --git a/test/form/removes-existing-sourcemap-comments/_expected/cjs.js b/test/form/removes-existing-sourcemap-comments/_expected/cjs.js index 8735c93..1152d76 100644 --- a/test/form/removes-existing-sourcemap-comments/_expected/cjs.js +++ b/test/form/removes-existing-sourcemap-comments/_expected/cjs.js @@ -2,6 +2,6 @@ var foo = function () { return 42; -} +}; console.log( foo() ); diff --git a/test/form/removes-existing-sourcemap-comments/_expected/es.js b/test/form/removes-existing-sourcemap-comments/_expected/es.js index 2a09cfb..0ef1000 100644 --- a/test/form/removes-existing-sourcemap-comments/_expected/es.js +++ b/test/form/removes-existing-sourcemap-comments/_expected/es.js @@ -1,5 +1,5 @@ var foo = function () { return 42; -} +}; console.log( foo() ); diff --git a/test/form/removes-existing-sourcemap-comments/_expected/iife.js b/test/form/removes-existing-sourcemap-comments/_expected/iife.js index 6381613..cac12fb 100644 --- a/test/form/removes-existing-sourcemap-comments/_expected/iife.js +++ b/test/form/removes-existing-sourcemap-comments/_expected/iife.js @@ -3,7 +3,7 @@ var foo = function () { return 42; - } + }; console.log( foo() ); diff --git a/test/form/removes-existing-sourcemap-comments/_expected/umd.js b/test/form/removes-existing-sourcemap-comments/_expected/umd.js index b291566..9a6706d 100644 --- a/test/form/removes-existing-sourcemap-comments/_expected/umd.js +++ b/test/form/removes-existing-sourcemap-comments/_expected/umd.js @@ -6,7 +6,7 @@ var foo = function () { return 42; - } + }; console.log( foo() ); diff --git a/test/function/adds-semicolons-if-necessary-c/_config.js b/test/function/adds-semicolons-if-necessary-c/_config.js new file mode 100644 index 0000000..bb17123 --- /dev/null +++ b/test/function/adds-semicolons-if-necessary-c/_config.js @@ -0,0 +1,3 @@ +module.exports = { + description: 'adds semi-colons if necessary' +}; diff --git a/test/function/adds-semicolons-if-necessary-c/foo.js b/test/function/adds-semicolons-if-necessary-c/foo.js new file mode 100644 index 0000000..7dc8833 --- /dev/null +++ b/test/function/adds-semicolons-if-necessary-c/foo.js @@ -0,0 +1,3 @@ +export default function () { + return 42; +} diff --git a/test/function/adds-semicolons-if-necessary-c/main.js b/test/function/adds-semicolons-if-necessary-c/main.js new file mode 100644 index 0000000..ae0bdb2 --- /dev/null +++ b/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 );