From 5a6e5df13730ab37eceef5a824afd61d32b7ee56 Mon Sep 17 00:00:00 2001 From: Nikita Stefaniak Date: Thu, 3 Dec 2020 21:10:57 +0100 Subject: [PATCH 1/7] feat(eslint-plugin): [prom-func-async] add fixer --- packages/eslint-plugin/README.md | 2 +- .../src/rules/promise-function-async.ts | 18 +++ .../rules/promise-function-async.test.ts | 120 +++++++++++++++++- 3 files changed, 138 insertions(+), 2 deletions(-) diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index 4cf27e64a83..1391af7e18d 100644 --- a/packages/eslint-plugin/README.md +++ b/packages/eslint-plugin/README.md @@ -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: | diff --git a/packages/eslint-plugin/src/rules/promise-function-async.ts b/packages/eslint-plugin/src/rules/promise-function-async.ts index 16134675d57..32072754621 100644 --- a/packages/eslint-plugin/src/rules/promise-function-async.ts +++ b/packages/eslint-plugin/src/rules/promise-function-async.ts @@ -3,6 +3,7 @@ import { TSESTree, } from '@typescript-eslint/experimental-utils'; import * as util from '../util'; +import * as ts from 'typescript'; type Options = [ { @@ -20,6 +21,7 @@ export default util.createRule({ name: 'promise-function-async', meta: { type: 'suggestion', + fixable: 'code', docs: { description: 'Requires any function or method that returns a Promise to be marked async', @@ -126,9 +128,25 @@ export default util.createRule({ 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.type === AST_NODE_TYPES.MethodDefinition) { + return fixer.insertTextBefore(node.key, 'async '); + } + return fixer.insertTextBefore(node, 'async '); + }, }); } diff --git a/packages/eslint-plugin/tests/rules/promise-function-async.test.ts b/packages/eslint-plugin/tests/rules/promise-function-async.test.ts index 47b5b6221d6..cb96167610e 100644 --- a/packages/eslint-plugin/tests/rules/promise-function-async.test.ts +++ b/packages/eslint-plugin/tests/rules/promise-function-async.test.ts @@ -191,6 +191,11 @@ const nonAsyncPromiseFunctionExpressionA = function (p: Promise) { messageId, }, ], + output: ` +const nonAsyncPromiseFunctionExpressionA = async function (p: Promise) { + return p; +}; + `, }, { code: ` @@ -203,6 +208,11 @@ const nonAsyncPromiseFunctionExpressionB = function () { messageId, }, ], + output: ` +const nonAsyncPromiseFunctionExpressionB = async function () { + return new Promise(); +}; + `, }, { code: ` @@ -215,6 +225,11 @@ function nonAsyncPromiseFunctionDeclarationA(p: Promise) { messageId, }, ], + output: ` +async function nonAsyncPromiseFunctionDeclarationA(p: Promise) { + return p; +} + `, }, { code: ` @@ -227,6 +242,11 @@ function nonAsyncPromiseFunctionDeclarationB() { messageId, }, ], + output: ` +async function nonAsyncPromiseFunctionDeclarationB() { + return new Promise(); +} + `, }, { code: ` @@ -237,6 +257,9 @@ const nonAsyncPromiseArrowFunctionA = (p: Promise) => p; messageId, }, ], + output: ` +const nonAsyncPromiseArrowFunctionA = async (p: Promise) => p; + `, }, { code: ` @@ -247,6 +270,9 @@ const nonAsyncPromiseArrowFunctionB = () => new Promise(); messageId, }, ], + output: ` +const nonAsyncPromiseArrowFunctionB = async () => new Promise(); + `, }, { code: ` @@ -255,7 +281,7 @@ class Test { return p; } - public nonAsyncPromiseMethodB() { + public static nonAsyncPromiseMethodB() { return new Promise(); } } @@ -270,6 +296,17 @@ class Test { messageId, }, ], + output: ` +class Test { + public async nonAsyncPromiseMethodA(p: Promise) { + return p; + } + + public static async nonAsyncPromiseMethodB() { + return new Promise(); + } +} + `, }, { code: ` @@ -308,6 +345,23 @@ class Test { messageId, }, ], + output: ` +const nonAsyncPromiseFunctionExpression = async function (p: Promise) { + return p; +}; + +async function nonAsyncPromiseFunctionDeclaration(p: Promise) { + return p; +} + +const nonAsyncPromiseArrowFunction = (p: Promise) => p; + +class Test { + public async nonAsyncPromiseMethod(p: Promise) { + return p; + } +} + `, }, { code: ` @@ -346,6 +400,23 @@ class Test { messageId, }, ], + output: ` +const nonAsyncPromiseFunctionExpression = async function (p: Promise) { + return p; +}; + +function nonAsyncPromiseFunctionDeclaration(p: Promise) { + return p; +} + +const nonAsyncPromiseArrowFunction = async (p: Promise) => p; + +class Test { + public async nonAsyncPromiseMethod(p: Promise) { + return p; + } +} + `, }, { code: ` @@ -384,6 +455,23 @@ class Test { messageId, }, ], + output: ` +const nonAsyncPromiseFunctionExpression = function (p: Promise) { + return p; +}; + +async function nonAsyncPromiseFunctionDeclaration(p: Promise) { + return p; +} + +const nonAsyncPromiseArrowFunction = async (p: Promise) => p; + +class Test { + public async nonAsyncPromiseMethod(p: Promise) { + return p; + } +} + `, }, { code: ` @@ -422,6 +510,23 @@ class Test { messageId, }, ], + output: ` +const nonAsyncPromiseFunctionExpression = async function (p: Promise) { + return p; +}; + +async function nonAsyncPromiseFunctionDeclaration(p: Promise) { + return p; +} + +const nonAsyncPromiseArrowFunction = async (p: Promise) => p; + +class Test { + public nonAsyncPromiseMethod(p: Promise) { + return p; + } +} + `, }, { code: ` @@ -440,6 +545,11 @@ const returnAllowedType = () => new PromiseType(); messageId, }, ], + output: ` +class PromiseType {} + +const returnAllowedType = async () => new PromiseType(); + `, }, { code: ` @@ -461,6 +571,14 @@ function foo(): Promise | SPromise { messageId, }, ], + output: ` +interface SPromise extends Promise {} +async function foo(): Promise | SPromise { + return Math.random() > 0.5 + ? Promise.resolve('value') + : Promise.resolve(false); +} + `, }, ], }); From 94574ce3896052d24bdc919074cb8de5c26d01fd Mon Sep 17 00:00:00 2001 From: Nikita Stefaniak Date: Thu, 10 Dec 2020 19:44:46 +0100 Subject: [PATCH 2/7] feat(eslint-plugin): report only head of function --- .../src/rules/promise-function-async.ts | 26 ++++--- .../src/util/explicitReturnTypeUtils.ts | 63 +--------------- .../src/util/getFunctionHeadLoc.ts | 73 +++++++++++++++++++ packages/eslint-plugin/src/util/index.ts | 1 + 4 files changed, 93 insertions(+), 70 deletions(-) create mode 100644 packages/eslint-plugin/src/util/getFunctionHeadLoc.ts diff --git a/packages/eslint-plugin/src/rules/promise-function-async.ts b/packages/eslint-plugin/src/rules/promise-function-async.ts index 32072754621..8a7fe57a119 100644 --- a/packages/eslint-plugin/src/rules/promise-function-async.ts +++ b/packages/eslint-plugin/src/rules/promise-function-async.ts @@ -2,8 +2,8 @@ import { AST_NODE_TYPES, TSESTree, } from '@typescript-eslint/experimental-utils'; -import * as util from '../util'; import * as ts from 'typescript'; +import * as util from '../util'; type Options = [ { @@ -91,14 +91,13 @@ export default util.createRule({ ]); 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 @@ -116,6 +115,7 @@ export default util.createRule({ allAllowedPromiseNames, ) ) { + // Return type is not a promise return; } @@ -125,6 +125,7 @@ export default util.createRule({ node.parent.type === AST_NODE_TYPES.MethodDefinition) && (node.parent.kind === 'get' || node.parent.kind === 'set') ) { + // Getters and setters can't be async return; } @@ -135,15 +136,20 @@ export default util.createRule({ 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.type === AST_NODE_TYPES.MethodDefinition) { - return fixer.insertTextBefore(node.key, 'async '); + if ( + node.parent && + node.parent.type === AST_NODE_TYPES.MethodDefinition + ) { + return fixer.insertTextBefore(node.parent.key, 'async '); } return fixer.insertTextBefore(node, 'async '); }, @@ -170,13 +176,15 @@ export default util.createRule({ ): 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); } }, diff --git a/packages/eslint-plugin/src/util/explicitReturnTypeUtils.ts b/packages/eslint-plugin/src/util/explicitReturnTypeUtils.ts index d2c3a684c86..cd2f0b11826 100644 --- a/packages/eslint-plugin/src/util/explicitReturnTypeUtils.ts +++ b/packages/eslint-plugin/src/util/explicitReturnTypeUtils.ts @@ -2,9 +2,9 @@ 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 = @@ -12,65 +12,6 @@ type FunctionExpression = | 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. * ``` @@ -327,7 +268,7 @@ function checkFunctionReturnType( return; } - report(getReporLoc(node, sourceCode)); + report(getFunctionHeadLoc(node, sourceCode)); } /** diff --git a/packages/eslint-plugin/src/util/getFunctionHeadLoc.ts b/packages/eslint-plugin/src/util/getFunctionHeadLoc.ts new file mode 100644 index 00000000000..e851da0788f --- /dev/null +++ b/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( + 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(), + }; +} diff --git a/packages/eslint-plugin/src/util/index.ts b/packages/eslint-plugin/src/util/index.ts index 86e0ec233fd..dbb2906231b 100644 --- a/packages/eslint-plugin/src/util/index.ts +++ b/packages/eslint-plugin/src/util/index.ts @@ -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'; From 85e6f61a5473a5f1272e395e74cfc72cf0ee3b04 Mon Sep 17 00:00:00 2001 From: Nikita Stefaniak Date: Thu, 10 Dec 2020 19:53:13 +0100 Subject: [PATCH 3/7] fix(eslint-plugin): ignore abstract methods --- .../src/rules/promise-function-async.ts | 5 +++++ .../tests/rules/promise-function-async.test.ts | 12 ++++++++++++ 2 files changed, 17 insertions(+) diff --git a/packages/eslint-plugin/src/rules/promise-function-async.ts b/packages/eslint-plugin/src/rules/promise-function-async.ts index 8a7fe57a119..ddd77a4e75f 100644 --- a/packages/eslint-plugin/src/rules/promise-function-async.ts +++ b/packages/eslint-plugin/src/rules/promise-function-async.ts @@ -119,6 +119,11 @@ export default util.createRule({ return; } + if (node.parent?.type === AST_NODE_TYPES.TSAbstractMethodDefinition) { + // Abstract method can't be async + return; + } + if ( node.parent && (node.parent.type === AST_NODE_TYPES.Property || diff --git a/packages/eslint-plugin/tests/rules/promise-function-async.test.ts b/packages/eslint-plugin/tests/rules/promise-function-async.test.ts index cb96167610e..cbdb27cd36e 100644 --- a/packages/eslint-plugin/tests/rules/promise-function-async.test.ts +++ b/packages/eslint-plugin/tests/rules/promise-function-async.test.ts @@ -141,6 +141,18 @@ const foo = (options: Options): Return => { code: ` function foo(): Promise | boolean { return Math.random() > 0.5 ? Promise.resolve('value') : false; +} + `, + }, + { + code: ` +abstract class Test { + abstract test1(): Promise; + + // abstract method with body is always an error but it still parses into valid AST + abstract test2(): Promise { + return Promise.resolve(1); + } } `, }, From fa2a1a3e233835ac35308f99778af41f3f71e59d Mon Sep 17 00:00:00 2001 From: Nikita Stefaniak Date: Thu, 10 Dec 2020 19:54:25 +0100 Subject: [PATCH 4/7] fix(eslint-plugin): fix fixer for object method --- .../src/rules/promise-function-async.ts | 4 +++- .../rules/promise-function-async.test.ts | 24 ++++++++++++++++++- 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/packages/eslint-plugin/src/rules/promise-function-async.ts b/packages/eslint-plugin/src/rules/promise-function-async.ts index ddd77a4e75f..ce624be643c 100644 --- a/packages/eslint-plugin/src/rules/promise-function-async.ts +++ b/packages/eslint-plugin/src/rules/promise-function-async.ts @@ -152,7 +152,9 @@ export default util.createRule({ fix: fixer => { if ( node.parent && - node.parent.type === AST_NODE_TYPES.MethodDefinition + (node.parent.type === AST_NODE_TYPES.MethodDefinition || + (node.parent.type === AST_NODE_TYPES.Property && + node.parent.method)) ) { return fixer.insertTextBefore(node.parent.key, 'async '); } diff --git a/packages/eslint-plugin/tests/rules/promise-function-async.test.ts b/packages/eslint-plugin/tests/rules/promise-function-async.test.ts index cbdb27cd36e..49f6e250b6c 100644 --- a/packages/eslint-plugin/tests/rules/promise-function-async.test.ts +++ b/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'; @@ -288,6 +288,28 @@ const nonAsyncPromiseArrowFunctionB = async () => new Promise(); }, { code: ` +const functions = { + nonAsyncPromiseMethod() { + return Promise.resolve(1); + }, +}; + `, + errors: [ + { + line: 3, + messageId, + }, + ], + output: ` +const functions = { + async nonAsyncPromiseMethod() { + return Promise.resolve(1); + }, +}; + `, + }, + { + code: ` class Test { public nonAsyncPromiseMethodA(p: Promise) { return p; From f568a9fedeb08a78ce9ec65a2784492b367fd464 Mon Sep 17 00:00:00 2001 From: Nikita Stefaniak Date: Sat, 12 Dec 2020 21:49:03 +0100 Subject: [PATCH 5/7] chore(eslint-plugin): revert: report function head --- .../src/rules/promise-function-async.ts | 3 - .../src/util/explicitReturnTypeUtils.ts | 63 +++++++++++++++- .../src/util/getFunctionHeadLoc.ts | 73 ------------------- packages/eslint-plugin/src/util/index.ts | 1 - 4 files changed, 61 insertions(+), 79 deletions(-) delete mode 100644 packages/eslint-plugin/src/util/getFunctionHeadLoc.ts diff --git a/packages/eslint-plugin/src/rules/promise-function-async.ts b/packages/eslint-plugin/src/rules/promise-function-async.ts index ce624be643c..00888c8f39f 100644 --- a/packages/eslint-plugin/src/rules/promise-function-async.ts +++ b/packages/eslint-plugin/src/rules/promise-function-async.ts @@ -91,7 +91,6 @@ export default util.createRule({ ]); const parserServices = util.getParserServices(context); const checker = parserServices.program.getTypeChecker(); - const sourceCode = context.getSourceCode(); function validateNode( node: @@ -141,14 +140,12 @@ export default util.createRule({ 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 && diff --git a/packages/eslint-plugin/src/util/explicitReturnTypeUtils.ts b/packages/eslint-plugin/src/util/explicitReturnTypeUtils.ts index cd2f0b11826..d2c3a684c86 100644 --- a/packages/eslint-plugin/src/util/explicitReturnTypeUtils.ts +++ b/packages/eslint-plugin/src/util/explicitReturnTypeUtils.ts @@ -2,9 +2,9 @@ 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 = @@ -12,6 +12,65 @@ type FunctionExpression = | 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. * ``` @@ -268,7 +327,7 @@ function checkFunctionReturnType( return; } - report(getFunctionHeadLoc(node, sourceCode)); + report(getReporLoc(node, sourceCode)); } /** diff --git a/packages/eslint-plugin/src/util/getFunctionHeadLoc.ts b/packages/eslint-plugin/src/util/getFunctionHeadLoc.ts deleted file mode 100644 index e851da0788f..00000000000 --- a/packages/eslint-plugin/src/util/getFunctionHeadLoc.ts +++ /dev/null @@ -1,73 +0,0 @@ -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( - 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(), - }; -} diff --git a/packages/eslint-plugin/src/util/index.ts b/packages/eslint-plugin/src/util/index.ts index dbb2906231b..86e0ec233fd 100644 --- a/packages/eslint-plugin/src/util/index.ts +++ b/packages/eslint-plugin/src/util/index.ts @@ -3,7 +3,6 @@ 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'; From 3b99f5e45a94a404cc562a49dc078d27a501a1c3 Mon Sep 17 00:00:00 2001 From: Nikita Stefaniak Date: Sat, 12 Dec 2020 22:00:09 +0100 Subject: [PATCH 6/7] feat(eslint-plugin): only report function head This reverts commit f568a9fedeb08a78ce9ec65a2784492b367fd464. --- .../src/rules/promise-function-async.ts | 3 + .../src/util/explicitReturnTypeUtils.ts | 63 +--------------- .../src/util/getFunctionHeadLoc.ts | 73 +++++++++++++++++++ packages/eslint-plugin/src/util/index.ts | 1 + 4 files changed, 79 insertions(+), 61 deletions(-) create mode 100644 packages/eslint-plugin/src/util/getFunctionHeadLoc.ts diff --git a/packages/eslint-plugin/src/rules/promise-function-async.ts b/packages/eslint-plugin/src/rules/promise-function-async.ts index 00888c8f39f..ce624be643c 100644 --- a/packages/eslint-plugin/src/rules/promise-function-async.ts +++ b/packages/eslint-plugin/src/rules/promise-function-async.ts @@ -91,6 +91,7 @@ export default util.createRule({ ]); const parserServices = util.getParserServices(context); const checker = parserServices.program.getTypeChecker(); + const sourceCode = context.getSourceCode(); function validateNode( node: @@ -140,12 +141,14 @@ export default util.createRule({ 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 && diff --git a/packages/eslint-plugin/src/util/explicitReturnTypeUtils.ts b/packages/eslint-plugin/src/util/explicitReturnTypeUtils.ts index d2c3a684c86..cd2f0b11826 100644 --- a/packages/eslint-plugin/src/util/explicitReturnTypeUtils.ts +++ b/packages/eslint-plugin/src/util/explicitReturnTypeUtils.ts @@ -2,9 +2,9 @@ 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 = @@ -12,65 +12,6 @@ type FunctionExpression = | 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. * ``` @@ -327,7 +268,7 @@ function checkFunctionReturnType( return; } - report(getReporLoc(node, sourceCode)); + report(getFunctionHeadLoc(node, sourceCode)); } /** diff --git a/packages/eslint-plugin/src/util/getFunctionHeadLoc.ts b/packages/eslint-plugin/src/util/getFunctionHeadLoc.ts new file mode 100644 index 00000000000..e851da0788f --- /dev/null +++ b/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( + 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(), + }; +} diff --git a/packages/eslint-plugin/src/util/index.ts b/packages/eslint-plugin/src/util/index.ts index 86e0ec233fd..dbb2906231b 100644 --- a/packages/eslint-plugin/src/util/index.ts +++ b/packages/eslint-plugin/src/util/index.ts @@ -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'; From b14ae2715901c62b7753a2f3719a4151f2cfae56 Mon Sep 17 00:00:00 2001 From: Nikita Stefaniak Date: Sun, 28 Feb 2021 01:22:34 +0100 Subject: [PATCH 7/7] fix(eslint-plugin): don't report decorators --- .../src/util/getFunctionHeadLoc.ts | 36 +++++++++++-------- .../rules/promise-function-async.test.ts | 19 ++++++++++ 2 files changed, 40 insertions(+), 15 deletions(-) diff --git a/packages/eslint-plugin/src/util/getFunctionHeadLoc.ts b/packages/eslint-plugin/src/util/getFunctionHeadLoc.ts index e851da0788f..dc18a832f94 100644 --- a/packages/eslint-plugin/src/util/getFunctionHeadLoc.ts +++ b/packages/eslint-plugin/src/util/getFunctionHeadLoc.ts @@ -31,31 +31,36 @@ export function getFunctionHeadLoc( 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 (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 ( - parent && - (parent.type === AST_NODE_TYPES.MethodDefinition || - (parent.type === AST_NODE_TYPES.Property && parent.method)) + node.parent && + node.parent.type === AST_NODE_TYPES.Property && + node.parent.method ) { - return parent.loc.start; + // 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; } - /** - * Returns end column position - * @param node - */ function getLocEnd(): TSESTree.LineAndColumnData { - /* highlight `=>` */ if (node.type === AST_NODE_TYPES.ArrowFunctionExpression) { + // find the end location for arrow function expression return sourceCode.getTokenBefore( node.body, token => @@ -63,6 +68,7 @@ export function getFunctionHeadLoc( )!.loc.end; } + // return the end location for a regular function return sourceCode.getTokenBefore(node.body)!.loc.end; } diff --git a/packages/eslint-plugin/tests/rules/promise-function-async.test.ts b/packages/eslint-plugin/tests/rules/promise-function-async.test.ts index 49f6e250b6c..961047138c9 100644 --- a/packages/eslint-plugin/tests/rules/promise-function-async.test.ts +++ b/packages/eslint-plugin/tests/rules/promise-function-async.test.ts @@ -611,6 +611,25 @@ async function foo(): Promise | SPromise { 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); + } } `, },