Skip to content

Commit

Permalink
Merge pull request #108 from endojs/mfig-107-eslint-await
Browse files Browse the repository at this point in the history
ESLint `await` rule cleanup
  • Loading branch information
michaelfig committed Jun 1, 2023
2 parents 5924ac7 + 2d5ee6f commit 36c9c20
Show file tree
Hide file tree
Showing 11 changed files with 333 additions and 139 deletions.
5 changes: 5 additions & 0 deletions packages/eslint-plugin/NEWS.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
User-visible changes in `@endo/eslint-plugin`

# v0.4.4 (2023-04-14)

- Separate rules into subsets (documented in README.md)
16 changes: 16 additions & 0 deletions packages/eslint-plugin/jsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
{
"compilerOptions": {
"baseUrl": ".",
"moduleResolution": "node",
"target": "esnext",
"module": "CommonJS",
"noImplicitAny": true,
"noEmit": true,
"noUnusedLocals": true,
"noUnusedParameters": true,
"alwaysStrict": true,
"downlevelIteration": true
},
"include": ["lib/**/*.js"],
"exclude": ["node_modules/**"]
}
3 changes: 3 additions & 0 deletions packages/eslint-plugin/lib/configs/recommended.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,7 @@
module.exports = {
plugins: ['@jessie.js'],
processor: '@jessie.js/use-jessie',
rules: {
'@jessie.js/safe-await-separator': 'warn',
},
};
113 changes: 15 additions & 98 deletions packages/eslint-plugin/lib/rules/no-nested-await.js
Original file line number Diff line number Diff line change
@@ -1,27 +1,20 @@
// @ts-check
/* eslint-env node */

/**
* @author Michael FIG
* See LICENSE file in root directory for full license.
*/

'use strict';

const isFunctionLike = node =>
[
'ArrowFunctionExpression',
'FunctionExpression',

'FunctionDeclaration',
'MethodDefinition',

'Program',
'StaticBlock',
].includes(node.type);
const { makeAwaitAllowedVisitor } = require('../tools.js');

/** @type {import('eslint').Rule.RuleModule} */
module.exports = {
meta: {
docs: {
description: `ensure the first \`await\` in an \`async\` function is non-nested so that it is clear when the synchronous portion of the function is finished`,
description: `ensure all \`await\`s in an \`async\` function are not nested`,
category: 'Possible Errors',
recommended: true,
url:
Expand All @@ -30,97 +23,21 @@ module.exports = {
type: 'problem',
fixable: null,
messages: {
unexpectedNestedAwait:
'The first `await` appearing in an async function must not be nested',
unexpectedNestedAwait: 'Nested `await`s are not permitted in Jessie',
},
schema: [],
supported: true,
},
create(context) {
const foundFirstAwait = new WeakSet();

const isAwaitAllowedInNode = node => {
// Climb the AST until finding the nearest enclosing function or
// function-like node.
let result = true;
for (; node && !isFunctionLike(node); node = node.parent) {
if (foundFirstAwait.has(node)) {
return true;
}
foundFirstAwait.add(node);

// Encountering anything other than an enclosing block is grounds for
// rejection.
if (node.type !== 'BlockStatement') {
result = false;
}
}

if (!node) {
// Should never reach here because Program is a FunctionLike for
// top-level await.
throw new Error(`unexpected non-Program AST!`);
}
return result;
};

return {
/**
* An `await` expression is treated as non-nested if it is:
* - a non-nested expression statement,
* - the right-hand side of a non-nested declarator, or
* - the right-hand side of an assignment expression.
*
* As a hint to future readers, the following ASTExplorer workspace
* reveals some of the relevant AST shapes (note that node.parent is not
* displayed in the ASTExplorer, but is available in the ESLint visitor):
*
* @see https://astexplorer.net/#/gist/4508eec25a8d5be1e0248c4cc06b9634/f6b22f2e8e3abd82a911ca6286a304ef0a3018c4
*
* This will need different handling for do expressions if they are ever
* added to the language.
* @see https://github.com/tc39/proposal-do-expressions
*
* @param {*} node
*/
AwaitExpression: node => {
let parent = node.parent;
if (parent.type === 'VariableDeclarator' && parent.init === node) {
// It's a declarator, so look up to the declaration statement's parent.
parent = parent.parent.parent;
} else if (
parent.type === 'AssignmentExpression' &&
parent.right === node &&
parent.operator === '='
) {
// It's an assignment, so look up to the assigment's parent.
parent = parent.parent;
}
if (parent.type === 'ExpressionStatement') {
// Try to find the parent block.
parent = parent.parent;
}

if (!isAwaitAllowedInNode(parent)) {
context.report({
node,
messageId: 'unexpectedNestedAwait',
});
}
},
/**
* A `for-await-of` loop is treated as a simple `await` statement.
*
* @param {*} node
*/
'ForOfStatement[await=true]': node => {
if (!isAwaitAllowedInNode(node.parent)) {
context.report({
node,
messageId: 'unexpectedNestedAwait',
});
}
},
/**
* @param {import('eslint').Rule.Node} node
*/
const makeReport = node => {
context.report({
node,
messageId: 'unexpectedNestedAwait',
});
};
return makeAwaitAllowedVisitor(context, makeReport);
},
};
60 changes: 60 additions & 0 deletions packages/eslint-plugin/lib/rules/safe-await-separator.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
// @ts-check
/* eslint-env node */

/**
* @author Michael FIG
* See LICENSE file in root directory for full license.
*/

'use strict';

const { isFunctionLike, makeAwaitAllowedVisitor } = require('../tools.js');

/** @type {import('eslint').Rule.RuleModule} */
module.exports = {
meta: {
docs: {
description: `ensure the first \`await\` in an \`async\` function is non-nested so that it is clear when the synchronous portion of the function is finished`,
category: 'Possible Errors',
recommended: true,
url:
'https://github.com/endojs/Jessie/blob/main/packages/eslint-plugin/lib/rules/safe-await-separator',
},
type: 'problem',
fixable: null,
hasSuggestions: true,
messages: {
unsafeAwaitSeparator:
'The first `await` appearing in an async function must not be nested',
insertAwaitNull: 'Insert `await null;` before the first `await`',
},
schema: [],
supported: true,
},
create(context) {
/**
* @param {import('eslint').Rule.Node} node
*/
const makeReport = node => {
context.report({
node,
messageId: 'unsafeAwaitSeparator',
suggest: [
{
messageId: 'insertAwaitNull',
fix: fixer => {
let cur = node;
let parent = cur.parent;
while (!isFunctionLike(parent.parent)) {
cur = parent;
parent = cur.parent;
}
return fixer.insertTextBefore(cur, 'await null;');
},
},
],
});
};
return makeAwaitAllowedVisitor(context, makeReport, true);
},
};
135 changes: 135 additions & 0 deletions packages/eslint-plugin/lib/tools.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
// @ts-check
/* eslint-env node */

'use strict';

/** @typedef {import('eslint').Rule.Node} Node */

const harden = Object.freeze;

/** @type {Node['type'][]} */
const functionLikeNodeTypes = [
'ArrowFunctionExpression',
'FunctionExpression',

'FunctionDeclaration',
'MethodDefinition',

'Program',
'StaticBlock',
];
harden(functionLikeNodeTypes);

/**
* @param {Node} node
*/
const isFunctionLike = node => functionLikeNodeTypes.includes(node.type);
harden(isFunctionLike);

/**
* @param {Node} node
* @param {WeakSet<Node>} already
* @returns {boolean}
*/
const isAwaitAllowedInNode = (node, already = new WeakSet()) => {
// Climb the AST until finding the nearest enclosing function or
// function-like node.
let result = true;
for (; node && !isFunctionLike(node); node = node.parent) {
if (already.has(node)) {
return true;
}
already.add(node);

if (node.type === 'BlockStatement') {
// do nothing
} else if (node.type === 'ForOfStatement' && node.await) {
// do nothing
} else {
// Encountering anything other than an enclosing block is grounds for
// rejection.
result = false;
}
}

if (!node) {
// Should never reach here because Program is a FunctionLike for
// top-level await.
throw new Error(`unexpected non-Program AST!`);
}

return result;
};
harden(isAwaitAllowedInNode);

/**
* @param {import('eslint').Rule.RuleContext} _context
* @param {(node: Node) => void} makeReport
* @param {boolean} [addToCache]
*/
const makeAwaitAllowedVisitor = (
_context,
makeReport,
addToCache = undefined,
) => {
const already = addToCache ? new WeakSet() : undefined;
return harden({
/**
* An `await` expression is treated as non-nested if it is:
* - a non-nested expression statement,
* - the right-hand side of a non-nested declarator, or
* - the right-hand side of an assignment expression.
*
* As a hint to future readers, the following ASTExplorer workspace
* reveals some of the relevant AST shapes (note that node.parent is not
* displayed in the ASTExplorer, but is available in the ESLint visitor):
*
* @see https://astexplorer.net/#/gist/4508eec25a8d5be1e0248c4cc06b9634/f6b22f2e8e3abd82a911ca6286a304ef0a3018c4
*
* This will need different handling for do expressions if they are ever
* added to the language.
* @see https://github.com/tc39/proposal-do-expressions
*
* @param {Node} node
*/
AwaitExpression: node => {
let parent = node.parent;
if (parent.type === 'VariableDeclarator' && parent.init === node) {
// It's a declarator, so look up to the declaration statement's parent.
parent = parent.parent.parent;
} else if (
parent.type === 'AssignmentExpression' &&
parent.right === node &&
parent.operator === '='
) {
// It's an assignment, so look up to the assigment's parent.
parent = parent.parent;
}
if (parent.type === 'ExpressionStatement') {
// Try to find the parent block.
parent = parent.parent;
}

if (!isAwaitAllowedInNode(parent, already)) {
makeReport(node);
}
},
/**
* A `for-await-of` loop is treated as a simple `await` statement.
*
* @param {Node} node
*/
'ForOfStatement[await=true]': node => {
if (!isAwaitAllowedInNode(node.parent, already)) {
makeReport(node);
}
},
});
};
harden(makeAwaitAllowedVisitor);

module.exports = harden({
isFunctionLike,
isAwaitAllowedInNode,
makeAwaitAllowedVisitor,
});
Loading

0 comments on commit 36c9c20

Please sign in to comment.