From d053cde3e8b5bf9ba1c22fd64a7456d672ef77ca Mon Sep 17 00:00:00 2001 From: Taeheon Kim Date: Mon, 17 Jan 2022 16:54:27 +0900 Subject: [PATCH] fix(eslint-plugin): [explicit-function-return-type] support AllowTypedFunctionExpression within AllowHigherOrderFunction (#4250) * refactor: move function to be used more widely * feat: find if the ancestor(top level) is typed * test: add more test cases * fix: lint fail on test case * fix: make working with option combos * chore: remove unnecessary comment --- .../rules/explicit-function-return-type.ts | 17 ++- .../rules/explicit-module-boundary-types.ts | 50 +------- .../src/util/explicitReturnTypeUtils.ts | 46 +++++++ .../explicit-function-return-type.test.ts | 120 ++++++++++++++++++ 4 files changed, 182 insertions(+), 51 deletions(-) diff --git a/packages/eslint-plugin/src/rules/explicit-function-return-type.ts b/packages/eslint-plugin/src/rules/explicit-function-return-type.ts index 233b07bbc6a..d9302a7775f 100644 --- a/packages/eslint-plugin/src/rules/explicit-function-return-type.ts +++ b/packages/eslint-plugin/src/rules/explicit-function-return-type.ts @@ -2,7 +2,8 @@ import { AST_NODE_TYPES, TSESTree } from '@typescript-eslint/utils'; import * as util from '../util'; import { checkFunctionReturnType, - checkFunctionExpressionReturnType, + isValidFunctionExpressionReturnType, + ancestorHasReturnType, } from '../util/explicitReturnTypeUtils'; type Options = [ @@ -78,7 +79,15 @@ export default util.createRule({ return; } - checkFunctionExpressionReturnType(node, options, sourceCode, loc => + if ( + options.allowTypedFunctionExpressions && + (isValidFunctionExpressionReturnType(node, options) || + ancestorHasReturnType(node)) + ) { + return; + } + + checkFunctionReturnType(node, options, sourceCode, loc => context.report({ node, loc, @@ -87,6 +96,10 @@ export default util.createRule({ ); }, FunctionDeclaration(node): void { + if (options.allowTypedFunctionExpressions && node.returnType) { + return; + } + checkFunctionReturnType(node, options, sourceCode, loc => context.report({ node, diff --git a/packages/eslint-plugin/src/rules/explicit-module-boundary-types.ts b/packages/eslint-plugin/src/rules/explicit-module-boundary-types.ts index 5bbfa7ebbea..30509bb660e 100644 --- a/packages/eslint-plugin/src/rules/explicit-module-boundary-types.ts +++ b/packages/eslint-plugin/src/rules/explicit-module-boundary-types.ts @@ -8,6 +8,7 @@ import { FunctionExpression, FunctionNode, isTypedFunctionExpression, + ancestorHasReturnType, } from '../util/explicitReturnTypeUtils'; type Options = [ @@ -387,55 +388,6 @@ export default util.createRule({ } } - /** - * Check whether any ancestor of the provided function has a valid return type. - * This function assumes that the function either: - * - belongs to an exported function chain validated by isExportedHigherOrderFunction - * - is directly exported itself - */ - function ancestorHasReturnType(node: FunctionNode): boolean { - let ancestor = node.parent; - - if (ancestor?.type === AST_NODE_TYPES.Property) { - ancestor = ancestor.value; - } - - // if the ancestor is not a return, then this function was not returned at all, so we can exit early - const isReturnStatement = - ancestor?.type === AST_NODE_TYPES.ReturnStatement; - const isBodylessArrow = - ancestor?.type === AST_NODE_TYPES.ArrowFunctionExpression && - ancestor.body.type !== AST_NODE_TYPES.BlockStatement; - if (!isReturnStatement && !isBodylessArrow) { - return false; - } - - while (ancestor) { - switch (ancestor.type) { - case AST_NODE_TYPES.ArrowFunctionExpression: - case AST_NODE_TYPES.FunctionExpression: - case AST_NODE_TYPES.FunctionDeclaration: - if (ancestor.returnType) { - return true; - } - // assume - break; - - // const x: Foo = () => {}; - // Assume that a typed variable types the function expression - case AST_NODE_TYPES.VariableDeclarator: - if (ancestor.id.typeAnnotation) { - return true; - } - break; - } - - ancestor = ancestor.parent; - } - - return false; - } - function checkEmptyBodyFunctionExpression( node: TSESTree.TSEmptyBodyFunctionExpression, ): void { diff --git a/packages/eslint-plugin/src/util/explicitReturnTypeUtils.ts b/packages/eslint-plugin/src/util/explicitReturnTypeUtils.ts index 3b737ad4f41..4d4ec6023a7 100644 --- a/packages/eslint-plugin/src/util/explicitReturnTypeUtils.ts +++ b/packages/eslint-plugin/src/util/explicitReturnTypeUtils.ts @@ -293,6 +293,50 @@ function checkFunctionExpressionReturnType( checkFunctionReturnType(node, options, sourceCode, report); } +/** + * Check whether any ancestor of the provided function has a valid return type. + */ +function ancestorHasReturnType(node: FunctionNode): boolean { + let ancestor = node.parent; + + if (ancestor?.type === AST_NODE_TYPES.Property) { + ancestor = ancestor.value; + } + + // if the ancestor is not a return, then this function was not returned at all, so we can exit early + const isReturnStatement = ancestor?.type === AST_NODE_TYPES.ReturnStatement; + const isBodylessArrow = + ancestor?.type === AST_NODE_TYPES.ArrowFunctionExpression && + ancestor.body.type !== AST_NODE_TYPES.BlockStatement; + if (!isReturnStatement && !isBodylessArrow) { + return false; + } + + while (ancestor) { + switch (ancestor.type) { + case AST_NODE_TYPES.ArrowFunctionExpression: + case AST_NODE_TYPES.FunctionExpression: + case AST_NODE_TYPES.FunctionDeclaration: + if (ancestor.returnType) { + return true; + } + break; + + // const x: Foo = () => {}; + // Assume that a typed variable types the function expression + case AST_NODE_TYPES.VariableDeclarator: + if (ancestor.id.typeAnnotation) { + return true; + } + break; + } + + ancestor = ancestor.parent; + } + + return false; +} + export { checkFunctionExpressionReturnType, checkFunctionReturnType, @@ -300,4 +344,6 @@ export { FunctionExpression, FunctionNode, isTypedFunctionExpression, + isValidFunctionExpressionReturnType, + ancestorHasReturnType, }; diff --git a/packages/eslint-plugin/tests/rules/explicit-function-return-type.test.ts b/packages/eslint-plugin/tests/rules/explicit-function-return-type.test.ts index ab98b4be27c..e1fae3cd116 100644 --- a/packages/eslint-plugin/tests/rules/explicit-function-return-type.test.ts +++ b/packages/eslint-plugin/tests/rules/explicit-function-return-type.test.ts @@ -400,6 +400,68 @@ new Foo(1, () => {}); code: 'const log = (message: string) => void console.log(message);', options: [{ allowConciseArrowFunctionExpressionsStartingWithVoid: true }], }, + { + filename: 'test.ts', + code: ` +type HigherOrderType = () => (arg1: string) => (arg2: number) => string; +const x: HigherOrderType = () => arg1 => arg2 => 'foo'; + `, + options: [ + { + allowTypedFunctionExpressions: true, + allowHigherOrderFunctions: true, + }, + ], + }, + { + filename: 'test.ts', + code: ` +type HigherOrderType = () => (arg1: string) => (arg2: number) => string; +const x: HigherOrderType = () => arg1 => arg2 => 'foo'; + `, + options: [ + { + allowTypedFunctionExpressions: true, + allowHigherOrderFunctions: false, + }, + ], + }, + { + filename: 'test.ts', + code: ` +interface Foo { + foo: string; + arrowFn: () => string; +} + +function foo(): Foo { + return { + foo: 'foo', + arrowFn: () => 'test', + }; +} + `, + options: [ + { + allowTypedFunctionExpressions: true, + allowHigherOrderFunctions: true, + }, + ], + }, + { + filename: 'test.ts', + code: ` +type Foo = (arg1: string) => string; +type Bar = (arg2: string) => T; +const x: Bar = arg1 => arg2 => arg1 + arg2; + `, + options: [ + { + allowTypedFunctionExpressions: true, + allowHigherOrderFunctions: true, + }, + ], + }, ], invalid: [ { @@ -1027,6 +1089,64 @@ foo({ { filename: 'test.ts', code: ` +type HigherOrderType = () => (arg1: string) => (arg2: number) => string; +const x: HigherOrderType = () => arg1 => arg2 => 'foo'; + `, + options: [ + { + allowTypedFunctionExpressions: false, + allowHigherOrderFunctions: true, + }, + ], + errors: [ + { + messageId: 'missingReturnType', + line: 3, + endLine: 3, + column: 42, + endColumn: 49, + }, + ], + }, + { + filename: 'test.ts', + code: ` +type HigherOrderType = () => (arg1: string) => (arg2: number) => string; +const x: HigherOrderType = () => arg1 => arg2 => 'foo'; + `, + options: [ + { + allowTypedFunctionExpressions: false, + allowHigherOrderFunctions: false, + }, + ], + errors: [ + { + messageId: 'missingReturnType', + line: 3, + endLine: 3, + column: 28, + endColumn: 33, + }, + { + messageId: 'missingReturnType', + line: 3, + endLine: 3, + column: 34, + endColumn: 41, + }, + { + messageId: 'missingReturnType', + line: 3, + endLine: 3, + column: 42, + endColumn: 49, + }, + ], + }, + { + filename: 'test.ts', + code: ` const func = (value: number) => ({ type: 'X', value } as any); const func = (value: number) => ({ type: 'X', value } as Action); `,