From a3be554109a3d834e2a4de4d921bee0d5f6ff051 Mon Sep 17 00:00:00 2001 From: Clement398 <90264924+Clement398@users.noreply.github.com> Date: Thu, 22 Feb 2024 08:00:00 +0100 Subject: [PATCH] Add `no-await-in-promise-methods` rule (#2259) Co-authored-by: fisker Co-authored-by: Sindre Sorhus --- configs/recommended.js | 1 + docs/rules/no-await-in-promise-methods.md | 34 +++ readme.md | 1 + rules/no-await-in-promise-methods.js | 68 +++++ test/no-await-in-promise-methods.mjs | 42 +++ .../no-await-in-promise-methods.mjs.md | 244 ++++++++++++++++++ .../no-await-in-promise-methods.mjs.snap | Bin 0 -> 723 bytes 7 files changed, 390 insertions(+) create mode 100644 docs/rules/no-await-in-promise-methods.md create mode 100644 rules/no-await-in-promise-methods.js create mode 100644 test/no-await-in-promise-methods.mjs create mode 100644 test/snapshots/no-await-in-promise-methods.mjs.md create mode 100644 test/snapshots/no-await-in-promise-methods.mjs.snap diff --git a/configs/recommended.js b/configs/recommended.js index 67f99a8472..b4f4e8f9d6 100644 --- a/configs/recommended.js +++ b/configs/recommended.js @@ -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', diff --git a/docs/rules/no-await-in-promise-methods.md b/docs/rules/no-await-in-promise-methods.md new file mode 100644 index 0000000000..1da102aa98 --- /dev/null +++ b/docs/rules/no-await-in-promise-methods.md @@ -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). + + + + +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]); +``` diff --git a/readme.md b/readme.md index 47c31feef5..e8194a8097 100644 --- a/readme.md +++ b/readme.md @@ -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. | ✅ | | | diff --git a/rules/no-await-in-promise-methods.js b/rules/no-await-in-promise-methods.js new file mode 100644 index 0000000000..4e428558bd --- /dev/null +++ b/rules/no-await-in-promise-methods.js @@ -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, + }, +}; diff --git a/test/no-await-in-promise-methods.mjs b/test/no-await-in-promise-methods.mjs new file mode 100644 index 0000000000..b439f2fd89 --- /dev/null +++ b/test/no-await-in-promise-methods.mjs @@ -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])', + ], +}); diff --git a/test/snapshots/no-await-in-promise-methods.mjs.md b/test/snapshots/no-await-in-promise-methods.mjs.md new file mode 100644 index 0000000000..eb098f193d --- /dev/null +++ b/test/snapshots/no-await-in-promise-methods.mjs.md @@ -0,0 +1,244 @@ +# Snapshot report for `test/no-await-in-promise-methods.mjs` + +The actual snapshot is saved in `no-await-in-promise-methods.mjs.snap`. + +Generated by [AVA](https://avajs.dev). + +## invalid(1): Promise.all([await promise]) + +> Input + + `␊ + 1 | Promise.all([await promise])␊ + ` + +> Error 1/1 + + `␊ + > 1 | Promise.all([await promise])␊ + | ^^^^^^^^^^^^^ Promise in \`Promise.all()\` should not be awaited.␊ + ␊ + --------------------------------------------------------------------------------␊ + Suggestion 1/1: Remove \`await\`.␊ + 1 | Promise.all([promise])␊ + ` + +## invalid(2): Promise.allSettled([await promise]) + +> Input + + `␊ + 1 | Promise.allSettled([await promise])␊ + ` + +> Error 1/1 + + `␊ + > 1 | Promise.allSettled([await promise])␊ + | ^^^^^^^^^^^^^ Promise in \`Promise.allSettled()\` should not be awaited.␊ + ␊ + --------------------------------------------------------------------------------␊ + Suggestion 1/1: Remove \`await\`.␊ + 1 | Promise.allSettled([promise])␊ + ` + +## invalid(3): Promise.any([await promise]) + +> Input + + `␊ + 1 | Promise.any([await promise])␊ + ` + +> Error 1/1 + + `␊ + > 1 | Promise.any([await promise])␊ + | ^^^^^^^^^^^^^ Promise in \`Promise.any()\` should not be awaited.␊ + ␊ + --------------------------------------------------------------------------------␊ + Suggestion 1/1: Remove \`await\`.␊ + 1 | Promise.any([promise])␊ + ` + +## invalid(4): Promise.race([await promise]) + +> Input + + `␊ + 1 | Promise.race([await promise])␊ + ` + +> Error 1/1 + + `␊ + > 1 | Promise.race([await promise])␊ + | ^^^^^^^^^^^^^ Promise in \`Promise.race()\` should not be awaited.␊ + ␊ + --------------------------------------------------------------------------------␊ + Suggestion 1/1: Remove \`await\`.␊ + 1 | Promise.race([promise])␊ + ` + +## invalid(5): Promise.all([, await promise]) + +> Input + + `␊ + 1 | Promise.all([, await promise])␊ + ` + +> Error 1/1 + + `␊ + > 1 | Promise.all([, await promise])␊ + | ^^^^^^^^^^^^^ Promise in \`Promise.all()\` should not be awaited.␊ + ␊ + --------------------------------------------------------------------------------␊ + Suggestion 1/1: Remove \`await\`.␊ + 1 | Promise.all([, promise])␊ + ` + +## invalid(6): Promise.all([await promise,]) + +> Input + + `␊ + 1 | Promise.all([await promise,])␊ + ` + +> Error 1/1 + + `␊ + > 1 | Promise.all([await promise,])␊ + | ^^^^^^^^^^^^^ Promise in \`Promise.all()\` should not be awaited.␊ + ␊ + --------------------------------------------------------------------------------␊ + Suggestion 1/1: Remove \`await\`.␊ + 1 | Promise.all([promise,])␊ + ` + +## invalid(7): Promise.all([await promise],) + +> Input + + `␊ + 1 | Promise.all([await promise],)␊ + ` + +> Error 1/1 + + `␊ + > 1 | Promise.all([await promise],)␊ + | ^^^^^^^^^^^^^ Promise in \`Promise.all()\` should not be awaited.␊ + ␊ + --------------------------------------------------------------------------------␊ + Suggestion 1/1: Remove \`await\`.␊ + 1 | Promise.all([promise],)␊ + ` + +## invalid(8): Promise.all([await (0, promise)],) + +> Input + + `␊ + 1 | Promise.all([await (0, promise)],)␊ + ` + +> Error 1/1 + + `␊ + > 1 | Promise.all([await (0, promise)],)␊ + | ^^^^^^^^^^^^^^^^^^ Promise in \`Promise.all()\` should not be awaited.␊ + ␊ + --------------------------------------------------------------------------------␊ + Suggestion 1/1: Remove \`await\`.␊ + 1 | Promise.all([(0, promise)],)␊ + ` + +## invalid(9): Promise.all([await (( promise ))]) + +> Input + + `␊ + 1 | Promise.all([await (( promise ))])␊ + ` + +> Error 1/1 + + `␊ + > 1 | Promise.all([await (( promise ))])␊ + | ^^^^^^^^^^^^^^^^^^^ Promise in \`Promise.all()\` should not be awaited.␊ + ␊ + --------------------------------------------------------------------------------␊ + Suggestion 1/1: Remove \`await\`.␊ + 1 | Promise.all([(( promise ))])␊ + ` + +## invalid(10): Promise.all([await await promise]) + +> Input + + `␊ + 1 | Promise.all([await await promise])␊ + ` + +> Error 1/1 + + `␊ + > 1 | Promise.all([await await promise])␊ + | ^^^^^^^^^^^^^^^^^^^ Promise in \`Promise.all()\` should not be awaited.␊ + ␊ + --------------------------------------------------------------------------------␊ + Suggestion 1/1: Remove \`await\`.␊ + 1 | Promise.all([await promise])␊ + ` + +## invalid(11): Promise.all([...foo, await promise1, await promise2]) + +> Input + + `␊ + 1 | Promise.all([...foo, await promise1, await promise2])␊ + ` + +> Error 1/2 + + `␊ + > 1 | Promise.all([...foo, await promise1, await promise2])␊ + | ^^^^^^^^^^^^^^ Promise in \`Promise.all()\` should not be awaited.␊ + ␊ + --------------------------------------------------------------------------------␊ + Suggestion 1/1: Remove \`await\`.␊ + 1 | Promise.all([...foo, promise1, await promise2])␊ + ` + +> Error 2/2 + + `␊ + > 1 | Promise.all([...foo, await promise1, await promise2])␊ + | ^^^^^^^^^^^^^^ Promise in \`Promise.all()\` should not be awaited.␊ + ␊ + --------------------------------------------------------------------------------␊ + Suggestion 1/1: Remove \`await\`.␊ + 1 | Promise.all([...foo, await promise1, promise2])␊ + ` + +## invalid(12): Promise.all([await /* comment*/ promise]) + +> Input + + `␊ + 1 | Promise.all([await /* comment*/ promise])␊ + ` + +> Error 1/1 + + `␊ + > 1 | Promise.all([await /* comment*/ promise])␊ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ Promise in \`Promise.all()\` should not be awaited.␊ + ␊ + --------------------------------------------------------------------------------␊ + Suggestion 1/1: Remove \`await\`.␊ + 1 | Promise.all([/* comment*/ promise])␊ + ` diff --git a/test/snapshots/no-await-in-promise-methods.mjs.snap b/test/snapshots/no-await-in-promise-methods.mjs.snap new file mode 100644 index 0000000000000000000000000000000000000000..86985f2171b422fb2f341e7ed7d7209eab414116 GIT binary patch literal 723 zcmV;^0xbPORzV|bhj*P#qNhS|C%yKvi0_yX_4dkp+O>PEEpHT`u#SjZys zT@T|^KMDigG4^0Tq5Uu=mLEm>XaAcYG8jz$IyAyG_6O++W$BNLAn=+0(Y@Zf)f#Q> z8?C$ThZ=wb=h6&~M1Ge<h`tz_u%}Jt?0?RTQe{2zo1sUS%ADIE^LGoomwU;_GFzPTa`L#%KTD ziActkIuXmcS|`S5^UOB7&U!C{du`^t62C_jz%~T1M}-qWLr}2?f|ynm!-KHCt2Y~G z%`e*IQtQ<^Ty3lPyfdMeZBt-d#U^+u1$A2%-0q;yymAscFdOmQGfVZ7^xL*{rD9Is zZ)gbWwj{XSu3xK|&0nmTDrQgRt4nhRW{2jC%tk>CH@mD=+05c*JGhy?o^`8%!tDrf zoocv35~{Z<=q*X~ifMI(3+;%PPJ+2xfvNKtFbr&Rs$+^tDCT1U^S%tTkfwyWxSD9~ zBrku?I?h?Uf>GvFS;eSxPW#rd*qnIn%ABt=&eF#h%d(CrEi3_@!f!V({PVByJ3;t4 zc7CeiDEhvD{t87`K0x&+0BFXoz$;N(