From 1e8caf8f50c0ab850e1f946c3803e0b4cb3b1a34 Mon Sep 17 00:00:00 2001 From: lonyele Date: Mon, 6 Dec 2021 00:50:20 +0900 Subject: [PATCH 1/6] refactor: move function to be used more widely --- .../rules/explicit-module-boundary-types.ts | 50 +------------------ .../src/util/explicitReturnTypeUtils.ts | 47 +++++++++++++++++ 2 files changed, 48 insertions(+), 49 deletions(-) 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 adaacaf363f..fa46415a14f 100644 --- a/packages/eslint-plugin/src/rules/explicit-module-boundary-types.ts +++ b/packages/eslint-plugin/src/rules/explicit-module-boundary-types.ts @@ -11,6 +11,7 @@ import { FunctionExpression, FunctionNode, isTypedFunctionExpression, + ancestorHasReturnType, } from '../util/explicitReturnTypeUtils'; type Options = [ @@ -390,55 +391,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 fd512749dfb..ae0b90ad2aa 100644 --- a/packages/eslint-plugin/src/util/explicitReturnTypeUtils.ts +++ b/packages/eslint-plugin/src/util/explicitReturnTypeUtils.ts @@ -287,6 +287,51 @@ 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; + } + // 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; +} + export { checkFunctionExpressionReturnType, checkFunctionReturnType, @@ -294,4 +339,6 @@ export { FunctionExpression, FunctionNode, isTypedFunctionExpression, + isValidFunctionExpressionReturnType, + ancestorHasReturnType, }; From bbe491f40e507edd433f8f5372701b3fa4b4ad4b Mon Sep 17 00:00:00 2001 From: lonyele Date: Mon, 6 Dec 2021 01:05:46 +0900 Subject: [PATCH 2/6] feat: find if the ancestor(top level) is typed --- .../src/rules/explicit-function-return-type.ts | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 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 e9a27b2b1fc..e557a9e0713 100644 --- a/packages/eslint-plugin/src/rules/explicit-function-return-type.ts +++ b/packages/eslint-plugin/src/rules/explicit-function-return-type.ts @@ -5,7 +5,8 @@ import { import * as util from '../util'; import { checkFunctionReturnType, - checkFunctionExpressionReturnType, + isValidFunctionExpressionReturnType, + ancestorHasReturnType, } from '../util/explicitReturnTypeUtils'; type Options = [ @@ -81,7 +82,14 @@ export default util.createRule({ return; } - checkFunctionExpressionReturnType(node, options, sourceCode, loc => + if ( + isValidFunctionExpressionReturnType(node, options) || + ancestorHasReturnType(node) + ) { + return; + } + + checkFunctionReturnType(node, options, sourceCode, loc => context.report({ node, loc, @@ -90,6 +98,10 @@ export default util.createRule({ ); }, FunctionDeclaration(node): void { + if (options.allowTypedFunctionExpressions && node.returnType) { + return; + } + checkFunctionReturnType(node, options, sourceCode, loc => context.report({ node, From c5651820678a1900c1525bcf5593738016668b8b Mon Sep 17 00:00:00 2001 From: lonyele Date: Mon, 6 Dec 2021 01:06:42 +0900 Subject: [PATCH 3/6] test: add more test cases --- .../explicit-function-return-type.test.ts | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) 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..ce84ed9a811 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,55 @@ 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: ` +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: [ { From 7804ba898ca2a9772ce0657842a9c08d37b86ce1 Mon Sep 17 00:00:00 2001 From: lonyele Date: Mon, 6 Dec 2021 02:06:46 +0900 Subject: [PATCH 4/6] fix: lint fail on test case --- .../tests/rules/explicit-function-return-type.test.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) 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 ce84ed9a811..f13b641765b 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 @@ -404,8 +404,8 @@ new Foo(1, () => {}); filename: 'test.ts', code: ` type HigherOrderType = () => (arg1: string) => (arg2: number) => string; -const x: HigherOrderType = () => (arg1) => (arg2) => 'foo'; - `, +const x: HigherOrderType = () => arg1 => arg2 => 'foo'; + `, options: [ { allowTypedFunctionExpressions: true, @@ -427,7 +427,7 @@ function foo(): Foo { arrowFn: () => 'test', }; } - `, + `, options: [ { allowTypedFunctionExpressions: true, @@ -441,7 +441,7 @@ function foo(): Foo { type Foo = (arg1: string) => string; type Bar = (arg2: string) => T; const x: Bar = arg1 => arg2 => arg1 + arg2; - `, + `, options: [ { allowTypedFunctionExpressions: true, From c732d95589e19fb9e64380fbc8efaca802670ecb Mon Sep 17 00:00:00 2001 From: lonyele Date: Mon, 20 Dec 2021 19:36:30 +0900 Subject: [PATCH 5/6] fix: make working with option combos --- .../rules/explicit-function-return-type.ts | 5 +- .../explicit-function-return-type.test.ts | 71 +++++++++++++++++++ 2 files changed, 74 insertions(+), 2 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 e557a9e0713..0c36f94ff1e 100644 --- a/packages/eslint-plugin/src/rules/explicit-function-return-type.ts +++ b/packages/eslint-plugin/src/rules/explicit-function-return-type.ts @@ -83,8 +83,9 @@ export default util.createRule({ } if ( - isValidFunctionExpressionReturnType(node, options) || - ancestorHasReturnType(node) + options.allowTypedFunctionExpressions && + (isValidFunctionExpressionReturnType(node, options) || + ancestorHasReturnType(node)) ) { return; } 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 f13b641765b..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 @@ -416,6 +416,19 @@ const x: HigherOrderType = () => arg1 => arg2 => 'foo'; { 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; @@ -1076,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); `, From 10922cdbc416af8eb1a8e2206d226a0fe7090409 Mon Sep 17 00:00:00 2001 From: lonyele Date: Mon, 20 Dec 2021 19:40:06 +0900 Subject: [PATCH 6/6] chore: remove unnecessary comment --- packages/eslint-plugin/src/util/explicitReturnTypeUtils.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/eslint-plugin/src/util/explicitReturnTypeUtils.ts b/packages/eslint-plugin/src/util/explicitReturnTypeUtils.ts index ae0b90ad2aa..cfaccd65782 100644 --- a/packages/eslint-plugin/src/util/explicitReturnTypeUtils.ts +++ b/packages/eslint-plugin/src/util/explicitReturnTypeUtils.ts @@ -314,7 +314,6 @@ function ancestorHasReturnType(node: FunctionNode): boolean { if (ancestor.returnType) { return true; } - // assume break; // const x: Foo = () => {};