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
3 changes: 3 additions & 0 deletions packages/eslint-plugin/src/rules/promise-function-async.ts
Expand Up @@ -91,6 +91,7 @@ export default util.createRule<Options, MessageIds>({
]);
const parserServices = util.getParserServices(context);
const checker = parserServices.program.getTypeChecker();
const sourceCode = context.getSourceCode();

function validateNode(
node:
Expand Down Expand Up @@ -140,12 +141,14 @@ export default util.createRule<Options, MessageIds>({
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 &&
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
79 changes: 79 additions & 0 deletions packages/eslint-plugin/src/util/getFunctionHeadLoc.ts
@@ -0,0 +1,79 @@
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 {
function getLocStart(): TSESTree.LineAndColumnData {
if (node.parent && node.parent.type === AST_NODE_TYPES.MethodDefinition) {
// return the start location for class method

if (node.parent.decorators && node.parent.decorators.length > 0) {
// exclude decorators
return sourceCode.getTokenAfter(
node.parent.decorators[node.parent.decorators.length - 1],
)!.loc.start;
}

return node.parent.loc.start;
}

if (
node.parent &&
node.parent.type === AST_NODE_TYPES.Property &&
node.parent.method
) {
// return the start location for object method shorthand
return node.parent.loc.start;
}

// return the start location for a regular function
return node.loc.start;
}

function getLocEnd(): TSESTree.LineAndColumnData {
if (node.type === AST_NODE_TYPES.ArrowFunctionExpression) {
// find the end location for arrow function expression
return sourceCode.getTokenBefore(
node.body,
token =>
token.type === AST_TOKEN_TYPES.Punctuator && token.value === '=>',
)!.loc.end;
}

// return the end location for a regular function
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
19 changes: 19 additions & 0 deletions packages/eslint-plugin/tests/rules/promise-function-async.test.ts
Expand Up @@ -611,6 +611,25 @@ async function foo(): Promise<string> | SPromise<boolean> {
return Math.random() > 0.5
? Promise.resolve('value')
: Promise.resolve(false);
}
`,
},
{
code: `
class Test {
@decorator
public test() {
return Promise.resolve(123);
}
}
`,
errors: [{ line: 4, column: 3, messageId }],
output: `
class Test {
@decorator
public async test() {
return Promise.resolve(123);
}
}
`,
},
Expand Down