From 9edd863b2a66ee44bd4a439903973e6c207480aa Mon Sep 17 00:00:00 2001 From: Ian MacLeod Date: Thu, 20 Feb 2020 08:32:53 -0800 Subject: [PATCH] feat(eslint-plugin): [require-await] add --fix support (#1561) --- packages/eslint-plugin/README.md | 2 +- .../eslint-plugin/src/rules/return-await.ts | 63 ++++++++++- packages/eslint-plugin/src/util/astUtils.ts | 20 ++++ .../tests/rules/return-await.test.ts | 107 ++++++++++++++++++ 4 files changed, 190 insertions(+), 2 deletions(-) diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index 0f55ace3d5d..fcc422386b4 100644 --- a/packages/eslint-plugin/README.md +++ b/packages/eslint-plugin/README.md @@ -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: | | diff --git a/packages/eslint-plugin/src/rules/return-await.ts b/packages/eslint-plugin/src/rules/return-await.ts index f77d80fe5d4..d18e41ef00e 100644 --- a/packages/eslint-plugin/src/rules/return-await.ts +++ b/packages/eslint-plugin/src/rules/return-await.ts @@ -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'; @@ -16,6 +17,7 @@ export default util.createRule({ requiresTypeChecking: true, extendsBaseRule: 'no-return-await', }, + fixable: 'code', type: 'problem', messages: { nonPromiseAwait: @@ -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; @@ -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); @@ -79,6 +135,7 @@ export default util.createRule({ context.report({ messageId: 'nonPromiseAwait', node, + fix: fixer => removeAwait(fixer, node), }); return; } @@ -88,6 +145,7 @@ export default util.createRule({ context.report({ messageId: 'requiredPromiseAwait', node, + fix: fixer => insertAwait(fixer, node), }); } @@ -99,6 +157,7 @@ export default util.createRule({ context.report({ messageId: 'disallowedPromiseAwait', node, + fix: fixer => removeAwait(fixer, node), }); } @@ -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), }); } diff --git a/packages/eslint-plugin/src/util/astUtils.ts b/packages/eslint-plugin/src/util/astUtils.ts index 367bd93350b..29dcdc0b459 100644 --- a/packages/eslint-plugin/src/util/astUtils.ts +++ b/packages/eslint-plugin/src/util/astUtils.ts @@ -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, diff --git a/packages/eslint-plugin/tests/rules/return-await.test.ts b/packages/eslint-plugin/tests/rules/return-await.test.ts index fc5f2597ebf..8599a937e19 100644 --- a/packages/eslint-plugin/tests/rules/return-await.test.ts +++ b/packages/eslint-plugin/tests/rules/return-await.test.ts @@ -164,6 +164,9 @@ ruleTester.run('return-await', rule, { code: `async function test() { return await 1; }`, + output: `async function test() { + return 1; + }`, errors: [ { line: 2, @@ -171,8 +174,52 @@ ruleTester.run('return-await', rule, { }, ], }, + { + 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, @@ -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, @@ -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, @@ -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, @@ -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, @@ -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, @@ -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, @@ -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, @@ -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, @@ -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, @@ -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, @@ -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, @@ -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, @@ -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, @@ -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,