Make sure annotations are removed together with the annotated nodes even

if this is not covered by normal comment removal.
rollup · Feb 22, 2019 · 1ff583c · 1ff583c
1 parent df1fef2
commit 1ff583c
Show file tree

Hide file tree

Showing 15 changed files with 161 additions and 44 deletions.
diff --git 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 { NodeRenderOptions, RenderOptions } from '../../utils/renderHelpers';
+import { removeAnnotations } from '../../utils/treeshakeNode';
 import CallOptions from '../CallOptions';
 import { DeoptimizableEntity } from '../DeoptimizableEntity';
 import { ExecutionPathOptions } from '../ExecutionPathOptions';
@@ -160,6 +161,7 @@ export default class ConditionalExpression extends NodeBase implements Deoptimiz
 		if (!this.test.included) {
 			code.remove(this.start, this.usedBranch.start);
 			code.remove(this.usedBranch.end, this.end);
+			removeAnnotations(this, code);
 			this.usedBranch.render(code, options, {
 				renderedParentType: renderedParentType || this.parent.type,
 				isCalleeOfRenderedParent: renderedParentType

diff --git a/src/ast/nodes/EmptyStatement.ts b/src/ast/nodes/EmptyStatement.ts
@@ -1,14 +1,10 @@
-import MagicString from 'magic-string';
-import { RenderOptions } from '../../utils/renderHelpers';
 import * as NodeType from './NodeType';
 import { StatementBase } from './shared/Node';
 
 export default class EmptyStatement extends StatementBase {
 	type: NodeType.tEmptyStatement;
 
-	render(code: MagicString, _options: RenderOptions) {
-		if (this.parent.type === NodeType.BlockStatement || this.parent.type === NodeType.Program) {
-			code.remove(this.start, this.end);
-		}
+	hasEffects(): boolean {
+		return false;
 	}
 }
diff --git a/src/ast/nodes/ExportAllDeclaration.ts b/src/ast/nodes/ExportAllDeclaration.ts
@@ -11,6 +11,10 @@ export default class ExportAllDeclaration extends NodeBase {
 
 	needsBoundaries: true;
 
+	hasEffects() {
+		return false;
+	}
+
 	initialise() {
 		this.included = false;
 		this.context.addExport(this);

diff --git a/src/ast/nodes/ExportDefaultDeclaration.ts b/src/ast/nodes/ExportDefaultDeclaration.ts
@@ -5,6 +5,7 @@ import {
 	NodeRenderOptions,
 	RenderOptions
 } from '../../utils/renderHelpers';
+import { treeshakeNode } from '../../utils/treeshakeNode';
 import ModuleScope from '../scopes/ModuleScope';
 import ExportDefaultVariable from '../variables/ExportDefaultVariable';
 import ClassDeclaration, { isClassDeclaration } from './ClassDeclaration';
@@ -88,7 +89,7 @@ export default class ExportDefaultDeclaration extends NodeBase {
 					`exports('${this.variable.exportName}', ${this.variable.getName()});`
 				);
 			} else {
-				code.remove(start, end);
+				treeshakeNode(this, code, start, end);
 			}
 			return;
 		} else if (this.variable.included) {

diff --git a/src/ast/nodes/IfStatement.ts b/src/ast/nodes/IfStatement.ts
@@ -1,5 +1,6 @@
 import MagicString from 'magic-string';
 import { RenderOptions } from '../../utils/renderHelpers';
+import { removeAnnotations } from '../../utils/treeshakeNode';
 import { DeoptimizableEntity } from '../DeoptimizableEntity';
 import { ExecutionPathOptions } from '../ExecutionPathOptions';
 import { EMPTY_IMMUTABLE_TRACKER } from '../utils/ImmutableEntityPathTracker';
@@ -83,6 +84,7 @@ export default class IfStatement extends StatementBase implements DeoptimizableE
 			const singleRetainedBranch = this.testValue ? this.consequent : this.alternate;
 			code.remove(this.start, singleRetainedBranch.start);
 			code.remove(singleRetainedBranch.end, this.end);
+			removeAnnotations(this, code);
 			singleRetainedBranch.render(code, options);
 		} else {
 			if (this.test.included) {

diff --git a/src/ast/nodes/LogicalExpression.ts b/src/ast/nodes/LogicalExpression.ts
@@ -1,6 +1,7 @@
 import MagicString from 'magic-string';
 import { BLANK } from '../../utils/blank';
 import { NodeRenderOptions, RenderOptions } from '../../utils/renderHelpers';
+import { removeAnnotations } from '../../utils/treeshakeNode';
 import CallOptions from '../CallOptions';
 import { DeoptimizableEntity } from '../DeoptimizableEntity';
 import { ExecutionPathOptions } from '../ExecutionPathOptions';
@@ -164,6 +165,7 @@ export default class LogicalExpression extends NodeBase implements Deoptimizable
 		if (!this.left.included || !this.right.included) {
 			code.remove(this.start, this.usedBranch.start);
 			code.remove(this.usedBranch.end, this.end);
+			removeAnnotations(this, code);
 			this.usedBranch.render(code, options, {
 				renderedParentType: renderedParentType || this.parent.type,
 				isCalleeOfRenderedParent: renderedParentType

diff --git a/src/ast/nodes/SequenceExpression.ts b/src/ast/nodes/SequenceExpression.ts
@@ -5,6 +5,7 @@ import {
 	NodeRenderOptions,
 	RenderOptions
 } from '../../utils/renderHelpers';
+import { treeshakeNode } from '../../utils/treeshakeNode';
 import CallOptions from '../CallOptions';
 import { DeoptimizableEntity } from '../DeoptimizableEntity';
 import { ExecutionPathOptions } from '../ExecutionPathOptions';
@@ -92,7 +93,7 @@ export default class SequenceExpression extends NodeBase {
 			this.end
 		)) {
 			if (!node.included) {
-				code.remove(start, end);
+				treeshakeNode(node, code, start, end);
 				continue;
 			}
 			includedNodes++;

diff --git a/src/ast/nodes/shared/Node.ts b/src/ast/nodes/shared/Node.ts
@@ -1,6 +1,6 @@
 import { locate } from 'locate-character';
 import MagicString from 'magic-string';
-import { AstContext } from '../../../Module';
+import { AstContext, CommentDescription } from '../../../Module';
 import { NodeRenderOptions, RenderOptions } from '../../../utils/renderHelpers';
 import CallOptions from '../../CallOptions';
 import { DeoptimizableEntity } from '../../DeoptimizableEntity';
@@ -19,15 +19,16 @@ export interface GenericEsTreeNode {
 }
 
 export interface Node extends Entity {
+	annotations?: CommentDescription[];
+	context: AstContext;
 	end: number;
 	included: boolean;
 	keys: string[];
-	context: AstContext;
+	needsBoundaries?: boolean;
 	parent: Node | { type?: string };
+	preventChildBlockScope?: boolean;
 	start: number;
 	type: string;
-	needsBoundaries?: boolean;
-	preventChildBlockScope?: boolean;
 	variable?: Variable | null;
 
 	/**

diff --git a/src/utils/pureComments.ts b/src/utils/pureComments.ts
@@ -4,41 +4,32 @@ import { base as basicWalker } from 'acorn-walk';
 import * as ESTree from 'estree';
 import { CommentDescription } from '../Module';
 
-type NodeHandler = (node: ESTree.Node) => void;
+type NodeHandler = (node: ESTree.Node, comment: CommentDescription) => void;
 
 function checkCommentsBeforeNode(
 	node: ESTree.Node & acorn.Node,
 	state: { handleNode: NodeHandler; commentIndex: number; commentNodes: CommentDescription[] },
 	type: string = node.type
 ) {
-	if (
-		state.commentIndex === state.commentNodes.length ||
-		state.commentNodes[state.commentIndex].end > node.end
-	)
-		return;
-	let isFound = false;
-	while (
-		state.commentIndex < state.commentNodes.length &&
-		node.start >= state.commentNodes[state.commentIndex].end
-	) {
-		if (!isFound) {
-			state.handleNode(node);
-			isFound = true;
-		}
-		state.commentIndex++;
+	let commentNode = state.commentNodes[state.commentIndex];
+	while (commentNode && node.start >= commentNode.end) {
+		state.handleNode(node, commentNode);
+		commentNode = state.commentNodes[++state.commentIndex];
+	}
+	if (commentNode && commentNode.end <= node.end) {
+		basicWalker[type](node, state, checkCommentsBeforeNode);
 	}
-	basicWalker[type](node, state, checkCommentsBeforeNode);
-}
-
-function forEachNodeAfterComment(
-	ast: ESTree.Program,
-	commentNodes: CommentDescription[],
-	handleNode: NodeHandler
-): void {
-	checkCommentsBeforeNode(<any>ast, { commentNodes, commentIndex: 0, handleNode });
 }
 
-function markPureNode(node: ESTree.Node) {
+function markPureNode(
+	node: ESTree.Node & { annotations?: CommentDescription[] },
+	comment: CommentDescription
+) {
+	if (node.annotations) {
+		node.annotations.push(comment);
+	} else {
+		node.annotations = [comment];
+	}
 	if (node.type === 'ExpressionStatement') {
 		node = node.expression;
 	}
@@ -48,11 +39,12 @@ function markPureNode(node: ESTree.Node) {
 }
 
 const pureCommentRegex = /[@#]__PURE__/;
+const isPureComment = (comment: CommentDescription) => pureCommentRegex.test(comment.text);
 
 export function markPureCallExpressions(comments: CommentDescription[], esTreeAst: ESTree.Program) {
-	forEachNodeAfterComment(
-		esTreeAst,
-		comments.filter(comment => pureCommentRegex.test(comment.text)),
-		markPureNode
-	);
+	checkCommentsBeforeNode(<any>esTreeAst, {
+		commentNodes: comments.filter(isPureComment),
+		commentIndex: 0,
+		handleNode: markPureNode
+	});
 }
diff --git a/src/utils/renderHelpers.ts b/src/utils/renderHelpers.ts
@@ -1,5 +1,6 @@
 import MagicString from 'magic-string';
 import { Node, StatementNode } from '../ast/nodes/shared/Node';
+import { treeshakeNode } from './treeshakeNode';
 
 export interface RenderOptions {
 	compact: boolean;
@@ -104,7 +105,7 @@ export function renderStatementList(
 					  })
 					: currentNode.render(code, options);
 			} else {
-				code.remove(currentNodeStart, nextNodeStart);
+				treeshakeNode(currentNode, code, currentNodeStart, nextNodeStart);
 			}
 		} else {
 			currentNode.render(code, options);

diff --git a/src/utils/treeshakeNode.ts b/src/utils/treeshakeNode.ts
@@ -0,0 +1,27 @@
+import MagicString from 'magic-string';
+import * as NodeType from '../ast/nodes/NodeType';
+import { Node } from '../ast/nodes/shared/Node';
+
+export function treeshakeNode(node: Node, code: MagicString, start: number, end: number) {
+	code.remove(start, end);
+	if (node.annotations) {
+		for (const annotation of node.annotations) {
+			if (annotation.start < start) {
+				code.remove(annotation.start, annotation.end);
+			} else {
+				return;
+			}
+		}
+	}
+}
+
+export function removeAnnotations(node: Node, code: MagicString) {
+	if (!node.annotations && node.parent.type === NodeType.ExpressionStatement) {
+		node = node.parent as Node;
+	}
+	if (node.annotations) {
+		for (const annotation of node.annotations) {
+			code.remove(annotation.start, annotation.end);
+		}
+	}
+}
diff --git a/test/form/samples/pure-comment-line-break/_config.js b/test/form/samples/pure-comment-line-break/_config.js
@@ -0,0 +1,3 @@
+module.exports = {
+	description: 'adjust line-break handling when dealing with pure annotations'
+};
diff --git a/test/form/samples/pure-comment-line-break/_expected.js b/test/form/samples/pure-comment-line-break/_expected.js
@@ -0,0 +1,37 @@
+const x = 'code';
+console.log('should remain impure');
+
+console.log('should remain impure');
+
+console.log('code');
+console.log('should remain impure');
+
+console.log('code');
+console.log('should remain impure');
+console.log('should remain impure');
+
+console.log('code');
+console.log('should remain impure');
+
+console.log('code'),
+console.log('should remain impure');
+
+console.log('code'),
+console.log('should remain impure');
+
+console.log('should remain impure');
+
+console.log('code');
+console.log('should remain impure');
+
+console.log('should remain impure');
+
+console.log('should remain impure');
+
+console.log('code');
+console.log('should remain impure', x);
+
+
+{
+	console.log('should remain impure');
+}
diff --git a/test/form/samples/pure-comment-line-break/dep.js b/test/form/samples/pure-comment-line-break/dep.js
@@ -0,0 +1,3 @@
+const x = 'code';//@__PURE__
+export default x;
+console.log('should remain impure');
diff --git a/test/form/samples/pure-comment-line-break/main.js b/test/form/samples/pure-comment-line-break/main.js
@@ -0,0 +1,45 @@
+/*@__PURE__*/
+(() => {})();
+console.log('should remain impure');
+
+console.log('code');//@__PURE__
+(() => {})();
+console.log('should remain impure');
+
+console.log('code')/*@__PURE__*/;
+(() => {})();
+console.log('should remain impure');
+
+(() => {})();//@__PURE__
+(() => {})();
+console.log('should remain impure');
+
+console.log('code');/*@__PURE__*///@__PURE__
+/*@__PURE__*/ (() => {})();
+console.log('should remain impure');
+
+console.log('code'),//@__PURE__
+(() => {})(),console.log('should remain impure');
+
+console.log('code')/*@__PURE__*/,
+(() => {})(),console.log('should remain impure');
+
+(() => {})(),//@__PURE__
+(() => {})(),console.log('should remain impure');
+
+console.log('code');//@__PURE__
+;console.log('should remain impure');
+
+/*@__PURE__*/true && console.log('should remain impure');
+
+/*@__PURE__*/true ? console.log('should remain impure') : console.log('code');
+
+console.log('code');//@__PURE__
+import code from './dep.js';
+
+console.log('should remain impure', code);
+
+/*@__PURE__*/
+if (true) {
+	console.log('should remain impure');
+}