Skip to content

Commit

Permalink
feat(eslint-plugin) Added await-promise rule
Browse files Browse the repository at this point in the history
Adds the equivalent of TSLint's `await-promise` rule.
  • Loading branch information
Josh Goldberg committed Feb 3, 2019
1 parent 3efb265 commit 951a4fc
Show file tree
Hide file tree
Showing 5 changed files with 390 additions and 14 deletions.
1 change: 1 addition & 0 deletions packages/eslint-plugin/README.md
Expand Up @@ -96,6 +96,7 @@ Install [`eslint-config-prettier`](https://github.com/prettier/eslint-config-pre
| --------------------------------------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------ | -------- |
| [`@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<T>` for arrays (`array-type` from TSLint) | :heavy_check_mark: | :wrench: |
| [`@typescript-eslint/await-promise`](./docs/rules/await-promise.md) | Disallow awaiting a value that is not a Promise (`await-promise` from TSLint) | :heavy_check_mark: | :wrench: |
| [`@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/camelcase`](./docs/rules/camelcase.md) | Enforce camelCase naming convention | :heavy_check_mark: | |
| [`@typescript-eslint/class-name-casing`](./docs/rules/class-name-casing.md) | Require PascalCased class and interface names (`class-name` from TSLint) | :heavy_check_mark: | |
Expand Down
29 changes: 15 additions & 14 deletions packages/eslint-plugin/ROADMAP.md
@@ -1,10 +1,10 @@
# Roadmap

✅ (28) = done
🌟 (79) = in ESLint core
🔌 (33) = in another plugin
🌓 (16) = implementations differ or ESLint version is missing functionality
🛑 (70) = unimplemented
✅ (28) = done
🌟 (79) = in ESLint core
🔌 (33) = in another plugin
🌓 (17) = implementations differ or ESLint version is missing functionality
🛑 (69) = unimplemented

## TSLint rules

Expand Down Expand Up @@ -39,7 +39,7 @@

| TSLint rule | | ESLint rule |
| ------------------------------------ | :-: | --------------------------------------------------------------------- |
| [`await-promise`] | 🛑 | N/A |
| [`await-promise`] | | [`@typescript-eslint/no-misused-new`] |

This comment has been minimized.

Copy link
@j-f1

j-f1 Feb 3, 2019

Contributor

Is this supposed to be @typescript-eslint/await-promise?

| [`ban-comma-operator`] | 🌟 | [`no-sequences`][no-sequences] |
| [`ban`] | 🌟 | [`no-restricted-properties`][no-restricted-properties] |
| [`curly`] | 🌟 | [`curly`][curly] |
Expand Down Expand Up @@ -96,7 +96,7 @@
| [`use-default-type-parameter`] | 🛑 | N/A |
| [`use-isnan`] | 🌟 | [`use-isnan`][use-isnan] |

<sup>[1]</sup> The ESLint rule also supports silencing with an extra set of parens (`if ((foo = bar)) {}`)
<sup>[1]</sup> The ESLint rule also supports silencing with an extra set of parens (`if ((foo = bar)) {}`)
<sup>[2]</sup> Missing private class member support. [`@typescript-eslint/no-unused-vars`] adds support for some TS-specific features.

### Maintainability
Expand All @@ -120,7 +120,7 @@
| [`prefer-readonly`] | 🛑 | N/A |
| [`trailing-comma`] | 🌓 | [`comma-dangle`][comma-dangle] or [Prettier] |

<sup>[1]</sup> Only warns when importing deprecated symbols
<sup>[1]</sup> Only warns when importing deprecated symbols
<sup>[2]</sup> Missing support for blank-line-delimited sections

### Style
Expand Down Expand Up @@ -179,7 +179,7 @@
| [`variable-name`] | 🌟 | <sup>[2]</sup> |
| [`whitespace`] | 🔌 | Use [Prettier] |

<sup>[1]</sup> Recommended config: `["error", { blankLine: "always", prev: "*", next: "return" }]`
<sup>[1]</sup> Recommended config: `["error", { blankLine: "always", prev: "*", next: "return" }]`
<sup>[2]</sup> [`camelcase`][camelcase], [`no-underscore-dangle`][no-underscore-dangle], [`id-blacklist`][id-blacklist], and/or [`id-match`][id-match]

## tslint-microsoft-contrib rules
Expand Down Expand Up @@ -245,10 +245,10 @@ Relevant plugins: [`chai-expect-keywords`](https://github.com/gavinaiken/eslint-
| `use-named-parameter` | 🛑 | N/A |
| `use-simple-attributes` | 🛑 | N/A |

<sup>[1]</sup> Enforces blank lines both at the beginning and end of a block
<sup>[2]</sup> Recommended config: `["error", "ForInStatement"]`
<sup>[3]</sup> Recommended config: `["error", "declaration", { "allowArrowFunctions": true }]`
<sup>[4]</sup> Recommended config: `["error", { "terms": ["BUG", "HACK", "FIXME", "LATER", "LATER2", "TODO"], "location": "anywhere" }]`
<sup>[1]</sup> Enforces blank lines both at the beginning and end of a block
<sup>[2]</sup> Recommended config: `["error", "ForInStatement"]`
<sup>[3]</sup> Recommended config: `["error", "declaration", { "allowArrowFunctions": true }]`
<sup>[4]</sup> Recommended config: `["error", { "terms": ["BUG", "HACK", "FIXME", "LATER", "LATER2", "TODO"], "location": "anywhere" }]`
<sup>[5]</sup> Does not check class fields.

[insecure-random]: https://github.com/desktop/desktop/blob/master/eslint-rules/insecure-random.js
Expand Down Expand Up @@ -310,7 +310,7 @@ Relevant plugins: [`chai-expect-keywords`](https://github.com/gavinaiken/eslint-
| `react-a11y-titles` | 🛑 | N/A |
| `react-anchor-blank-noopener` | 🛑 | N/A |

<sup>[1]</sup> TSLint rule is more strict
<sup>[1]</sup> TSLint rule is more strict
<sup>[2]</sup> ESLint rule only reports for click handlers

[prettier]: https://prettier.io
Expand Down Expand Up @@ -558,6 +558,7 @@ Relevant plugins: [`chai-expect-keywords`](https://github.com/gavinaiken/eslint-
<!-- @typescript-eslint/eslint-plugin -->

[`@typescript-eslint/adjacent-overload-signatures`]: https://github.com/bradzacher/@typescript-eslint/eslint-plugin/blob/master/docs/rules/adjacent-overload-signatures.md
[`@typescript-eslint/await-promise`]: https://github.com/bradzacher/@typescript-eslint/eslint-plugin/blob/master/docs/rules/await-promise.md
[`@typescript-eslint/ban-types`]: https://github.com/bradzacher/@typescript-eslint/eslint-plugin/blob/master/docs/rules/ban-types.md
[`@typescript-eslint/explicit-member-accessibility`]: https://github.com/bradzacher/@typescript-eslint/eslint-plugin/blob/master/docs/rules/explicit-member-accessibility.md
[`@typescript-eslint/member-ordering`]: https://github.com/bradzacher/@typescript-eslint/eslint-plugin/blob/master/docs/rules/member-ordering.md
Expand Down
83 changes: 83 additions & 0 deletions packages/eslint-plugin/docs/rules/await-promise.md
@@ -0,0 +1,83 @@
# Disallows awaiting a value that is not a Promise (await-promise)

This rule disallows awaiting a value that is not 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();
```

```ts
// An array of Promises is not the same as an AsyncIterable
async function incorrect(arrayOfPromises: Array<Promise<string>) {
for await (const element of arrayOfPromises) {}
}
```
Examples of **correct** code for this rule:
```ts
await Promise.resolve('value');

const createValue = (async() = 'value');
await createValue();
```
```ts
async function overIterable(iterable: AsyncIterable<string>) {
for await (const element of iterable) {
}
}

async function overIterableIterator(iterable: AsyncIterableIterator<string>) {
for await (const element of iterable) {
}
}
```
## Options
The rule accepts an options object with the following property:
- `allowedPromiseNames` any extra names of classes or interfaces to be considered "awaitable" in `await` statements.
Classes named `Promise` may always be awaited.
`allowedPromiseNames` does not affect `for-await-of` statements.
### allowedPromiseNames
Examples of **incorrect** code for this rule with `{ allowedPromiseNames: ["Thenable"] }`:
```ts
class Thenable {
/* ... */
}

await new Thenable();
```
Examples of **incorrect** code for this rule with `{ allowedPromiseNames: ["Thenable"] }`:
```ts
class OtherClass {
/* ... */
}

await new OtherClass();
```
## 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)
124 changes: 124 additions & 0 deletions packages/eslint-plugin/lib/rules/await-promise.js
@@ -0,0 +1,124 @@
/**
* @fileoverview Disallows awaiting a value that is not a Promise
* @author Josh Goldberg
*/
'use strict';
const tsutils = require('tsutils');
const ts = require('typescript');
const util = require('../util');

//------------------------------------------------------------------------------
// Rule Definition
//------------------------------------------------------------------------------

const defaultOptions = [
{
allowedPromiseNames: []
}
];

/**
* @type {import("eslint").Rule.RuleModule}
*/
module.exports = {
meta: {
docs: {
description: 'Disallows awaiting a value that is not a Promise',
category: 'Functionality',
recommended: 'error',
extraDescription: [util.tslintRule('await-promise')],
url: util.metaDocsUrl('await-promise')
},
fixable: null,
messages: {
await: 'Invalid `await` of a non-Promise value.',
forOf: 'Invalid `for-await-of` of a non-AsyncIterable value.'
},
schema: [
{
type: 'object',
properties: {
allowedPromiseNames: {
type: 'array',
items: {
type: 'string'
}
}
},
additionalProperties: false
}
],
type: 'problem'
},

create(context) {
const options = util.applyDefault(defaultOptions, context.options)[0];

const allowedAsyncIterableNames = new Set([
'AsyncIterable',
'AsyncIterableIterator'
]);

const allowedPromiseNames = new Set([
'Promise',
...options.allowedPromiseNames
]);

const parserServices = util.getParserServices(context);
const checker = parserServices.program.getTypeChecker();

function validateNode(node, allowedSymbolNames, messageId) {
const originalNode = parserServices.esTreeNodeToTSNodeMap.get(node);
const type = checker.getTypeAtLocation(originalNode.expression);

if (!containsType(type, allowedSymbolNames)) {
context.report({
messageId,
node
});
}
}

return {
AwaitExpression(node) {
validateNode(node, allowedPromiseNames, 'await');
},
ForOfStatement(node) {
if (node.await) {
validateNode(node, allowedAsyncIterableNames, 'forOf');
}
}
};
}
};

/**
* @param {string} type Type being awaited upon.
* @param {Set<string>} allowedNames Symbol names being checked for.
*/
function containsType(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 => containsType(t, allowedNames));
}

const bases = type.getBaseTypes();
return (
typeof bases !== 'undefined' &&
bases.some(t => containsType(t, allowedNames))
);
}

0 comments on commit 951a4fc

Please sign in to comment.