diff --git a/TODO.md b/TODO.md new file mode 100644 index 00000000..70192269 --- /dev/null +++ b/TODO.md @@ -0,0 +1,10 @@ +# TODO + +* Blocks in serializer code rename internal vars which aren't frozen, even in prescence of `eval()` +* No point setting `isFrozenName` on external vars passed to serializer? (as it depends entirely on `containsEval` anyway) +* `serialize/blocks` + `serialize/functions` + `serialize/parseFunction` could have changes stripped out if so. +* TODO comment in `visitors/eval` +* TODO comment in `serialize/blocks` +* TODO comment in `serialize/functions` +* TODO comment in `serialize/parseFunction` +* Deal with duplicate bindings - `for (let x of x)` and `x => { var x; }` diff --git a/experiment/index.js b/experiment/index.js new file mode 100644 index 00000000..ea17cc90 --- /dev/null +++ b/experiment/index.js @@ -0,0 +1,27 @@ +/* eslint-disable no-console */ + +'use strict'; + +const options = { + cache: false, + esm: false, + jsx: false, + minify: false, + mangle: true, + inline: true, + comments: true, + format: 'cjs', + strictEnv: undefined +}; + +const {cache, esm, jsx, ...serializeOpts} = options; + +require('../register.js')({cache, esm, jsx}); + +const {serialize} = require('../index.js'); + +let fn = require('./src/index.js'); + +if (esm) fn = fn.default; + +console.log(serialize(fn, serializeOpts)); diff --git a/experiment/instrument.js b/experiment/instrument.js new file mode 100644 index 00000000..eddfe6ad --- /dev/null +++ b/experiment/instrument.js @@ -0,0 +1,29 @@ +/* eslint-disable no-console */ + +'use strict'; + +const {readFileSync} = require('fs'), + pathJoin = require('path').join, + {instrumentCodeImpl} = require('../lib/instrument/instrument.js'); + +const filename = pathJoin(__dirname, 'src/index.js'); +const codeIn = readFileSync(filename, 'utf8'); + +// eeslint-disable-next-line no-unused-vars +const options = { + isEsm: false, + isCommonJs: true, + isJsx: false, + isStrict: false, + sourceMaps: true, + retainLines: false +}; + +const startTime = new Date(); +const codeOut = instrumentCodeImpl( + codeIn, filename, options.isEsm, options.isCommonJs, options.isJsx, options.isStrict, + options.sourceMaps, undefined, options.retainLines +).code; +const endTime = new Date(); +console.log(codeOut); +console.log(`-----\nInstrumented in ${endTime - startTime} ms`); diff --git a/experiment/src/index.js b/experiment/src/index.js new file mode 100644 index 00000000..7b77b904 --- /dev/null +++ b/experiment/src/index.js @@ -0,0 +1,18 @@ +/* eslint-disable no-eval, no-unused-vars */ + +'use strict'; + +class S {} +class X extends S { + constructor(module, exports) { + super(); + eval('0'); + this.x = X; + } +} + +{ + const Klass = X; + X = 1; // eslint-disable-line no-class-assign + module.exports = Klass; +} diff --git a/lib/init/eval.js b/lib/init/eval.js index 3e77d910..3dd35346 100644 --- a/lib/init/eval.js +++ b/lib/init/eval.js @@ -143,7 +143,7 @@ function evalDirect(args, filename, blockIdCounter, externalPrefixNum, evalDirec } else { // Whether var is function is not relevant because it will always be in external scope // of the functions it's being recorded on, and value of `isFunction` only has any effect - // for internal vars + // for internal vars. Ditto `isFrozenName`. createBindingWithoutNameCheck( block, varName, {isConst: !!isConst, isSilentConst: !!isSilentConst, argNames} ); diff --git a/lib/instrument/blocks.js b/lib/instrument/blocks.js index 347f0d7d..d927f825 100644 --- a/lib/instrument/blocks.js +++ b/lib/instrument/blocks.js @@ -101,6 +101,8 @@ function createBlockId(state) { * @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`) * @param {Object} state - State object * @returns {Object} - Binding object @@ -125,6 +127,8 @@ function createBinding(block, varName, props, state) { * @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`) * @returns {Object} - Binding object */ @@ -136,8 +140,9 @@ function createBindingWithoutNameCheck(block, varName, props) { 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 + trails: [] // Trails of usages within same function as binding, where var not function or frozen }; } @@ -161,7 +166,8 @@ const thisNode = t.thisExpression(); * @returns {Object} - Binding object */ function createArgumentsBinding(block, isConst, argNames) { - return createBindingWithoutNameCheck(block, 'arguments', {isConst, argNames}); + // `isFrozenName: true` because it cannot be renamed within function in which it's created + return createBindingWithoutNameCheck(block, 'arguments', {isConst, isFrozenName: true, argNames}); } /** @@ -201,6 +207,7 @@ function getOrCreateExternalVar(externalVars, block, varName, binding) { binding, isReadFrom: false, isAssignedTo: false, + isFrozenName: false, trails: [] }; blockVars[varName] = externalVar; diff --git a/lib/instrument/visitors/eval.js b/lib/instrument/visitors/eval.js index c3acc889..1febbb65 100644 --- a/lib/instrument/visitors/eval.js +++ b/lib/instrument/visitors/eval.js @@ -127,6 +127,17 @@ function instrumentEvalCall(callNode, block, fn, isStrict, canUseSuper, superIsP varNamesUsed.add(varName); + // `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; + } + // Create array for var `[varName, isConst, isSilentConst, argNames, tempVarValue]`. // NB: Assignment to var called `arguments` or `eval` is illegal in strict mode. // This is relevant as it affects whether var is flagged as mutable or not. @@ -152,6 +163,12 @@ function instrumentEvalCall(callNode, block, fn, isStrict, canUseSuper, superIsP const externalVar = getOrCreateExternalVar(externalVars, block, varName, binding); externalVar.isReadFrom = true; if (!isConst) externalVar.isAssignedTo = true; + if (!externalVar.isFrozenName) { + // TODO: Should also do same for all functions above + // TODO: Should also do this for `super` and `new.target` + externalVar.isFrozenName = true; + externalVar.trails.length = 0; + } } } diff --git a/lib/instrument/visitors/function.js b/lib/instrument/visitors/function.js index e8b16661..e68aa649 100644 --- a/lib/instrument/visitors/function.js +++ b/lib/instrument/visitors/function.js @@ -723,16 +723,24 @@ function insertTrackerComment(fnId, fnType, commentHolderNode, commentType, stat */ function createFunctionInfoFunction(fn, state) { // Compile internal vars - const internalVars = Object.create(null); - for (const {name: varName, trails, isFunction} of fn.bindings) { + 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. + // So skip them here. `parseFunction` adds them to list of reserved var names. if (isFunction) continue; - const internalVar = internalVars[varName] || (internalVars[varName] = []); - internalVar.push(...trails); + + if (isFrozenName) { + reservedVarNames.add(varName); + } else { + const internalVar = internalVars[varName] || (internalVars[varName] = []); + internalVar.push(...trails); + } } + // TODO: Remove from `reservedVarNames` where internal var with same name + // Create JSON function info string const {children} = fn; let argNames; @@ -741,11 +749,13 @@ function createFunctionInfoFunction(fn, state) { blockId: block.id, blockName: block.name, vars: mapValues(vars, (varProps, varName) => { - if (varName === 'arguments') argNames = varProps.binding.argNames; + const {binding} = varProps; + if (varName === 'arguments') argNames = binding.argNames; return { isReadFrom: varProps.isReadFrom || undefined, isAssignedTo: varProps.isAssignedTo || undefined, - isFrozenInternalName: varProps.binding.isFunction || undefined, + isFrozenName: varProps.isFrozenName || undefined, + isFrozenInternalName: binding.isFrozenName || binding.isFunction || undefined, trails: varProps.trails }; }) @@ -756,6 +766,7 @@ function createFunctionInfoFunction(fn, state) { containsImport: fn.containsImport || undefined, argNames, internalVars, + reservedVarNames: reservedVarNames.size !== 0 ? [...reservedVarNames] : undefined, globalVarNames: fn.globalVarNames.size !== 0 ? [...fn.globalVarNames] : undefined, amendments: fn.amendments.length !== 0 ? fn.amendments.map(({type, blockId, trail}) => [type, blockId, ...trail]).reverse() diff --git a/lib/instrument/visitors/identifier.js b/lib/instrument/visitors/identifier.js index d21b8d0e..120898e4 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.isFunction && !binding.argNames) binding.trails.push(trail); + if (!binding.isFrozenName && !binding.isFunction) binding.trails.push(trail); return; } @@ -209,5 +209,5 @@ function recordExternalVar(binding, block, varName, fn, trail, isReadFrom, isAss const externalVar = getOrCreateExternalVar(fn.externalVars, block, varName, binding); if (isReadFrom) externalVar.isReadFrom = true; if (isAssignedTo) externalVar.isAssignedTo = true; - externalVar.trails.push(trail); + if (!externalVar.isFrozenName) externalVar.trails.push(trail); } diff --git a/lib/serialize/blocks.js b/lib/serialize/blocks.js index d6d219bc..f0ebd1f8 100644 --- a/lib/serialize/blocks.js +++ b/lib/serialize/blocks.js @@ -59,10 +59,14 @@ module.exports = { const returnNodes = []; // Init param objects - const paramsByName = Object.create(null); - const params = [...block.params.keys()].map((name) => { + const {containsEval} = block, + paramsByName = Object.create(null), + blockFrozenNames = []; + const params = [...block.params].map(([name, {isFrozenName}]) => { + if (containsEval) isFrozenName = true; const param = { name, + isFrozenName, // Identifier nodes referring to this param localVarNodes: [], // If param always contains another function defined in this block, @@ -79,6 +83,8 @@ module.exports = { injectionVarNode: null }; paramsByName[name] = param; + // `super` and `new.target` are not actually frozen + if (isFrozenName && !['super', 'new.target'].includes(name)) blockFrozenNames.push(name); return param; }); @@ -93,7 +99,7 @@ module.exports = { // Also count which params are most used in calls to create scope function, so params // which are most often undefined (or circular, and so undefined in initial call to scope function) // go last. - const {functions: blockFunctions, scopes: blockScopes, children: childBlocks, containsEval} = block, + const {functions: blockFunctions, scopes: blockScopes, children: childBlocks} = block, localFunctions = new Map(); for (const fnDef of blockFunctions) { for (const [scope, fnRecord] of fnDef.scopes) { @@ -342,8 +348,10 @@ module.exports = { const isSingular = !isRoot && returnNodeIndex + blockFunctions.length - numInternalOnlyFunctions + childBlocks.length === 1; - // If block contains `eval()`, freeze all param names - if (containsEval) frozenNames = new Set([...frozenNames, ...params.map(param => param.name)]); + // Flag any frozen var names. + // Var names are frozen because potentially accessed by `eval()`. + // TODO: Can this be done earlier when `blockFrozenNames` is filled? + if (blockFrozenNames.length > 0) frozenNames = new Set([...frozenNames, ...blockFrozenNames]); // Init vars to track strict/sloppy children const strictFns = []; @@ -641,7 +649,7 @@ module.exports = { if (paramName === 'new.target') { // `new.target` is always renamed as it can't be provided for `eval()` anyway - newName = transformVarName('newTarget', containsEval); + newName = transformVarName('newTarget', param.isFrozenName); // Convert `MetaProperty` nodes into `Identifier`s with new name for (const varNode of param.localVarNodes) { @@ -653,9 +661,10 @@ module.exports = { // Rename injection node renameInjectionVarNode(); - } else if (!containsEval || paramName === 'super') { - // NB: `super` var is always renamed - newName = transformVarName(paramName, containsEval); + } else if (!param.isFrozenName || paramName === 'super') { + // Param can be renamed. + // NB: `super` param is always renamed. + newName = transformVarName(paramName, param.isFrozenName); if (newName !== paramName) { // Rename all nodes for (const varNode of param.localVarNodes) { @@ -668,7 +677,7 @@ module.exports = { } else { // Frozen var name (potentially used in `eval()`) // eslint-disable-next-line no-lonely-if - if (paramName === 'this' || (paramName === 'arguments' && paramsByName.this)) { + 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. diff --git a/lib/serialize/functions.js b/lib/serialize/functions.js index 99b95ba9..2593b3ef 100644 --- a/lib/serialize/functions.js +++ b/lib/serialize/functions.js @@ -150,12 +150,13 @@ module.exports = { } // Add var names to block - for (const [varName, {isAssignedTo}] of Object.entries(vars)) { + for (const [varName, {isAssignedTo, isFrozenName}] of Object.entries(vars)) { const param = params.get(varName); if (!param) { - params.set(varName, {isMutable: isAssignedTo || argNames?.has(varName)}); - } else if (isAssignedTo) { - param.isMutable = true; + params.set(varName, {isMutable: isAssignedTo || argNames?.has(varName), isFrozenName}); + } else { + if (isAssignedTo) param.isMutable = true; + if (isFrozenName) param.isFrozenName = true; } } diff --git a/lib/serialize/parseFunction.js b/lib/serialize/parseFunction.js index 33fe2aab..f30e38ea 100644 --- a/lib/serialize/parseFunction.js +++ b/lib/serialize/parseFunction.js @@ -82,7 +82,11 @@ module.exports = function parseFunction( blockName, vars: mapValues(vars, (varProps, varName) => { externalVars[varName] = []; - return {isReadFrom: !!varProps.isReadFrom, isAssignedTo: !!varProps.isAssignedTo}; + return { + isReadFrom: !!varProps.isReadFrom, + isAssignedTo: !!varProps.isAssignedTo, + isFrozenName: !!varProps.isFrozenName + }; }) }); } @@ -873,19 +877,24 @@ function resolveFunctionInfo( const {blockId} = scope; if (blockId < fnId) { // External var - for (const [varName, {isReadFrom, isAssignedTo, trails}] of Object.entries(scope.vars)) { + for ( + const [varName, {isReadFrom, isAssignedTo, isFrozenName, trails}] + of Object.entries(scope.vars) + ) { if (isNestedFunction) { const scopeDefVar = scopeDefs.get(blockId).vars[varName]; if (isReadFrom) scopeDefVar.isReadFrom = true; if (isAssignedTo) scopeDefVar.isAssignedTo = true; + if (isFrozenName) scopeDefVar.isFrozenName = true; } - externalVars[varName].push(...trailsToNodes(fnNode, trails, varName)); + if (!isFrozenName) externalVars[varName].push(...trailsToNodes(fnNode, trails, varName)); } } else { // Var which is external to current function, but internal to function being serialized for (const [varName, {isFrozenInternalName, trails}] of Object.entries(scope.vars)) { if (!isFrozenInternalName) { + // TODO: Could remove the `?.` if flagged `super` and `new.target` as `isFrozenInternalName` internalVars[varName]?.push(...trailsToNodes(fnNode, trails, varName)); } } @@ -897,7 +906,8 @@ function resolveFunctionInfo( internalVars[varName].push(...trailsToNodes(fnNode, trails, varName)); } - // Get global var names + // Get reserved internal var names + global var names + if (fnInfo.reservedVarNames) setAddFrom(reservedVarNames, fnInfo.reservedVarNames); if (fnInfo.globalVarNames) setAddFrom(globalVarNames, fnInfo.globalVarNames); // Get amendments (const violations and `super`).