From 53113421d85b94be6518cea35dca6d1211436961 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Wed, 10 Apr 2019 20:59:47 -0400 Subject: [PATCH] feat(eslint-plugin): added new rule await-promise (#192) --- packages/eslint-plugin/README.md | 1 + packages/eslint-plugin/ROADMAP.md | 3 +- .../docs/rules/await-thenable.md | 33 +++ .../eslint-plugin/src/rules/await-thenable.ts | 47 ++++ .../tests/rules/await-thenable.test.ts | 223 ++++++++++++++++++ 5 files changed, 306 insertions(+), 1 deletion(-) create mode 100644 packages/eslint-plugin/docs/rules/await-thenable.md create mode 100644 packages/eslint-plugin/src/rules/await-thenable.ts create mode 100644 packages/eslint-plugin/tests/rules/await-thenable.test.ts diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index 22754a977c4..00d11b52834 100644 --- a/packages/eslint-plugin/README.md +++ b/packages/eslint-plugin/README.md @@ -112,6 +112,7 @@ Then you should add `airbnb` (or `airbnb-base`) to your `extends` section of `.e | --------------------------------------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------ | -------- | ----------------- | | [`@typescript-eslint/adjacent-overload-signatures`](./docs/rules/adjacent-overload-signatures.md) | Require that member overloads be consecutive (`adjacent-overload-signatures` from TSLint) | :heavy_check_mark: | | | | [`@typescript-eslint/array-type`](./docs/rules/array-type.md) | Requires using either `T[]` or `Array` for arrays (`array-type` from TSLint) | :heavy_check_mark: | :wrench: | | +| [`@typescript-eslint/await-thenable`](./docs/rules/await-thenable.md) | Disallow awaiting a value that is not a Promise (`await-promise` from TSLint) | :heavy_check_mark: | | :thought_balloon: | | [`@typescript-eslint/ban-types`](./docs/rules/ban-types.md) | Enforces that types will not to be used (`ban-types` from TSLint) | :heavy_check_mark: | :wrench: | | | [`@typescript-eslint/ban-ts-ignore`](./docs/rules/ban-ts-ignore.md) | Bans β€œ// @ts-ignore” comments from being used (`ban-ts-ignore` from TSLint) | | | | | [`@typescript-eslint/camelcase`](./docs/rules/camelcase.md) | Enforce camelCase naming convention | :heavy_check_mark: | | | diff --git a/packages/eslint-plugin/ROADMAP.md b/packages/eslint-plugin/ROADMAP.md index 6df8aa7fe31..9622f367f57 100644 --- a/packages/eslint-plugin/ROADMAP.md +++ b/packages/eslint-plugin/ROADMAP.md @@ -40,7 +40,7 @@ | TSLint rule | | ESLint rule | | ------------------------------------ | :-: | --------------------------------------------------------------------- | -| [`await-promise`] | πŸ›‘ | N/A | +| [`await-promise`] | βœ… | [`@typescript-eslint/await-thenable`] | | [`ban-comma-operator`] | 🌟 | [`no-sequences`][no-sequences] | | [`ban`] | 🌟 | [`no-restricted-properties`][no-restricted-properties] | | [`curly`] | 🌟 | [`curly`][curly] | @@ -574,6 +574,7 @@ Relevant plugins: [`chai-expect-keywords`](https://github.com/gavinaiken/eslint- [`@typescript-eslint/adjacent-overload-signatures`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/adjacent-overload-signatures.md +[`@typescript-eslint/await-thenable`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/await-thenable.md [`@typescript-eslint/ban-types`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/ban-types.md [`@typescript-eslint/ban-ts-ignore`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/ban-ts-ignore.md [`@typescript-eslint/explicit-member-accessibility`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/explicit-member-accessibility.md diff --git a/packages/eslint-plugin/docs/rules/await-thenable.md b/packages/eslint-plugin/docs/rules/await-thenable.md new file mode 100644 index 00000000000..47780d1db42 --- /dev/null +++ b/packages/eslint-plugin/docs/rules/await-thenable.md @@ -0,0 +1,33 @@ +# Disallows awaiting a value that is not a Promise (await-thenable) + +This rule disallows awaiting a value that is not a "Thenable" (an object which has `then` method, such as a Promise). +While it is valid JavaScript to await a non-`Promise`-like value (it will resolve immediately), this pattern is often a programmer error, such as forgetting to add parenthesis to call a function that returns a Promise. + +## Rule Details + +Examples of **incorrect** code for this rule: + +```ts +await 'value'; + +const createValue = () => 'value'; +await createValue(); +``` + +Examples of **correct** code for this rule: + +```ts +await Promise.resolve('value'); + +const createValue = async () => 'value'; +await createValue(); +``` + +## When Not To Use It + +If you want to allow code to `await` non-Promise values. +This is generally not preferred, but can sometimes be useful for visual consistency. + +## Related to + +- TSLint: ['await-promise'](https://palantir.github.io/tslint/rules/await-promise) diff --git a/packages/eslint-plugin/src/rules/await-thenable.ts b/packages/eslint-plugin/src/rules/await-thenable.ts new file mode 100644 index 00000000000..a52469ef6e3 --- /dev/null +++ b/packages/eslint-plugin/src/rules/await-thenable.ts @@ -0,0 +1,47 @@ +import * as tsutils from 'tsutils'; +import * as ts from 'typescript'; + +import * as util from '../util'; + +export default util.createRule({ + name: 'await-thenable', + meta: { + docs: { + description: 'Disallows awaiting a value that is not a Thenable', + category: 'Best Practices', + recommended: 'error', + tslintName: 'await-thenable', + }, + messages: { + await: 'Unexpected `await` of a non-Promise (non-"Thenable") value.', + }, + schema: [], + type: 'problem', + }, + defaultOptions: [], + + create(context) { + const parserServices = util.getParserServices(context); + const checker = parserServices.program.getTypeChecker(); + + return { + AwaitExpression(node) { + const originalNode = parserServices.esTreeNodeToTSNodeMap.get( + node, + ) as ts.AwaitExpression; + const type = checker.getTypeAtLocation(originalNode.expression); + + if ( + !tsutils.isTypeFlagSet(type, ts.TypeFlags.Any) && + !tsutils.isTypeFlagSet(type, ts.TypeFlags.Unknown) && + !tsutils.isThenableType(checker, originalNode.expression, type) + ) { + context.report({ + messageId: 'await', + node, + }); + } + }, + }; + }, +}); diff --git a/packages/eslint-plugin/tests/rules/await-thenable.test.ts b/packages/eslint-plugin/tests/rules/await-thenable.test.ts new file mode 100644 index 00000000000..deca521aefa --- /dev/null +++ b/packages/eslint-plugin/tests/rules/await-thenable.test.ts @@ -0,0 +1,223 @@ +import rule from '../../src/rules/await-thenable'; +import { RuleTester, getFixturesRootDir } from '../RuleTester'; + +const rootDir = getFixturesRootDir(); +const parserOptions = { + ecmaVersion: 2018, + tsconfigRootDir: rootDir, + project: './tsconfig.json', +}; + +const messageId = 'await'; + +const ruleTester = new RuleTester({ + parserOptions, + parser: '@typescript-eslint/parser', +}); + +ruleTester.run('await-promise', rule, { + valid: [ + ` +async function test() { + await Promise.resolve("value"); + await Promise.reject(new Error("message")); +} +`, + ` +async function test() { + await (async () => true)(); +} +`, + ` +async function test() { + function returnsPromise() { + return Promise.resolve("value"); + } + await returnsPromise(); +} +`, + ` +async function test() { + async function returnsPromiseAsync() {} + await returnsPromiseAsync(); +} +`, + ` +async function test() { + let anyValue: any; + await anyValue; +} +`, + ` +async function test() { + let unknownValue: unknown; + await unknownValue; +} +`, + ` +async function test() { + const numberPromise: Promise; + await numberPromise; +} +`, + ` +async function test() { + class Foo extends Promise {} + const foo: Foo = Foo.resolve(2); + await foo; + + class Bar extends Foo {} + const bar: Bar = Bar.resolve(2); + await bar; +} +`, + ` +async function test() { + await (Math.random() > 0.5 ? numberPromise : 0); + await (Math.random() > 0.5 ? foo : 0); + await (Math.random() > 0.5 ? bar : 0); + + const intersectionPromise: Promise & number; + await intersectionPromise; +} +`, + ` +async function test() { + class Thenable { then(callback: () => {}) { } }; + const thenable = new Thenable(); + + await thenable; +} +`, + ` +// https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/promise-polyfill/index.d.ts +// Type definitions for promise-polyfill 6.0 +// Project: https://github.com/taylorhakes/promise-polyfill +// Definitions by: Steve Jenkins +// Daniel Cassidy +// Definitions: https://github.com/DefinitelyTyped/DefinitelyTyped + +interface PromisePolyfillConstructor extends PromiseConstructor { + _immediateFn?: (handler: (() => void) | string) => void; +} + +declare const PromisePolyfill: PromisePolyfillConstructor; + +async function test() { + const promise = new PromisePolyfill(() => {}); + + await promise; +} +`, + ` +// https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/bluebird/index.d.ts +// Type definitions for bluebird 3.5 +// Project: https://github.com/petkaantonov/bluebird +// Definitions by: Leonard Hecker +// Definitions: https://github.com/DefinitelyTyped/DefinitelyTyped +// TypeScript Version: 2.8 + +/*! + * The code following this comment originates from: + * https://github.com/types/npm-bluebird + * + * Note for browser users: use bluebird-global typings instead of this one + * if you want to use Bluebird via the global Promise symbol. + * + * Licensed under: + * The MIT License (MIT) + * + * Copyright (c) 2016 unional + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +type Constructor = new (...args: any[]) => E; +type CatchFilter = ((error: E) => boolean) | (object & E); +type IterableItem = R extends Iterable ? U : never; +type IterableOrNever = Extract>; +type Resolvable = R | PromiseLike; +type IterateFunction = (item: T, index: number, arrayLength: number) => Resolvable; + +declare class Bluebird implements PromiseLike { + then(onFulfill?: (value: R) => Resolvable, onReject?: (error: any) => Resolvable): Bluebird; // For simpler signature help. + then( + onfulfilled?: ((value: R) => Resolvable) | null, + onrejected?: ((reason: any) => Resolvable) | null + ): Bluebird; +} + +declare const bluebird: Bluebird; + +async function test() { + await bluebird; +} +`, + ], + + invalid: [ + { + code: ` +async function test() { + await 0; + await "value"; + + await (Math.random() > 0.5 ? "" : 0); + + class NonPromise extends Array {} + await new NonPromise(); +} +`, + errors: [ + { + line: 3, + messageId, + }, + { + line: 4, + messageId, + }, + { + line: 6, + messageId, + }, + { + line: 9, + messageId, + }, + ], + }, + { + code: ` +async function test() { + class IncorrectThenable { then() { } }; + const thenable = new IncorrectThenable(); + + await thenable; +} + `, + errors: [ + { + line: 6, + messageId, + }, + ], + }, + ], +});