Skip to content

Commit

Permalink
Add no-useless-promise-resolve-reject rule (#1623)
Browse files Browse the repository at this point in the history
Co-authored-by: Sindre Sorhus <sindresorhus@gmail.com>
Co-authored-by: fisker Cheung <lionkay@gmail.com>
  • Loading branch information
3 people committed Dec 27, 2021
1 parent c318644 commit 054436e
Show file tree
Hide file tree
Showing 5 changed files with 688 additions and 0 deletions.
1 change: 1 addition & 0 deletions configs/recommended.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ module.exports = {
'unicorn/no-unused-properties': 'off',
'unicorn/no-useless-fallback-in-spread': 'error',
'unicorn/no-useless-length-check': 'error',
'unicorn/no-useless-promise-resolve-reject': 'error',
'unicorn/no-useless-spread': 'error',
'unicorn/no-useless-undefined': 'error',
'unicorn/no-zero-fractions': 'error',
Expand Down
41 changes: 41 additions & 0 deletions docs/rules/no-useless-promise-resolve-reject.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
# Disallow returning/yielding `Promise.resolve/reject()` in async functions

*This rule is part of the [recommended](https://github.com/sindresorhus/eslint-plugin-unicorn#recommended-config) config.*

🔧 *This rule is [auto-fixable](https://eslint.org/docs/user-guide/command-line-interface#fixing-problems).*

Wrapping a return value in `Promise.resolve` in an async function is unnecessary as all return values of an async function are already wrapped in a `Promise`. Similarly, returning an error wrapped in `Promise.reject` is equivalent to simply `throw`ing the error. This is the same for `yield`ing in async generators as well.

## Fail

```js
const main = async foo => {
if (foo > 4) {
return Promise.reject(new Error('🤪'));
}

return Promise.resolve(result);
};

async function * generator() {
yield Promise.resolve(result);
yield Promise.reject(error);
}
```

## Pass

```js
const main = async foo => {
if (foo > 4) {
throw new Error('🤪');
}

return result;
};

async function * generator() {
yield result;
throw error;
}
```
2 changes: 2 additions & 0 deletions readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ Configure it in `package.json`.
"unicorn/no-unused-properties": "off",
"unicorn/no-useless-fallback-in-spread": "error",
"unicorn/no-useless-length-check": "error",
"unicorn/no-useless-promise-resolve-reject": "error",
"unicorn/no-useless-spread": "error",
"unicorn/no-useless-undefined": "error",
"unicorn/no-zero-fractions": "error",
Expand Down Expand Up @@ -195,6 +196,7 @@ Each rule has emojis denoting:
| [no-unused-properties](docs/rules/no-unused-properties.md) | Disallow unused object properties. | | | |
| [no-useless-fallback-in-spread](docs/rules/no-useless-fallback-in-spread.md) | Forbid useless fallback when spreading in object literals. || 🔧 | |
| [no-useless-length-check](docs/rules/no-useless-length-check.md) | Disallow useless array length check. || 🔧 | |
| [no-useless-promise-resolve-reject](docs/rules/no-useless-promise-resolve-reject.md) | Disallow returning/yielding `Promise.resolve/reject()` in async functions || 🔧 | |
| [no-useless-spread](docs/rules/no-useless-spread.md) | Disallow unnecessary spread. || 🔧 | |
| [no-useless-undefined](docs/rules/no-useless-undefined.md) | Disallow useless `undefined`. || 🔧 | |
| [no-zero-fractions](docs/rules/no-zero-fractions.md) | Disallow number literals with zero fractions or dangling dots. || 🔧 | |
Expand Down
171 changes: 171 additions & 0 deletions rules/no-useless-promise-resolve-reject.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,171 @@
'use strict';
const {matches, methodCallSelector} = require('./selectors/index.js');
const {getParenthesizedRange} = require('./utils/parentheses.js');

const MESSAGE_ID_RESOLVE = 'resolve';
const MESSAGE_ID_REJECT = 'reject';
const messages = {
[MESSAGE_ID_RESOLVE]: 'Prefer `{{type}} value` over `{{type}} Promise.resolve(error)`.',
[MESSAGE_ID_REJECT]: 'Prefer `throw error` over `{{type}} Promise.reject(error)`.',
};

const selector = [
methodCallSelector({
object: 'Promise',
methods: ['resolve', 'reject'],
}),
matches([
'ArrowFunctionExpression[async=true] > .body',
'ReturnStatement > .argument',
'YieldExpression[delegate=false] > .argument',
]),
].join('');

const functionTypes = new Set([
'ArrowFunctionExpression',
'FunctionDeclaration',
'FunctionExpression',
]);
function getFunctionNode(node) {
let isInTryStatement = false;
let functionNode;
for (; node; node = node.parent) {
if (functionTypes.has(node.type)) {
functionNode = node;
break;
}

if (node.type === 'TryStatement') {
isInTryStatement = true;
}
}

return {
functionNode,
isInTryStatement,
};
}

function createProblem(callExpression, fix) {
const {callee, parent} = callExpression;
const method = callee.property.name;
const type = parent.type === 'YieldExpression' ? 'yield' : 'return';

return {
node: callee,
messageId: method,
data: {type},
fix,
};
}

function fix(callExpression, isInTryStatement, sourceCode) {
if (callExpression.arguments.length > 1) {
return;
}

const {callee, parent, arguments: [errorOrValue]} = callExpression;
if (errorOrValue && errorOrValue.type === 'SpreadElement') {
return;
}

const isReject = callee.property.name === 'reject';
const isYieldExpression = parent.type === 'YieldExpression';
if (
isReject
&& (
isInTryStatement
|| (isYieldExpression && parent.parent.type !== 'ExpressionStatement')
)
) {
return;
}

return function (fixer) {
const isArrowFunctionBody = parent.type === 'ArrowFunctionExpression';

let text = errorOrValue ? sourceCode.getText(errorOrValue) : '';

if (errorOrValue && errorOrValue.type === 'SequenceExpression') {
text = `(${text})`;
}

if (isReject) {
// `return Promise.reject()` -> `throw undefined`
text = text || 'undefined';
text = `throw ${text}`;

if (isYieldExpression) {
return fixer.replaceTextRange(
getParenthesizedRange(parent, sourceCode),
text,
);
}

text += ';';

// `=> Promise.reject(error)` -> `=> { throw error; }`
if (isArrowFunctionBody) {
text = `{ ${text} }`;
return fixer.replaceTextRange(
getParenthesizedRange(callExpression, sourceCode),
text,
);
}
} else {
// eslint-disable-next-line no-lonely-if
if (isYieldExpression) {
text = `yield${text ? ' ' : ''}${text}`;
} else if (parent.type === 'ReturnStatement') {
text = `return${text ? ' ' : ''}${text};`;
} else {
if (errorOrValue && errorOrValue.type === 'ObjectExpression') {
text = `(${text})`;
}

// `=> Promise.resolve()` -> `=> {}`
text = text || '{}';
}
}

return fixer.replaceText(
isArrowFunctionBody ? callExpression : parent,
text,
);
};
}

/** @param {import('eslint').Rule.RuleContext} context */
const create = context => {
const sourceCode = context.getSourceCode();

return {
[selector](callExpression) {
const {functionNode, isInTryStatement} = getFunctionNode(callExpression);
if (!functionNode || !functionNode.async) {
return;
}

return createProblem(
callExpression,
fix(callExpression, isInTryStatement, sourceCode),
);
},
};
};

const schema = [];

/** @type {import('eslint').Rule.RuleModule} */
module.exports = {
create,
meta: {
type: 'suggestion',
docs: {
description: 'Disallow returning/yielding `Promise.resolve/reject()` in async functions',
},
fixable: 'code',
schema,
messages,
},
};

0 comments on commit 054436e

Please sign in to comment.