Skip to content

Commit

Permalink
[eslint] prevent async Promise constructor mistakes (elastic#110349)
Browse files Browse the repository at this point in the history
Co-authored-by: spalger <spalger@users.noreply.github.com>
  • Loading branch information
2 people authored and kibanamachine committed Aug 31, 2021
1 parent 2c5613b commit 81be66a
Show file tree
Hide file tree
Showing 32 changed files with 1,404 additions and 773 deletions.
2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -657,6 +657,7 @@
"@types/yauzl": "^2.9.1",
"@types/zen-observable": "^0.8.0",
"@typescript-eslint/eslint-plugin": "^4.14.1",
"@typescript-eslint/typescript-estree": "^4.14.1",
"@typescript-eslint/parser": "^4.14.1",
"@yarnpkg/lockfile": "^1.1.0",
"abab": "^2.0.4",
Expand Down Expand Up @@ -727,6 +728,7 @@
"eslint-plugin-react": "^7.20.3",
"eslint-plugin-react-hooks": "^4.2.0",
"eslint-plugin-react-perf": "^3.2.3",
"eslint-traverse": "^1.0.0",
"expose-loader": "^0.7.5",
"faker": "^5.1.0",
"fancy-log": "^1.3.2",
Expand Down
2 changes: 2 additions & 0 deletions packages/elastic-eslint-config-kibana/.eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -90,5 +90,7 @@ module.exports = {
},
],
],

'@kbn/eslint/no_async_promise_body': 'error',
},
};
1 change: 1 addition & 0 deletions packages/kbn-eslint-plugin-eslint/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,6 @@ module.exports = {
'disallow-license-headers': require('./rules/disallow_license_headers'),
'no-restricted-paths': require('./rules/no_restricted_paths'),
module_migration: require('./rules/module_migration'),
no_async_promise_body: require('./rules/no_async_promise_body'),
},
};
165 changes: 165 additions & 0 deletions packages/kbn-eslint-plugin-eslint/rules/no_async_promise_body.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,165 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

const { parseExpression } = require('@babel/parser');
const { default: generate } = require('@babel/generator');
const tsEstree = require('@typescript-eslint/typescript-estree');
const traverse = require('eslint-traverse');
const esTypes = tsEstree.AST_NODE_TYPES;
const babelTypes = require('@babel/types');

/** @typedef {import("eslint").Rule.RuleModule} Rule */
/** @typedef {import("@typescript-eslint/parser").ParserServices} ParserServices */
/** @typedef {import("@typescript-eslint/typescript-estree").TSESTree.Expression} Expression */
/** @typedef {import("@typescript-eslint/typescript-estree").TSESTree.ArrowFunctionExpression} ArrowFunctionExpression */
/** @typedef {import("@typescript-eslint/typescript-estree").TSESTree.FunctionExpression} FunctionExpression */
/** @typedef {import("@typescript-eslint/typescript-estree").TSESTree.TryStatement} TryStatement */
/** @typedef {import("@typescript-eslint/typescript-estree").TSESTree.NewExpression} NewExpression */
/** @typedef {import("typescript").ExportDeclaration} ExportDeclaration */
/** @typedef {import("eslint").Rule.RuleFixer} Fixer */

const ERROR_MSG =
'Passing an async function to the Promise constructor leads to a hidden promise being created and prevents handling rejections';

/**
* @param {Expression} node
*/
const isPromise = (node) => node.type === esTypes.Identifier && node.name === 'Promise';

/**
* @param {Expression} node
* @returns {node is ArrowFunctionExpression | FunctionExpression}
*/
const isFunc = (node) =>
node.type === esTypes.ArrowFunctionExpression || node.type === esTypes.FunctionExpression;

/**
* @param {any} context
* @param {ArrowFunctionExpression | FunctionExpression} node
*/
const isFuncBodySafe = (context, node) => {
// if the body isn't wrapped in a blockStatement it can't have a try/catch at the root
if (node.body.type !== esTypes.BlockStatement) {
return false;
}

// when the entire body is wrapped in a try/catch it is the only node
if (node.body.body.length !== 1) {
return false;
}

const tryNode = node.body.body[0];
// ensure we have a try node with a handler
if (tryNode.type !== esTypes.TryStatement || !tryNode.handler) {
return false;
}

// ensure the handler doesn't throw
let hasThrow = false;
traverse(context, tryNode.handler, (path) => {
if (path.node.type === esTypes.ThrowStatement) {
hasThrow = true;
return traverse.STOP;
}
});
return !hasThrow;
};

/**
* @param {string} code
*/
const wrapFunctionInTryCatch = (code) => {
// parse the code with babel so we can mutate the AST
const ast = parseExpression(code, {
plugins: ['typescript', 'jsx'],
});

// validate that the code reperesents an arrow or function expression
if (!babelTypes.isArrowFunctionExpression(ast) && !babelTypes.isFunctionExpression(ast)) {
throw new Error('expected function to be an arrow or function expression');
}

// ensure that the function receives the second argument, and capture its name if already defined
let rejectName = 'reject';
if (ast.params.length === 0) {
ast.params.push(babelTypes.identifier('resolve'), babelTypes.identifier(rejectName));
} else if (ast.params.length === 1) {
ast.params.push(babelTypes.identifier(rejectName));
} else if (ast.params.length === 2) {
if (babelTypes.isIdentifier(ast.params[1])) {
rejectName = ast.params[1].name;
} else {
throw new Error('expected second param of promise definition function to be an identifier');
}
}

// ensure that the body of the function is a blockStatement
let block = ast.body;
if (!babelTypes.isBlockStatement(block)) {
block = babelTypes.blockStatement([babelTypes.returnStatement(block)]);
}

// redefine the body of the function as a new blockStatement containing a tryStatement
// which catches errors and forwards them to reject() when caught
ast.body = babelTypes.blockStatement([
// try {
babelTypes.tryStatement(
block,
// catch (error) {
babelTypes.catchClause(
babelTypes.identifier('error'),
babelTypes.blockStatement([
// reject(error)
babelTypes.expressionStatement(
babelTypes.callExpression(babelTypes.identifier(rejectName), [
babelTypes.identifier('error'),
])
),
])
)
),
]);

return generate(ast).code;
};

/** @type {Rule} */
module.exports = {
meta: {
fixable: 'code',
schema: [],
},
create: (context) => ({
NewExpression(_) {
const node = /** @type {NewExpression} */ (_);

// ensure we are newing up a promise with a single argument
if (!isPromise(node.callee) || node.arguments.length !== 1) {
return;
}

const func = node.arguments[0];
// ensure the argument is an arrow or function expression and is async
if (!isFunc(func) || !func.async) {
return;
}

// body must be a blockStatement, try/catch can't exist outside of a block
if (!isFuncBodySafe(context, func)) {
context.report({
message: ERROR_MSG,
loc: func.loc,
fix(fixer) {
const source = context.getSourceCode();
return fixer.replaceText(func, wrapFunctionInTryCatch(source.getText(func)));
},
});
}
},
}),
};
Loading

0 comments on commit 81be66a

Please sign in to comment.