-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
feat(eslint-plugin): add new rule require-await
- Loading branch information
1 parent
af70a59
commit d3d7830
Showing
6 changed files
with
336 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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> |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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( | ||
node, | ||
) as ts.ReturnStatement; | ||
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 | ||
>, | ||
}; | ||
}, | ||
}); |
123 changes: 123 additions & 0 deletions
123
packages/eslint-plugin/tests/rules/require-await.test.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,123 @@ | ||
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 arrow function expression with promise return | ||
code: `const numberOne = async (): Promise<number> => 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); | ||
}`, | ||
}, | ||
/*{ | ||
// Async arrow function expression with async function return | ||
code: `const numberOne = async (): Promise<number> => getAsyncNumber(1); | ||
const getAsyncNumber = async (x: number): Promise<number> => 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], | ||
}, | ||
], | ||
}); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters