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.

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

feat(eslint-plugin): [prom-func-async] add automatic fix #2845

Merged
merged 7 commits into from Dec 21, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Expand Up @@ -163,7 +163,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
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
@@ -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);
}
`,
},
],
});