Skip to content

Commit

Permalink
Reformat TODO code comments [nocode]
Browse files Browse the repository at this point in the history
  • Loading branch information
overlookmotel committed Aug 29, 2023
1 parent 0035a7f commit 30ed7b4
Show file tree
Hide file tree
Showing 21 changed files with 49 additions and 49 deletions.
2 changes: 1 addition & 1 deletion lib/init/eval.js
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ function evalDirect(args, tracker, filename, evalDirectLocal) {
} else if (varName === 'new.target') {
createNewTargetBinding(block);
} else {
// TODO Also need to set `superIsProto` and create a new temp var for `super` target
// TODO: Also need to set `superIsProto` and create a new temp var for `super` target
// (the external var could be shadowed inside `eval()` if prefix num is changing)
if (varName === 'super') state.currentSuperBlock = block;

Expand Down
2 changes: 1 addition & 1 deletion lib/init/globals.js
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ function addToQueue(val, type, parent, key, noPreload, queue) {

/*
// For debugging
// TODO Remove this
// TODO: Remove this
function trace(val, globals) {
const record = globals.get(val);
if (!record) return '<not found>';
Expand Down
4 changes: 2 additions & 2 deletions lib/init/weak.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ function getWeakSets() {
// Could make `refs`, `mapping` and `finalizationRegistry` private class fields, and that would likely
// be more performant. However, don't want to do that now as intent is that Livepack can build itself,
// and currently Livepack can't serialize private fields.
// TODO Switch to private fields once Livepack can serialize them.
// TODO: Switch to private fields once Livepack can serialize them.
WeakSet = class WeakSet { // eslint-disable-line no-global-assign
constructor(...iterables) {
const refs = new Set();
Expand Down Expand Up @@ -100,7 +100,7 @@ function getWeakMaps() {
// Could make `refs`, `mapping` and `finalizationRegistry` private class fields, and that would likely
// be more performant. However, don't want to do that now as intent is that Livepack can build itself,
// and currently Livepack can't serialize private fields.
// TODO Switch to private fields once Livepack can serialize them.
// TODO: Switch to private fields once Livepack can serialize them.
const WeakMapOriginal = WeakMap;
WeakMap = class WeakMap { // eslint-disable-line no-global-assign
constructor(...iterables) {
Expand Down
6 changes: 3 additions & 3 deletions lib/instrument/modify.js
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ function modifyAst(ast, filename, isCommonJs, isStrict, sources, evalState) {
// Create program block
const programBlock = createAndEnterBlock(blockName, true, state);
state.programBlock = programBlock;
// TODO If code is a script or in indirect `(0, eval)(...)`,
// TODO: If code is a script or in indirect `(0, eval)(...)`,
// `var` declarations and function declarations (including async functions + generator functions)
// create globals, not local bindings.
// In direct `eval()` they create bindings in hoist block external to the `eval()`.
Expand All @@ -129,7 +129,7 @@ function modifyAst(ast, filename, isCommonJs, isStrict, sources, evalState) {
// NB Only remaining possibility is direct `eval()` code, where `this` is already defined
// in parent blocks from the outer context.
// Create binding for `this` (which will be `undefined` in ESM, or `globalThis` in script context).
// TODO Uncomment next line.
// TODO: Uncomment next line.
// It is correct, but producing excessively verbose output where indirect eval is used.
// Need to remove references to `globalThis` where in output the code is run inside `(0, eval)()`
// anyway, so `this` is already `globalThis` and unnecessary to inject it.
Expand All @@ -139,7 +139,7 @@ function modifyAst(ast, filename, isCommonJs, isStrict, sources, evalState) {
state.currentThisBlock = fileBlock;
} else if (!state.currentThisBlock) {
// Code from direct `eval()` which doesn't provide a `this` binding.
// TODO This can be removed once `createThisBinding(fileBlock)` above is uncommented,
// TODO: This can be removed once `createThisBinding(fileBlock)` above is uncommented,
// as then `state.currentThisBlock` will always be defined already in parent scope.
state.currentThisBlock = fileBlock;
}
Expand Down
14 changes: 7 additions & 7 deletions lib/instrument/visitors/class.js
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ function visitClass(classNode, parent, key, className, state) {
methodIndexes.push(index);
}
} else if (type === 'ClassPrivateMethod') {
// TODO Will be missed out when serializing.
// TODO: Will be missed out when serializing.
// Flag the class that it has private methods so serialization can throw error?
methodIndexes.push(index);
} else if (type === 'ClassProperty') {
Expand All @@ -153,7 +153,7 @@ function visitClass(classNode, parent, key, className, state) {
protoPropertyIndexes.push(index);
}
} else if (type === 'ClassPrivateProperty') {
// TODO Will be missed out when serializing.
// TODO: Will be missed out when serializing.
// Flag the class that it has private properties so serialization can throw error?
if (memberNode.static) {
staticPropertyOrBlockIndexes.push(index);
Expand Down Expand Up @@ -196,7 +196,7 @@ function visitClass(classNode, parent, key, className, state) {
// Visit prototype properties
if (protoPropertyIndexes.length !== 0) {
// Create and enter block for `this` in the context of prototype properties
// TODO This should be a vars block and all prototype methods should be wrapped
// TODO: This should be a vars block and all prototype methods should be wrapped
// in closures. See https://github.com/overlookmotel/livepack/issues/305
state.currentBlock = superBlock;
protoThisBlock = createAndEnterBlock(className, false, state);
Expand Down Expand Up @@ -248,7 +248,7 @@ function visitClass(classNode, parent, key, className, state) {
state.currentSuperIsProto = parentSuperIsProto;

// Visit computed keys
// TODO Pass values of computed prototype property keys to serializer so it can recreate them
// TODO: Pass values of computed prototype property keys to serializer so it can recreate them
if (computedKeys.length !== 0) {
state.currentBlock = nameBlock || parentBlock;
for (const {memberNode, index} of computedKeys) {
Expand Down Expand Up @@ -454,7 +454,7 @@ function instrumentClass(
const commentHolderNode = classNode.id || classNode.superClass || classNode.body;
insertTrackerComment(fn.id, FN_TYPE_CLASS, commentHolderNode, 'leading', state);

// TODO Handle prototype properties
// TODO: Handle prototype properties
}

/**
Expand Down Expand Up @@ -591,7 +591,7 @@ function wrapAnonymousClassExpression(classNode, parent, key, superBlock, state)
// `+ ''` is added in case `.toString()` method has side effects,
// so need to prevent it being called multiple times too.
// `{ [f()]: class {} }` -> `{ [temp_2 = fn() + '']: temp_1 = { [temp_2]: class {} }[temp_2] }`
// TODO Conversion to string won't work for Symbols.
// TODO: Conversion to string won't work for Symbols.
// Needs to be `{ [temp_2 = livepack_getKey(fn())]: temp_1 = { [temp_2]: class {} }[temp_2] }`
// where `livepack_getKey` is defined as:
// `k => (typeof k === 'symbol' || require('util').types.isSymbolObject(k)) ? k : k + '';`
Expand All @@ -617,7 +617,7 @@ function wrapAnonymousClassExpression(classNode, parent, key, superBlock, state)
// NB Unlike object properties, computed key in class property does not name class.
// `(class { static ['x'] = class {} } ).x.name === ''`
// and `new (class { ['x'] = class {} })().x.name === ''`
// TODO Add tests for this.
// TODO: Add tests for this.
const keyNode = parent.key;
return wrapInNamingObject(classNode, keyNode, false, keyNode.type !== 'Identifier');
}
Expand Down
2 changes: 1 addition & 1 deletion lib/instrument/visitors/eval.js
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ function activateSuperIfIsUsable(fn, state) {
setSuperIsProtoOnFunctions(superBlock, fn, state);
}

// TODO Also need to pass `superIsProto` and `superVarNode` to `evalDirect()`
// TODO: Also need to pass `superIsProto` and `superVarNode` to `evalDirect()`
// for it to set inside `eval()`
activateSuperBinding(superBlock, state);
return true;
Expand Down
2 changes: 1 addition & 1 deletion lib/instrument/visitors/function.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ function FunctionDeclaration(node, state, parent, key) {
const fn = visitFunction(node, parent, key, fnName, true, true, state);

// Create binding for function name
// TODO Whether the value of the function is hoisted depends on whether is a top level statement
// TODO: Whether the value of the function is hoisted depends on whether is a top level statement
// (including in a labeled statement)
// `() => { console.log(typeof x); function x() {} }` -> Logs 'function'
// `() => { console.log(typeof x); q: function x() {} }` -> Logs 'function'
Expand Down
10 changes: 5 additions & 5 deletions lib/instrument/visitors/loop.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ function ForStatement(node, state) {
// Create blocks for init clause and body.
// NB Init block is required even if no `const` / `let` declaration,
// if initializer contains a class/object expression with method using `super`.
// TODO Add scope ID var and temp vars to init `const` / `let` initializer, rather than in block
// TODO Actually it's more complicated than this:
// TODO: Add scope ID var and temp vars to init `const` / `let` initializer, rather than in block
// TODO: Actually it's more complicated than this:
// e.g. `for (let i = 0, getI = () => i; i < 3; i++) {}`
// Initial vars declaration is only evaluated once, and `getI()` returns `0` in every turn of the loop,
// whereas `i` evaluates to a different value of `i` in each turn of loop.
Expand Down Expand Up @@ -80,7 +80,7 @@ function ForInitializer(node, state, parent) {
* @returns {undefined}
*/
function ForXStatement(node, state) {
// TODO Move init clause into body if contains functions which reference init vars
// TODO: Move init clause into body if contains functions which reference init vars
// e.g. `for ( const [ x, getX = () => x ] of [ [1], [2] ] ) { ... }`

// Create blocks.
Expand Down Expand Up @@ -140,7 +140,7 @@ function ForXInitializer(node, state, parent) {
function WhileStatement(node, state) {
// Init block is required to hold scope ID and temp vars if test clause contains
// a class/object expression with method using `super`
// TODO Convert to a `for (...; ...; ...)` loop if init block is used for `super` temp vars
// TODO: Convert to a `for (...; ...; ...)` loop if init block is used for `super` temp vars
const initBlock = createAndEnterBlock('while', false, state);
const bodyBlock = createBlock('while', true, state);
initBlock.varsBlock = bodyBlock;
Expand All @@ -161,7 +161,7 @@ function WhileStatement(node, state) {
function DoWhileStatement(node, state) {
// Init block is required to hold scope ID and temp vars if test clause contains
// a class/object expression with method using `super`
// TODO Convert to a `for (...; ...; ...)` loop if init block is used for `super` temp vars
// TODO: Convert to a `for (...; ...; ...)` loop if init block is used for `super` temp vars
const initBlock = createAndEnterBlock('doWhile', false, state);
const bodyBlock = createAndEnterBlock('doWhile', true, state);
initBlock.varsBlock = bodyBlock;
Expand Down
2 changes: 1 addition & 1 deletion lib/instrument/visitors/statement.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ function ReturnStatement(node, state) {
}

function WithStatement(node, state) {
// TODO Maintain a state property `currentWithBlock` which can be used in `resolveBinding()`
// 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);
Expand Down
2 changes: 1 addition & 1 deletion lib/instrument/visitors/super.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ function Super(node, state, parent, key) {
// `super()` call.
// We must be inside class constructor as `super()` is illegal anywhere else,
// however could be inside an arrow function nested within the constructor.
// TODO Handle `super()` in class constructor params
// TODO: Handle `super()` in class constructor params
const {trail} = state,
// `trail[3] === 'body'` ensures `super()` is not in constructor's params
isTopLevelStatement = !isInArrowFunction && trail.length === 8 && trail[3] === 'body';
Expand Down
2 changes: 1 addition & 1 deletion lib/serialize/arguments.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,6 @@ module.exports = function serializeArguments(args, record) {
};

function argumentsShouldSkipKey(key) {
// TODO Would be better to include `Symbol.iterator` in default props
// TODO: Would be better to include `Symbol.iterator` in default props
return key === 'callee' || key === Symbol.iterator;
}
6 changes: 3 additions & 3 deletions lib/serialize/blocks.js
Original file line number Diff line number Diff line change
Expand Up @@ -713,14 +713,14 @@ module.exports = {
// If uses frozen `this` or `arguments`, wrap return value in an IIFE
// to inject these values as actual `this` / `arguments`.
// `() => 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
// 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.
// TODO: Ensure scope function using `this` is strict mode if value of `this` is not an object.
// In sloppy mode literals passed as `this` gets 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.
// TODO: Also doesn't work where `this` or `arguments` is circular and is injected late.
if (frozenThisVarName || frozenArgumentsVarName) {
const callArgsNodes = [];
let functionNode;
Expand Down
12 changes: 6 additions & 6 deletions lib/serialize/functions.js
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ module.exports = {
// In sloppy mode, `arguments` and the argument vars themselves are linked.
// e.g. `function f(x) { arguments[0] = 1; return () => x; }` sets `x` to `1`.
// Therefore any use of `arguments` makes all the vars in that function's arguments mutable.
// TODO Skip this if `arguments` is strict mode.
// TODO: Skip this if `arguments` is strict mode.
// NB Whether `arguments` is sloppy mode depends on whether function which *originates*
// `arguments` is strict mode, not on whether function in which `arguments[0] =`
// appears is strict mode.
Expand Down Expand Up @@ -202,7 +202,7 @@ module.exports = {
// `undefined` is ignored as it's a special case - may be omitted from `createScope()` call.
if (val !== undefined) createDependency(scopeRecord, valRecord, undefined, undefined);
} else if (!scopeValue.isCircular && recordIsCircular(valRecord)) {
// TODO I think this should be executed even if first check for existence of
// TODO: I think this should be executed even if first check for existence of
// `scopeValues[varName]` fails
scopeValue.isCircular = true;
}
Expand Down Expand Up @@ -472,15 +472,15 @@ module.exports = {
const fnVarName = record.varNode.name;
const fnRecord = this.serializeValue(fn, `${fnVarName}Unpromisified`, '<unpromisified>');

// TODO Handle circular references
// TODO: Handle circular references
assert(!recordIsCircular(fnRecord), 'Cannot handle circular-referenced promisified functions');

const promisifyRecord = this.serializeValue(util.promisify);
const node = t.callExpression(promisifyRecord.varNode, [fnRecord.varNode]);
createDependency(record, promisifyRecord, node, 'callee');
createDependency(record, fnRecord, node.arguments, 0);

// TODO Add additional properties
// TODO: Add additional properties
return node;
},

Expand All @@ -496,7 +496,7 @@ module.exports = {
* @returns {Object} - Node for debuglog function
*/
serializeDebuglogFunction(info, record) {
// TODO Handle circular references
// TODO: Handle circular references
const setRecord = this.serializeValue(info.set, 'debuglogSet', '<debuglog set argument>');
const argumentsNodes = [setRecord.varNode];
createDependency(record, setRecord, argumentsNodes, 0);
Expand All @@ -512,7 +512,7 @@ module.exports = {
const node = t.callExpression(debuglogRecord.varNode, argumentsNodes);
createDependency(record, debuglogRecord, node, 'callee');

// TODO Add additional properties
// TODO: Add additional properties
return node;
},

Expand Down
4 changes: 2 additions & 2 deletions lib/serialize/output.js
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,7 @@ module.exports = {

if (minify) {
// Patch generator to remove final semi-colon if not required
// TODO Remove this if https://github.com/babel/babel/issues/14160 is resolved
// TODO: Remove this if https://github.com/babel/babel/issues/14160 is resolved
const {Program} = generator;
generator.Program = function(programNode) {
Program.call(this, programNode);
Expand Down Expand Up @@ -451,7 +451,7 @@ module.exports = {
// - Async or generator function
// - Function is named (as it may refer to itself internally)
// - Function has parameters (as may use these params internally)
// TODO Could optimize last 2 cases by checking if function name/params
// TODO: Could optimize last 2 cases by checking if function name/params
// are referred to in function body, and unwrapping if not.
const node = exportNode.expression.callee;
if (
Expand Down
2 changes: 1 addition & 1 deletion lib/serialize/parseFunction.js
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ module.exports = function parseFunction(
// }`
const argsVarNode = t.identifier('args');
internalVars.args = [argsVarNode];
// TODO Can using `.unshift()` interfere with replacing const violations further on?
// TODO: Can using `.unshift()` interfere with replacing const violations further on?
// Indexes of any class properties will be altered.
memberNodes.unshift(t.classMethod(
'constructor',
Expand Down
2 changes: 1 addition & 1 deletion lib/serialize/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ const JS_ID_REGEX = /^[A-Za-z$_][A-Za-z0-9$_]*$/u;
/**
* Determine if string is valid number object key.
* e.g. `{0: 'a'}`, `{22: 'a'}`
* TODO Return true for other valid number keys e.g. `{1.2: 'a'}`
* TODO: Return true for other valid number keys e.g. `{1.2: 'a'}`
* @param {string} name - Input string
* @returns {boolean} - `true` if is valid number object key
*/
Expand Down
2 changes: 1 addition & 1 deletion lib/shared/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

// Exports

// TODO `COMMON_JS_LOCAL_VAR_NAMES` and `COMMON_JS_VAR_NAMES` don't need to be in shared consts
// TODO: `COMMON_JS_LOCAL_VAR_NAMES` and `COMMON_JS_VAR_NAMES` don't need to be in shared consts
const COMMON_JS_LOCAL_VAR_NAMES = ['module', 'exports', 'require'],
COMMON_JS_VAR_NAMES = [...COMMON_JS_LOCAL_VAR_NAMES, '__filename', '__dirname'];

Expand Down
4 changes: 2 additions & 2 deletions test/eval.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -444,7 +444,7 @@ describe('eval', () => {
});

// These tests don't work due to https://github.com/overlookmotel/livepack/issues/102
// TODO Uncomment these tests once that issue resolved.
// TODO: Uncomment these tests once that issue resolved.
// eslint-disable-next-line jest/no-commented-out-tests
/*
describe('defined in method key', () => {
Expand Down Expand Up @@ -768,7 +768,7 @@ describe('eval', () => {
// is *not* the arguments object of the outer function.
// It doesn't test what it is, because it's not performing exactly right.
// It's picking up the `arguments` object of the CJS loader function wrapping this module.
// TODO Fix this.
// TODO: Fix this.
in() {
function outer() {
return (0, eval)("() => typeof arguments === 'undefined' ? undefined : arguments");
Expand Down
Loading

0 comments on commit 30ed7b4

Please sign in to comment.