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] report only function head #2872

Merged
merged 11 commits into from Feb 28, 2021
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
45 changes: 39 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 @@ -89,14 +91,13 @@ export default util.createRule<Options, MessageIds>({
]);
const parserServices = util.getParserServices(context);
const checker = parserServices.program.getTypeChecker();
const sourceCode = context.getSourceCode();

function validateNode(
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 +115,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 +130,36 @@ 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,
loc: util.getFunctionHeadLoc(node, sourceCode),
});
}

context.report({
messageId: 'missingAsync',
node,
loc: util.getFunctionHeadLoc(node, sourceCode),
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 +183,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
63 changes: 2 additions & 61 deletions packages/eslint-plugin/src/util/explicitReturnTypeUtils.ts
Expand Up @@ -2,75 +2,16 @@ import {
TSESTree,
AST_NODE_TYPES,
TSESLint,
AST_TOKEN_TYPES,
} from '@typescript-eslint/experimental-utils';
import { isTypeAssertion, isConstructor, isSetter } from './astUtils';
import { getFunctionHeadLoc } from './getFunctionHeadLoc';
import { nullThrows, NullThrowsReasons } from './nullThrows';

type FunctionExpression =
| TSESTree.ArrowFunctionExpression
| TSESTree.FunctionExpression;
type FunctionNode = FunctionExpression | TSESTree.FunctionDeclaration;

/**
* Creates a report location for the given function.
* The location only encompasses the "start" of the function, and not the body
*
* eg.
* function foo(args) {}
* ^^^^^^^^^^^^^^^^^^
*
* get y(args) {}
* ^^^^^^^^^^^
*
* const x = (args) => {}
* ^^^^^^^^^
*/
function getReporLoc(
node: FunctionNode,
sourceCode: TSESLint.SourceCode,
): TSESTree.SourceLocation {
/**
* Returns start column position
* @param node
*/
function getLocStart(): TSESTree.LineAndColumnData {
/* highlight method name */
const parent = node.parent;
if (
parent &&
(parent.type === AST_NODE_TYPES.MethodDefinition ||
(parent.type === AST_NODE_TYPES.Property && parent.method))
) {
return parent.loc.start;
}

return node.loc.start;
}

/**
* Returns end column position
* @param node
*/
function getLocEnd(): TSESTree.LineAndColumnData {
/* highlight `=>` */
if (node.type === AST_NODE_TYPES.ArrowFunctionExpression) {
return sourceCode.getTokenBefore(
node.body,
token =>
token.type === AST_TOKEN_TYPES.Punctuator && token.value === '=>',
)!.loc.end;
}

return sourceCode.getTokenBefore(node.body)!.loc.end;
}

return {
start: getLocStart(),
end: getLocEnd(),
};
}

/**
* Checks if a node is a variable declarator with a type annotation.
* ```
Expand Down Expand Up @@ -327,7 +268,7 @@ function checkFunctionReturnType(
return;
}

report(getReporLoc(node, sourceCode));
report(getFunctionHeadLoc(node, sourceCode));
}

/**
Expand Down
73 changes: 73 additions & 0 deletions packages/eslint-plugin/src/util/getFunctionHeadLoc.ts
@@ -0,0 +1,73 @@
import {
AST_NODE_TYPES,
AST_TOKEN_TYPES,
TSESLint,
TSESTree,
} from '@typescript-eslint/experimental-utils';

type FunctionNode =
| TSESTree.ArrowFunctionExpression
| TSESTree.FunctionExpression
| TSESTree.FunctionDeclaration;

/**
* Creates a report location for the given function.
* The location only encompasses the "start" of the function, and not the body
*
* eg.
*
* ```
* function foo(args) {}
* ^^^^^^^^^^^^^^^^^^
*
* get y(args) {}
* ^^^^^^^^^^^
*
* const x = (args) => {}
* ^^^^^^^^^
* ```
*/
export function getFunctionHeadLoc(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

while we're here - one case I believe this code doesn't correctly handle is decorators because decorators are included within the method's start.

Eg this code will highlight:

class Foo {
  @deco
  ^^^^^
  foo() {}
  ^^^^^
}

this has confused some people in the past (eg #751)

node: FunctionNode,
sourceCode: TSESLint.SourceCode,
): TSESTree.SourceLocation {
/**
* Returns start column position
* @param node
*/
function getLocStart(): TSESTree.LineAndColumnData {
/* highlight method name */
const parent = node.parent;
if (
parent &&
(parent.type === AST_NODE_TYPES.MethodDefinition ||
(parent.type === AST_NODE_TYPES.Property && parent.method))
) {
return parent.loc.start;
}

return node.loc.start;
}

/**
* Returns end column position
* @param node
*/
function getLocEnd(): TSESTree.LineAndColumnData {
/* highlight `=>` */
if (node.type === AST_NODE_TYPES.ArrowFunctionExpression) {
return sourceCode.getTokenBefore(
node.body,
token =>
token.type === AST_TOKEN_TYPES.Punctuator && token.value === '=>',
)!.loc.end;
}

return sourceCode.getTokenBefore(node.body)!.loc.end;
}

return {
start: getLocStart(),
end: getLocEnd(),
};
}
1 change: 1 addition & 0 deletions packages/eslint-plugin/src/util/index.ts
Expand Up @@ -3,6 +3,7 @@ import { ESLintUtils } from '@typescript-eslint/experimental-utils';
export * from './astUtils';
export * from './collectUnusedVariables';
export * from './createRule';
export * from './getFunctionHeadLoc';
export * from './isTypeReadonly';
export * from './misc';
export * from './nullThrows';
Expand Down