From d7d4eeb03f2918d5d9e361fdb47c2d42e83bd593 Mon Sep 17 00:00:00 2001 From: Ifiok Jr Date: Sat, 30 May 2020 19:40:21 +0100 Subject: [PATCH] fix(eslint-plugin): [explicit-module-boundary-types] don't check returned functions if parent function has return type (#2084) --- .../rules/explicit-module-boundary-types.ts | 18 ++- .../src/util/explicitReturnTypeUtils.ts | 136 ++++++++++++---- .../explicit-module-boundary-types.test.ts | 148 ++++++++++++++++++ 3 files changed, 268 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 265010adc6d..63f94d1d1a4 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 2725a6ab3d3..220cd081c0a 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,112 @@ 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 { + // 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; + + while (ancestor) { + switch (ancestor.type) { + case AST_NODE_TYPES.ArrowFunctionExpression: + case AST_NODE_TYPES.FunctionExpression: + return ( + isValidFunctionExpressionReturnType(ancestor, options) || + isValidFunctionReturnType(ancestor, options, isParentCheck) + ); + case AST_NODE_TYPES.FunctionDeclaration: + return isValidFunctionReturnType(ancestor, options, isParentCheck); + } + + ancestor = ancestor.parent; + } + + /* istanbul ignore next */ + return false; +} + 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 4d20b7b9a3f..313fe533e72 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, + }, + ], + }, ], });