diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index 52776b44c72..eb78f4a0de5 100644 --- a/packages/eslint-plugin/README.md +++ b/packages/eslint-plugin/README.md @@ -164,7 +164,7 @@ Pro Tip: For larger codebases you may want to consider splitting our linting int | [`@typescript-eslint/prefer-regexp-exec`](./docs/rules/prefer-regexp-exec.md) | Enforce that `RegExp#exec` is used instead of `String#match` if no global flag is provided | :heavy_check_mark: | | :thought_balloon: | | [`@typescript-eslint/prefer-string-starts-ends-with`](./docs/rules/prefer-string-starts-ends-with.md) | Enforce the use of `String#startsWith` and `String#endsWith` instead of other equivalent methods of checking substrings | | :wrench: | :thought_balloon: | | [`@typescript-eslint/prefer-ts-expect-error`](./docs/rules/prefer-ts-expect-error.md) | Recommends using `@ts-expect-error` over `@ts-ignore` | | :wrench: | | -| [`@typescript-eslint/promise-function-async`](./docs/rules/promise-function-async.md) | Requires any function or method that returns a Promise to be marked async | | | :thought_balloon: | +| [`@typescript-eslint/promise-function-async`](./docs/rules/promise-function-async.md) | Requires any function or method that returns a Promise to be marked async | | :wrench: | :thought_balloon: | | [`@typescript-eslint/require-array-sort-compare`](./docs/rules/require-array-sort-compare.md) | Requires `Array#sort` calls to always provide a `compareFunction` | | | :thought_balloon: | | [`@typescript-eslint/restrict-plus-operands`](./docs/rules/restrict-plus-operands.md) | When adding two variables, operands must both be of type number or of type string | :heavy_check_mark: | | :thought_balloon: | | [`@typescript-eslint/restrict-template-expressions`](./docs/rules/restrict-template-expressions.md) | Enforce template literal expressions to be of string type | :heavy_check_mark: | | :thought_balloon: | diff --git a/packages/eslint-plugin/src/rules/promise-function-async.ts b/packages/eslint-plugin/src/rules/promise-function-async.ts index 16134675d57..00888c8f39f 100644 --- a/packages/eslint-plugin/src/rules/promise-function-async.ts +++ b/packages/eslint-plugin/src/rules/promise-function-async.ts @@ -2,6 +2,7 @@ import { AST_NODE_TYPES, TSESTree, } from '@typescript-eslint/experimental-utils'; +import * as ts from 'typescript'; import * as util from '../util'; type Options = [ @@ -20,6 +21,7 @@ export default util.createRule({ name: 'promise-function-async', meta: { type: 'suggestion', + fixable: 'code', docs: { description: 'Requires any function or method that returns a Promise to be marked async', @@ -94,9 +96,7 @@ export default util.createRule({ node: | TSESTree.ArrowFunctionExpression | TSESTree.FunctionDeclaration - | TSESTree.FunctionExpression - | TSESTree.MethodDefinition - | TSESTree.TSAbstractMethodDefinition, + | TSESTree.FunctionExpression, ): void { const originalNode = parserServices.esTreeNodeToTSNodeMap.get(node); const signatures = checker @@ -114,6 +114,12 @@ export default util.createRule({ allAllowedPromiseNames, ) ) { + // Return type is not a promise + return; + } + + if (node.parent?.type === AST_NODE_TYPES.TSAbstractMethodDefinition) { + // Abstract method can't be async return; } @@ -123,12 +129,34 @@ export default util.createRule({ node.parent.type === AST_NODE_TYPES.MethodDefinition) && (node.parent.kind === 'get' || node.parent.kind === 'set') ) { + // Getters and setters can't be async return; } + if ( + util.isTypeFlagSet(returnType, ts.TypeFlags.Any | ts.TypeFlags.Unknown) + ) { + // Report without auto fixer because the return type is unknown + return context.report({ + messageId: 'missingAsync', + node, + }); + } + context.report({ messageId: 'missingAsync', node, + fix: fixer => { + if ( + node.parent && + (node.parent.type === AST_NODE_TYPES.MethodDefinition || + (node.parent.type === AST_NODE_TYPES.Property && + node.parent.method)) + ) { + return fixer.insertTextBefore(node.parent.key, 'async '); + } + return fixer.insertTextBefore(node, 'async '); + }, }); } @@ -152,13 +180,15 @@ export default util.createRule({ ): void { if ( node.parent && - 'kind' in node.parent && + node.parent.type === AST_NODE_TYPES.MethodDefinition && node.parent.kind === 'method' ) { if (checkMethodDeclarations) { - validateNode(node.parent); + validateNode(node); } - } else if (checkFunctionExpressions) { + return; + } + if (checkFunctionExpressions) { validateNode(node); } }, diff --git a/packages/eslint-plugin/tests/rules/promise-function-async.test.ts b/packages/eslint-plugin/tests/rules/promise-function-async.test.ts index 47b5b6221d6..49f6e250b6c 100644 --- a/packages/eslint-plugin/tests/rules/promise-function-async.test.ts +++ b/packages/eslint-plugin/tests/rules/promise-function-async.test.ts @@ -1,5 +1,5 @@ import rule from '../../src/rules/promise-function-async'; -import { RuleTester, getFixturesRootDir } from '../RuleTester'; +import { getFixturesRootDir, RuleTester } from '../RuleTester'; const rootDir = getFixturesRootDir(); const messageId = 'missingAsync'; @@ -141,6 +141,18 @@ const foo = (options: Options): Return => { code: ` function foo(): Promise | boolean { return Math.random() > 0.5 ? Promise.resolve('value') : false; +} + `, + }, + { + code: ` +abstract class Test { + abstract test1(): Promise; + + // abstract method with body is always an error but it still parses into valid AST + abstract test2(): Promise { + return Promise.resolve(1); + } } `, }, @@ -191,6 +203,11 @@ const nonAsyncPromiseFunctionExpressionA = function (p: Promise) { messageId, }, ], + output: ` +const nonAsyncPromiseFunctionExpressionA = async function (p: Promise) { + return p; +}; + `, }, { code: ` @@ -203,6 +220,11 @@ const nonAsyncPromiseFunctionExpressionB = function () { messageId, }, ], + output: ` +const nonAsyncPromiseFunctionExpressionB = async function () { + return new Promise(); +}; + `, }, { code: ` @@ -215,6 +237,11 @@ function nonAsyncPromiseFunctionDeclarationA(p: Promise) { messageId, }, ], + output: ` +async function nonAsyncPromiseFunctionDeclarationA(p: Promise) { + return p; +} + `, }, { code: ` @@ -227,6 +254,11 @@ function nonAsyncPromiseFunctionDeclarationB() { messageId, }, ], + output: ` +async function nonAsyncPromiseFunctionDeclarationB() { + return new Promise(); +} + `, }, { code: ` @@ -237,6 +269,9 @@ const nonAsyncPromiseArrowFunctionA = (p: Promise) => p; messageId, }, ], + output: ` +const nonAsyncPromiseArrowFunctionA = async (p: Promise) => p; + `, }, { code: ` @@ -247,6 +282,31 @@ const nonAsyncPromiseArrowFunctionB = () => new Promise(); messageId, }, ], + output: ` +const nonAsyncPromiseArrowFunctionB = async () => new Promise(); + `, + }, + { + code: ` +const functions = { + nonAsyncPromiseMethod() { + return Promise.resolve(1); + }, +}; + `, + errors: [ + { + line: 3, + messageId, + }, + ], + output: ` +const functions = { + async nonAsyncPromiseMethod() { + return Promise.resolve(1); + }, +}; + `, }, { code: ` @@ -255,7 +315,7 @@ class Test { return p; } - public nonAsyncPromiseMethodB() { + public static nonAsyncPromiseMethodB() { return new Promise(); } } @@ -270,6 +330,17 @@ class Test { messageId, }, ], + output: ` +class Test { + public async nonAsyncPromiseMethodA(p: Promise) { + return p; + } + + public static async nonAsyncPromiseMethodB() { + return new Promise(); + } +} + `, }, { code: ` @@ -308,6 +379,23 @@ class Test { messageId, }, ], + output: ` +const nonAsyncPromiseFunctionExpression = async function (p: Promise) { + return p; +}; + +async function nonAsyncPromiseFunctionDeclaration(p: Promise) { + return p; +} + +const nonAsyncPromiseArrowFunction = (p: Promise) => p; + +class Test { + public async nonAsyncPromiseMethod(p: Promise) { + return p; + } +} + `, }, { code: ` @@ -346,6 +434,23 @@ class Test { messageId, }, ], + output: ` +const nonAsyncPromiseFunctionExpression = async function (p: Promise) { + return p; +}; + +function nonAsyncPromiseFunctionDeclaration(p: Promise) { + return p; +} + +const nonAsyncPromiseArrowFunction = async (p: Promise) => p; + +class Test { + public async nonAsyncPromiseMethod(p: Promise) { + return p; + } +} + `, }, { code: ` @@ -384,6 +489,23 @@ class Test { messageId, }, ], + output: ` +const nonAsyncPromiseFunctionExpression = function (p: Promise) { + return p; +}; + +async function nonAsyncPromiseFunctionDeclaration(p: Promise) { + return p; +} + +const nonAsyncPromiseArrowFunction = async (p: Promise) => p; + +class Test { + public async nonAsyncPromiseMethod(p: Promise) { + return p; + } +} + `, }, { code: ` @@ -422,6 +544,23 @@ class Test { messageId, }, ], + output: ` +const nonAsyncPromiseFunctionExpression = async function (p: Promise) { + return p; +}; + +async function nonAsyncPromiseFunctionDeclaration(p: Promise) { + return p; +} + +const nonAsyncPromiseArrowFunction = async (p: Promise) => p; + +class Test { + public nonAsyncPromiseMethod(p: Promise) { + return p; + } +} + `, }, { code: ` @@ -440,6 +579,11 @@ const returnAllowedType = () => new PromiseType(); messageId, }, ], + output: ` +class PromiseType {} + +const returnAllowedType = async () => new PromiseType(); + `, }, { code: ` @@ -461,6 +605,14 @@ function foo(): Promise | SPromise { messageId, }, ], + output: ` +interface SPromise extends Promise {} +async function foo(): Promise | SPromise { + return Math.random() > 0.5 + ? Promise.resolve('value') + : Promise.resolve(false); +} + `, }, ], });