From 1059e99c7c64a712404b80b8cff61fa0eeed3586 Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Tue, 6 Aug 2019 13:22:24 +0200 Subject: [PATCH] Prevent ASI errors for conditional expressions --- src/ast/nodes/ConditionalExpression.ts | 29 +++++++++++++------ src/ast/nodes/ReturnStatement.ts | 2 +- src/utils/renderHelpers.ts | 1 + .../_config.js | 4 +++ .../_expected.js | 7 +++++ .../keep-tree-shaking-comments-no-asi/main.js | 9 ++++++ .../prevent-tree-shaking-asi/_config.js | 9 ++++++ .../samples/prevent-tree-shaking-asi/main.js | 15 ++++++++++ 8 files changed, 66 insertions(+), 10 deletions(-) create mode 100644 test/form/samples/keep-tree-shaking-comments-no-asi/_config.js create mode 100644 test/form/samples/keep-tree-shaking-comments-no-asi/_expected.js create mode 100644 test/form/samples/keep-tree-shaking-comments-no-asi/main.js create mode 100644 test/function/samples/prevent-tree-shaking-asi/_config.js create mode 100644 test/function/samples/prevent-tree-shaking-asi/main.js diff --git a/src/ast/nodes/ConditionalExpression.ts b/src/ast/nodes/ConditionalExpression.ts index 4cb71af5cb8..0d990838be8 100644 --- a/src/ast/nodes/ConditionalExpression.ts +++ b/src/ast/nodes/ConditionalExpression.ts @@ -1,6 +1,7 @@ import MagicString from 'magic-string'; import { BLANK } from '../../utils/blank'; import { + findFirstLineBreakOutsideComment, findFirstOccurrenceOutsideComment, NodeRenderOptions, RenderOptions @@ -151,20 +152,30 @@ export default class ConditionalExpression extends NodeBase implements Deoptimiz render( code: MagicString, options: RenderOptions, - { renderedParentType, isCalleeOfRenderedParent }: NodeRenderOptions = BLANK + { renderedParentType, isCalleeOfRenderedParent, preventASI }: NodeRenderOptions = BLANK ) { if (!this.test.included) { const colonPos = findFirstOccurrenceOutsideComment(code.original, ':', this.consequent.end); + const inclusionStart = + (this.consequent.included + ? findFirstOccurrenceOutsideComment(code.original, '?', this.test.end) + : colonPos) + 1; + if (preventASI) { + const branchStart = (this.usedBranch as ExpressionNode).start; + let lineBreakPos = inclusionStart; + do { + lineBreakPos = findFirstLineBreakOutsideComment(code.original, lineBreakPos); + if (lineBreakPos >= 0 && lineBreakPos < branchStart) { + code.remove(lineBreakPos, lineBreakPos + 1); + lineBreakPos++; + } else { + lineBreakPos = -1; + } + } while (lineBreakPos >= 0); + } + code.remove(this.start, inclusionStart); if (this.consequent.included) { - const questionmarkPos = findFirstOccurrenceOutsideComment( - code.original, - '?', - this.test.end - ); - code.remove(this.start, questionmarkPos + 1); code.remove(colonPos, this.end); - } else { - code.remove(this.start, colonPos + 1); } removeAnnotations(this, code); (this.usedBranch as ExpressionNode).render(code, options, { diff --git a/src/ast/nodes/ReturnStatement.ts b/src/ast/nodes/ReturnStatement.ts index d8f41c8370e..7f2998b5ea5 100644 --- a/src/ast/nodes/ReturnStatement.ts +++ b/src/ast/nodes/ReturnStatement.ts @@ -22,7 +22,7 @@ export default class ReturnStatement extends StatementBase { render(code: MagicString, options: RenderOptions) { if (this.argument) { - this.argument.render(code, options); + this.argument.render(code, options, { preventASI: true }); if (this.argument.start === this.start + 6 /* 'return'.length */) { code.prependLeft(this.start + 6, ' '); } diff --git a/src/utils/renderHelpers.ts b/src/utils/renderHelpers.ts index 2a2af20ba64..cf0dbba649f 100644 --- a/src/utils/renderHelpers.ts +++ b/src/utils/renderHelpers.ts @@ -17,6 +17,7 @@ export interface NodeRenderOptions { isCalleeOfRenderedParent?: boolean; isNoStatement?: boolean; isShorthandProperty?: boolean; + preventASI?: boolean; renderedParentType?: string; // also serves as a flag if the rendered parent is different from the actual parent start?: number; } diff --git a/test/form/samples/keep-tree-shaking-comments-no-asi/_config.js b/test/form/samples/keep-tree-shaking-comments-no-asi/_config.js new file mode 100644 index 00000000000..74cf0990a83 --- /dev/null +++ b/test/form/samples/keep-tree-shaking-comments-no-asi/_config.js @@ -0,0 +1,4 @@ +module.exports = { + description: + 'always keep leading comments when tree-shaking and no automatic semicolons are inserted' +}; diff --git a/test/form/samples/keep-tree-shaking-comments-no-asi/_expected.js b/test/form/samples/keep-tree-shaking-comments-no-asi/_expected.js new file mode 100644 index 00000000000..4e8a40b8b1b --- /dev/null +++ b/test/form/samples/keep-tree-shaking-comments-no-asi/_expected.js @@ -0,0 +1,7 @@ +console.log( + /* keep me */ + 'expected'); + +console.log( + /* keep me */ + 'expected' ); diff --git a/test/form/samples/keep-tree-shaking-comments-no-asi/main.js b/test/form/samples/keep-tree-shaking-comments-no-asi/main.js new file mode 100644 index 00000000000..18a6a0f3bac --- /dev/null +++ b/test/form/samples/keep-tree-shaking-comments-no-asi/main.js @@ -0,0 +1,9 @@ +console.log(false ? + 'unexpected' : + /* keep me */ + 'expected'); + +console.log(true ? + /* keep me */ + 'expected' : + 'unexpected'); diff --git a/test/function/samples/prevent-tree-shaking-asi/_config.js b/test/function/samples/prevent-tree-shaking-asi/_config.js new file mode 100644 index 00000000000..b3139391407 --- /dev/null +++ b/test/function/samples/prevent-tree-shaking-asi/_config.js @@ -0,0 +1,9 @@ +const assert = require('assert'); + +module.exports = { + description: 'prevent automatic semicolon insertion from changing behaviour when tree-shaking', + exports(exports) { + assert.strictEqual(exports.test1(), 'expected'); + assert.strictEqual(exports.test2(), 'expected'); + } +}; diff --git a/test/function/samples/prevent-tree-shaking-asi/main.js b/test/function/samples/prevent-tree-shaking-asi/main.js new file mode 100644 index 00000000000..eb69cf7aff0 --- /dev/null +++ b/test/function/samples/prevent-tree-shaking-asi/main.js @@ -0,0 +1,15 @@ +export function test1() { + return true ? + + + 'expected' : + 'unexpected'; +} + +export function test2() { + return false ? + 'unexpected' : + + + 'expected'; +}