Always use a new inclusion context when including declarations of var…

…iables, always inlcude labels when not treeshaking, resolves #3473 (#3492)

src/Chunk.ts

@@ -1,6 +1,5 @@
 import MagicString, { Bundle as MagicStringBundle, SourceMap } from 'magic-string';
 import { relative } from '../browser/path';
-import { createInclusionContext } from './ast/ExecutionContext';
 import ExportDefaultDeclaration from './ast/nodes/ExportDefaultDeclaration';
 import FunctionDeclaration from './ast/nodes/FunctionDeclaration';
 import { UNDEFINED_EXPRESSION } from './ast/values';
@@ -1051,10 +1050,9 @@ export default class Chunk {
 				}
 			}
 		}
-		const context = createInclusionContext();
 		for (const { node, resolution } of module.dynamicImports) {
 			if (node.included && resolution instanceof Module && resolution.chunk === this)
-				resolution.getOrCreateNamespace().include(context);
+				resolution.getOrCreateNamespace().include();
 		}
 	}
 }

src/Module.ts

@@ -2,11 +2,7 @@ import * as acorn from 'acorn';
 import { locate } from 'locate-character';
 import MagicString from 'magic-string';
 import extractAssignedNames from 'rollup-pluginutils/src/extractAssignedNames';
-import {
-	createHasEffectsContext,
-	createInclusionContext,
-	InclusionContext
-} from './ast/ExecutionContext';
+import { createHasEffectsContext, createInclusionContext } from './ast/ExecutionContext';
 import ExportAllDeclaration from './ast/nodes/ExportAllDeclaration';
 import ExportDefaultDeclaration from './ast/nodes/ExportDefaultDeclaration';
 import ExportNamedDeclaration from './ast/nodes/ExportNamedDeclaration';
@@ -100,9 +96,9 @@ export interface AstContext {
 	getModuleName: () => string;
 	getReexports: () => string[];
 	importDescriptions: { [name: string]: ImportDescription };
-	includeAndGetAdditionalMergedNamespaces: (context: InclusionContext) => Variable[];
+	includeAndGetAdditionalMergedNamespaces: () => Variable[];
 	includeDynamicImport: (node: ImportExpression) => void;
-	includeVariable: (context: InclusionContext, variable: Variable) => void;
+	includeVariable: (variable: Variable) => void;
 	magicString: MagicString;
 	module: Module; // not to be used for tree-shaking
 	moduleContext: string;
@@ -565,12 +561,11 @@ export default class Module {
 			markModuleAndImpureDependenciesAsExecuted(this);
 		}
 
-		const context = createInclusionContext();
 		for (const exportName of this.getExports()) {
 			const variable = this.getVariableForExportName(exportName);
 			variable.deoptimizePath(UNKNOWN_PATH);
 			if (!variable.included) {
-				variable.include(context);
+				variable.include();
 				this.graph.needsTreeshakingPass = true;
 			}
 		}
@@ -579,7 +574,7 @@ export default class Module {
 			const variable = this.getVariableForExportName(name);
 			variable.deoptimizePath(UNKNOWN_PATH);
 			if (!variable.included) {
-				variable.include(context);
+				variable.include();
 				this.graph.needsTreeshakingPass = true;
 			}
 			if (variable instanceof ExternalVariable) {
@@ -905,7 +900,7 @@ export default class Module {
 		}
 	}
 
-	private includeAndGetAdditionalMergedNamespaces(context: InclusionContext): Variable[] {
+	private includeAndGetAdditionalMergedNamespaces(): Variable[] {
 		const mergedNamespaces: Variable[] = [];
 		for (const module of this.exportAllModules) {
 			if (module instanceof ExternalModule) {
@@ -915,7 +910,7 @@ export default class Module {
 				mergedNamespaces.push(externalVariable);
 			} else if (module.syntheticNamedExports) {
 				const syntheticNamespace = module.getDefaultExport();
-				syntheticNamespace.include(context);
+				syntheticNamespace.include();
 				this.imports.add(syntheticNamespace);
 				mergedNamespaces.push(syntheticNamespace);
 			}
@@ -933,10 +928,10 @@ export default class Module {
 		}
 	}
 
-	private includeVariable(context: InclusionContext, variable: Variable) {
+	private includeVariable(variable: Variable) {
 		const variableModule = variable.module;
 		if (!variable.included) {
-			variable.include(context);
+			variable.include();
 			this.graph.needsTreeshakingPass = true;
 		}
 		if (variableModule && variableModule !== this) {

src/ast/nodes/BreakStatement.ts

@@ -27,7 +27,7 @@ export default class BreakStatement extends StatementBase {
 	include(context: InclusionContext) {
 		this.included = true;
 		if (this.label) {
-			this.label.include(context);
+			this.label.include();
 			context.includedLabels.add(this.label.name);
 		}
 		context.brokenFlow = this.label ? BROKEN_FLOW_ERROR_RETURN_LABEL : BROKEN_FLOW_BREAK_CONTINUE;

src/ast/nodes/ContinueStatement.ts

@@ -27,7 +27,7 @@ export default class ContinueStatement extends StatementBase {
 	include(context: InclusionContext) {
 		this.included = true;
 		if (this.label) {
-			this.label.include(context);
+			this.label.include();
 			context.includedLabels.add(this.label.name);
 		}
 		context.brokenFlow = this.label ? BROKEN_FLOW_ERROR_RETURN_LABEL : BROKEN_FLOW_BREAK_CONTINUE;

src/ast/nodes/ExportDefaultDeclaration.ts

@@ -46,7 +46,7 @@ export default class ExportDefaultDeclaration extends NodeBase {
 	include(context: InclusionContext, includeChildrenRecursively: IncludeChildren) {
 		super.include(context, includeChildrenRecursively);
 		if (includeChildrenRecursively) {
-			this.context.includeVariable(context, this.variable);
+			this.context.includeVariable(this.variable);
 		}
 	}
 

src/ast/nodes/Identifier.ts

@@ -123,11 +123,11 @@ export default class Identifier extends NodeBase implements PatternNode {
 		return !this.variable || this.variable.hasEffectsWhenCalledAtPath(path, callOptions, context);
 	}
 
-	include(context: InclusionContext) {
+	include() {
 		if (!this.included) {
 			this.included = true;
 			if (this.variable !== null) {
-				this.context.includeVariable(context, this.variable);
+				this.context.includeVariable(this.variable);
 			}
 		}
 	}

src/ast/nodes/LabeledStatement.ts

@@ -26,8 +26,8 @@ export default class LabeledStatement extends StatementBase {
 		this.included = true;
 		const brokenFlow = context.brokenFlow;
 		this.body.include(context, includeChildrenRecursively);
-		if (context.includedLabels.has(this.label.name)) {
-			this.label.include(context);
+		if (includeChildrenRecursively || context.includedLabels.has(this.label.name)) {
+			this.label.include();
 			context.includedLabels.delete(this.label.name);
 			context.brokenFlow = brokenFlow;
 		}

src/ast/nodes/MemberExpression.ts

@@ -213,7 +213,7 @@ export default class MemberExpression extends NodeBase implements DeoptimizableE
 		if (!this.included) {
 			this.included = true;
 			if (this.variable !== null) {
-				this.context.includeVariable(context, this.variable);
+				this.context.includeVariable(this.variable);
 			}
 		}
 		this.object.include(context, includeChildrenRecursively);

src/ast/nodes/shared/FunctionNode.ts

@@ -92,7 +92,7 @@ export default class FunctionNode extends NodeBase {
 
 	include(context: InclusionContext, includeChildrenRecursively: IncludeChildren) {
 		this.included = true;
-		if (this.id) this.id.include(context);
+		if (this.id) this.id.include();
 		const hasArguments = this.scope.argumentsVariable.included;
 		for (const param of this.params) {
 			if (!(param instanceof Identifier) || hasArguments) {

src/ast/variables/LocalVariable.ts

@@ -2,7 +2,7 @@ import Module, { AstContext } from '../../Module';
 import { markModuleAndImpureDependenciesAsExecuted } from '../../utils/traverseStaticDependencies';
 import { CallOptions } from '../CallOptions';
 import { DeoptimizableEntity } from '../DeoptimizableEntity';
-import { HasEffectsContext, InclusionContext } from '../ExecutionContext';
+import { createInclusionContext, HasEffectsContext, InclusionContext } from '../ExecutionContext';
 import ExportDefaultDeclaration from '../nodes/ExportDefaultDeclaration';
 import Identifier from '../nodes/Identifier';
 import * as NodeType from '../nodes/NodeType';
@@ -156,15 +156,15 @@ export default class LocalVariable extends Variable {
 		return (this.init && this.init.hasEffectsWhenCalledAtPath(path, callOptions, context))!;
 	}
 
-	include(context: InclusionContext) {
+	include() {
 		if (!this.included) {
 			this.included = true;
 			if (!this.module.isExecuted) {
 				markModuleAndImpureDependenciesAsExecuted(this.module);
 			}
 			for (const declaration of this.declarations) {
 				// If node is a default export, it can save a tree-shaking run to include the full declaration now
-				if (!declaration.included) declaration.include(context, false);
+				if (!declaration.included) declaration.include(createInclusionContext(), false);
 				let node = declaration.parent as Node;
 				while (!node.included) {
 					// We do not want to properly include parents in case they are part of a dead branch

src/ast/variables/NamespaceVariable.ts

@@ -1,7 +1,6 @@
 import Module, { AstContext } from '../../Module';
 import { RenderOptions } from '../../utils/renderHelpers';
 import { RESERVED_NAMES } from '../../utils/reservedNames';
-import { InclusionContext } from '../ExecutionContext';
 import Identifier from '../nodes/Identifier';
 import { UNKNOWN_PATH } from '../utils/PathTracker';
 import Variable from './Variable';
@@ -39,7 +38,7 @@ export default class NamespaceVariable extends Variable {
 		}
 	}
 
-	include(context: InclusionContext) {
+	include() {
 		if (!this.included) {
 			this.included = true;
 			for (const identifier of this.references) {
@@ -48,13 +47,13 @@ export default class NamespaceVariable extends Variable {
 					break;
 				}
 			}
-			this.mergedNamespaces = this.context.includeAndGetAdditionalMergedNamespaces(context);
+			this.mergedNamespaces = this.context.includeAndGetAdditionalMergedNamespaces();
 			if (this.context.preserveModules) {
 				for (const memberName of Object.keys(this.memberVariables))
-					this.memberVariables[memberName].include(context);
+					this.memberVariables[memberName].include();
 			} else {
 				for (const memberName of Object.keys(this.memberVariables))
-					this.context.includeVariable(context, this.memberVariables[memberName]);
+					this.context.includeVariable(this.memberVariables[memberName]);
 			}
 		}
 	}

src/ast/variables/SyntheticNamedExportVariable.ts

@@ -1,5 +1,4 @@
 import Module, { AstContext } from '../../Module';
-import { InclusionContext } from '../ExecutionContext';
 import ExportDefaultVariable from './ExportDefaultVariable';
 import Variable from './Variable';
 
@@ -25,10 +24,10 @@ export default class SyntheticNamedExportVariable extends Variable {
 		return this.defaultVariable.getOriginalVariable();
 	}
 
-	include(context: InclusionContext) {
+	include() {
 		if (!this.included) {
 			this.included = true;
-			this.context.includeVariable(context, this.defaultVariable);
+			this.context.includeVariable(this.defaultVariable);
 		}
 	}
 }

src/ast/variables/Variable.ts

@@ -82,7 +82,7 @@ export default class Variable implements ExpressionEntity {
 	 * previously.
 	 * Once a variable is included, it should take care all its declarations are included.
 	 */
-	include(_context: InclusionContext) {
+	include() {
 		this.included = true;
 	}
 
@@ -102,8 +102,4 @@ export default class Variable implements ExpressionEntity {
 	setSafeName(name: string | null) {
 		this.renderName = name;
 	}
-
-	toString() {
-		return this.name;
-	}
 }

test/form/samples/no-treeshake-include-labels/_config.js

@@ -0,0 +1,7 @@
+module.exports = {
+	description: 'always includes labels when tree-shaking is turned off (#3473)',
+	expectedWarnings: ['CIRCULAR_DEPENDENCY'],
+	options: {
+		treeshake: false
+	}
+};

test/form/samples/no-treeshake-include-labels/_expected.js

@@ -0,0 +1,20 @@
+function foo() {
+	loop: while (unknown) {
+		if (unknown) {
+			break loop;
+		}
+		bar();
+	}
+}
+
+function bar() {
+	loop: while (unknown) {
+		if (unknown) {
+			break loop;
+		}
+		foo();
+	}
+}
+
+unused: {
+}

test/form/samples/no-treeshake-include-labels/dep1.js

@@ -0,0 +1,10 @@
+import foo from './dep2.js';
+
+export default function bar() {
+	loop: while (unknown) {
+		if (unknown) {
+			break loop;
+		}
+		foo();
+	}
+}

test/form/samples/no-treeshake-include-labels/dep2.js

@@ -0,0 +1,10 @@
+import bar from './dep1.js';
+
+export default function foo() {
+	loop: while (unknown) {
+		if (unknown) {
+			break loop;
+		}
+		bar();
+	}
+}

test/form/samples/no-treeshake-include-labels/main.js

@@ -0,0 +1,4 @@
+import './dep1.js';
+
+unused: {
+}