Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(eslint-plugin): add new rule require-await #674

Merged
merged 3 commits into from Jul 19, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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