Reimplement variable deconflicting logic (#2689)

* Refactor scope structure * Initial draft for new deconflicting solution with variable access tracking * Slightly improve some names * Deconflict CJS interop function * Cache findVariable via accessedOutsideVariables * Get rid of unnecessary parameter and improve name * Use objects instead of Sets to track used names * Only track global outside variables on module scopes, re-unify external variable handling with other import handling * Move warning about missing exports into Graph * Test variables are no longer needlessly deconflicted due to globals in other chunks * Use tree-shaking information when deconflicting * Deconflict build-in names, resolves #2680 * Make sure it resolves #2683 * Clean up chunk deconflicting code * Change priorities so that chunk variable names take precedence over imported variable names * Fix acorn import fix
rollup · Feb 17, 2019 · 19a7727 · 19a7727
1 parent fe84e00
commit 19a7727
Show file tree

Hide file tree

Showing 220 changed files with 1,598 additions and 816 deletions.
diff --git a/rollup.config.js b/rollup.config.js
@@ -40,7 +40,7 @@ const onwarn = warning => {
 	throw new Error(warning.message);
 };
 
-const expectedAcornImport = "import acorn__default, { tokTypes, Parser } from 'acorn';";
+const expectedAcornImport = /import acorn__default, { tokTypes, Parser( as [\w$]+)? } from 'acorn';/;
 const newAcornImport =
 	"import * as acorn__default from 'acorn';\nimport { tokTypes, Parser } from 'acorn';";
 

diff --git a/src/Chunk.ts b/src/Chunk.ts
@@ -22,6 +22,7 @@ import {
 import { Addons } from './utils/addons';
 import { toBase64 } from './utils/base64';
 import collapseSourcemaps from './utils/collapseSourcemaps';
+import { deconflictChunk } from './utils/deconflictChunk';
 import { error } from './utils/error';
 import { sortByExecutionOrder } from './utils/executionOrder';
 import getIndentString from './utils/getIndentString';
@@ -30,7 +31,7 @@ import { basename, dirname, isAbsolute, normalize, relative, resolve } from './u
 import renderChunk from './utils/renderChunk';
 import { RenderOptions } from './utils/renderHelpers';
 import { makeUnique, renderNamePattern } from './utils/renderNamePattern';
-import { sanitizeFileName } from './utils/sanitize-file-name';
+import { sanitizeFileName } from './utils/sanitizeFileName';
 import { timeEnd, timeStart } from './utils/timers';
 import { MISSING_EXPORT_SHIM_VARIABLE } from './utils/variableNames';
 
@@ -410,126 +411,41 @@ export default class Chunk {
 	}
 
 	private setIdentifierRenderResolutions(options: OutputOptions) {
-		this.needsExportsShim = false;
-		const used = Object.create(null);
-		const esm = options.format === 'es' || options.format === 'system';
-
-		// ensure no conflicts with globals
-		Object.keys(this.graph.scope.variables).forEach(name => (used[name] = 1));
-
-		function getSafeName(name: string): string {
-			let safeName = name;
-			while (used[safeName]) {
-				safeName = `${name}$${toBase64(used[name]++, true)}`;
-			}
-			used[safeName] = 1;
-			return safeName;
-		}
-
-		// reserved internal binding names for system format wiring
-		if (options.format === 'system') {
-			used._setter = used._starExcludes = used._$p = 1;
-		}
-
-		const toDeshadow: Set<string> = new Set();
-
-		if (!esm) {
-			this.dependencies.forEach(module => {
-				const safeName = getSafeName(module.variableName);
-				toDeshadow.add(safeName);
-				module.variableName = safeName;
-			});
-		}
-
 		for (const exportName of Object.keys(this.exportNames)) {
 			const exportVariable = this.exportNames[exportName];
-			if (exportVariable instanceof ExportShimVariable) this.needsExportsShim = true;
-			if (exportVariable && exportVariable.exportName !== exportName) {
-				exportVariable.exportName = exportName;
-			}
-		}
-
-		for (const variable of Array.from(this.imports)) {
-			let safeName;
-
-			if (variable.module instanceof ExternalModule) {
-				if (variable.name === '*') {
-					safeName = variable.module.variableName;
-				} else if (variable.name === 'default') {
-					if (
-						options.interop !== false &&
-						(variable.module.exportsNamespace || (!esm && variable.module.exportsNames))
-					) {
-						safeName = `${variable.module.variableName}__default`;
-					} else {
-						safeName = variable.module.variableName;
-					}
-				} else {
-					safeName = esm ? variable.name : `${variable.module.variableName}.${variable.name}`;
+			if (exportVariable) {
+				if (exportVariable instanceof ExportShimVariable) {
+					this.needsExportsShim = true;
 				}
-				if (esm) {
-					safeName = getSafeName(safeName);
-					toDeshadow.add(safeName);
-				}
-			} else if (esm) {
-				safeName = getSafeName(variable.name);
-				toDeshadow.add(safeName);
-			} else {
-				const chunk = variable.module.chunk;
+				exportVariable.exportName = exportName;
 				if (
-					chunk.exportMode === 'default' ||
-					(this.graph.preserveModules && variable.isNamespace)
+					options.format !== 'es' &&
+					options.format !== 'system' &&
+					exportVariable.isReassigned &&
+					!exportVariable.isId &&
+					(!isExportDefaultVariable(exportVariable) || !exportVariable.hasId)
 				) {
-					safeName = chunk.variableName;
+					exportVariable.setRenderNames('exports', exportName);
 				} else {
-					safeName = `${chunk.variableName}.${variable.module.chunk.getVariableExportName(
-						variable
-					)}`;
+					exportVariable.setRenderNames(null, null);
 				}
 			}
-			if (safeName) variable.setSafeName(safeName);
 		}
 
-		this.orderedModules.forEach(module => {
-			this.deconflictExportsOfModule(module, getSafeName, esm);
-		});
-
-		this.graph.scope.deshadow(toDeshadow, this.orderedModules.map(module => module.scope));
-	}
-
-	private deconflictExportsOfModule(
-		module: Module,
-		getSafeName: (name: string) => string,
-		esm: boolean
-	) {
-		Object.keys(module.scope.variables).forEach(variableName => {
-			const variable = module.scope.variables[variableName];
-			if (isExportDefaultVariable(variable) && variable.referencesOriginal()) {
-				variable.setSafeName(null);
-				return;
-			}
-			if (!(isExportDefaultVariable(variable) && variable.hasId)) {
-				let safeName;
-				if (esm || !variable.isReassigned || variable.isId) {
-					safeName = getSafeName(variable.name);
-				} else {
-					const safeExportName = variable.exportName;
-					if (safeExportName) {
-						safeName = `exports.${safeExportName}`;
-					} else {
-						safeName = getSafeName(variable.name);
-					}
-				}
-				variable.setSafeName(safeName);
-			}
-		});
-		module.exportShimVariable.setSafeName(null);
-
-		// deconflict reified namespaces
-		const namespace = module.getOrCreateNamespace();
-		if (namespace.included) {
-			namespace.setSafeName(getSafeName(namespace.name));
+		const usedNames = Object.create(null);
+		if (this.needsExportsShim) {
+			usedNames[MISSING_EXPORT_SHIM_VARIABLE] = true;
 		}
+
+		deconflictChunk(
+			this.orderedModules,
+			this.dependencies,
+			this.imports,
+			usedNames,
+			options.format,
+			options.interop !== false,
+			this.graph.preserveModules
+		);
 	}
 
 	private getChunkDependencyDeclarations(
@@ -573,13 +489,11 @@ export default class Chunk {
 			if (importsFromDependency.length) {
 				imports = [];
 				for (const variable of importsFromDependency) {
-					const local = variable.safeName || variable.name;
-					let imported;
-					if (variable.module instanceof ExternalModule) {
-						imported = variable.name;
-					} else {
-						imported = variable.module.chunk.getVariableExportName(variable);
-					}
+					const local = variable.getName();
+					const imported =
+						variable.module instanceof ExternalModule
+							? variable.name
+							: variable.module.chunk.getVariableExportName(variable);
 					imports.push({ local, imported });
 				}
 			}

diff --git a/src/Graph.ts b/src/Graph.ts
@@ -31,7 +31,6 @@ import { createPluginDriver, PluginDriver } from './utils/pluginDriver';
 import relativeId, { getAliasName } from './utils/relativeId';
 import { timeEnd, timeStart } from './utils/timers';
 import transform from './utils/transform';
-import { MISSING_EXPORT_SHIM_VARIABLE } from './utils/variableNames';
 
 function makeOnwarn() {
 	const warned = Object.create(null);
@@ -157,12 +156,7 @@ export default class Graph {
 		}
 
 		this.shimMissingExports = options.shimMissingExports;
-
 		this.scope = new GlobalScope();
-		for (const name of ['module', 'exports', '_interopDefault', MISSING_EXPORT_SHIM_VARIABLE]) {
-			this.scope.findVariable(name); // creates global variable as side-effect
-		}
-
 		this.context = String(options.context);
 
 		const optionsModuleContext = options.moduleContext;
@@ -255,6 +249,31 @@ export default class Graph {
 		for (const module of this.modules) {
 			module.bindReferences();
 		}
+		this.warnForMissingExports();
+	}
+
+	private warnForMissingExports() {
+		for (const module of this.modules) {
+			for (const importName of Object.keys(module.importDescriptions)) {
+				const importDescription = module.importDescriptions[importName];
+				if (
+					importDescription.name !== '*' &&
+					!importDescription.module.getVariableForExportName(importDescription.name)
+				) {
+					module.warn(
+						{
+							code: 'NON_EXISTENT_EXPORT',
+							name: importDescription.name,
+							source: importDescription.module.id,
+							message: `Non-existent export '${
+								importDescription.name
+							}' is imported from ${relativeId(importDescription.module.id)}`
+						},
+						importDescription.start
+					);
+				}
+			}
+		}
 	}
 
 	includeMarked(modules: Module[]) {

diff --git a/src/Module.ts b/src/Module.ts
@@ -751,7 +751,7 @@ export default class Module {
 		}
 	}
 
-	private warn(warning: RollupWarning, pos: number) {
+	warn(warning: RollupWarning, pos: number) {
 		if (pos !== undefined) {
 			warning.pos = pos;
 

diff --git a/src/ast/nodes/BlockStatement.ts b/src/ast/nodes/BlockStatement.ts
@@ -2,6 +2,7 @@ import MagicString from 'magic-string';
 import { RenderOptions, renderStatementList } from '../../utils/renderHelpers';
 import { ExecutionPathOptions } from '../ExecutionPathOptions';
 import BlockScope from '../scopes/BlockScope';
+import ChildScope from '../scopes/ChildScope';
 import Scope from '../scopes/Scope';
 import { UNKNOWN_EXPRESSION } from '../values';
 import * as NodeType from './NodeType';
@@ -24,7 +25,7 @@ export default class BlockStatement extends StatementBase {
 
 	createScope(parentScope: Scope) {
 		this.scope = (<Node>this.parent).preventChildBlockScope
-			? parentScope
+			? <ChildScope>parentScope
 			: new BlockScope(parentScope);
 	}
 

diff --git a/src/ast/nodes/ClassDeclaration.ts b/src/ast/nodes/ClassDeclaration.ts
@@ -1,5 +1,6 @@
 import MagicString from 'magic-string';
 import { RenderOptions } from '../../utils/renderHelpers';
+import ChildScope from '../scopes/ChildScope';
 import Identifier from './Identifier';
 import * as NodeType from './NodeType';
 import ClassNode from './shared/ClassNode';
@@ -22,10 +23,10 @@ export default class ClassDeclaration extends ClassNode {
 
 	parseNode(esTreeNode: GenericEsTreeNode) {
 		if (esTreeNode.id !== null) {
-			this.id = <Identifier>new this.context.nodeConstructors.Identifier(
-				esTreeNode.id,
-				this,
-				this.scope.parent
+			this.id = <Identifier>(
+				new this.context.nodeConstructors.Identifier(esTreeNode.id, this, <ChildScope>(
+					this.scope.parent
+				))
 			);
 		}
 		super.parseNode(esTreeNode);

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 ModuleScope from '../scopes/ModuleScope';
 import ExportDefaultVariable from '../variables/ExportDefaultVariable';
 import ClassDeclaration, { isClassDeclaration } from './ClassDeclaration';
 import FunctionDeclaration, { isFunctionDeclaration } from './FunctionDeclaration';
@@ -39,6 +40,7 @@ export function isExportDefaultDeclaration(node: Node): node is ExportDefaultDec
 export default class ExportDefaultDeclaration extends NodeBase {
 	type: NodeType.tExportDefaultDeclaration;
 	declaration: FunctionDeclaration | ClassDeclaration | ExpressionNode;
+	scope: ModuleScope;
 
 	needsBoundaries: true;
 	variable: ExportDefaultVariable;

diff --git a/src/ast/nodes/FunctionDeclaration.ts b/src/ast/nodes/FunctionDeclaration.ts
@@ -1,3 +1,4 @@
+import ChildScope from '../scopes/ChildScope';
 import Identifier from './Identifier';
 import * as NodeType from './NodeType';
 import FunctionNode from './shared/FunctionNode';
@@ -19,10 +20,10 @@ export default class FunctionDeclaration extends FunctionNode {
 
 	parseNode(esTreeNode: GenericEsTreeNode) {
 		if (esTreeNode.id !== null) {
-			this.id = <Identifier>new this.context.nodeConstructors.Identifier(
-				esTreeNode.id,
-				this,
-				this.scope.parent
+			this.id = <Identifier>(
+				new this.context.nodeConstructors.Identifier(esTreeNode.id, this, <ChildScope>(
+					this.scope.parent
+				))
 			);
 		}
 		super.parseNode(esTreeNode);

diff --git a/src/ast/nodes/MemberExpression.ts b/src/ast/nodes/MemberExpression.ts
@@ -60,6 +60,14 @@ function getPathIfNotComputed(memberExpression: MemberExpression): PathWithPosit
 	return null;
 }
 
+function getStringFromPath(path: PathWithPositions): string {
+	let pathString = path[0].key;
+	for (let index = 1; index < path.length; index++) {
+		pathString += '.' + path[index].key;
+	}
+	return pathString;
+}
+
 export function isMemberExpression(node: Node): node is MemberExpression {
 	return node.type === NodeType.MemberExpression;
 }
@@ -92,6 +100,7 @@ export default class MemberExpression extends NodeBase implements DeoptimizableE
 					(<ExternalVariable>resolvedVariable).module.suggestName(path[0].key);
 				}
 				this.variable = resolvedVariable;
+				this.scope.addNamespaceMemberAccess(getStringFromPath(path), resolvedVariable);
 			}
 		} else {
 			super.bind();

diff --git a/src/ast/nodes/ObjectExpression.ts b/src/ast/nodes/ObjectExpression.ts
@@ -1,6 +1,7 @@
 import MagicString from 'magic-string';
 import { BLANK } from '../../utils/blank';
 import { NodeRenderOptions, RenderOptions } from '../../utils/renderHelpers';
+import { NameCollection } from '../../utils/reservedNames';
 import CallOptions from '../CallOptions';
 import { DeoptimizableEntity } from '../DeoptimizableEntity';
 import { ExecutionPathOptions } from '../ExecutionPathOptions';
@@ -45,7 +46,7 @@ export default class ObjectExpression extends NodeBase {
 	private propertyMap: PropertyMap | null;
 	private unmatchablePropertiesRead: (Property | SpreadElement)[] | null;
 	private unmatchablePropertiesWrite: Property[] | null;
-	private deoptimizedPaths: { [key: string]: true };
+	private deoptimizedPaths: NameCollection;
 	private hasUnknownDeoptimizedProperty: boolean;
 	private expressionsToBeDeoptimized: { [key: string]: DeoptimizableEntity[] };
 

diff --git a/src/ast/nodes/ThisExpression.ts b/src/ast/nodes/ThisExpression.ts
@@ -1,6 +1,7 @@
 import MagicString from 'magic-string';
 import { RenderOptions } from '../../utils/renderHelpers';
 import { ExecutionPathOptions } from '../ExecutionPathOptions';
+import ModuleScope from '../scopes/ModuleScope';
 import { ObjectPath } from '../values';
 import ThisVariable from '../variables/ThisVariable';
 import * as NodeType from './NodeType';
@@ -28,7 +29,8 @@ export default class ThisExpression extends NodeBase {
 	initialise() {
 		this.included = false;
 		this.variable = null;
-		this.alias = this.scope.findLexicalBoundary().isModuleScope ? this.context.moduleContext : null;
+		this.alias =
+			this.scope.findLexicalBoundary() instanceof ModuleScope ? this.context.moduleContext : null;
 		if (this.alias === 'undefined') {
 			this.context.warn(
 				{