Skip to content

Commit

Permalink
feat(eslint-plugin): [prom-func-async] add automatic fix (#2845)
Browse files Browse the repository at this point in the history
  • Loading branch information
@phaux
phaux committed Dec 21, 2020
1 parent 1ef0d64 commit 717e718
Show file tree
Hide file tree
Showing 3 changed files with 191 additions and 9 deletions.
2 changes: 1 addition & 1 deletion packages/eslint-plugin/README.md
View file
Expand Up @@ -164,7 +164,7 @@ Pro Tip: For larger codebases you may want to consider splitting our linting int
| [`@typescript-eslint/prefer-regexp-exec`](./docs/rules/prefer-regexp-exec.md) | Enforce that `RegExp#exec` is used instead of `String#match` if no global flag is provided | :heavy_check_mark: | | :thought_balloon: |
| [`@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/prefer-ts-expect-error`](./docs/rules/prefer-ts-expect-error.md) | Recommends using `@ts-expect-error` over `@ts-ignore` | | :wrench: | |
| [`@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/promise-function-async`](./docs/rules/promise-function-async.md) | Requires any function or method that returns a Promise to be marked async | | :wrench: | :thought_balloon: |
| [`@typescript-eslint/require-array-sort-compare`](./docs/rules/require-array-sort-compare.md) | Requires `Array#sort` calls to always provide a `compareFunction` | | | :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 | :heavy_check_mark: | | :thought_balloon: |
| [`@typescript-eslint/restrict-template-expressions`](./docs/rules/restrict-template-expressions.md) | Enforce template literal expressions to be of string type | :heavy_check_mark: | | :thought_balloon: |
Expand Down
42 changes: 36 additions & 6 deletions packages/eslint-plugin/src/rules/promise-function-async.ts
View file
Expand Up @@ -2,6 +2,7 @@ import {
AST_NODE_TYPES,
TSESTree,
} from '@typescript-eslint/experimental-utils';
import * as ts from 'typescript';
import * as util from '../util';

type Options = [
Expand All @@ -20,6 +21,7 @@ export default util.createRule<Options, MessageIds>({
name: 'promise-function-async',
meta: {
type: 'suggestion',
fixable: 'code',
docs: {
description:
'Requires any function or method that returns a Promise to be marked async',
Expand Down Expand Up @@ -94,9 +96,7 @@ export default util.createRule<Options, MessageIds>({
node:
| TSESTree.ArrowFunctionExpression
| TSESTree.FunctionDeclaration
| TSESTree.FunctionExpression
| TSESTree.MethodDefinition
| TSESTree.TSAbstractMethodDefinition,
| TSESTree.FunctionExpression,
): void {
const originalNode = parserServices.esTreeNodeToTSNodeMap.get(node);
const signatures = checker
Expand All @@ -114,6 +114,12 @@ export default util.createRule<Options, MessageIds>({
allAllowedPromiseNames,
)
) {
// Return type is not a promise
return;
}

if (node.parent?.type === AST_NODE_TYPES.TSAbstractMethodDefinition) {
// Abstract method can't be async
return;
}

Expand All @@ -123,12 +129,34 @@ export default util.createRule<Options, MessageIds>({
node.parent.type === AST_NODE_TYPES.MethodDefinition) &&
(node.parent.kind === 'get' || node.parent.kind === 'set')
) {
// Getters and setters can't be async
return;
}

if (
util.isTypeFlagSet(returnType, ts.TypeFlags.Any | ts.TypeFlags.Unknown)
) {
// Report without auto fixer because the return type is unknown
return context.report({
messageId: 'missingAsync',
node,
});
}

context.report({
messageId: 'missingAsync',
node,
fix: fixer => {
if (
node.parent &&
(node.parent.type === AST_NODE_TYPES.MethodDefinition ||
(node.parent.type === AST_NODE_TYPES.Property &&
node.parent.method))
) {
return fixer.insertTextBefore(node.parent.key, 'async ');
}
return fixer.insertTextBefore(node, 'async ');
},
});
}

Expand All @@ -152,13 +180,15 @@ export default util.createRule<Options, MessageIds>({
): void {
if (
node.parent &&
'kind' in node.parent &&
node.parent.type === AST_NODE_TYPES.MethodDefinition &&
node.parent.kind === 'method'
) {
if (checkMethodDeclarations) {
validateNode(node.parent);
validateNode(node);
}
} else if (checkFunctionExpressions) {
return;
}
if (checkFunctionExpressions) {
validateNode(node);
}
},
Expand Down
156 changes: 154 additions & 2 deletions packages/eslint-plugin/tests/rules/promise-function-async.test.ts
View file
@@ -1,5 +1,5 @@
import rule from '../../src/rules/promise-function-async';
import { RuleTester, getFixturesRootDir } from '../RuleTester';
import { getFixturesRootDir, RuleTester } from '../RuleTester';

const rootDir = getFixturesRootDir();
const messageId = 'missingAsync';
Expand Down Expand Up @@ -141,6 +141,18 @@ const foo = (options: Options): Return => {
code: `
function foo(): Promise<string> | boolean {
return Math.random() > 0.5 ? Promise.resolve('value') : false;
}
`,
},
{
code: `
abstract class Test {
abstract test1(): Promise<number>;
// abstract method with body is always an error but it still parses into valid AST
abstract test2(): Promise<number> {
return Promise.resolve(1);
}
}
`,
},
Expand Down Expand Up @@ -191,6 +203,11 @@ const nonAsyncPromiseFunctionExpressionA = function (p: Promise<void>) {
messageId,
},
],
output: `
const nonAsyncPromiseFunctionExpressionA = async function (p: Promise<void>) {
return p;
};
`,
},
{
code: `
Expand All @@ -203,6 +220,11 @@ const nonAsyncPromiseFunctionExpressionB = function () {
messageId,
},
],
output: `
const nonAsyncPromiseFunctionExpressionB = async function () {
return new Promise<void>();
};
`,
},
{
code: `
Expand All @@ -215,6 +237,11 @@ function nonAsyncPromiseFunctionDeclarationA(p: Promise<void>) {
messageId,
},
],
output: `
async function nonAsyncPromiseFunctionDeclarationA(p: Promise<void>) {
return p;
}
`,
},
{
code: `
Expand All @@ -227,6 +254,11 @@ function nonAsyncPromiseFunctionDeclarationB() {
messageId,
},
],
output: `
async function nonAsyncPromiseFunctionDeclarationB() {
return new Promise<void>();
}
`,
},
{
code: `
Expand All @@ -237,6 +269,9 @@ const nonAsyncPromiseArrowFunctionA = (p: Promise<void>) => p;
messageId,
},
],
output: `
const nonAsyncPromiseArrowFunctionA = async (p: Promise<void>) => p;
`,
},
{
code: `
Expand All @@ -247,6 +282,31 @@ const nonAsyncPromiseArrowFunctionB = () => new Promise<void>();
messageId,
},
],
output: `
const nonAsyncPromiseArrowFunctionB = async () => new Promise<void>();
`,
},
{
code: `
const functions = {
nonAsyncPromiseMethod() {
return Promise.resolve(1);
},
};
`,
errors: [
{
line: 3,
messageId,
},
],
output: `
const functions = {
async nonAsyncPromiseMethod() {
return Promise.resolve(1);
},
};
`,
},
{
code: `
Expand All @@ -255,7 +315,7 @@ class Test {
return p;
}
public nonAsyncPromiseMethodB() {
public static nonAsyncPromiseMethodB() {
return new Promise<void>();
}
}
Expand All @@ -270,6 +330,17 @@ class Test {
messageId,
},
],
output: `
class Test {
public async nonAsyncPromiseMethodA(p: Promise<void>) {
return p;
}
public static async nonAsyncPromiseMethodB() {
return new Promise<void>();
}
}
`,
},
{
code: `
Expand Down Expand Up @@ -308,6 +379,23 @@ class Test {
messageId,
},
],
output: `
const nonAsyncPromiseFunctionExpression = async function (p: Promise<void>) {
return p;
};
async function nonAsyncPromiseFunctionDeclaration(p: Promise<void>) {
return p;
}
const nonAsyncPromiseArrowFunction = (p: Promise<void>) => p;
class Test {
public async nonAsyncPromiseMethod(p: Promise<void>) {
return p;
}
}
`,
},
{
code: `
Expand Down Expand Up @@ -346,6 +434,23 @@ class Test {
messageId,
},
],
output: `
const nonAsyncPromiseFunctionExpression = async function (p: Promise<void>) {
return p;
};
function nonAsyncPromiseFunctionDeclaration(p: Promise<void>) {
return p;
}
const nonAsyncPromiseArrowFunction = async (p: Promise<void>) => p;
class Test {
public async nonAsyncPromiseMethod(p: Promise<void>) {
return p;
}
}
`,
},
{
code: `
Expand Down Expand Up @@ -384,6 +489,23 @@ class Test {
messageId,
},
],
output: `
const nonAsyncPromiseFunctionExpression = function (p: Promise<void>) {
return p;
};
async function nonAsyncPromiseFunctionDeclaration(p: Promise<void>) {
return p;
}
const nonAsyncPromiseArrowFunction = async (p: Promise<void>) => p;
class Test {
public async nonAsyncPromiseMethod(p: Promise<void>) {
return p;
}
}
`,
},
{
code: `
Expand Down Expand Up @@ -422,6 +544,23 @@ class Test {
messageId,
},
],
output: `
const nonAsyncPromiseFunctionExpression = async function (p: Promise<void>) {
return p;
};
async function nonAsyncPromiseFunctionDeclaration(p: Promise<void>) {
return p;
}
const nonAsyncPromiseArrowFunction = async (p: Promise<void>) => p;
class Test {
public nonAsyncPromiseMethod(p: Promise<void>) {
return p;
}
}
`,
},
{
code: `
Expand All @@ -440,6 +579,11 @@ const returnAllowedType = () => new PromiseType();
messageId,
},
],
output: `
class PromiseType {}
const returnAllowedType = async () => new PromiseType();
`,
},
{
code: `
Expand All @@ -461,6 +605,14 @@ function foo(): Promise<string> | SPromise<boolean> {
messageId,
},
],
output: `
interface SPromise<T> extends Promise<T> {}
async function foo(): Promise<string> | SPromise<boolean> {
return Math.random() > 0.5
? Promise.resolve('value')
: Promise.resolve(false);
}
`,
},
],
});

0 comments on commit 717e718

Please sign in to comment.