diff --git a/lib/instrument/blocks.js b/lib/instrument/blocks.js index a97ca1aa..367f3d20 100644 --- a/lib/instrument/blocks.js +++ b/lib/instrument/blocks.js @@ -99,8 +99,6 @@ function createBlockId(state) { * @param {boolean} [props.isSilentConst=false] - `true` if is function expression name, * assignment to which fails silently in sloppy mode * @param {boolean} [props.isVar=false] - `true` if defined with `var` statement or function declaration - * @param {boolean} [props.isFunction=false] - `true` if is function or class name - * (relevant as these canot be renamed) * @param {boolean} [props.isFrozenName=false] - `true` if is var which is not renamed when used * inside function in which it's bound * @param {Array} [props.argNames] - Linked argument names (only relevant for `arguments`) @@ -125,8 +123,6 @@ function createBinding(block, varName, props, state) { * @param {boolean} [props.isSilentConst=false] - `true` if is function expression name, * assignment to which fails silently in sloppy mode * @param {boolean} [props.isVar=false] - `true` if defined with `var` statement or function declaration - * @param {boolean} [props.isFunction=false] - `true` if is function or class name - * (relevant as these canot be renamed) * @param {boolean} [props.isFrozenName=false] - `true` if is var which is not renamed when used * inside function in which it's bound * @param {Array} [props.argNames] - Linked argument names (only relevant for `arguments`) @@ -139,7 +135,6 @@ function createBindingWithoutNameCheck(block, varName, props) { isConst: !!props.isConst, isSilentConst: !!props.isSilentConst, isVar: !!props.isVar, - isFunction: !!props.isFunction, isFrozenName: !!props.isFrozenName, argNames: props.argNames, trails: [] // Trails of usages within same function as binding, where var not function or frozen diff --git a/lib/instrument/visitors/assignee.js b/lib/instrument/visitors/assignee.js index 177f2d01..f6f02c3e 100644 --- a/lib/instrument/visitors/assignee.js +++ b/lib/instrument/visitors/assignee.js @@ -197,11 +197,7 @@ function IdentifierVar(node, state) { } if (fn) fn.bindings.push(binding); - } else if (binding.isFunction) { - // NB: No need to check `binding.isFrozenName` because it's only set in 2nd pass - // so will always be `false` at this point. - // Exception would be if it was a shared binding with `arguments`, but this doesn't work yet. - // TODO: Add `|| binding.isFrozenName` to `if` condition when that problem is fixed. + } else if (binding.isFrozenName) { return; } diff --git a/lib/instrument/visitors/class.js b/lib/instrument/visitors/class.js index 7e034af2..9951728c 100644 --- a/lib/instrument/visitors/class.js +++ b/lib/instrument/visitors/class.js @@ -49,7 +49,7 @@ function ClassDeclaration(node, state, parent, key) { assertNoCommonJsVarsClash(block, className, state); // No need to check for internal var name clash here as will be checked when creating // the binding for class name accessed from inside class - createBindingWithoutNameCheck(block, className, {isFunction: true}); + createBindingWithoutNameCheck(block, className, {isFrozenName: true}); // Visit class visitClass(node, parent, key, className, state); diff --git a/lib/instrument/visitors/function.js b/lib/instrument/visitors/function.js index 36b5d3f6..371b932d 100644 --- a/lib/instrument/visitors/function.js +++ b/lib/instrument/visitors/function.js @@ -86,7 +86,7 @@ function FunctionDeclaration(node, state, parent, key) { let binding = block.bindings[fnName]; if (!binding) { const isTopLevel = block === hoistBlock; - binding = createBinding(block, fnName, {isVar: isTopLevel, isFunction: true}, state); + binding = createBinding(block, fnName, {isVar: isTopLevel, isFrozenName: 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. @@ -99,10 +99,10 @@ function FunctionDeclaration(node, state, parent, key) { parentFn: state.currentFunction }); } - } else if (!binding.isFunction) { + } else if (!binding.isFrozenName) { // Existing binding is a `var` declaration. - // Flag binding as a function so `var` declarations' identifiers don't get renamed. - binding.isFunction = true; + // Flag binding as frozen so `var` declarations' identifiers don't get renamed. + binding.isFrozenName = true; binding.trails.length = 0; } @@ -153,7 +153,7 @@ function FunctionExpression(node, state, parent, key) { */ function createAndEnterFunctionOrClassNameBlock(fnName, isSilentConst, state) { const block = createAndEnterBlock(fnName, false, state); - createBinding(block, fnName, {isConst: true, isSilentConst, isFunction: true}, state); + createBinding(block, fnName, {isConst: true, isSilentConst, isFrozenName: true}, state); return block; } @@ -576,10 +576,10 @@ function hoistSloppyFunctionDeclaration(declaration) { // Can be hoisted if (!hoistBlockBinding) { hoistBlock.bindings[varName] = {...declaration.binding, isVar: true}; - } else if (!hoistBlockBinding.isFunction) { + } else if (!hoistBlockBinding.isFrozenName) { // Existing binding is a `var` declaration. - // Flag binding as a function so `var` declarations' identifiers don't get renamed. - hoistBlockBinding.isFunction = true; + // Flag binding as frozen so `var` declarations' identifiers don't get renamed. + hoistBlockBinding.isFrozenName = true; hoistBlockBinding.trails.length = 0; } } @@ -726,12 +726,7 @@ function createFunctionInfoFunction(fn, state) { // Compile internal vars and reserved var names const internalVars = Object.create(null), reservedVarNames = new Set(); - for (const {name: varName, trails, isFunction, isFrozenName} of fn.bindings) { - // Function name bindings are not usually added to `fn.bindings`, - // but can be included if binding is initially a `var` statement e.g. `var x; function x() {}`. - // So skip them here. `parseFunction` adds them to list of reserved var names. - if (isFunction) continue; - + for (const {name: varName, trails, isFrozenName} of fn.bindings) { if (isFrozenName && varName !== 'this') { if (!internalVars[varName]) reservedVarNames.add(varName); } else { @@ -762,13 +757,12 @@ function createFunctionInfoFunction(fn, state) { blockId: block.id, blockName: block.name, vars: mapValues(vars, (varProps, varName) => { - const {binding} = varProps; - if (varName === 'arguments') argNames = binding.argNames; + if (varName === 'arguments') argNames = varProps.binding.argNames; return { isReadFrom: varProps.isReadFrom || undefined, isAssignedTo: varProps.isAssignedTo || undefined, isFrozenName: varProps.isFrozenName || undefined, - isFrozenInternalName: binding.isFrozenName || binding.isFunction || undefined, + isFrozenInternalName: varProps.binding.isFrozenName || undefined, trails: varProps.trails }; }) diff --git a/lib/instrument/visitors/identifier.js b/lib/instrument/visitors/identifier.js index a4fb9d55..43c277f2 100644 --- a/lib/instrument/visitors/identifier.js +++ b/lib/instrument/visitors/identifier.js @@ -167,7 +167,7 @@ function resolveIdentifier(node, block, varName, fn, trail, isReadFrom, isAssign // Record if internal var if (block.id >= fn.id) { - if (!binding.isFrozenName && !binding.isFunction) binding.trails.push(trail); + if (!binding.isFrozenName) binding.trails.push(trail); return; } @@ -179,15 +179,14 @@ function resolveIdentifier(node, block, varName, fn, trail, isReadFrom, isAssign // it will need to be added to `internalVars`. // This is not the case if var is being read from, as an external var is created below, // which will be converted to an internal var. - // It's also not needed if binding is a function name or frozen var, as they're not renamed - // and so aren't treated as internal vars. + // It's also not needed if binding is frozen, as they're not treated as internal vars. // Not possible to know at this point whether binding is frozen or not (an `eval()` might be later // in the function), so store the binding, and type will be changed to `CONST_VIOLATION_NEEDS_NO_VAR` // later on if the binding is frozen. fn.amendments.push({ type: binding.isSilentConst && !isStrict ? CONST_VIOLATION_SILENT - : isReadFrom || binding.isFrozenName || binding.isFunction + : isReadFrom || binding.isFrozenName ? CONST_VIOLATION_NEEDS_NO_VAR : CONST_VIOLATION_NEEDS_VAR, blockId: block.id,