From 1accd815f17dafb2c926bd8c52c70c23e0abd3e8 Mon Sep 17 00:00:00 2001 From: Ifiok Jr Date: Sun, 24 May 2020 05:36:51 +0200 Subject: [PATCH 1/3] fix(eslint-plugin): explicit-module-boundary-types Was complaining about inner function expressions. Fixes #1845 --- .../rules/explicit-module-boundary-types.ts | 18 ++- .../src/util/explicitReturnTypeUtils.ts | 138 ++++++++++++---- .../explicit-module-boundary-types.test.ts | 148 ++++++++++++++++++ 3 files changed, 270 insertions(+), 34 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 265010adc6d0..63f94d1d1a4d 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 { checkFunctionExpressionReturnType, checkFunctionReturnType, isTypedFunctionExpression, + ancestorHasReturnType, } from '../util/explicitReturnTypeUtils'; type Options = [ @@ -74,6 +75,10 @@ export default util.createRule({ create(context, [options]) { const sourceCode = context.getSourceCode(); + /** + * Steps up the nodes parents to check if any ancestor nodes are export + * declarations. + */ function isUnexported(node: TSESTree.Node | undefined): boolean { let isReturnedValue = false; while (node) { @@ -111,6 +116,9 @@ export default util.createRule({ return true; } + /** + * Returns true when the argument node is not typed. + */ function isArgumentUntyped(node: TSESTree.Identifier): boolean { return ( !node.typeAnnotation || @@ -265,7 +273,7 @@ export default util.createRule({ } /** - * Checks if a function expression follow the rule + * Checks if a function expression follow the rule. */ function checkFunctionExpression( node: TSESTree.ArrowFunctionExpression | TSESTree.FunctionExpression, @@ -280,7 +288,8 @@ export default util.createRule({ if ( isAllowedName(node.parent) || - isTypedFunctionExpression(node, options) + isTypedFunctionExpression(node, options) || + ancestorHasReturnType(node.parent, options) ) { return; } @@ -300,7 +309,10 @@ export default util.createRule({ * Checks if a function follow the rule */ function checkFunction(node: TSESTree.FunctionDeclaration): void { - if (isAllowedName(node.parent)) { + if ( + isAllowedName(node.parent) || + ancestorHasReturnType(node.parent, options) + ) { return; } diff --git a/packages/eslint-plugin/src/util/explicitReturnTypeUtils.ts b/packages/eslint-plugin/src/util/explicitReturnTypeUtils.ts index 2725a6ab3d3b..2e26f69b1cab 100644 --- a/packages/eslint-plugin/src/util/explicitReturnTypeUtils.ts +++ b/packages/eslint-plugin/src/util/explicitReturnTypeUtils.ts @@ -240,31 +240,8 @@ interface Options { } /** - * Checks if a function declaration/expression has a return type. + * True when the provided function expression is typed. */ -function checkFunctionReturnType( - node: - | TSESTree.ArrowFunctionExpression - | TSESTree.FunctionDeclaration - | TSESTree.FunctionExpression, - options: Options, - sourceCode: TSESLint.SourceCode, - report: (loc: TSESTree.SourceLocation) => void, -): void { - if ( - options.allowHigherOrderFunctions && - doesImmediatelyReturnFunctionExpression(node) - ) { - return; - } - - if (node.returnType || isConstructor(node.parent) || isSetter(node.parent)) { - return; - } - - report(getReporLoc(node, sourceCode)); -} - function isTypedFunctionExpression( node: TSESTree.ArrowFunctionExpression | TSESTree.FunctionExpression, options: Options, @@ -286,16 +263,15 @@ function isTypedFunctionExpression( } /** - * Checks if a function declaration/expression has a return type. + * Check whether the function expression return type is either typed or valid + * with the provided options. */ -function checkFunctionExpressionReturnType( +function isValidFunctionExpressionReturnType( node: TSESTree.ArrowFunctionExpression | TSESTree.FunctionExpression, options: Options, - sourceCode: TSESLint.SourceCode, - report: (loc: TSESTree.SourceLocation) => void, -): void { +): boolean { if (isTypedFunctionExpression(node, options)) { - return; + return true; } const parent = nullThrows(node.parent, NullThrowsReasons.MissingParent); @@ -306,7 +282,7 @@ function checkFunctionExpressionReturnType( parent.type !== AST_NODE_TYPES.ExportDefaultDeclaration && parent.type !== AST_NODE_TYPES.ClassProperty ) { - return; + return true; } // https://github.com/typescript-eslint/typescript-eslint/issues/653 @@ -315,14 +291,114 @@ function checkFunctionExpressionReturnType( node.type === AST_NODE_TYPES.ArrowFunctionExpression && returnsConstAssertionDirectly(node) ) { + return true; + } + + return false; +} + +/** + * Check that the function expression or declaration is valid. + */ +function isValidFunctionReturnType( + node: + | TSESTree.ArrowFunctionExpression + | TSESTree.FunctionDeclaration + | TSESTree.FunctionExpression, + options: Options, + isParentCheck = false, +): boolean { + if ( + !isParentCheck && + options.allowHigherOrderFunctions && + doesImmediatelyReturnFunctionExpression(node) + ) { + return true; + } + + if (node.returnType || isConstructor(node.parent) || isSetter(node.parent)) { + return true; + } + + return false; +} + +/** + * Checks if a function declaration/expression has a return type. + */ +function checkFunctionReturnType( + node: + | TSESTree.ArrowFunctionExpression + | TSESTree.FunctionDeclaration + | TSESTree.FunctionExpression, + options: Options, + sourceCode: TSESLint.SourceCode, + report: (loc: TSESTree.SourceLocation) => void, +): void { + if (isValidFunctionReturnType(node, options)) { + return; + } + + report(getReporLoc(node, sourceCode)); +} + +/** + * Checks if a function declaration/expression has a return type. + */ +function checkFunctionExpressionReturnType( + node: TSESTree.ArrowFunctionExpression | TSESTree.FunctionExpression, + options: Options, + sourceCode: TSESLint.SourceCode, + report: (loc: TSESTree.SourceLocation) => void, +): void { + if (isValidFunctionExpressionReturnType(node, options)) { return; } checkFunctionReturnType(node, options, sourceCode, report); } +/** + * Check whether any ancestor of the provided node has a valid return type, with + * the given options. + */ +function ancestorHasReturnType( + ancestor: TSESTree.Node | undefined, + options: Options, +): boolean { + // This boolean tells the `isValidFunctionReturnType` that it is being called + // by an ancestor check. + const isParentCheck = true; + + // Has a valid type been found in any of the ancestors. + let hasType = false; + + while (ancestor) { + switch (ancestor.type) { + case AST_NODE_TYPES.ArrowFunctionExpression: + case AST_NODE_TYPES.FunctionExpression: + hasType = + isValidFunctionExpressionReturnType(ancestor, options) || + isValidFunctionReturnType(ancestor, options, isParentCheck); + break; + case AST_NODE_TYPES.FunctionDeclaration: + hasType = isValidFunctionReturnType(ancestor, options, isParentCheck); + break; + } + + if (hasType) { + return hasType; + } + + ancestor = ancestor.parent; + } + + return hasType; +} + export { checkFunctionReturnType, checkFunctionExpressionReturnType, isTypedFunctionExpression, + ancestorHasReturnType, }; diff --git a/packages/eslint-plugin/tests/rules/explicit-module-boundary-types.test.ts b/packages/eslint-plugin/tests/rules/explicit-module-boundary-types.test.ts index 4d20b7b9a3f6..313fe533e724 100644 --- a/packages/eslint-plugin/tests/rules/explicit-module-boundary-types.test.ts +++ b/packages/eslint-plugin/tests/rules/explicit-module-boundary-types.test.ts @@ -437,6 +437,89 @@ export default { Foo }; `, options: [{ shouldTrackReferences: true }], }, + { + code: ` +export function foo(): (n: number) => string { + return n => String(n); +} + `, + }, + { + code: ` +export const foo = (a: string): ((n: number) => string) => { + return function (n) { + return String(n); + }; +}; + `, + }, + { + code: ` +export function a(): void { + function b() {} + const x = () => {}; + (function () {}); + + function c() { + return () => {}; + } + + return; +} + `, + }, + { + code: ` +export function a(): void { + function b() { + function c() {} + } + const x = () => { + return () => 100; + }; + (function () { + (function () {}); + }); + + function c() { + return () => { + (function () {}); + }; + } + + return; +} + `, + }, + { + code: ` +export function a() { + return function b(): () => void { + return function c() {}; + }; +} + `, + options: [{ allowHigherOrderFunctions: true }], + }, + { + code: ` +export var arrowFn = () => (): void => {}; + `, + }, + { + code: ` +export function fn() { + return function (): void {}; +} + `, + }, + { + code: ` +export function foo(outer: string) { + return function (inner: string): void {}; +} + `, + }, ], invalid: [ { @@ -1280,5 +1363,70 @@ export default test; }, ], }, + { + code: ` +export const foo = () => (a: string): ((n: number) => string) => { + return function (n) { + return String(n); + }; +}; + `, + options: [{ allowHigherOrderFunctions: false }], + errors: [ + { + messageId: 'missingReturnType', + line: 2, + column: 20, + }, + ], + }, + { + code: ` +export var arrowFn = () => () => {}; + `, + options: [{ allowHigherOrderFunctions: true }], + errors: [ + { + messageId: 'missingReturnType', + line: 2, + column: 28, + }, + ], + }, + { + code: ` +export function fn() { + return function () {}; +} + `, + options: [{ allowHigherOrderFunctions: true }], + errors: [ + { + messageId: 'missingReturnType', + line: 3, + column: 10, + }, + ], + }, + { + code: ` +export function foo(outer) { + return function (inner): void {}; +} + `, + options: [{ allowHigherOrderFunctions: true }], + errors: [ + { + messageId: 'missingArgType', + line: 2, + column: 8, + }, + { + messageId: 'missingArgType', + line: 3, + column: 10, + }, + ], + }, ], }); From 4560790900160f00cfcdeddc79b9121b0dd46633 Mon Sep 17 00:00:00 2001 From: Ifiok Jr Date: Tue, 26 May 2020 09:37:27 +0200 Subject: [PATCH 2/3] fix(eslint-plugin): explicit-module-boundary-types Exit early when the type of the ancestor of the node being checked is not a return statement. --- .../src/util/explicitReturnTypeUtils.ts | 23 ++++++++----------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/packages/eslint-plugin/src/util/explicitReturnTypeUtils.ts b/packages/eslint-plugin/src/util/explicitReturnTypeUtils.ts index 2e26f69b1cab..56d4a267a2d1 100644 --- a/packages/eslint-plugin/src/util/explicitReturnTypeUtils.ts +++ b/packages/eslint-plugin/src/util/explicitReturnTypeUtils.ts @@ -366,34 +366,31 @@ function ancestorHasReturnType( ancestor: TSESTree.Node | undefined, options: Options, ): boolean { + // Exit early if this ancestor is not a ReturnStatement. + if (ancestor?.type !== AST_NODE_TYPES.ReturnStatement) { + return false; + } + // This boolean tells the `isValidFunctionReturnType` that it is being called // by an ancestor check. const isParentCheck = true; - // Has a valid type been found in any of the ancestors. - let hasType = false; - while (ancestor) { switch (ancestor.type) { case AST_NODE_TYPES.ArrowFunctionExpression: case AST_NODE_TYPES.FunctionExpression: - hasType = + return ( isValidFunctionExpressionReturnType(ancestor, options) || - isValidFunctionReturnType(ancestor, options, isParentCheck); - break; + isValidFunctionReturnType(ancestor, options, isParentCheck) + ); case AST_NODE_TYPES.FunctionDeclaration: - hasType = isValidFunctionReturnType(ancestor, options, isParentCheck); - break; - } - - if (hasType) { - return hasType; + return isValidFunctionReturnType(ancestor, options, isParentCheck); } ancestor = ancestor.parent; } - return hasType; + return false; } export { From 9f8d026c42e780e5815a293560bbbd692e978ab6 Mon Sep 17 00:00:00 2001 From: Ifiok Jr Date: Wed, 27 May 2020 13:41:21 +0200 Subject: [PATCH 3/3] test(eslint-plugin): explicit-module-boundary-types Ignore final return code path --- packages/eslint-plugin/src/util/explicitReturnTypeUtils.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/eslint-plugin/src/util/explicitReturnTypeUtils.ts b/packages/eslint-plugin/src/util/explicitReturnTypeUtils.ts index 56d4a267a2d1..220cd081c0ac 100644 --- a/packages/eslint-plugin/src/util/explicitReturnTypeUtils.ts +++ b/packages/eslint-plugin/src/util/explicitReturnTypeUtils.ts @@ -390,6 +390,7 @@ function ancestorHasReturnType( ancestor = ancestor.parent; } + /* istanbul ignore next */ return false; }