From 674d8cd2ad2e5c565399620c3e7ea58f81ffc6b8 Mon Sep 17 00:00:00 2001 From: Tiger Oakes Date: Wed, 29 May 2024 09:14:16 -0700 Subject: [PATCH] Add prefer-promise-static-methods rule --- README.md | 35 +++--- __tests__/prefer-promise-static-methods.js | 69 +++++++++++ docs/rules/prefer-promise-static-methods.md | 42 +++++++ index.js | 1 + rules/prefer-promise-static-methods.js | 131 ++++++++++++++++++++ 5 files changed, 261 insertions(+), 17 deletions(-) create mode 100644 __tests__/prefer-promise-static-methods.js create mode 100644 docs/rules/prefer-promise-static-methods.md create mode 100644 rules/prefer-promise-static-methods.js diff --git a/README.md b/README.md index 30a82e7c..9f1306fa 100644 --- a/README.md +++ b/README.md @@ -85,23 +85,24 @@ or start with the recommended rule set: 🔧 Automatically fixable by the [`--fix` CLI option](https://eslint.org/docs/user-guide/command-line-interface#--fix). -| Name                      | Description | 💼 | ⚠️ | 🚫 | 🔧 | -| :------------------------------------------------------------------- | :------------------------------------------------------------------------------------- | :-- | :-- | :-- | :-- | -| [always-return](docs/rules/always-return.md) | Require returning inside each `then()` to create readable and reusable Promise chains. | ✅ | | | | -| [avoid-new](docs/rules/avoid-new.md) | Disallow creating `new` promises outside of utility libs (use [pify][] instead). | | | ✅ | | -| [catch-or-return](docs/rules/catch-or-return.md) | Enforce the use of `catch()` on un-returned promises. | ✅ | | | | -| [no-callback-in-promise](docs/rules/no-callback-in-promise.md) | Disallow calling `cb()` inside of a `then()` (use [nodeify][] instead). | | ✅ | | | -| [no-multiple-resolved](docs/rules/no-multiple-resolved.md) | Disallow creating new promises with paths that resolve multiple times. | | | | | -| [no-native](docs/rules/no-native.md) | Require creating a `Promise` constructor before using it in an ES5 environment. | | | ✅ | | -| [no-nesting](docs/rules/no-nesting.md) | Disallow nested `then()` or `catch()` statements. | | ✅ | | | -| [no-new-statics](docs/rules/no-new-statics.md) | Disallow calling `new` on a Promise static method. | ✅ | | | 🔧 | -| [no-promise-in-callback](docs/rules/no-promise-in-callback.md) | Disallow using promises inside of callbacks. | | ✅ | | | -| [no-return-in-finally](docs/rules/no-return-in-finally.md) | Disallow return statements in `finally()`. | | ✅ | | | -| [no-return-wrap](docs/rules/no-return-wrap.md) | Disallow wrapping values in `Promise.resolve` or `Promise.reject` when not needed. | ✅ | | | | -| [param-names](docs/rules/param-names.md) | Enforce consistent param names and ordering when creating new promises. | ✅ | | | | -| [prefer-await-to-callbacks](docs/rules/prefer-await-to-callbacks.md) | Prefer async/await to the callback pattern. | | | | | -| [prefer-await-to-then](docs/rules/prefer-await-to-then.md) | Prefer `await` to `then()`/`catch()`/`finally()` for reading Promise values. | | | | | -| [valid-params](docs/rules/valid-params.md) | Enforces the proper number of arguments are passed to Promise functions. | | ✅ | | | +| Name                          | Description | 💼 | ⚠️ | 🚫 | 🔧 | +| :--------------------------------------------------------------------------- | :------------------------------------------------------------------------------------- | :-- | :-- | :-- | :-- | +| [always-return](docs/rules/always-return.md) | Require returning inside each `then()` to create readable and reusable Promise chains. | ✅ | | | | +| [avoid-new](docs/rules/avoid-new.md) | Disallow creating `new` promises outside of utility libs (use [pify][] instead). | | | ✅ | | +| [catch-or-return](docs/rules/catch-or-return.md) | Enforce the use of `catch()` on un-returned promises. | ✅ | | | | +| [no-callback-in-promise](docs/rules/no-callback-in-promise.md) | Disallow calling `cb()` inside of a `then()` (use [nodeify][] instead). | | ✅ | | | +| [no-multiple-resolved](docs/rules/no-multiple-resolved.md) | Disallow creating new promises with paths that resolve multiple times. | | | | | +| [no-native](docs/rules/no-native.md) | Require creating a `Promise` constructor before using it in an ES5 environment. | | | ✅ | | +| [no-nesting](docs/rules/no-nesting.md) | Disallow nested `then()` or `catch()` statements. | | ✅ | | | +| [no-new-statics](docs/rules/no-new-statics.md) | Disallow calling `new` on a Promise static method. | ✅ | | | 🔧 | +| [no-promise-in-callback](docs/rules/no-promise-in-callback.md) | Disallow using promises inside of callbacks. | | ✅ | | | +| [no-return-in-finally](docs/rules/no-return-in-finally.md) | Disallow return statements in `finally()`. | | ✅ | | | +| [no-return-wrap](docs/rules/no-return-wrap.md) | Disallow wrapping values in `Promise.resolve` or `Promise.reject` when not needed. | ✅ | | | | +| [param-names](docs/rules/param-names.md) | Enforce consistent param names and ordering when creating new promises. | ✅ | | | | +| [prefer-await-to-callbacks](docs/rules/prefer-await-to-callbacks.md) | Prefer async/await to the callback pattern. | | | | | +| [prefer-await-to-then](docs/rules/prefer-await-to-then.md) | Prefer `await` to `then()`/`catch()`/`finally()` for reading Promise values. | | | | | +| [prefer-promise-static-methods](docs/rules/prefer-promise-static-methods.md) | Prefer `Promise.resolve(foo)` to `new Promise((resolve) => resolve(foo))`. | | | | 🔧 | +| [valid-params](docs/rules/valid-params.md) | Enforces the proper number of arguments are passed to Promise functions. | | ✅ | | | diff --git a/__tests__/prefer-promise-static-methods.js b/__tests__/prefer-promise-static-methods.js new file mode 100644 index 00000000..b86fe955 --- /dev/null +++ b/__tests__/prefer-promise-static-methods.js @@ -0,0 +1,69 @@ +'use strict' + +const rule = require('../rules/prefer-promise-static-methods') +const { RuleTester } = require('./rule-tester') +const ruleTester = new RuleTester({ + parserOptions: { + ecmaVersion: 6, + }, +}) + +const resolveErrorMessage = + 'Prefer `Promise.resolve()` to `new Promise()`. The static method is faster, more readable and less verbose.' +const rejectErrorMessage = + 'Prefer `Promise.reject()` to `new Promise()`. The static method is faster, more readable and less verbose.' + +ruleTester.run('prefer-promise-static-methods', rule, { + valid: [ + `Promise.resolve(foo)`, + `Promise.reject(bar)`, + `new Promise(() => {})`, + `new Promise((resolve) => setTimeout(resolve, 100))`, + `new Promise(process.nextTick)`, + `new Promise((resolve) => { resolve(foo); resolve(bar) })`, + `new Promise((resolve, reject) => { foo(bar) })`, + `new Promise((resolve, reject) => { foo && resolve(bar) })`, + `new Promise((...args) => {})`, + // This is a type error but the rule wouldn't check it + `new Promise(([foo, bar]) => {})`, + `new Promise(([foo, bar] = []) => {})`, + ], + + invalid: [ + { + code: `new Promise((resolve) => resolve(foo))`, + output: `Promise.resolve(foo)`, + errors: [{ message: resolveErrorMessage }], + }, + { + code: `new Promise((resolve) => { resolve(foo) })`, + output: `Promise.resolve(foo)`, + errors: [{ message: resolveErrorMessage }], + }, + { + code: `new Promise((resolve) => resolve())`, + output: `Promise.resolve()`, + errors: [{ message: resolveErrorMessage }], + }, + { + code: `new Promise((_, reject) => reject(foo))`, + output: `Promise.reject(foo)`, + errors: [{ message: rejectErrorMessage }], + }, + { + code: `new Promise((resolve, reject) => { reject(foo) })`, + output: `Promise.reject(foo)`, + errors: [{ message: rejectErrorMessage }], + }, + { + code: `new Promise(function foo(resolve, reject) { reject(bar) })`, + output: `Promise.reject(bar)`, + errors: [{ message: rejectErrorMessage }], + }, + { + code: `new Promise((resolve = unusedDefault) => resolve())`, + output: `Promise.resolve()`, + errors: [{ message: resolveErrorMessage }], + }, + ], +}) diff --git a/docs/rules/prefer-promise-static-methods.md b/docs/rules/prefer-promise-static-methods.md new file mode 100644 index 00000000..a6554cb7 --- /dev/null +++ b/docs/rules/prefer-promise-static-methods.md @@ -0,0 +1,42 @@ +# Prefer `Promise.resolve(foo)` to `new Promise((resolve) => resolve(foo))` (`promise/prefer-promise-static-methods`) + +🔧 This rule is automatically fixable by the +[`--fix` CLI option](https://eslint.org/docs/latest/user-guide/command-line-interface#--fix). + + + +Using `new Promise()` for simple use cases is a common mistake when developers +first start working with promises. When `resolve` or `reject` is immediately +called in the promise executor, it's better to just use the static +`Promise.resolve()` or `Promise.reject()` methods. The static methods are +faster, more readable and less verbose. + +#### Valid + +```js +Promise.resolve(foo) +Promise.reject(bar) + +new Promise((resolve) => setTimeout(resolve, 100)) +new Promise((resolve, reject) => { + if (foo) { + resolve(bar) + } +}) +``` + +#### Invalid + +```js +new Promise((resolve) => resolve(foo)) +new Promise((resolve) => { + resolve(foo) +}) +// autofix to Promise.resolve(foo); + +new Promise((_, reject) => reject(foo)) +new Promise(function (_, reject) { + reject(foo) +}) +// autofix to Promise.reject(foo); +``` diff --git a/index.js b/index.js index 40b81054..31e447d2 100644 --- a/index.js +++ b/index.js @@ -8,6 +8,7 @@ module.exports = { 'catch-or-return': require('./rules/catch-or-return'), 'prefer-await-to-callbacks': require('./rules/prefer-await-to-callbacks'), 'prefer-await-to-then': require('./rules/prefer-await-to-then'), + 'prefer-promise-static-methods': require('./rules/prefer-promise-static-methods'), 'no-native': require('./rules/no-native'), 'no-callback-in-promise': require('./rules/no-callback-in-promise'), 'no-promise-in-callback': require('./rules/no-promise-in-callback'), diff --git a/rules/prefer-promise-static-methods.js b/rules/prefer-promise-static-methods.js new file mode 100644 index 00000000..22c4ba9d --- /dev/null +++ b/rules/prefer-promise-static-methods.js @@ -0,0 +1,131 @@ +/** + * Rule: prefer-promise-static-methods + * Prefer `Promise.resolve(foo)` to `new Promise((resolve) => resolve(foo))`. + */ + +'use strict' + +const { getSourceCode } = require('./lib/eslint-compat') +const getDocsUrl = require('./lib/get-docs-url') +const { + isPromiseConstructorWithInlineExecutor, +} = require('./lib/is-promise-constructor') + +module.exports = { + meta: { + type: 'suggestion', + docs: { + description: + 'Prefer `Promise.resolve(foo)` to `new Promise((resolve) => resolve(foo))`.', + url: getDocsUrl('prefer-promise-static-methods'), + }, + messages: { + replaceWithStaticMethod: + 'Prefer `Promise.{{ method }}()` to `new Promise()`. The static method is faster, more readable and less verbose.', + }, + fixable: 'code', + schema: [], + }, + create(context) { + const sourceCode = getSourceCode(context) + /** + * Report an error if the given node is a call to `resolve` or `reject`. + * @param {import('estree').SimpleCallExpression} callNode + * @param {import('estree').NewExpression} constructorNode + * @param {Array} parameterNames + */ + function reportIfIsPromiseCall(callNode, constructorNode, parameterNames) { + if ( + callNode.callee.type === 'Identifier' && + callNode.arguments.length <= 1 + ) { + /** @type {'resolve' | 'reject'} */ + let method + if (callNode.callee.name === parameterNames[0]) { + method = 'resolve' + } else if (callNode.callee.name === parameterNames[1]) { + method = 'reject' + } else { + return + } + + // Value passed to resolve/reject method + const valueNode = callNode.arguments[0] + const valueText = valueNode + ? sourceCode.getText(callNode.arguments[0]) + : '' + context.report({ + node: callNode, + messageId: 'replaceWithStaticMethod', + data: { method }, + fix: (fixer) => + fixer.replaceText( + constructorNode, + `Promise.${method}(${valueText})` + ), + }) + } + } + + return { + NewExpression(node) { + if (isPromiseConstructorWithInlineExecutor(node)) { + const func = node.arguments[0] + const parameterNames = getParameterNames(func.params) + + if (func.body.type === 'CallExpression') { + // (resolve) => resolve(foo) + reportIfIsPromiseCall(func.body, node, parameterNames) + } else if ( + func.body.type === 'BlockStatement' && + func.body.body.length === 1 + ) { + const statement = func.body.body[0] + if ( + statement.type === 'ExpressionStatement' && + statement.expression.type === 'CallExpression' + ) { + // (resolve) => { resolve(foo) } + reportIfIsPromiseCall(statement.expression, node, parameterNames) + } + } + } + }, + } + }, +} + +/** + * Given AST for `(resolve, reject) => {...}` params, return `['resolve', 'reject']`. + * @param {import('estree').Pattern[]} params + */ +function getParameterNames(params) { + /** @type {Array} */ + const names = [] + for (const param of params) { + switch (param.type) { + // (resolve) => + case 'Identifier': + names.push(param.name) + break + // (resolve = foo) => + case 'AssignmentPattern': + if (param.left.type === 'Identifier') { + names.push(param.left.name) + } else { + names.push(undefined) + } + break + // (...args) => + case 'RestElement': + // there won't be any more valid names + return names + // ([resolve]) => + default: + names.push(undefined) + break + } + } + + return names +}