From 3648b82ac8db4d73b78eb65a4cd4e3cc9ff6a379 Mon Sep 17 00:00:00 2001 From: overlookmotel Date: Thu, 16 Nov 2023 22:36:50 +0000 Subject: [PATCH] `with` statements WIP 1 --- README.md | 2 - lib/init/eval.js | 8 ++- lib/init/tracker.js | 51 +++++++++++++++- lib/instrument/modify.js | 1 + lib/instrument/visitors/eval.js | 56 ++++++++++-------- lib/instrument/visitors/identifier.js | 81 ++++++++++++++++++++++++-- lib/instrument/visitors/statement.js | 8 +-- lib/instrument/visitors/super.js | 19 +++++- lib/instrument/visitors/with.js | 83 +++++++++++++++++++++++++++ lib/serialize/blocks.js | 82 ++++++++++++++++++-------- lib/serialize/functions.js | 6 +- lib/serialize/parseFunction.js | 4 +- lib/shared/tracker.js | 23 +++++++- 13 files changed, 353 insertions(+), 71 deletions(-) create mode 100644 lib/instrument/visitors/with.js diff --git a/README.md b/README.md index 1c1f2aa6..bc0b2bd1 100644 --- a/README.md +++ b/README.md @@ -573,8 +573,6 @@ NB Applications can *use* any of these within functions, just that instances of * Unsupported: `export default Promise.resolve();` (Promise instance serialized directly) * Unsupported: `const p = Promise.resolve(); export default function f() { return p; };` (Promise instance in outer scope of exported function) -`with (...) {...}` is also not supported where it alters the scope of a function being serialized. - ### Browser code This works in part. You can, for example, build a simple React app with Livepack. diff --git a/lib/init/eval.js b/lib/init/eval.js index 7d133eec..fb6b1fb3 100644 --- a/lib/init/eval.js +++ b/lib/init/eval.js @@ -119,7 +119,8 @@ function evalDirect(args, filename, blockIdCounter, externalPrefixNum, evalDirec currentBlock: undefined, currentThisBlock: undefined, currentSuperBlock: undefined, - currentSuperIsProto: false + currentSuperIsProto: false, + currentWithBlock: undefined }; const tempVars = []; let allowNewTarget = false; @@ -147,6 +148,11 @@ function evalDirect(args, filename, blockIdCounter, externalPrefixNum, evalDirec createBindingWithoutNameCheck( block, varName, {isConst: !!isConst, isSilentConst: !!isSilentConst, argNames} ); + + if (varName === 'with') { + state.currentWithBlock ||= block; + tempVars.push({value: tempVarValue, block, varName}); + } } } } diff --git a/lib/init/tracker.js b/lib/init/tracker.js index e2942da7..d63c5eca 100644 --- a/lib/init/tracker.js +++ b/lib/init/tracker.js @@ -11,7 +11,8 @@ module.exports = createTracker; // Imports const addEvalFunctionsToTracker = require('./eval.js'), - {tracker} = require('../shared/tracker.js'); + {tracker, getIsGettingScope} = require('../shared/tracker.js'), + {INTERNAL_VAR_NAMES_PREFIX} = require('../shared/constants.js'); // Exports @@ -25,5 +26,53 @@ const addEvalFunctionsToTracker = require('./eval.js'), function createTracker(filename, blockIdCounter, prefixNum) { const localTracker = (getFnInfo, getScopes) => tracker(getFnInfo, getScopes); addEvalFunctionsToTracker(localTracker, filename, blockIdCounter, prefixNum); + addWrapWithFunctionToTracker(localTracker, prefixNum); return localTracker; } + +function addWrapWithFunctionToTracker(localTracker, prefixNum) { + // `.wrapWith` wraps an object used as object of a `with ()` statement. + // + // Filter out any accesses to Livepack's internal vars, to prevent the `with` object + // obscuring access to them. + // + // There can be no valid accesses to Livepack's internal vars, otherwise they'd have + // been renamed to avoid clashes with any existing vars. + // Only exception is in `eval()`, where var names cannot be predicted in advance + // e.g. `with ({livepack_tracker: 1}) { eval('livepack_tracker = 2'); }`. + // This should set the property on the `with` object. + // However, this is a pre-existing problem with `eval()`, and can manifest without `with`, + // so not going to try to solve it here either. + // TODO: Try to solve this. + // + // Also always returns false from `has` trap if getting scope vars for a function. + // This renders the `with` object transparent, so tracker can get values of variables + // outside of the `with () {}` block. e.g. `let f, x = 123; with ({x: 1}) { f = () => x; }` + // Tracker in `f` needs to be able to get the value of `x` in outer scope. + // + // Proxy an empty object, and forward operations to the `with` object, + // rather than proxying the `with` object directly, to avoid breaking Proxy `has` trap's invariant + // that cannot report a property as non-existent if it's non-configurable. + // e.g. `with (Object.freeze({livepack_tracker: 123})) { ... }` + const internalVarsPrefix = `${INTERNAL_VAR_NAMES_PREFIX}${prefixNum || ''}_`; + localTracker.wrapWith = withObj => new Proxy(Object.create(null), { + // NB: These are the only traps which can be triggered + has(target, key) { + // Act as if object has no properties if currently getting scope vars for a function + if (getIsGettingScope()) return false; + // Act as if properties named like Livepack's internal vars don't exist + if (key.startsWith(internalVarsPrefix)) return false; + // Forward to `with` object + return Reflect.has(withObj, key); + }, + get(target, key) { + return Reflect.get(withObj, key, withObj); + }, + set(target, key, value) { + return Reflect.set(withObj, key, value, withObj); + }, + deleteProperty(target, key) { + return Reflect.deleteProperty(withObj, key); + } + }); +} diff --git a/lib/instrument/modify.js b/lib/instrument/modify.js index 891e65ba..19846bd6 100644 --- a/lib/instrument/modify.js +++ b/lib/instrument/modify.js @@ -74,6 +74,7 @@ function modifyAst(ast, filename, isCommonJs, isStrict, sources, evalState) { currentThisBlock: undefined, currentSuperBlock: undefined, currentHoistBlock: undefined, + currentWithBlock: undefined, fileBlock: undefined, programBlock: undefined, currentFunction: undefined, diff --git a/lib/instrument/visitors/eval.js b/lib/instrument/visitors/eval.js index 514e1df8..64f0e979 100644 --- a/lib/instrument/visitors/eval.js +++ b/lib/instrument/visitors/eval.js @@ -16,6 +16,7 @@ const { activateSuperBinding, getParentFullFunction, createInternalVarForThis, createExternalVarForThis, setSuperIsProtoOnFunctions } = require('./super.js'), + {activateWithBinding} = require('./with.js'), {getOrCreateExternalVar, activateBlock, activateBinding} = require('../blocks.js'), {createTempVarNode} = require('../internalVars.js'), {copyLocAndComments} = require('../utils.js'), @@ -138,35 +139,40 @@ function instrumentEvalCall(callNode, block, fn, isStrict, canUseSuper, superIsP const varDefsNodes = []; for (const [varName, binding] of Object.entries(block.bindings)) { - // Freeze binding to prevent var being renamed when used internally in a function. - // All vars in scope need to be frozen, even if not accessible to `eval()` - // because if they're renamed, they would become accessible to `eval()` when they weren't before. - // `this` should not be frozen internally, as it can be replaced with a variable - // in a class constructor when `super()` is transpiled. - // For `super` and `new.target`, the var name can't actually be frozen, but flagged here - // as frozen anyway, to indicate that replacement var name should be prefixed with `_`. - // Frozen `new.target` is not currently supported. - // https://github.com/overlookmotel/livepack/issues/448 - if (varName !== 'this') { - binding.isFrozenName = true; - binding.trails.length = 0; - } + if (varName === 'with') { + // Variables accessed from within `eval()` may pass through this with block + activateWithBinding(block, state); + } else { + // Freeze binding to prevent var being renamed when used internally in a function. + // All vars in scope need to be frozen, even if not accessible to `eval()` + // because if they're renamed, they would become accessible to `eval()` when they weren't before. + // `this` should not be frozen internally, as it can be replaced with a variable + // in a class constructor when `super()` is transpiled. + // For `super` and `new.target`, the var name can't actually be frozen, but flagged here + // as frozen anyway, to indicate that replacement var name should be prefixed with `_`. + // Frozen `new.target` is not currently supported. + // https://github.com/overlookmotel/livepack/issues/448 + if (varName !== 'this') { + binding.isFrozenName = true; + binding.trails.length = 0; + } - // Skip var if it's shadowed by var with same name in higher scope - if (varNamesUsed.has(varName)) continue; + // Skip var if it's shadowed by var with same name in higher scope + if (varNamesUsed.has(varName)) continue; - // If `eval()` is strict mode, skip vars which are illegal to use in strict mode (e.g. `package`) - // as `eval()` won't be able to access them - if (isStrict && varName !== 'this' && varName !== 'super' && isReservedWord(varName)) continue; + // If `eval()` is strict mode, skip vars which are illegal to use in strict mode (e.g. `package`) + // as `eval()` won't be able to access them + if (isStrict && varName !== 'this' && varName !== 'super' && isReservedWord(varName)) continue; - // Ignore `require` as it would render the function containing `eval()` un-serializable. - // Also ignore CommonJS wrapper function's `arguments` as that contains `require` too. - if ((varName === 'require' || varName === 'arguments') && block === state.fileBlock) continue; + // Ignore `require` as it would render the function containing `eval()` un-serializable. + // Also ignore CommonJS wrapper function's `arguments` as that contains `require` too. + if ((varName === 'require' || varName === 'arguments') && block === state.fileBlock) continue; - // Ignore `super` if it's not accessible - if (varName === 'super' && !canUseSuper) continue; + // Ignore `super` if it's not accessible + if (varName === 'super' && !canUseSuper) continue; - varNamesUsed.add(varName); + varNamesUsed.add(varName); + } // Create array for var `[varName, isConst, isSilentConst, argNames, tempVarValue]`. // NB: Assignment to var called `arguments` or `eval` is illegal in strict mode. @@ -179,7 +185,7 @@ function instrumentEvalCall(callNode, block, fn, isStrict, canUseSuper, superIsP while (varDefNodes.length !== 3) varDefNodes.push(null); varDefNodes.push(t.arrayExpression(binding.argNames.map(argName => t.stringLiteral(argName)))); } - if (varName === 'super') { + if (varName === 'super' || varName === 'with') { while (varDefNodes.length !== 4) varDefNodes.push(null); varDefNodes.push(binding.varNode); } diff --git a/lib/instrument/visitors/identifier.js b/lib/instrument/visitors/identifier.js index ebb514b5..3ece8e5d 100644 --- a/lib/instrument/visitors/identifier.js +++ b/lib/instrument/visitors/identifier.js @@ -16,6 +16,7 @@ module.exports = { // Imports const visitEval = require('./eval.js'), + {activateWithBinding} = require('./with.js'), {getOrCreateExternalVar, activateBlock, activateBinding} = require('../blocks.js'), {checkInternalVarNameClash} = require('../internalVars.js'), { @@ -80,7 +81,18 @@ function ThisExpression(node, state) { // Ignore if internal to function, unless in class constructor or prototype class property // of class with super class (where `this` will be substituted when `super()` is transpiled). if (block.id < fn.id) { - recordExternalVar(binding, block, 'this', fn, [...state.trail], true, false, state); + // Check if a `with` binding in between, because if so, `this` needs to be frozen, to avoid whatever + // var is used to stand in for `this` being blocked by a property of the `with` object. + // No need to activate the `with` binding, as `this` doesn't read from it. + // Actually, `this` only needs to be frozen if the `with` block ends up in the output's blocks tree. + // e.g. `with (o) return () => this` does not require `this` to be frozen. + // But this can't be determined until serialization time, as it could be *another* function + // in same block which causes the `with` block to be used. + // To avoid complicating matters for this edge case, don't worry about it, and always freeze `this`. + // `this` may be frozen unnecessarily, which produces longer output, but it won't be incorrect. + const isBehindWith = !!state.currentWithBlock && state.currentWithBlock.id > block.id; + + recordExternalVar(binding, block, 'this', fn, [...state.trail], true, false, isBehindWith, state); } else if (fn.hasSuperClass) { binding.trails.push([...state.trail]); } @@ -103,8 +115,19 @@ function NewTargetExpression(node, state) { // parser will have already rejected the code as syntax error. const block = state.currentThisBlock; if (block.id < fn.id) { + // Check if a `with` binding in between, because if so, `new.target` needs to be frozen, + // to avoid whatever var is used to stand in for `new.target` being blocked by a property + // of the `with` object. + // At present, `new.target` won't actually be frozen, but setting `isFrozenName` will at least + // cause var name to be prefixed with "_". + // No need to activate the `with` binding, as `new.target` doesn't read from it. + // Same as with `this`, `new.target` only actually needs to be frozen if the `with` block ends + // up in the output's blocks tree. But to simplify matters, just freeze it anyway. + const isBehindWith = !!state.currentWithBlock && state.currentWithBlock.id > block.id; + recordExternalVar( - block.bindings['new.target'], block, 'new.target', fn, [...state.trail], true, false, state + block.bindings['new.target'], block, 'new.target', fn, [...state.trail], + true, false, isBehindWith, state ); } } @@ -126,7 +149,7 @@ function visitIdentifier(node, varName, isReadFrom, isAssignedTo, state) { // Resolve the binding var refers to in 2nd pass state.secondPass( resolveIdentifier, - node, state.currentBlock, varName, fn, [...state.trail], + node, state.currentBlock, state.currentWithBlock, varName, fn, [...state.trail], isReadFrom, isAssignedTo, state.isStrict, state ); } else { @@ -140,6 +163,7 @@ function visitIdentifier(node, varName, isReadFrom, isAssignedTo, state) { * Resolve binding an identifier refers to and record its usage on function it's used within. * @param {Object} node - Identifier AST node (not needed but passed for debug reasons) * @param {Object} block - Block object + * @param {Object} [withBlock] - Block object for first `with ()` block above identifier * @param {string} varName - Variable name * @param {Object} fn - Function object * @param {Array} trail - Trail @@ -149,13 +173,27 @@ function visitIdentifier(node, varName, isReadFrom, isAssignedTo, state) { * @param {Object} state - State object * @returns {undefined} */ -function resolveIdentifier(node, block, varName, fn, trail, isReadFrom, isAssignedTo, isStrict, state) { +function resolveIdentifier( + node, block, withBlock, varName, fn, trail, isReadFrom, isAssignedTo, isStrict, state +) { // Find binding let binding; do { binding = block.bindings[varName]; } while (!binding && (block = block.parent)); // eslint-disable-line no-cond-assign + // If `with ()` block between identifier and its binding, the `with ()` could block access + // to the binding. Create external vars for `with` blocks in function, and flag the `binding` + // as frozen name, so var isn't renamed in output. + const isBehindWith = !!withBlock && (!block || withBlock.id > block.id); + if (isBehindWith) { + if (binding) { + binding.isFrozenName = true; + binding.trails.length = 0; + } + createExternalVarsForWiths(withBlock, block, fn, state); + } + // Record if global var if (!binding) { // Vars which are local bindings have var name checked for internal var name clashes @@ -198,7 +236,34 @@ function resolveIdentifier(node, block, varName, fn, trail, isReadFrom, isAssign isAssignedTo = false; } - recordExternalVar(binding, block, varName, fn, trail, isReadFrom, isAssignedTo, state); + recordExternalVar(binding, block, varName, fn, trail, isReadFrom, isAssignedTo, isBehindWith, state); +} + +/** + * Find all `with` blocks above identifier's binding. + * Activate those `with` blocks and create external vars for them in function. + * @param {Object} withBlock - Block object for `with` block + * @param {Object} [bindingBlock] - Block object for variable binding (which will be above `with` block) + * or `undefined` if no binding (global var) + * @param {Object} fn - Function object + * @param {Object} state - State object + * @returns {undefined} + */ +function createExternalVarsForWiths(withBlock, bindingBlock, fn, state) { + while (true) { // eslint-disable-line no-constant-condition + if (withBlock.id < fn.id) { + // `with ()` is external to function - create external var + const binding = activateWithBinding(withBlock, state); + const externalVar = getOrCreateExternalVar(fn, withBlock, 'with', binding); + externalVar.isReadFrom = true; + } + + // Find next `with` block above binding (if any) + do { + withBlock = withBlock.parent; + if (withBlock === bindingBlock) return; + } while (!withBlock.bindings.with); + } } /** @@ -210,15 +275,19 @@ function resolveIdentifier(node, block, varName, fn, trail, isReadFrom, isAssign * @param {Array} trail - Trail * @param {boolean} isReadFrom - `true` if variable is read from * @param {boolean} isAssignedTo - `true` if variable is assigned to + * @param {boolean} isBehindWith - `true` if variable may be shadowed by a `with () {}` statement * @param {Object} state - State object * @returns {undefined} */ -function recordExternalVar(binding, block, varName, fn, trail, isReadFrom, isAssignedTo, state) { +function recordExternalVar( + binding, block, varName, fn, trail, isReadFrom, isAssignedTo, isBehindWith, state +) { activateBlock(block, state); activateBinding(binding, varName); const externalVar = getOrCreateExternalVar(fn, block, varName, binding); if (isReadFrom) externalVar.isReadFrom = true; if (isAssignedTo) externalVar.isAssignedTo = true; + if (isBehindWith) externalVar.isFrozenName = true; // `new.target` isn't actually frozen so needs trail recorded if (!externalVar.isFrozenName || varName === 'new.target') externalVar.trails.push(trail); } diff --git a/lib/instrument/visitors/statement.js b/lib/instrument/visitors/statement.js index 1122f4d4..5d1a9048 100644 --- a/lib/instrument/visitors/statement.js +++ b/lib/instrument/visitors/statement.js @@ -19,6 +19,7 @@ const VariableDeclaration = require('./variableDeclaration.js'), SwitchStatement = require('./switch.js'), TryStatement = require('./try.js'), ThrowStatement = require('./unary.js'), + {WithStatement} = require('./with.js'), {visitKey, visitKeyMaybe} = require('../visit.js'); // Exports @@ -71,13 +72,6 @@ function ReturnStatement(node, state) { visitKeyMaybe(node, 'argument', Expression, state); } -function WithStatement(node, state) { - // TODO: Maintain a state property `currentWithBlock` which can be used in `resolveBinding()` - // to flag functions which access a var which would be affected by `with` - visitKey(node, 'object', Expression, state); - visitKey(node, 'body', Statement, state); -} - function LabeledStatement(node, state) { visitKey(node, 'body', Statement, state); } diff --git a/lib/instrument/visitors/super.js b/lib/instrument/visitors/super.js index 51a220af..a99f90de 100644 --- a/lib/instrument/visitors/super.js +++ b/lib/instrument/visitors/super.js @@ -84,6 +84,8 @@ function Super(node, state, parent, key) { } } else if (isInArrowFunction) { // `super` expression in arrow function + // TODO: Need to freeze `this` if behind `with` block. This is possible in object methods + // as they can be sloppy mode. createExternalVarForThis(fn, state); // If inside class constructor or prototype class property of class with super class, @@ -97,7 +99,19 @@ function Super(node, state, parent, key) { // Create external var for `super` const superBlock = state.currentSuperBlock; - recordSuper(superBlock, fn, state); + const externalVar = recordSuper(superBlock, fn, state); + + // Check if a `with` binding in between, because if so, `super` needs to be frozen, + // to avoid whatever var is used to stand in for `super` being blocked by a property + // of the `with` object. + // `super` var won't actually be frozen, but setting `isFrozenName` will at least + // cause var name to be prefixed with "_". + // No need to activate the `with` binding, as `super` doesn't read from it. + // Same as with `this`, `super` only actually needs to be frozen if the `with` block ends + // up in the output's blocks tree. But to simplify matters, just freeze it anyway. + if (state.currentWithBlock && state.currentWithBlock.id > superBlock.id) { + externalVar.isFrozenName = true; + } // Set `superIsProto` on all functions between this one and the `super` target setSuperIsProtoOnFunctions(superBlock, fn, state); @@ -112,12 +126,13 @@ function Super(node, state, parent, key) { * @param {Object} superBlock - `super` target block * @param {Object} fn - Function object * @param {Object} state - State object - * @returns {undefined} + * @returns {Object} - External var object for `super` */ function recordSuper(superBlock, fn, state) { const binding = activateSuperBinding(superBlock, state); const superExternalVar = getOrCreateExternalVar(fn, superBlock, 'super', binding); superExternalVar.isReadFrom = true; + return superExternalVar; } /** diff --git a/lib/instrument/visitors/with.js b/lib/instrument/visitors/with.js new file mode 100644 index 00000000..8da9d405 --- /dev/null +++ b/lib/instrument/visitors/with.js @@ -0,0 +1,83 @@ +/* -------------------- + * livepack module + * Code instrumentation visitor for `with` statements + * ------------------*/ + +'use strict'; + +// Export +module.exports = { + WithStatement, + activateWithBinding +}; + +// Modules +const t = require('@babel/types'); + +// Imports +const Expression = require('./expression.js'), + Statement = require('./statement.js'), + { + createAndEnterBlock, createBindingWithoutNameCheck, activateBlock, createBlockTempVar + } = require('../blocks.js'), + {createTrackerVarNode} = require('../internalVars.js'), + {visitKey} = require('../visit.js'); + +// Exports + +/** + * Visitor for `with () {}` statement. + * @param {Object} node - Statement AST node + * @param {Object} state - State object + * @returns {undefined} + */ +function WithStatement(node, state) { + // Visit object i.e. expression inside `with (...)` + visitKey(node, 'object', Expression, state); + + // Create block for `with` object. + // `isFrozenName: true` because it shouldn't have an an internal var created for it. + const parentBlock = state.currentBlock, + parentWithBlock = state.currentWithBlock; + const block = createAndEnterBlock('with', false, state); + const binding = createBindingWithoutNameCheck( + block, 'with', {isConst: true, isFrozenName: true}, state + ); + state.currentWithBlock = block; + + // Visit body + visitKey(node, 'body', Statement, state); + + // Exit block + state.currentBlock = parentBlock; + state.currentWithBlock = parentWithBlock; + + // Queue action to wrap `with` object + state.secondPass(instrumentWithObj, node, binding, state); +} + +function instrumentWithObj(node, binding, state) { + // `with (o) {}` -> `with ( livepack_tracker.wrapWith(livepack_temp2 = o) ) {}` + node.object = t.callExpression( + t.memberExpression(createTrackerVarNode(state), t.identifier('wrapWith')), + [ + binding.varNode + ? t.assignmentExpression('=', binding.varNode, node.object) + : node.object + ] + ); +} + +/** + * Activate `with ()` object binding. + * Create a temp var node to have `with` object assigned to it. + * @param {Object} block - `with` object block object + * @param {Object} state - State object + * @returns {Object} - Binding + */ +function activateWithBinding(block, state) { + activateBlock(block, state); + const binding = block.bindings.with; + if (!binding.varNode) binding.varNode = createBlockTempVar(block, state); + return binding; +} diff --git a/lib/serialize/blocks.js b/lib/serialize/blocks.js index 83464dd3..00ddba1a 100644 --- a/lib/serialize/blocks.js +++ b/lib/serialize/blocks.js @@ -84,8 +84,8 @@ module.exports = { }; paramsByName[name] = param; - // `super` and `new.target` are not actually frozen - if (isFrozenName && !['super', 'new.target'].includes(name)) { + // `super`, `new.target` and `with` are not actually frozen + if (isFrozenName && !['super', 'new.target', 'with'].includes(name)) { if (!frozenNamesIsCloned) { frozenNames = new Set(frozenNames); frozenNamesIsCloned = true; @@ -626,7 +626,7 @@ module.exports = { const paramNodes = [], {mangle} = this.options; let hasArgumentsOrEvalParam = false, - frozenThisVarName, frozenArgumentsVarName; + frozenThisVarName, frozenArgumentsVarName, withVarName; for (const param of params) { const paramName = param.name; let newName; @@ -651,9 +651,11 @@ module.exports = { // Rename injection node renameInjectionVarNode(); - } else if (!param.isFrozenName || paramName === 'super') { + } else if (!param.isFrozenName || paramName === 'super' || paramName === 'with') { + // TODO: `with` can be handled separately, as no nodes to rename. + // TODO: Should also throw error if `with` is circular, as can't handle that. // Param can be renamed. - // NB: `super` param is always renamed. + // NB: `super` and `with` params are always renamed. newName = transformVarName(paramName, param.isFrozenName); if (newName !== paramName) { // Rename all nodes @@ -663,17 +665,30 @@ module.exports = { // Rename injection node renameInjectionVarNode(); + + // Record var name for `with` object + if (paramName === 'with') withVarName = newName; } } else { - // Frozen var name (potentially used in `eval()`) + // Frozen var name (potentially used in `eval()`, or behind a `with (...)` block) // eslint-disable-next-line no-lonely-if if (paramName === 'this' || (paramName === 'arguments' && paramsByName.this?.isFrozenName)) { // `this` or `arguments` captured from function. // `arguments` is only injected with a function wrapper if `this` is frozen too. // Otherwise, can just make `arguments` a normal param. - // This can be the case if `arguments` is a user-defined variable, - // not `arguments` created by a function. - newName = transformVarName(paramName, true); + // This can be the case if `arguments` is behind a `with (...)` block, + // or `arguments` is a user-defined variable, not `arguments` created by a function. + // TODO: `arguments` could also be renamed in some cases, even if `this` is frozen. + // e.g.: ``` + // function outer() { + // let f; + // with ({x: 1}) f = () => [x, this]; + // return [f, () => arguments]; + // } + // module.exports = outer.call({y: 2}, 3, 4); + // ``` + // Actually I think this case is already handled. Write a test for it. + newName = transformVarName(paramName, param.isFrozenName); if (paramName === 'this') { frozenThisVarName = newName; } else { @@ -682,7 +697,7 @@ module.exports = { assert( !param.injectionVarNode, - 'Cannot handle circular `this` or `arguments` where `this` cannot be renamed due to use of `eval()`' + 'Cannot handle circular `this` or `arguments` where `this` cannot be renamed due to use of `eval()` or `with ()`' ); } else { newName = paramName; @@ -702,10 +717,12 @@ module.exports = { // Handle strict/sloppy mode let isStrict; if (!isRoot) { - if (hasArgumentsOrEvalParam) { + if (hasArgumentsOrEvalParam || withVarName) { // If param named `arguments` or `eval`, scope function must be sloppy mode // or it's a syntax error. - // NB: Only way param will be called `arguments` or `eval` is if it's frozen by an `eval()`. + // NB: Only way param will be called `arguments` or `eval` is if it's frozen by an `eval()` + // or behind a `with (...)` block. + // `with (...)` requires sloppy mode too. isStrict = false; } else if (strictFns.length === 0) { // No strict child functions or child blocks. Block is sloppy if any sloppy children, @@ -734,18 +751,18 @@ module.exports = { returnNode = t.sequenceExpression(internalFunctionNodes); } - // If uses frozen `this`, wrap return value in an IIFE to inject actual `this` - // (and `arguments`, if it's used too). - // `() => eval(x)` -> `(function() { return () => eval(x); }).apply(this$0, arguments$0)` - // TODO: In sloppy mode, it's possible for `arguments` to be re-defined as a non-iterable object - // which would cause an error when this function is called. - // A better solution when outputting sloppy mode code would be to just use a var called `arguments`, - // rather than injecting. Of course this isn't possible in ESM. - // TODO: Ensure scope function using `this` is strict mode if value of `this` is not an object. - // In sloppy mode, literals passed as `this` get boxed. - // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Strict_mode#securing_javascript - // TODO: Also doesn't work where `this` or `arguments` is circular and is injected late. if (frozenThisVarName) { + // Uses frozen `this`. + // Wrap return value in an IIFE to inject actual `this` (and `arguments`, if it's used too). + // `() => eval(x)` -> `(function() { return () => eval(x); }).apply(this$0, arguments$0)` + // TODO: In sloppy mode, it's possible for `arguments` to be re-defined as a non-iterable object + // which would cause an error when this function is called. + // A better solution when outputting sloppy mode code would be to just use a var called + // `arguments`, rather than injecting. Of course this isn't possible in ESM. + // TODO: Ensure scope function using `this` is strict mode if value of `this` is not an object. + // In sloppy mode, literals passed as `this` get boxed. + // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Strict_mode#securing_javascript + // TODO: Also doesn't work where `this` or `arguments` is circular and is injected late. returnNode = t.callExpression( t.memberExpression( t.functionExpression(null, [], t.blockStatement([t.returnStatement(returnNode)])), @@ -756,6 +773,25 @@ module.exports = { ...(frozenArgumentsVarName ? [t.identifier(frozenArgumentsVarName)] : []) ] ); + } else if (withVarName) { + // Wrap in `{ with (with$0) return ...; }`. + // NB: It's not possible for the `with` object to be a circular reference. + // TODO: But it can have circular properties: + // ` + // const o = {}; + // let f, f2; + // with (o) { + // let y; + // f = () => x; + // f2 = () => [x, y]; + // } + // o.x = f; + // o.x2 = f2; + // module.exports = f; + // ` + returnNode = t.blockStatement([ + t.withStatement(t.identifier(withVarName), t.returnStatement(returnNode)) + ]); } const node = t.arrowFunctionExpression(paramNodes, returnNode); diff --git a/lib/serialize/functions.js b/lib/serialize/functions.js index 2593b3ef..9c5c8487 100644 --- a/lib/serialize/functions.js +++ b/lib/serialize/functions.js @@ -15,7 +15,9 @@ const util = require('util'), t = require('@babel/types'); // Imports -const {activateTracker, getTrackerResult, trackerError} = require('../shared/tracker.js'), +const { + activateTracker, getTrackerResult, trackerError, setIsGettingScope + } = require('../shared/tracker.js'), specialFunctions = require('../shared/internal.js').functions, { TRACKER_COMMENT_PREFIX, @@ -319,11 +321,13 @@ module.exports = { // Call `getScopes()` to get scope vars let scopeVars; if (!errorMessage) { + setIsGettingScope(true); try { scopeVars = getScopes(); } catch (err) { errorMessage = getErrorMessage(err); } + setIsGettingScope(false); } assertBug( diff --git a/lib/serialize/parseFunction.js b/lib/serialize/parseFunction.js index 75e8c068..1f222e75 100644 --- a/lib/serialize/parseFunction.js +++ b/lib/serialize/parseFunction.js @@ -47,7 +47,7 @@ const RUNTIME_DIR_PATH = pathJoin(__dirname, '../runtime/'); * {Object} .internalVars - Object keyed by var name, values are arrays of identifier nodes * {Set} .globalVarNames - Set of names of global vars used * {Set} .reservedVarNames - Set of reserved var names - * (function names and vars accesible to `eval()` which are frozen) + * (function names and vars accesible to `eval()` or `with()` which are frozen) * {string} .name - `.name` of created function * {number} .numParams - `.length` of created function * {boolean} .isClass - `true` if is class @@ -833,7 +833,7 @@ function createTempVarNode(name, internalVars) { * @param {Object} internalVars - Map of var name to array of var nodes * @param {Set} globalVarNames - Set of global var names * @param {Set} reservedVarNames - Set of var names which are reserved - * (function names and vars accesible to `eval()` which are frozen) + * (function names and vars accesible to `eval()` or `with()` which are frozen) * @param {Array} amendments - Array of amendments (const violations or `super`) * @returns {undefined} */ diff --git a/lib/shared/tracker.js b/lib/shared/tracker.js index 60ef7a8f..8d677303 100644 --- a/lib/shared/tracker.js +++ b/lib/shared/tracker.js @@ -14,10 +14,13 @@ module.exports = { tracker, activateTracker, getTrackerResult, - trackerError + trackerError, + getIsGettingScope, + setIsGettingScope }; let trackerIsActive = false, + isGettingScope = false, trackerResult; /** @@ -62,3 +65,21 @@ function getTrackerResult() { trackerResult = undefined; return result || {getFnInfo: undefined, getScopes: undefined}; } + +/** + * Get whether currently getting scope vars for function. + * Used by `livepack_tracker.wrapWith()`. + * @returns {boolean} - `true` if currently getting scope vars + */ +function getIsGettingScope() { + return isGettingScope; +} + +/** + * Set whether currently getting scope vars for function. + * @param {boolean} isGetting - `true` if currently getting scope vars + * @returns {undefined} + */ +function setIsGettingScope(isGetting) { + isGettingScope = isGetting; +}