Skip to content

Commit

Permalink
feat(eslint-plugin): [require-await] add --fix support (#1561)
Browse files Browse the repository at this point in the history
  • Loading branch information
@nevir
nevir committed Feb 20, 2020
1 parent 1aa7135 commit 9edd863
Show file tree
Hide file tree
Showing 4 changed files with 190 additions and 2 deletions.
2 changes: 1 addition & 1 deletion packages/eslint-plugin/README.md
View file
Expand Up @@ -185,7 +185,7 @@ In these cases, we create what we call an extension rule; a rule within our plug
| [`@typescript-eslint/no-useless-constructor`](./docs/rules/no-useless-constructor.md) | Disallow unnecessary constructors | | | |
| [`@typescript-eslint/quotes`](./docs/rules/quotes.md) | Enforce the consistent use of either backticks, double, or single quotes | | :wrench: | |
| [`@typescript-eslint/require-await`](./docs/rules/require-await.md) | Disallow async functions which have no `await` expression | :heavy_check_mark: | | :thought_balloon: |
| [`@typescript-eslint/return-await`](./docs/rules/return-await.md) | Enforces consistent returning of awaited values | | | :thought_balloon: |
| [`@typescript-eslint/return-await`](./docs/rules/return-await.md) | Enforces consistent returning of awaited values | | :wrench: | :thought_balloon: |
| [`@typescript-eslint/semi`](./docs/rules/semi.md) | Require or disallow semicolons instead of ASI | | :wrench: | |
| [`@typescript-eslint/space-before-function-paren`](./docs/rules/space-before-function-paren.md) | Enforces consistent spacing before function parenthesis | | :wrench: | |

Expand Down
63 changes: 62 additions & 1 deletion packages/eslint-plugin/src/rules/return-await.ts
View file
@@ -1,6 +1,7 @@
import {
AST_NODE_TYPES,
TSESTree,
TSESLint,
} from '@typescript-eslint/experimental-utils';
import * as tsutils from 'tsutils';
import * as ts from 'typescript';
Expand All @@ -16,6 +17,7 @@ export default util.createRule({
requiresTypeChecking: true,
extendsBaseRule: 'no-return-await',
},
fixable: 'code',
type: 'problem',
messages: {
nonPromiseAwait:
Expand All @@ -36,6 +38,7 @@ export default util.createRule({
create(context, [option]) {
const parserServices = util.getParserServices(context);
const checker = parserServices.program.getTypeChecker();
const sourceCode = context.getSourceCode();

function inTryCatch(node: ts.Node): boolean {
let ancestor = node.parent;
Expand All @@ -54,13 +57,66 @@ export default util.createRule({
return false;
}

// function findTokensToRemove()

function removeAwait(
fixer: TSESLint.RuleFixer,
node: TSESTree.ReturnStatement | TSESTree.ArrowFunctionExpression,
): TSESLint.RuleFix | null {
const awaitNode =
node.type === AST_NODE_TYPES.ReturnStatement
? node.argument
: node.body;
// Should always be an await node; but let's be safe.
/* istanbul ignore if */ if (!util.isAwaitExpression(awaitNode)) {
return null;
}

const awaitToken = sourceCode.getFirstToken(
awaitNode,
util.isAwaitKeyword,
);
// Should always be the case; but let's be safe.
/* istanbul ignore if */ if (!awaitToken) {
return null;
}

const startAt = awaitToken.range[0];
let endAt = awaitToken.range[1];
// Also remove any extraneous whitespace after `await`, if there is any.
const nextToken = sourceCode.getTokenAfter(awaitToken, {
includeComments: true,
});
if (nextToken) {
endAt = nextToken.range[0];
}

return fixer.removeRange([startAt, endAt]);
}

function insertAwait(
fixer: TSESLint.RuleFixer,
node: TSESTree.ReturnStatement | TSESTree.ArrowFunctionExpression,
): TSESLint.RuleFix | null {
const targetNode =
node.type === AST_NODE_TYPES.ReturnStatement
? node.argument
: node.body;
// There should always be a target node; but let's be safe.
/* istanbul ignore if */ if (!targetNode) {
return null;
}

return fixer.insertTextBefore(targetNode, 'await ');
}

function test(
node: TSESTree.ReturnStatement | TSESTree.ArrowFunctionExpression,
expression: ts.Node,
): void {
let child: ts.Node;

const isAwait = expression.kind === ts.SyntaxKind.AwaitExpression;
const isAwait = tsutils.isAwaitExpression(expression);

if (isAwait) {
child = expression.getChildAt(1);
Expand All @@ -79,6 +135,7 @@ export default util.createRule({
context.report({
messageId: 'nonPromiseAwait',
node,
fix: fixer => removeAwait(fixer, node),
});
return;
}
Expand All @@ -88,6 +145,7 @@ export default util.createRule({
context.report({
messageId: 'requiredPromiseAwait',
node,
fix: fixer => insertAwait(fixer, node),
});
}

Expand All @@ -99,6 +157,7 @@ export default util.createRule({
context.report({
messageId: 'disallowedPromiseAwait',
node,
fix: fixer => removeAwait(fixer, node),
});
}

Expand All @@ -111,11 +170,13 @@ export default util.createRule({
context.report({
messageId: 'disallowedPromiseAwait',
node,
fix: fixer => removeAwait(fixer, node),
});
} else if (!isAwait && isInTryCatch) {
context.report({
messageId: 'requiredPromiseAwait',
node,
fix: fixer => insertAwait(fixer, node),
});
}

Expand Down
20 changes: 20 additions & 0 deletions packages/eslint-plugin/src/util/astUtils.ts
View file
Expand Up @@ -114,7 +114,27 @@ function isIdentifier(
return node?.type === AST_NODE_TYPES.Identifier;
}

/**
* Checks if a node represents an `await …` expression.
*/
function isAwaitExpression(
node: TSESTree.Node | undefined | null,
): node is TSESTree.AwaitExpression {
return node?.type === AST_NODE_TYPES.AwaitExpression;
}

/**
* Checks if a possible token is the `await` keyword.
*/
function isAwaitKeyword(
node: TSESTree.Token | TSESTree.Comment | undefined | null,
): node is TSESTree.KeywordToken & { value: 'await' } {
return node?.type === AST_TOKEN_TYPES.Identifier && node.value === 'await';
}

export {
isAwaitExpression,
isAwaitKeyword,
isConstructor,
isIdentifier,
isLogicalOrOperator,
Expand Down
107 changes: 107 additions & 0 deletions packages/eslint-plugin/tests/rules/return-await.test.ts
View file
Expand Up @@ -164,15 +164,62 @@ ruleTester.run('return-await', rule, {
code: `async function test() {
return await 1;
}`,
output: `async function test() {
return 1;
}`,
errors: [
{
line: 2,
messageId: 'nonPromiseAwait',
},
],
},
{
code: `async function test() {
const foo = 1;
return await{foo};
}`,
output: `async function test() {
const foo = 1;
return {foo};
}`,
errors: [
{
line: 3,
messageId: 'nonPromiseAwait',
},
],
},
{
code: `async function test() {
const foo = 1;
return await
foo;
}`,
output: `async function test() {
const foo = 1;
return foo;
}`,
errors: [
{
line: 3,
messageId: 'nonPromiseAwait',
},
],
},
{
code: `const test = async () => await 1;`,
output: `const test = async () => 1;`,
errors: [
{
line: 1,
messageId: 'nonPromiseAwait',
},
],
},
{
code: `const test = async () => await/* comment */1;`,
output: `const test = async () => /* comment */1;`,
errors: [
{
line: 1,
Expand All @@ -182,6 +229,7 @@ ruleTester.run('return-await', rule, {
},
{
code: `const test = async () => await Promise.resolve(1);`,
output: `const test = async () => Promise.resolve(1);`,
errors: [
{
line: 1,
Expand All @@ -199,6 +247,15 @@ ruleTester.run('return-await', rule, {
console.log('cleanup');
}
}`,
output: `async function test() {
try {
return await Promise.resolve(1);
} catch (e) {
return await Promise.resolve(2);
} finally {
console.log('cleanup');
}
}`,
errors: [
{
line: 3,
Expand All @@ -214,6 +271,9 @@ ruleTester.run('return-await', rule, {
code: `async function test() {
return await Promise.resolve(1);
}`,
output: `async function test() {
return Promise.resolve(1);
}`,
errors: [
{
line: 2,
Expand All @@ -226,6 +286,9 @@ ruleTester.run('return-await', rule, {
code: `async function test() {
return await 1;
}`,
output: `async function test() {
return 1;
}`,
errors: [
{
line: 2,
Expand All @@ -236,6 +299,7 @@ ruleTester.run('return-await', rule, {
{
options: ['in-try-catch'],
code: `const test = async () => await 1;`,
output: `const test = async () => 1;`,
errors: [
{
line: 1,
Expand All @@ -246,6 +310,7 @@ ruleTester.run('return-await', rule, {
{
options: ['in-try-catch'],
code: `const test = async () => await Promise.resolve(1);`,
output: `const test = async () => Promise.resolve(1);`,
errors: [
{
line: 1,
Expand All @@ -264,6 +329,15 @@ ruleTester.run('return-await', rule, {
console.log('cleanup');
}
}`,
output: `async function test() {
try {
return await Promise.resolve(1);
} catch (e) {
return await Promise.resolve(2);
} finally {
console.log('cleanup');
}
}`,
errors: [
{
line: 3,
Expand All @@ -280,6 +354,9 @@ ruleTester.run('return-await', rule, {
code: `async function test() {
return await Promise.resolve(1);
}`,
output: `async function test() {
return Promise.resolve(1);
}`,
errors: [
{
line: 2,
Expand All @@ -292,6 +369,9 @@ ruleTester.run('return-await', rule, {
code: `async function test() {
return await 1;
}`,
output: `async function test() {
return 1;
}`,
errors: [
{
line: 2,
Expand All @@ -310,6 +390,15 @@ ruleTester.run('return-await', rule, {
console.log('cleanup');
}
}`,
output: `async function test() {
try {
return Promise.resolve(1);
} catch (e) {
return Promise.resolve(2);
} finally {
console.log('cleanup');
}
}`,
errors: [
{
line: 3,
Expand All @@ -326,6 +415,9 @@ ruleTester.run('return-await', rule, {
code: `async function test() {
return await Promise.resolve(1);
}`,
output: `async function test() {
return Promise.resolve(1);
}`,
errors: [
{
line: 2,
Expand All @@ -338,6 +430,9 @@ ruleTester.run('return-await', rule, {
code: `async function test() {
return await 1;
}`,
output: `async function test() {
return 1;
}`,
errors: [
{
line: 2,
Expand All @@ -356,6 +451,15 @@ ruleTester.run('return-await', rule, {
console.log('cleanup');
}
}`,
output: `async function test() {
try {
return await Promise.resolve(1);
} catch (e) {
return await Promise.resolve(2);
} finally {
console.log('cleanup');
}
}`,
errors: [
{
line: 3,
Expand All @@ -372,6 +476,9 @@ ruleTester.run('return-await', rule, {
code: `async function test() {
return Promise.resolve(1);
}`,
output: `async function test() {
return await Promise.resolve(1);
}`,
errors: [
{
line: 2,
Expand Down

0 comments on commit 9edd863

Please sign in to comment.