Fix mangling function names [fix]

Fixes #494.

lib/instrument/blocks.js

@@ -17,6 +17,7 @@ module.exports = {
 	createArgumentsBinding,
 	createNewTargetBinding,
 	getOrCreateExternalVar,
+	createInternalVar,
 	activateBlock,
 	activateBinding,
 	createBlockTempVar
@@ -207,6 +208,25 @@ function getOrCreateExternalVar(externalVars, block, varName, bindingOrExternalV
 	return externalVar;
 }
 
+/**
+ * Create an internal var in a function.
+ * @param {Object} fn - Function object
+ * @param {string} varName - Var name
+ * @param {Object} binding - Binding object
+ * @param {Object} block - Block object
+ * @param {Array} [trail] - Trail for where binding appears (optional)
+ * @returns {undefined}
+ */
+function createInternalVar(fn, varName, binding, block, trail) {
+	const {internalVars} = fn;
+	let internalVar = internalVars.get(binding);
+	if (!internalVar) {
+		internalVar = {varName, blockId: block.id, trails: []};
+		internalVars.set(binding, internalVar);
+	}
+	if (trail) internalVar.trails.push(trail);
+}
+
 /**
  * Activate block.
  * Called when block contains a binding which is referenced within a function.

lib/instrument/visitors/assignee.js

@@ -25,9 +25,8 @@ const assert = require('simple-invariant');
 const Expression = require('./expression.js'),
 	{IdentifierAssignOnly, IdentifierReadAndAssign} = require('./identifier.js'),
 	MemberExpression = require('./memberExpression.js'),
-	{createBinding} = require('../blocks.js'),
-	{visitKey, visitKeyContainer, visitKeyContainerWithEmptyMembers} = require('../visit.js'),
-	{createArrayOrPush} = require('../../shared/functions.js');
+	{createBinding, createInternalVar} = require('../blocks.js'),
+	{visitKey, visitKeyContainer, visitKeyContainerWithEmptyMembers} = require('../visit.js');
 
 // Exports
 
@@ -157,11 +156,11 @@ function visitConstOrLetIdentifier(node, isConst, state) {
 	const varName = node.name,
 		block = state.currentBlock;
 	assertNoCommonJsVarsClash(block, varName, state);
-	createBinding(block, varName, {isConst}, state);
+	const binding = createBinding(block, varName, {isConst}, state);
 
 	// Record as internal var
 	const fn = state.currentFunction;
-	if (fn) createArrayOrPush(fn.internalVars, varName, [...state.trail]);
+	if (fn) createInternalVar(fn, varName, binding, block, [...state.trail]);
 }
 
 /**
@@ -183,24 +182,10 @@ function IdentifierVar(node, state) {
 		return;
 	}
 
-	// Leave until later to add to `internalVars` in case binding is redeclared later
-	// as a function declaration. Vars defined as functions must not be
-	// added to `internalVars` to avoid their names being mangled.
+	// Record as internal var.
+	// If a function declaration reuses the same binding later, internal var will be deleted again.
 	const fn = state.currentFunction;
-	if (fn) state.secondPass(addVarIdentifierToInternalVars, node, binding, varName, fn, [...state.trail]);
-}
-
-/**
- * Add identifier to parent function's internal vars.
- * @param {Object} node - Identifier AST node (not needed but passed for debug reasons)
- * @param {Object} binding - Binding object
- * @param {string} varName - Variable name
- * @param {Object} fn - Function object
- * @param {Array<string|number>} trail - Trail
- * @returns {undefined}
- */
-function addVarIdentifierToInternalVars(node, binding, varName, fn, trail) {
-	if (!binding.isFunction) createArrayOrPush(fn.internalVars, varName, trail);
+	if (fn) createInternalVar(fn, varName, binding, block, [...state.trail]);
 }
 
 /**

lib/instrument/visitors/eval.js

@@ -245,7 +245,7 @@ function activateSuperIfIsUsable(fn, state) {
 				const maybeConstructorNode = getProp(fn.node, trail, 3);
 				if (maybeConstructorNode.type === 'ClassMethod' && maybeConstructorNode.kind === 'constructor') {
 					// In class constructor - need internal var for `this` as `eval()` code might include `super()`
-					createInternalVarForThis(fn);
+					createInternalVarForThis(fn, state);
 				}
 			}
 		}

lib/instrument/visitors/function.js

@@ -82,18 +82,26 @@ function FunctionDeclaration(node, state, parent, key) {
 	// `() => { console.log(typeof x); if (true) function x() {} }` -> Logs 'undefined'
 	const {currentBlock: block, currentHoistBlock: hoistBlock} = state;
 	let binding = block.bindings[fnName];
-	if (binding) {
-		// Existing binding must be a `var` declaration or another function declaration
-		binding.isFunction = true;
-	} else {
+	if (!binding) {
 		const isTopLevel = block === hoistBlock;
 		binding = createBinding(block, fnName, {isVar: isTopLevel, isFunction: true}, state);
 
 		// If is a sloppy-mode function declaration which may need to be hoisted, record that.
 		// Determine whether to hoist after 1st pass is complete, once all other bindings created.
 		if (!isTopLevel && !state.isStrict && !node.async && !node.generator) {
-			state.sloppyFunctionDeclarations.push({varName: fnName, binding, block, hoistBlock});
+			state.sloppyFunctionDeclarations.push({
+				varName: fnName,
+				binding,
+				block,
+				hoistBlock,
+				parentFn: state.currentFunction
+			});
 		}
+	} else if (!binding.isFunction) {
+		// Existing binding is a `var` declaration.
+		// Remove from parent function's `internalVars` so `var` declarations' identifiers don't get renamed.
+		binding.isFunction = true;
+		state.currentFunction?.internalVars.delete(binding);
 	}
 
 	// Insert tracker comment
@@ -423,7 +431,7 @@ function createFunction(id, node, isStrict, state) {
 		parent: parentFunction,
 		children: [],
 		trail: parentFunction ? [...state.trail] : undefined,
-		internalVars: Object.create(null), // Keyed by var name
+		internalVars: new Map(), // Keyed by binding
 		externalVars: new Map(), // Keyed by block
 		globalVarNames: new Set(),
 		amendments: [],
@@ -535,6 +543,7 @@ function hoistSloppyFunctionDeclarations(state) {
  * @param {Object} declaration.binding - Binding object for binding in block where originally declared
  * @param {Object} declaration.block - Block object for block where originally declared
  * @param {Object} declaration.hoistBlock - Block object for block where could be hoisted to
+ * @param {Object} [declaration.parentFn] - Function object for parent function
  * @returns {undefined}
  */
 function hoistSloppyFunctionDeclaration(declaration) {
@@ -561,10 +570,13 @@ function hoistSloppyFunctionDeclaration(declaration) {
 	}
 
 	// Can be hoisted
-	if (hoistBlockBinding) {
-		hoistBlockBinding.isFunction = true;
-	} else {
+	if (!hoistBlockBinding) {
 		hoistBlock.bindings[varName] = {...declaration.binding, isVar: true};
+	} else if (!hoistBlockBinding.isFunction) {
+		// Existing binding is a `var` declaration.
+		// Remove from parent function's `internalVars` so `var` declarations' identifiers don't get renamed.
+		hoistBlockBinding.isFunction = true;
+		declaration.parentFn?.internalVars.delete(hoistBlockBinding);
 	}
 }
 
@@ -716,6 +728,14 @@ function insertTrackerComment(fnId, fnType, commentHolderNode, commentType, stat
  * @returns {undefined}
  */
 function createFunctionInfoFunction(fn, scopes, astJson, fnInfoVarNode, state) {
+	// Compile internal vars
+	const internalVars = Object.create(null);
+	for (const {varName, blockId, trails} of fn.internalVars.values()) {
+		const internalVar = internalVars[varName] || (internalVars[varName] = {blockIds: [], trails: []});
+		internalVar.blockIds.push(blockId);
+		internalVar.trails.push(...trails);
+	}
+
 	// Create JSON function info string
 	const {children} = fn;
 	let argNames;
@@ -737,7 +757,7 @@ function createFunctionInfoFunction(fn, scopes, astJson, fnInfoVarNode, state) {
 		containsEval: fn.containsEval || undefined,
 		containsImport: fn.containsImport || undefined,
 		argNames,
-		internalVars: fn.internalVars,
+		internalVars,
 		globalVarNames: fn.globalVarNames.size !== 0 ? [...fn.globalVarNames] : undefined,
 		amendments: fn.amendments.length !== 0
 			? fn.amendments.map(({type, blockId, trail}) => [type, blockId, ...trail]).reverse()

lib/instrument/visitors/identifier.js

@@ -16,9 +16,8 @@ module.exports = {
 
 // Imports
 const visitEval = require('./eval.js'),
-	{getOrCreateExternalVar, activateBlock, activateBinding} = require('../blocks.js'),
+	{getOrCreateExternalVar, activateBlock, activateBinding, createInternalVar} = require('../blocks.js'),
 	{checkInternalVarNameClash} = require('../internalVars.js'),
-	{createArrayOrPush} = require('../../shared/functions.js'),
 	{
 		CONST_VIOLATION_CONST, CONST_VIOLATION_FUNCTION_THROWING, CONST_VIOLATION_FUNCTION_SILENT
 	} = require('../../shared/constants.js');
@@ -83,7 +82,7 @@ function ThisExpression(node, state) {
 	if (block.id < fn.id) {
 		recordExternalVar(binding, block, 'this', fn, [...state.trail], true, false, state);
 	} else if (fn.hasSuperClass) {
-		createArrayOrPush(fn.internalVars, 'this', [...state.trail]);
+		createInternalVar(fn, 'this', binding, block, [...state.trail]);
 	}
 }
 
@@ -183,7 +182,7 @@ function resolveIdentifier(node, block, varName, fn, trail, isReadFrom, isAssign
 
 	// Record if internal var
 	if (block.id >= fn.id) {
-		if (!binding.isFunction && !binding.argNames) createArrayOrPush(fn.internalVars, varName, trail);
+		if (!binding.isFunction && !binding.argNames) fn.internalVars.get(binding).trails.push(trail);
 		return;
 	}
 

lib/instrument/visitors/super.js

@@ -19,7 +19,8 @@ module.exports = {
 
 // Imports
 const {
-		createBindingWithoutNameCheck, getOrCreateExternalVar, activateBlock, createBlockTempVar
+		createBindingWithoutNameCheck, getOrCreateExternalVar, activateBlock, createBlockTempVar,
+		createInternalVar
 	} = require('../blocks.js'),
 	{getProp} = require('../../shared/functions.js'),
 	{SUPER_CALL, SUPER_EXPRESSION} = require('../../shared/constants.js');
@@ -63,7 +64,7 @@ function Super(node, state, parent, key) {
 			} else if (fn.firstSuperStatementIndex === undefined) {
 				// First `super()` statement - record statement index
 				fn.firstSuperStatementIndex = statementIndex;
-				createInternalVarForThis(fn);
+				createInternalVarForThis(fn, state);
 			}
 		} else if (
 			// Need to check parent node type as `ThrowStatement` also has an `argument` property
@@ -80,7 +81,7 @@ function Super(node, state, parent, key) {
 		} else {
 			// `super()` appears in constructor, but not as top level statement
 			fn.firstSuperStatementIndex = -1;
-			createInternalVarForThis(fn);
+			createInternalVarForThis(fn, state);
 		}
 	} else if (isInArrowFunction) {
 		// `super` expression in arrow function
@@ -89,10 +90,10 @@ function Super(node, state, parent, key) {
 		// If inside class constructor or prototype class property of class with super class,
 		// class requires `this$0`. `this$0` not needed in methods.
 		const fullFn = getParentFullFunction(fn);
-		if (fullFn && fullFn.hasSuperClass) createInternalVarForThis(fullFn);
+		if (fullFn && fullFn.hasSuperClass) createInternalVarForThis(fullFn, state);
 	} else if (fn.hasSuperClass) {
 		// `super` expression in class constructor or class property of class with super class
-		createInternalVarForThis(fn);
+		createInternalVarForThis(fn, state);
 	}
 
 	// Create external var for `super`
@@ -143,11 +144,12 @@ function activateSuperBinding(superBlock, state) {
 /**
  * Create internal var for `this` on function.
  * @param {Object} fn - Function object
+ * @param {Object} state - State object
  * @returns {undefined}
  */
-function createInternalVarForThis(fn) {
-	const {internalVars} = fn;
-	if (!internalVars.this) internalVars.this = [];
+function createInternalVarForThis(fn, state) {
+	const block = state.currentThisBlock;
+	createInternalVar(fn, 'this', block.bindings.this, block, null);
 }
 
 /**

lib/serialize/parseFunction.js

@@ -89,13 +89,17 @@ module.exports = function parseFunction(
 
 	// Add child functions into AST, get external/internal var nodes, and get amendments to be made
 	// (const violations / incidences of `super` to be transpiled)
-	const internalVars = Object.create(null),
+	const internalVarsWithBlockIds = Object.create(null),
 		globalVarNames = new Set(),
 		functionNames = new Set(),
 		amendments = [];
 	resolveFunctionInfo(
 		fnInfo, getChildFnInfos, false, fnId, scopeDefs,
-		externalVars, internalVars, globalVarNames, functionNames, amendments
+		externalVars, internalVarsWithBlockIds, globalVarNames, functionNames, amendments
+	);
+
+	const internalVars = Object.fromEntries(
+		Object.entries(internalVarsWithBlockIds).map(([varName, {nodes}]) => [varName, nodes])
 	);
 
 	let node = fnInfo.ast;
@@ -836,8 +840,11 @@ function resolveFunctionInfo(
 	// Initializing properties of `internalVars` needs to happen before processing child functions,
 	// as external vars in child functions can become internal vars in this function.
 	const internalVarTrails = [];
-	for (const [varName, trails] of Object.entries(fnInfo.internalVars)) {
-		if (!internalVars[varName]) internalVars[varName] = [];
+	for (const [varName, {blockIds, trails}] of Object.entries(fnInfo.internalVars)) {
+		let internalVar = internalVars[varName];
+		if (!internalVar) internalVar = internalVars[varName] = {blockIds: new Set(), nodes: []};
+		blockIds.forEach(blockId => internalVar.blockIds.add(blockId));
+
 		internalVarTrails.push({varName, trails});
 	}
 
@@ -878,15 +885,17 @@ function resolveFunctionInfo(
 		} else {
 			// Var which is external to current function, but internal to function being serialized
 			for (const [varName, {trails}] of Object.entries(scope.vars)) {
-				const internalVarNodes = internalVars[varName];
-				if (internalVarNodes) internalVarNodes.push(...trailsToNodes(fnNode, trails, varName));
+				const internalVar = internalVars[varName];
+				if (internalVar && internalVar.blockIds.has(blockId)) {
+					internalVar.nodes.push(...trailsToNodes(fnNode, trails, varName));
+				}
 			}
 		}
 	}
 
 	// Get internal var nodes
 	for (const {varName, trails} of internalVarTrails) {
-		internalVars[varName].push(...trailsToNodes(fnNode, trails, varName));
+		internalVars[varName].nodes.push(...trailsToNodes(fnNode, trails, varName));
 	}
 
 	// Get global var names
@@ -905,7 +914,7 @@ function resolveFunctionInfo(
 				// Ignore CONST_VIOLATION_FUNCTION_THROWING and CONST_VIOLATION_FUNCTION_SILENT violation types
 				// because they refer to function names, which are not treated as internal vars.
 				const node = getProp(fnNode, trail);
-				internalVars[node.name].push(node);
+				internalVars[node.name].nodes.push(node);
 			}
 		}
 	}