with statements WIP 1

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.

TODO.md

@@ -0,0 +1,5 @@
+# TODO
+
+* Tests
+* Deal with `with` in `eval()`
+* TODO comments

lib/init/index.js

@@ -11,8 +11,8 @@
 const getScopeId = require('./getScopeId.js'),
 	addEvalFunctionsToTracker = require('./eval.js'),
 	internal = require('../shared/internal.js'),
-	{tracker} = require('../shared/tracker.js'),
-	{COMMON_JS_MODULE} = require('../shared/constants.js');
+	{tracker, getIsGettingScope} = require('../shared/tracker.js'),
+	{COMMON_JS_MODULE, INTERNAL_VAR_NAMES_PREFIX} = require('../shared/constants.js');
 
 // Exports
 
@@ -43,11 +43,59 @@ module.exports = (filename, module, require, nextBlockId, prefixNum) => {
 	localTracker.nextBlockId = nextBlockId;
 	localTracker.prefixNum = prefixNum;
 	addEvalFunctionsToTracker(localTracker, filename);
+	addWrapWithFunctionToTracker(localTracker, prefixNum);
 
 	// Return tracker and `getScopeId` functions
 	return [localTracker, getScopeId];
 };
 
+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), {
+		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);
+		}
+		// NB: These are the only traps which can be triggered
+	});
+}
+
 // Imports
 // These imports are after export to avoid circular requires in Jest tests
 const captureFunctions = require('./functions.js'),

lib/instrument/blocks.js

@@ -136,6 +136,7 @@ function createBindingWithoutNameCheck(block, varName, props) {
 		isSilentConst: !!props.isSilentConst,
 		isVar: !!props.isVar,
 		isFunction: !!props.isFunction,
+		isBehindWith: false,
 		argNames: props.argNames
 	};
 }
@@ -200,6 +201,7 @@ function getOrCreateExternalVar(externalVars, block, varName, binding) {
 			binding,
 			isReadFrom: false,
 			isAssignedTo: false,
+			isFrozenName: false,
 			trails: []
 		};
 		blockVars[varName] = externalVar;

lib/instrument/visitors/eval.js

@@ -115,6 +115,7 @@ function instrumentEvalCall(callNode, block, fn, isStrict, canUseSuper, state) {
 
 		const varDefsNodes = [];
 		for (const [varName, binding] of Object.entries(block.bindings)) {
+			// TODO: Capture `with` object
 			if (varNamesUsed.has(varName)) continue;
 			if (isStrict && varName !== 'this' && isReservedWord(varName)) continue;
 
@@ -145,6 +146,7 @@ function instrumentEvalCall(callNode, block, fn, isStrict, canUseSuper, state) {
 			if (blockIsExternalToFunction && varName !== 'new.target') {
 				activateBinding(binding, varName);
 				const externalVar = getOrCreateExternalVar(externalVars, block, varName, binding);
+				// TODO: Set `isFrozenName`
 				externalVar.isReadFrom = true;
 				if (!isConst) externalVar.isAssignedTo = true;
 			}

lib/instrument/visitors/function.js

@@ -722,10 +722,15 @@ function insertTrackerComment(fnId, fnType, commentHolderNode, commentType, stat
  */
 function createFunctionInfoFunction(fn, state) {
 	// Compile internal vars
-	const internalVars = Object.create(null);
-	for (const {varName, trails} of fn.internalVars.values()) {
-		const internalVar = internalVars[varName] || (internalVars[varName] = []);
-		internalVar.push(...trails);
+	const internalVars = Object.create(null),
+		reservedVarNames = new Set();
+	for (const [binding, {varName, trails}] of fn.internalVars.entries()) {
+		if (binding.isBehindWith) {
+			reservedVarNames.add(varName);
+		} else {
+			const internalVar = internalVars[varName] || (internalVars[varName] = []);
+			internalVar.push(...trails);
+		}
 	}
 
 	// Create JSON function info string
@@ -736,11 +741,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,
-					isFunction: varProps.binding.isFunction || undefined,
+					isFrozenName: varProps.isFrozenName || undefined,
+					isFrozenInternalName: binding.isFunction || binding.isBehindWith || undefined,
 					trails: varProps.trails
 				};
 			})
@@ -751,6 +758,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()

lib/instrument/visitors/identifier.js

@@ -16,7 +16,9 @@ module.exports = {
 
 // Imports
 const visitEval = require('./eval.js'),
-	{getOrCreateExternalVar, activateBlock, activateBinding, createInternalVar} = require('../blocks.js'),
+	{
+		getOrCreateExternalVar, activateBlock, activateBinding, createInternalVar, createBlockTempVar
+	} = require('../blocks.js'),
 	{checkInternalVarNameClash} = require('../internalVars.js'),
 	{
 		CONST_VIOLATION_CONST, CONST_VIOLATION_FUNCTION_THROWING, CONST_VIOLATION_FUNCTION_SILENT
@@ -80,7 +82,7 @@ function ThisExpression(node, state) {
 	// Ignore if internal to function, unless in class constructor or prototype class property
 	// of class with super class.
 	if (block.id < fn.id) {
-		recordExternalVar(binding, block, 'this', fn, [...state.trail], true, false, state);
+		recordExternalVar(binding, block, 'this', fn, [...state.trail], true, false, false, state);
 	} else if (fn.hasSuperClass) {
 		createInternalVar(fn, 'this', binding, [...state.trail]);
 	}
@@ -104,7 +106,7 @@ function NewTargetExpression(node, state) {
 	const block = state.currentThisBlock;
 	if (block.id < fn.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, false, state
 		);
 	}
 }
@@ -147,7 +149,7 @@ function visitIdentifier(node, varName, isReadFrom, isAssignedTo, state) {
 function resolveIdentifierInSecondPass(node, block, varName, fn, isReadFrom, isAssignedTo, state) {
 	state.secondPass(
 		resolveIdentifier,
-		node, block, varName, fn, [...state.trail], isReadFrom, isAssignedTo, state.isStrict, state
+		node, block, varName, fn, [...state.trail], isReadFrom, isAssignedTo, false, state.isStrict, state
 	);
 }
 
@@ -160,14 +162,25 @@ function resolveIdentifierInSecondPass(node, block, varName, fn, isReadFrom, isA
  * @param {Array<string|number>} 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 {boolean} isStrict - `true` if variable used in strict mode
  * @param {Object} state - State object
  * @returns {undefined}
  */
-function resolveIdentifier(node, block, varName, fn, trail, isReadFrom, isAssignedTo, isStrict, state) {
+function resolveIdentifier(
+	node, block, varName, fn, trail, isReadFrom, isAssignedTo, isBehindWith, isStrict, state
+) {
 	// Find binding
-	let binding;
+	let binding,
+		isWithBinding = false;
 	do {
+		// Check for `with () {}` block which can intercept any variable access
+		binding = block.bindings.with;
+		if (binding) {
+			isWithBinding = true;
+			break;
+		}
+
 		binding = block.bindings[varName];
 	} while (!binding && (block = block.parent)); // eslint-disable-line no-cond-assign
 
@@ -180,9 +193,36 @@ function resolveIdentifier(node, block, varName, fn, trail, isReadFrom, isAssign
 		return;
 	}
 
+	// Flag binding as behind `with` if it is
+	if (isBehindWith) binding.isBehindWith = true;
+
 	// Record if internal var
 	if (block.id >= fn.id) {
-		if (!binding.isFunction && !binding.argNames) fn.internalVars.get(binding).trails.push(trail);
+		if (isWithBinding) {
+			// Continue searching for binding further down the scope chain
+			resolveIdentifier(
+				node, block.parent, varName, fn, trail, isReadFrom, isAssignedTo, true, isStrict, state
+			);
+			return;
+		}
+
+		if (!binding.isFunction && !binding.argNames && !binding.isBehindWith) {
+			fn.internalVars.get(binding).trails.push(trail);
+		}
+		return;
+	}
+
+	// If is a `with () {}` block, activate block
+	// and continue to search for binding further down the scope chain
+	if (isWithBinding) {
+		activateBlock(block, state);
+		if (!binding.varNode) binding.varNode = createBlockTempVar(block, state);
+		const externalVar = getOrCreateExternalVar(fn.externalVars, block, 'with', binding);
+		externalVar.isReadFrom = true;
+
+		resolveIdentifier(
+			node, block.parent, varName, fn, trail, isReadFrom, isAssignedTo, true, isStrict, state
+		);
 		return;
 	}
 
@@ -203,7 +243,7 @@ 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);
 }
 
 /**
@@ -215,14 +255,22 @@ function resolveIdentifier(node, block, varName, fn, trail, isReadFrom, isAssign
  * @param {Array<string|number>} 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.externalVars, block, varName, binding);
 	if (isReadFrom) externalVar.isReadFrom = true;
 	if (isAssignedTo) externalVar.isAssignedTo = true;
-	externalVar.trails.push(trail);
+	if (isBehindWith) {
+		externalVar.isFrozenName = true;
+		externalVar.trails.length = 0;
+	} else if (!externalVar.isFrozenName) {
+		externalVar.trails.push(trail);
+	}
 }

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);
 }

lib/instrument/visitors/with.js

@@ -0,0 +1,58 @@
+/* --------------------
+ * livepack module
+ * Code instrumentation visitor for `with` statements
+ * ------------------*/
+
+'use strict';
+
+// Export
+module.exports = WithStatement;
+
+// Modules
+const t = require('@babel/types');
+
+// Imports
+const Expression = require('./expression.js'),
+	Statement = require('./statement.js'),
+	{createAndEnterBlock, createBindingWithoutNameCheck} = 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
+	const parentBlock = state.currentBlock;
+	const block = createAndEnterBlock('with', false, state);
+	const binding = createBindingWithoutNameCheck(block, 'with', {isConst: true}, state);
+
+	// Visit body
+	visitKey(node, 'body', Statement, state);
+
+	// Exit block
+	state.currentBlock = parentBlock;
+
+	// 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
+		]
+	);
+}