Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add no-await-in-promise-methods rule #2259

Merged
merged 18 commits into from
Feb 22, 2024
Merged
1 change: 1 addition & 0 deletions configs/recommended.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ module.exports = {
'unicorn/no-array-push-push': 'error',
'unicorn/no-array-reduce': 'error',
'unicorn/no-await-expression-member': 'error',
'unicorn/no-await-in-promise-methods': 'error',
'unicorn/no-console-spaces': 'error',
'unicorn/no-document-cookie': 'error',
'unicorn/no-empty-file': 'error',
Expand Down
34 changes: 34 additions & 0 deletions docs/rules/no-await-in-promise-methods.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# Disallow using `await` in `Promise` method parameters

💼 This rule is enabled in the ✅ `recommended` [config](https://github.com/sindresorhus/eslint-plugin-unicorn#preset-configs).

💡 This rule is manually fixable by [editor suggestions](https://eslint.org/docs/developer-guide/working-with-rules#providing-suggestions).

<!-- end auto-generated rule header -->
<!-- Do not manually modify this header. Run: `npm run fix:eslint-docs` -->

Using `await` on promises passed as arguments to `Promise.all()`, `Promise.allSettled()`, `Promise.any()`, or `Promise.race()` is likely a mistake.

## Fail

```js
Promise.all([await promise, anotherPromise]);

Promise.allSettled([await promise, anotherPromise]);

Promise.any([await promise, anotherPromise]);

Promise.race([await promise, anotherPromise]);
```

## Pass

```js
Promise.all([promise, anotherPromise]);

Promise.allSettled([promise, anotherPromise]);

Promise.any([promise, anotherPromise]);

Promise.race([promise, anotherPromise]);
```
1 change: 1 addition & 0 deletions readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ If you don't use the preset, ensure you use the same `env` and `parserOptions` c
| [no-array-push-push](docs/rules/no-array-push-push.md) | Enforce combining multiple `Array#push()` into one call. | ✅ | 🔧 | 💡 |
| [no-array-reduce](docs/rules/no-array-reduce.md) | Disallow `Array#reduce()` and `Array#reduceRight()`. | ✅ | | |
| [no-await-expression-member](docs/rules/no-await-expression-member.md) | Disallow member access from await expression. | ✅ | 🔧 | |
| [no-await-in-promise-methods](docs/rules/no-await-in-promise-methods.md) | Disallow using `await` in `Promise` method parameters. | ✅ | | 💡 |
| [no-console-spaces](docs/rules/no-console-spaces.md) | Do not use leading/trailing space between `console.log` parameters. | ✅ | 🔧 | |
| [no-document-cookie](docs/rules/no-document-cookie.md) | Do not use `document.cookie` directly. | ✅ | | |
| [no-empty-file](docs/rules/no-empty-file.md) | Disallow empty files. | ✅ | | |
Expand Down
68 changes: 68 additions & 0 deletions rules/no-await-in-promise-methods.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
'use strict';
const {isMethodCall} = require('./ast/index.js');
const {removeSpacesAfter} = require('./fix/index.js');

const MESSAGE_ID_ERROR = 'no-await-in-promise-methods/error';
const MESSAGE_ID_SUGGESTION = 'no-await-in-promise-methods/suggestion';
const messages = {
[MESSAGE_ID_ERROR]: 'Promise in `Promise.{{method}}()` should not be awaited.',
[MESSAGE_ID_SUGGESTION]: 'Remove `await`.',
};
const METHODS = ['all', 'allSettled', 'any', 'race'];

const isPromiseMethodCallWithArrayExpression = node =>
isMethodCall(node, {
object: 'Promise',
methods: METHODS,
optionalMember: false,
optionalCall: false,
argumentsLength: 1,
})
&& node.arguments[0].type === 'ArrayExpression';

/** @param {import('eslint').Rule.RuleContext} context */
const create = context => ({
* CallExpression(callExpression) {
if (!isPromiseMethodCallWithArrayExpression(callExpression)) {
return;
}

for (const element of callExpression.arguments[0].elements) {
if (element?.type !== 'AwaitExpression') {
continue;
}

yield {
node: element,
messageId: MESSAGE_ID_ERROR,
data: {
method: callExpression.callee.property.name,
},
suggest: [
{
messageId: MESSAGE_ID_SUGGESTION,
* fix(fixer) {
const {sourceCode} = context;
const awaitToken = context.sourceCode.getFirstToken(element);
yield fixer.remove(awaitToken);
yield removeSpacesAfter(awaitToken, sourceCode, fixer);
},
},
],
};
}
},
});

/** @type {import('eslint').Rule.RuleModule} */
module.exports = {
create,
meta: {
type: 'suggestion',
docs: {
description: 'Disallow using `await` in `Promise` method parameters.',
},
hasSuggestions: true,
messages,
},
};
42 changes: 42 additions & 0 deletions test/no-await-in-promise-methods.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
import {getTester} from './utils/test.mjs';

const {test} = getTester(import.meta);

test.snapshot({
valid: [
'Promise.all([promise1, promise2, promise3, promise4])',
'Promise.allSettled([promise1, promise2, promise3, promise4])',
'Promise.any([promise1, promise2, promise3, promise4])',
'Promise.race([promise1, promise2, promise3, promise4])',
'Promise.all(...[await promise])',
'Promise.all([await promise], extraArguments)',
'Promise.all()',
'Promise.all(notArrayExpression)',
'Promise.all([,])',
'Promise[all]([await promise])',
'Promise.all?.([await promise])',
'Promise?.all([await promise])',
'Promise.notListedMethod([await promise])',
'NotPromise.all([await promise])',
'Promise.all([(await promise, 0)])',
'new Promise.all([await promise])',

// We are not checking these cases
'globalThis.Promise.all([await promise])',
'Promise["all"]([await promise])',
],
invalid: [
'Promise.all([await promise])',
'Promise.allSettled([await promise])',
'Promise.any([await promise])',
'Promise.race([await promise])',
'Promise.all([, await promise])',
'Promise.all([await promise,])',
'Promise.all([await promise],)',
'Promise.all([await (0, promise)],)',
'Promise.all([await (( promise ))])',
'Promise.all([await await promise])',
'Promise.all([...foo, await promise1, await promise2])',
'Promise.all([await /* comment*/ promise])',
],
});
Loading
Loading