Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
feat(eslint-plugin): add new rule require-await (#674)
  • Loading branch information
scottohara authored and bradzacher committed Jul 19, 2019
1 parent f949355 commit 807bc2d
Show file tree
Hide file tree
Showing 8 changed files with 329 additions and 0 deletions.
1 change: 1 addition & 0 deletions packages/eslint-plugin/README.md
Expand Up @@ -175,6 +175,7 @@ Then you should add `airbnb` (or `airbnb-base`) to your `extends` section of `.e
| [`@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/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/require-array-sort-compare`](./docs/rules/require-array-sort-compare.md) | Enforce giving `compare` argument to `Array#sort` | | | :thought_balloon: |
| [`@typescript-eslint/require-await`](./docs/rules/require-await.md) | Disallow async functions which have no `await` expression | | | :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 | | | :thought_balloon: |
| [`@typescript-eslint/semi`](./docs/rules/semi.md) | Require or disallow semicolons instead of ASI | | :wrench: | |
| [`@typescript-eslint/strict-boolean-expressions`](./docs/rules/strict-boolean-expressions.md) | Restricts the types allowed in boolean expressions | | | :thought_balloon: |
Expand Down
49 changes: 49 additions & 0 deletions packages/eslint-plugin/docs/rules/require-await.md
@@ -0,0 +1,49 @@
# Disallow async functions which have no await expression (@typescript-eslint/require-await)

Asynchronous functions that don’t use `await` might not need to be asynchronous functions and could be the unintentional result of refactoring.

## Rule Details

The `@typescript-eslint/require-await` rule extends the `require-await` rule from ESLint core, and allows for cases where the additional typing information can prevent false positives that would otherwise trigger the rule.

One example is when a function marked as `async` returns a value that is:

1. already a promise; or
2. the result of calling another `async` function

```typescript
async function numberOne(): Promise<number> {
return Promise.resolve(1);
}

async function getDataFromApi(endpoint: string): Promise<Response> {
return fetch(endpoint);
}
```

In the above examples, the core `require-await` triggers the following warnings:

```
async function 'numberOne' has no 'await' expression
async function 'getDataFromApi' has no 'await' expression
```

One way to resolve these errors is to remove the `async` keyword. However doing so can cause a conflict with the [`@typescript-eslint/promise-function-async`](https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/promise-function-async.md) rule (if enabled), which requires any function returning a promise to be marked as `async`.

Another way to resolve these errors is to add an `await` keyword to the return statements. However doing so can cause a conflict with the [`no-return-await`](https://eslint.org/docs/rules/no-return-await) rule (if enabled), which warns against using `return await` since the return value of an `async` function is always wrapped in `Promise.resolve` anyway.

With the additional typing information available in Typescript code, this extension to the `require-await` rule is able to look at the _actual_ return types of an `async` function (before being implicitly wrapped in `Promise.resolve`), and avoid the need for an `await` expression when the return value is already a promise.

See the [ESLint documentation](https://eslint.org/docs/rules/require-await) for more details on the `require-await` rule.

## Rule Changes

```cjson
{
// note you must disable the base rule as it can report incorrect errors
"require-await": "off",
"@typescript-eslint/require-await": "error"
}
```

<sup>Taken with ❤️ [from ESLint core](https://github.com/eslint/eslint/blob/master/docs/rules/require-await.md)</sup>
1 change: 1 addition & 0 deletions packages/eslint-plugin/src/configs/all.json
Expand Up @@ -62,6 +62,7 @@
"@typescript-eslint/prefer-string-starts-ends-with": "error",
"@typescript-eslint/promise-function-async": "error",
"@typescript-eslint/require-array-sort-compare": "error",
"@typescript-eslint/require-await": "error",
"@typescript-eslint/restrict-plus-operands": "error",
"semi": "off",
"@typescript-eslint/semi": "error",
Expand Down
1 change: 1 addition & 0 deletions packages/eslint-plugin/src/configs/recommended.json
Expand Up @@ -7,6 +7,7 @@
"camelcase": "off",
"@typescript-eslint/camelcase": "error",
"@typescript-eslint/class-name-casing": "error",
"@typescript-eslint/consistent-type-definitions": "error",
"@typescript-eslint/explicit-function-return-type": "warn",
"@typescript-eslint/explicit-member-accessibility": "error",
"indent": "off",
Expand Down
2 changes: 2 additions & 0 deletions packages/eslint-plugin/src/rules/index.ts
Expand Up @@ -52,6 +52,7 @@ import preferRegexpExec from './prefer-regexp-exec';
import preferStringStartsEndsWith from './prefer-string-starts-ends-with';
import promiseFunctionAsync from './promise-function-async';
import requireArraySortCompare from './require-array-sort-compare';
import requireAwait from './require-await';
import restrictPlusOperands from './restrict-plus-operands';
import semi from './semi';
import strictBooleanExpressions from './strict-boolean-expressions';
Expand Down Expand Up @@ -115,6 +116,7 @@ export default {
'prefer-string-starts-ends-with': preferStringStartsEndsWith,
'promise-function-async': promiseFunctionAsync,
'require-array-sort-compare': requireArraySortCompare,
'require-await': requireAwait,
'restrict-plus-operands': restrictPlusOperands,
semi: semi,
'strict-boolean-expressions': strictBooleanExpressions,
Expand Down
138 changes: 138 additions & 0 deletions packages/eslint-plugin/src/rules/require-await.ts
@@ -0,0 +1,138 @@
import {
TSESTree,
TSESLint,
AST_NODE_TYPES,
} from '@typescript-eslint/experimental-utils';
import baseRule from 'eslint/lib/rules/require-await';
import * as tsutils from 'tsutils';
import ts from 'typescript';
import * as util from '../util';

type Options = util.InferOptionsTypeFromRule<typeof baseRule>;
type MessageIds = util.InferMessageIdsTypeFromRule<typeof baseRule>;

interface ScopeInfo {
upper: ScopeInfo | null;
returnsPromise: boolean;
}

export default util.createRule<Options, MessageIds>({
name: 'require-await',
meta: {
type: 'suggestion',
docs: {
description: 'Disallow async functions which have no `await` expression',
category: 'Best Practices',
recommended: false,
},
schema: baseRule.meta.schema,
messages: baseRule.meta.messages,
},
defaultOptions: [],
create(context) {
const rules = baseRule.create(context);
const parserServices = util.getParserServices(context);
const checker = parserServices.program.getTypeChecker();

let scopeInfo: ScopeInfo | null = null;

/**
* Push the scope info object to the stack.
*
* @returns {void}
*/
function enterFunction(
node:
| TSESTree.FunctionDeclaration
| TSESTree.FunctionExpression
| TSESTree.ArrowFunctionExpression,
) {
scopeInfo = {
upper: scopeInfo,
returnsPromise: false,
};

switch (node.type) {
case AST_NODE_TYPES.FunctionDeclaration:
rules.FunctionDeclaration(node);
break;

case AST_NODE_TYPES.FunctionExpression:
rules.FunctionExpression(node);
break;

case AST_NODE_TYPES.ArrowFunctionExpression:
rules.ArrowFunctionExpression(node);
break;
}
}

/**
* Pop the top scope info object from the stack.
* Passes through to the base rule if the function doesn't return a promise
*
* @param {ASTNode} node - The node exiting
* @returns {void}
*/
function exitFunction(
node:
| TSESTree.FunctionDeclaration
| TSESTree.FunctionExpression
| TSESTree.ArrowFunctionExpression,
) {
if (scopeInfo) {
if (!scopeInfo.returnsPromise) {
switch (node.type) {
case AST_NODE_TYPES.FunctionDeclaration:
rules['FunctionDeclaration:exit'](node);
break;

case AST_NODE_TYPES.FunctionExpression:
rules['FunctionExpression:exit'](node);
break;

case AST_NODE_TYPES.ArrowFunctionExpression:
rules['ArrowFunctionExpression:exit'](node);
break;
}
}

scopeInfo = scopeInfo.upper;
}
}

return {
'FunctionDeclaration[async = true]': enterFunction,
'FunctionExpression[async = true]': enterFunction,
'ArrowFunctionExpression[async = true]': enterFunction,
'FunctionDeclaration[async = true]:exit': exitFunction,
'FunctionExpression[async = true]:exit': exitFunction,
'ArrowFunctionExpression[async = true]:exit': exitFunction,

ReturnStatement(node: TSESTree.ReturnStatement) {
if (!scopeInfo) {
return;
}

const { expression } = parserServices.esTreeNodeToTSNodeMap.get<
ts.ReturnStatement
>(node);
if (!expression) {
return;
}

const type = checker.getTypeAtLocation(expression);
if (tsutils.isThenableType(checker, expression, type)) {
scopeInfo.returnsPromise = true;
}
},

AwaitExpression: rules.AwaitExpression as TSESLint.RuleFunction<
TSESTree.Node
>,
ForOfStatement: rules.ForOfStatement as TSESLint.RuleFunction<
TSESTree.Node
>,
};
},
});
114 changes: 114 additions & 0 deletions packages/eslint-plugin/tests/rules/require-await.test.ts
@@ -0,0 +1,114 @@
import rule from '../../src/rules/require-await';
import { RuleTester, getFixturesRootDir } from '../RuleTester';

const rootDir = getFixturesRootDir();

const ruleTester = new RuleTester({
parserOptions: {
ecmaVersion: 2018,
tsconfigRootDir: rootDir,
project: './tsconfig.json',
},
parser: '@typescript-eslint/parser',
});

const noAwaitFunctionDeclaration: any = {
message: "Async function 'numberOne' has no 'await' expression.",
};

const noAwaitFunctionExpression: any = {
message: "Async function has no 'await' expression.",
};

const noAwaitAsyncFunctionExpression: any = {
message: "Async arrow function has no 'await' expression.",
};

ruleTester.run('require-await', rule, {
valid: [
{
// Non-async function declaration
code: `function numberOne(): number {
return 1;
}`,
},
{
// Non-async function expression
code: `const numberOne = function(): number {
return 1;
}`,
},
{
// Non-async arrow function expression
code: `const numberOne = (): number => 1;`,
},
{
// Async function declaration with await
code: `async function numberOne(): Promise<number> {
return await 1;
}`,
},
{
// Async function expression with await
code: `const numberOne = async function(): Promise<number> {
return await 1;
}`,
},
{
// Async arrow function expression with await
code: `const numberOne = async (): Promise<number> => await 1;`,
},
{
// Async function declaration with promise return
code: `async function numberOne(): Promise<number> {
return Promise.resolve(1);
}`,
},
{
// Async function expression with promise return
code: `const numberOne = async function(): Promise<number> {
return Promise.resolve(1);
}`,
},
{
// Async function declaration with async function return
code: `async function numberOne(): Promise<number> {
return getAsyncNumber(1);
}
async function getAsyncNumber(x: number): Promise<number> {
return Promise.resolve(x);
}`,
},
{
// Async function expression with async function return
code: `const numberOne = async function(): Promise<number> {
return getAsyncNumber(1);
}
const getAsyncNumber = async function(x: number): Promise<number> {
return Promise.resolve(x);
}`,
},
],

invalid: [
{
// Async function declaration with no await
code: `async function numberOne(): Promise<number> {
return 1;
}`,
errors: [noAwaitFunctionDeclaration],
},
{
// Async function expression with no await
code: `const numberOne = async function(): Promise<number> {
return 1;
}`,
errors: [noAwaitFunctionExpression],
},
{
// Async arrow function expression with no await
code: `const numberOne = async (): Promise<number> => 1;`,
errors: [noAwaitAsyncFunctionExpression],
},
],
});
23 changes: 23 additions & 0 deletions packages/eslint-plugin/typings/eslint-rules.d.ts
Expand Up @@ -409,6 +409,29 @@ declare module 'eslint/lib/rules/no-extra-parens' {
export = rule;
}

declare module 'eslint/lib/rules/require-await' {
import { TSESLint, TSESTree } from '@typescript-eslint/experimental-utils';

const rule: TSESLint.RuleModule<
never,
[],
{
FunctionDeclaration(node: TSESTree.FunctionDeclaration): void;
FunctionExpression(node: TSESTree.FunctionExpression): void;
ArrowFunctionExpression(node: TSESTree.ArrowFunctionExpression): void;
'FunctionDeclaration:exit'(node: TSESTree.FunctionDeclaration): void;
'FunctionExpression:exit'(node: TSESTree.FunctionExpression): void;
'ArrowFunctionExpression:exit'(
node: TSESTree.ArrowFunctionExpression,
): void;
ReturnStatement(node: TSESTree.ReturnStatement): void;
AwaitExpression(node: TSESTree.AwaitExpression): void;
ForOfStatement(node: TSESTree.ForOfStatement): void;
}
>;
export = rule;
}

declare module 'eslint/lib/rules/semi' {
import { TSESLint, TSESTree } from '@typescript-eslint/experimental-utils';

Expand Down

0 comments on commit 807bc2d

Please sign in to comment.