with statements WIP 1

TODO.md

@@ -0,0 +1,9 @@
+# TODO
+
+* Create `with () {}` block in output
+  * Make sure that scope function is sloppy mode
+* Prevent vars below `with () {}` having their names mangled
+  * Including vars internal to functions
+* Deal with `with` in `eval()`
+* Tests
+* Remove note in README that `with` is not supported

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

@@ -199,6 +199,7 @@ function getOrCreateExternalVar(externalVars, block, varName, bindingOrExternalV
 			varNode: bindingOrExternalVar.varNode,
 			isReadFrom: false,
 			isAssignedTo: false,
+			isNameLocked: false,
 			argNames: bindingOrExternalVar.argNames,
 			trails: []
 		};

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 `isNameLocked`
 				externalVar.isReadFrom = true;
 				if (!isConst) externalVar.isAssignedTo = true;
 			}

lib/instrument/visitors/function.js

@@ -728,6 +728,7 @@ function createFunctionInfoFunction(fn, scopes, astJson, fnInfoVarNode, state) {
 				return {
 					isReadFrom: varProps.isReadFrom || undefined,
 					isAssignedTo: varProps.isAssignedTo || undefined,
+					isNameLocked: varProps.isNameLocked || undefined,
 					trails: varProps.trails
 				};
 			})

lib/instrument/visitors/identifier.js

@@ -16,7 +16,7 @@ module.exports = {
 
 // Imports
 const visitEval = require('./eval.js'),
-	{getOrCreateExternalVar, activateBlock, activateBinding} = require('../blocks.js'),
+	{getOrCreateExternalVar, activateBlock, activateBinding, createBlockTempVar} = require('../blocks.js'),
 	{checkInternalVarNameClash} = require('../internalVars.js'),
 	{createArrayOrPush} = require('../../shared/functions.js'),
 	{
@@ -81,7 +81,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) {
 		createArrayOrPush(fn.internalVars, 'this', [...state.trail]);
 	}
@@ -105,7 +105,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
 		);
 	}
 }
@@ -148,7 +148,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
 	);
 }
 
@@ -161,14 +161,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
 
@@ -183,10 +194,33 @@ function resolveIdentifier(node, block, varName, fn, trail, isReadFrom, isAssign
 
 	// Record if internal var
 	if (block.id >= fn.id) {
+		if (isWithBinding) {
+			// Continue searching for binding further down the scope chain
+			resolveIdentifier(
+				node, block.parent, varName, fn, trail, isReadFrom, isAssignedTo, true, isStrict, state
+			);
+			return;
+		}
+
+		// TODO: Freeze var name if behind `with () {}`
 		if (!binding.isFunction && !binding.argNames) createArrayOrPush(fn.internalVars, varName, 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;
+	}
+
 	// Record external var
 	if (isAssignedTo && binding.isConst) {
 		// Record const violation
@@ -204,7 +238,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);
 }
 
 /**
@@ -216,14 +250,18 @@ 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;
+	if (isBehindWith) externalVar.isNameLocked = true;
 	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
+		]
+	);
+}

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,
@@ -309,11 +311,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(

lib/serialize/parseFunction.js

@@ -81,7 +81,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,
+					isNameLocked: !!varProps.isNameLocked
+				};
 			})
 		});
 	}
@@ -865,11 +869,15 @@ 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, isNameLocked, 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 (isNameLocked) scopeDefVar.isNameLocked = true;
 				}
 
 				externalVars[varName].push(...trailsToNodes(fnNode, trails, varName));