Skip to content

Commit

Permalink
Add no-await-in-promise-methods rule (#2259)
Browse files Browse the repository at this point in the history
Co-authored-by: fisker <[email protected]>
Co-authored-by: Sindre Sorhus <[email protected]>
  • Loading branch information
3 people authored Feb 22, 2024
1 parent 4594020 commit a3be554
Show file tree
Hide file tree
Showing 7 changed files with 390 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 @@ -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

0 comments on commit a3be554

Please sign in to comment.