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.

Sign up for GitHub

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

Jump to bottom

feat(eslint-plugin): [require-await] add --fix support #1561

Merged
merged 1 commit into from Feb 20, 2020
Merged

feat(eslint-plugin): [require-await] add --fix support #1561

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion packages/eslint-plugin/README.md
View file
Open in desktop
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
Open in desktop
@@ -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;
bradzacher marked this conversation as resolved.
Show resolved Hide resolved
}

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
Open in desktop
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
Open in desktop
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