diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index 3807877a073e..90da943b887c 100644 --- a/packages/eslint-plugin/README.md +++ b/packages/eslint-plugin/README.md @@ -145,6 +145,7 @@ Then you should add `airbnb` (or `airbnb-base`) to your `extends` section of `.e | [`@typescript-eslint/no-var-requires`](./docs/rules/no-var-requires.md) | Disallows the use of require statements except in import statements (`no-var-requires` from TSLint) | :heavy_check_mark: | | | [`@typescript-eslint/prefer-interface`](./docs/rules/prefer-interface.md) | Prefer an interface declaration over a type literal (type T = { ... }) (`interface-over-type-literal` from TSLint) | :heavy_check_mark: | :wrench: | | [`@typescript-eslint/prefer-namespace-keyword`](./docs/rules/prefer-namespace-keyword.md) | Require the use of the `namespace` keyword instead of the `module` keyword to declare custom TypeScript modules. (`no-internal-module` from TSLint) | :heavy_check_mark: | :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. (`promise-function-async` from TSLint) | :heavy_check_mark: | | | [`@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. (`restrict-plus-operands` from TSLint) | | | | [`@typescript-eslint/type-annotation-spacing`](./docs/rules/type-annotation-spacing.md) | Require consistent spacing around type annotations (`typedef-whitespace` from TSLint) | :heavy_check_mark: | :wrench: | diff --git a/packages/eslint-plugin/ROADMAP.md b/packages/eslint-plugin/ROADMAP.md index 48ccaa900b63..444700439669 100644 --- a/packages/eslint-plugin/ROADMAP.md +++ b/packages/eslint-plugin/ROADMAP.md @@ -30,7 +30,7 @@ | [`no-var-requires`] | ✅ | [`@typescript-eslint/no-var-requires`] | | [`only-arrow-functions`] | 🔌 | [`prefer-arrow/prefer-arrow-functions`] | | [`prefer-for-of`] | 🛑 | N/A | -| [`promise-function-async`] | 🛑 | N/A ([relevant plugin][plugin:promise]) | +| [`promise-function-async`] | ✅ | [`@typescript-eslint/promise-function-async`] | | [`typedef`] | 🛑 | N/A | | [`typedef-whitespace`] | ✅ | [`@typescript-eslint/type-annotation-spacing`] | | [`unified-signatures`] | 🛑 | N/A | diff --git a/packages/eslint-plugin/docs/rules/promise-function-async.md b/packages/eslint-plugin/docs/rules/promise-function-async.md new file mode 100644 index 000000000000..c64c25c11b5a --- /dev/null +++ b/packages/eslint-plugin/docs/rules/promise-function-async.md @@ -0,0 +1,65 @@ +# Functions that return promises must be async (promise-function-async) + +Requires any function or method that returns a Promise to be marked async. +Ensures that each function is only capable of: + +- returning a rejected promise, or +- throwing an Error object. + +In contrast, non-`async` `Promise`-returning functions are technically capable of either. +Code that handles the results of those functions will often need to handle both cases, which can get complex. +This rule's practice removes a requirement for creating code to handle both cases. + +## Rule Details + +Examples of **incorrect** code for this rule + +```ts +const arrowFunctionReturnsPromise = () => Promise.resolve('value'); + +function functionDeturnsPromise() { + return Math.random() > 0.5 ? Promise.resolve('value') : false; +} +``` + +Examples of **correct** code for this rule + +```ts +const arrowFunctionReturnsPromise = async () => 'value'; + +async function functionDeturnsPromise() { + return Math.random() > 0.5 ? 'value' : false; +} +``` + +## Options + +Options may be provided as an object with: + +- `allowedPromiseNames` to indicate any extra names of classes or interfaces to be considered Promises when returned. + +In addition, each of the following properties may be provided, and default to `true`: + +- `checkArrowFunctions` +- `checkFunctionDeclarations` +- `checkFunctionExpressions` +- `checkMethodDeclarations` + +```json +{ + "@typescript-eslint/promise-function-async": [ + "error", + { + "allowedPromiseNames": ["Thenable"], + "checkArrowFunctions": true, + "checkFunctionDeclarations": true, + "checkFunctionExpressions": true, + "checkMethodDeclarations": true + } + ] +} +``` + +## Related To + +- TSLint: [promise-function-async](https://palantir.github.io/tslint/rules/promise-function-async) diff --git a/packages/eslint-plugin/lib/rules/promise-function-async.js b/packages/eslint-plugin/lib/rules/promise-function-async.js new file mode 100644 index 000000000000..febbbc2a012b --- /dev/null +++ b/packages/eslint-plugin/lib/rules/promise-function-async.js @@ -0,0 +1,125 @@ +/** + * @fileoverview Requires any function or method that returns a Promise to be marked async + * @author Josh Goldberg + */ +'use strict'; + +const util = require('../util'); +const types = require('../utils/types'); + +//------------------------------------------------------------------------------ +// Rule Definition +//------------------------------------------------------------------------------ + +const defaultOptions = [ + { + allowedPromiseNames: [], + checkArrowFunctions: true, + checkFunctionDeclarations: true, + checkFunctionExpressions: true, + checkMethodDeclarations: true + } +]; + +/** + * @type {import("eslint").Rule.RuleModule} + */ +module.exports = { + meta: { + type: 'suggestion', + docs: { + description: + 'Requires any function or method that returns a Promise to be marked async.', + extraDescription: [util.tslintRule('promise-function-async')], + category: 'TypeScript', + url: util.metaDocsUrl('promise-function-async'), + recommended: 'error' + }, + fixable: null, + messages: { + missingAsync: 'Functions that return promises must be async.' + }, + schema: [ + { + type: 'object', + properties: { + allowedPromiseNames: { + type: 'array', + items: { + type: 'string' + } + }, + checkArrowFunctions: { + type: 'boolean' + }, + checkFunctionDeclarations: { + type: 'boolean' + }, + checkFunctionExpressions: { + type: 'boolean' + }, + checkMethodDeclarations: { + type: 'boolean' + } + }, + additionalProperties: false + } + ] + }, + + create(context) { + const { + allowedPromiseNames, + checkArrowFunctions, + checkFunctionDeclarations, + checkFunctionExpressions, + checkMethodDeclarations + } = util.applyDefault(defaultOptions, context.options)[0]; + + const allAllowedPromiseNames = new Set(['Promise', ...allowedPromiseNames]); + const parserServices = util.getParserServices(context); + const checker = parserServices.program.getTypeChecker(); + + /** + * @param {import("estree").Function} node + */ + function validateNode(node) { + const originalNode = parserServices.esTreeNodeToTSNodeMap.get(node); + const [callSignature] = checker + .getTypeAtLocation(originalNode) + .getCallSignatures(); + const returnType = checker.getReturnTypeOfSignature(callSignature); + + if (!types.containsTypeByName(returnType, allAllowedPromiseNames)) { + return; + } + + context.report({ + messageId: 'missingAsync', + node + }); + } + + return { + ArrowFunctionExpression(node) { + if (checkArrowFunctions && !node.async) { + validateNode(node); + } + }, + FunctionDeclaration(node) { + if (checkFunctionDeclarations && !node.async) { + validateNode(node); + } + }, + FunctionExpression(node) { + if (!!node.parent && node.parent.kind === 'method') { + if (checkMethodDeclarations && !node.async) { + validateNode(node.parent); + } + } else if (checkFunctionExpressions && !node.async) { + validateNode(node); + } + } + }; + } +}; diff --git a/packages/eslint-plugin/lib/utils/types.js b/packages/eslint-plugin/lib/utils/types.js new file mode 100644 index 000000000000..adfb402b72ae --- /dev/null +++ b/packages/eslint-plugin/lib/utils/types.js @@ -0,0 +1,38 @@ +'use strict'; + +const tsutils = require('tsutils'); +const ts = require('typescript'); + +/** + * @param {string} type Type being checked by name. + * @param {Set} allowedNames Symbol names checking on the type. + * @returns {boolean} Whether the type is, extends, or contains any of the allowed names. + */ +function containsTypeByName(type, allowedNames) { + if (tsutils.isTypeFlagSet(type, ts.TypeFlags.Any | ts.TypeFlags.Unknown)) { + return true; + } + + if (tsutils.isTypeReference(type)) { + type = type.target; + } + + if ( + typeof type.symbol !== 'undefined' && + allowedNames.has(type.symbol.name) + ) { + return true; + } + + if (tsutils.isUnionOrIntersectionType(type)) { + return type.types.some(t => containsTypeByName(t, allowedNames)); + } + + const bases = type.getBaseTypes(); + return ( + typeof bases !== 'undefined' && + bases.some(t => containsTypeByName(t, allowedNames)) + ); +} + +exports.containsTypeByName = containsTypeByName; diff --git a/packages/eslint-plugin/tests/lib/rules/promise-function-async.js b/packages/eslint-plugin/tests/lib/rules/promise-function-async.js new file mode 100644 index 000000000000..dddb26046435 --- /dev/null +++ b/packages/eslint-plugin/tests/lib/rules/promise-function-async.js @@ -0,0 +1,275 @@ +/** + * @fileoverview Requires any function or method that returns a Promise to be marked async + * @author Josh Goldberg + */ +'use strict'; + +//------------------------------------------------------------------------------ +// Requirements +//------------------------------------------------------------------------------ + +const rule = require('../../../lib/rules/promise-function-async'), + RuleTester = require('eslint').RuleTester, + path = require('path'); + +//------------------------------------------------------------------------------ +// Tests +//------------------------------------------------------------------------------ + +const rootDir = path.join(process.cwd(), 'tests/fixtures/'); +const parserOptions = { + ecmaVersion: 2018, + tsconfigRootDir: rootDir, + project: './tsconfig.json' +}; + +const messageId = 'missingAsync'; + +const ruleTester = new RuleTester({ + parserOptions, + parser: '@typescript-eslint/parser' +}); + +ruleTester.run('promise-function-async', rule, { + valid: [ + ` +const nonAsyncNonPromiseArrowFunction = (n: number) => n; + +function nonAsyncNonPromiseFunctionDeclaration(n: number) { return n; } + +const asyncPromiseFunctionExpressionA = async function(p: Promise) { return p; }; +const asyncPromiseFunctionExpressionB = async function() { return new Promise(); }; + +class Test { + public nonAsyncNonPromiseArrowFunction = (n: number) => n; + + public nonAsyncNonPromiseMethod() { + return 0; + } + + public async asyncPromiseMethodA(p: Promise) { + return p; + } + + public async asyncPromiseMethodB() { + return new Promise(); + } +} +` + ], + invalid: [ + { + code: ` +const nonAsyncPromiseFunctionExpressionA = function(p: Promise) { return p; }; + +const nonAsyncPromiseFunctionExpressionB = function() { return new Promise(); }; + +function nonAsyncPromiseFunctionDeclarationA(p: Promise) { return p; } + +function nonAsyncPromiseFunctionDeclarationB() { return new Promise(); } + +const nonAsyncPromiseArrowFunctionA = (p: Promise) => p; + +const nonAsyncPromiseArrowFunctionB = () => new Promise(); + +class Test { + public nonAsyncPromiseMethodA(p: Promise) { + return p; + } + + public nonAsyncPromiseMethodB() { + return new Promise(); + } +} +`, + errors: [ + { + line: 2, + messageId + }, + { + line: 4, + messageId + }, + { + line: 6, + messageId + }, + { + line: 8, + messageId + }, + { + line: 10, + messageId + }, + { + line: 12, + messageId + }, + { + line: 15, + messageId + }, + { + line: 19, + messageId + } + ] + }, + { + code: ` +const nonAsyncPromiseFunctionExpression = function(p: Promise) { return p; }; + +function nonAsyncPromiseFunctionDeclaration(p: Promise) { return p; } + +const nonAsyncPromiseArrowFunction = (p: Promise) => p; + +class Test { + public nonAsyncPromiseMethod(p: Promise) { + return p; + } +} +`, + options: [ + { + checkArrowFunctions: false + } + ], + errors: [ + { + line: 2, + messageId + }, + { + line: 4, + messageId + }, + { + line: 9, + messageId + } + ] + }, + { + code: ` +const nonAsyncPromiseFunctionExpression = function(p: Promise) { return p; }; + +function nonAsyncPromiseFunctionDeclaration(p: Promise) { return p; } + +const nonAsyncPromiseArrowFunction = (p: Promise) => p; + +class Test { + public nonAsyncPromiseMethod(p: Promise) { + return p; + } +} +`, + options: [ + { + checkFunctionDeclarations: false + } + ], + errors: [ + { + line: 2, + messageId + }, + { + line: 6, + messageId + }, + { + line: 9, + messageId + } + ] + }, + { + code: ` +const nonAsyncPromiseFunctionExpression = function(p: Promise) { return p; }; + +function nonAsyncPromiseFunctionDeclaration(p: Promise) { return p; } + +const nonAsyncPromiseArrowFunction = (p: Promise) => p; + +class Test { + public nonAsyncPromiseMethod(p: Promise) { + return p; + } +} +`, + options: [ + { + checkFunctionExpressions: false + } + ], + errors: [ + { + line: 4, + messageId + }, + { + line: 6, + messageId + }, + { + line: 9, + messageId + } + ] + }, + { + code: ` +const nonAsyncPromiseFunctionExpression = function(p: Promise) { return p; }; + +function nonAsyncPromiseFunctionDeclaration(p: Promise) { return p; } + +const nonAsyncPromiseArrowFunction = (p: Promise) => p; + +class Test { + public nonAsyncPromiseMethod(p: Promise) { + return p; + } +} +`, + options: [ + { + checkMethodDeclarations: false + } + ], + errors: [ + { + line: 2, + messageId + }, + { + line: 4, + messageId + }, + { + line: 6, + messageId + } + ] + }, + { + code: ` +class PromiseType { } + +const returnAllowedType = () => new PromiseType(); +`, + options: [ + { + allowedPromiseNames: ['PromiseType'] + } + ], + errors: [ + { + line: 4, + messageId + } + ] + } + ] +});