From 50a493e4c98f06b768aa9234c29e9da2630b2fc4 Mon Sep 17 00:00:00 2001 From: Torleif Berger Date: Mon, 3 Jun 2019 15:42:24 +0200 Subject: [PATCH] feat(eslint-plugin): [explicit-function-return-type] allowHigherOrderFunctions (#193) (#538) --- .../rules/explicit-function-return-type.md | 33 +++- .../rules/explicit-function-return-type.ts | 59 +++++- .../explicit-function-return-type.test.ts | 186 ++++++++++++++++++ 3 files changed, 275 insertions(+), 3 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/explicit-function-return-type.md b/packages/eslint-plugin/docs/rules/explicit-function-return-type.md index 2812023ffba..a20aa83c7a5 100644 --- a/packages/eslint-plugin/docs/rules/explicit-function-return-type.md +++ b/packages/eslint-plugin/docs/rules/explicit-function-return-type.md @@ -65,13 +65,16 @@ The rule accepts an options object with the following properties: type Options = { // if true, only functions which are part of a declaration will be checked allowExpressions?: boolean; - // if true, type annotations are also allowed on the variable of a function expression rather than on the function directly. + // if true, type annotations are also allowed on the variable of a function expression rather than on the function directly allowTypedFunctionExpressions?: boolean; + // if true, functions immediately returning another function expression will not be checked + allowHigherOrderFunctions?: boolean; }; const defaults = { allowExpressions: false, allowTypedFunctionExpressions: false, + allowHigherOrderFunctions: false, }; ``` @@ -121,7 +124,7 @@ let funcExpr: FuncType = function() { }; let asTyped = (() => '') as () => string; -let caasTyped = <() => string>(() => ''); +let castTyped = <() => string>(() => ''); interface ObjectType { foo(): number; @@ -137,6 +140,32 @@ let objectPropCast = { }; ``` +### allowHigherOrderFunctions + +Examples of **incorrect** code for this rule with `{ allowHigherOrderFunctions: true }`: + +```ts +var arrowFn = (x: number) => (y: number) => x + y; + +function fn(x: number) { + return function(y: number) { + return x + y; + }; +} +``` + +Examples of **correct** code for this rule with `{ allowHigherOrderFunctions: true }`: + +```ts +var arrowFn = (x: number) => (y: number): number => x + y; + +function fn(x: number) { + return function(y: number): number { + return x + y; + }; +} +``` + ## When Not To Use It If you don't wish to prevent calling code from using function return values in unexpected ways, then 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 be6bb14c448..876f42517d6 100644 --- a/packages/eslint-plugin/src/rules/explicit-function-return-type.ts +++ b/packages/eslint-plugin/src/rules/explicit-function-return-type.ts @@ -8,6 +8,7 @@ type Options = [ { allowExpressions?: boolean; allowTypedFunctionExpressions?: boolean; + allowHigherOrderFunctions?: boolean; } ]; type MessageIds = 'missingReturnType'; @@ -35,6 +36,9 @@ export default util.createRule({ allowTypedFunctionExpressions: { type: 'boolean', }, + allowHigherOrderFunctions: { + type: 'boolean', + }, }, additionalProperties: false, }, @@ -44,6 +48,7 @@ export default util.createRule({ { allowExpressions: false, allowTypedFunctionExpressions: false, + allowHigherOrderFunctions: false, }, ], create(context, [options]) { @@ -138,6 +143,50 @@ export default util.createRule({ ); } + /** + * Checks if a function belongs to: + * `() => () => ...` + * `() => function () { ... }` + * `() => { return () => ... }` + * `() => { return function () { ... } }` + * `function fn() { return () => ... }` + * `function fn() { return function() { ... } }` + */ + function doesImmediatelyReturnFunctionExpression({ + body, + }: + | TSESTree.ArrowFunctionExpression + | TSESTree.FunctionDeclaration + | TSESTree.FunctionExpression): boolean { + // Should always have a body; really checking just in case + /* istanbul ignore if */ if (!body) { + return false; + } + + // Check if body is a block with a single statement + if ( + body.type === AST_NODE_TYPES.BlockStatement && + body.body.length === 1 + ) { + const [statement] = body.body; + + // Check if that statement is a return statement with an argument + if ( + statement.type === AST_NODE_TYPES.ReturnStatement && + !!statement.argument + ) { + // If so, check that returned argument as body + body = statement.argument; + } + } + + // Check if the body being returned is a function expression + return ( + body.type === AST_NODE_TYPES.ArrowFunctionExpression || + body.type === AST_NODE_TYPES.FunctionExpression + ); + } + /** * Checks if a function declaration/expression has a return type. */ @@ -147,6 +196,13 @@ export default util.createRule({ | TSESTree.FunctionDeclaration | TSESTree.FunctionExpression, ): void { + if ( + options.allowHigherOrderFunctions && + doesImmediatelyReturnFunctionExpression(node) + ) { + return; + } + if ( node.returnType || isConstructor(node.parent) || @@ -169,7 +225,8 @@ export default util.createRule({ function checkFunctionExpressionReturnType( node: TSESTree.ArrowFunctionExpression | TSESTree.FunctionExpression, ): void { - if (node.parent) { + // Should always have a parent; checking just in case + /* istanbul ignore else */ if (node.parent) { if (options.allowTypedFunctionExpressions) { if ( isTypeCast(node.parent) || 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 936b300134b..1b9209b7abf 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 @@ -180,6 +180,71 @@ const myObj = { }; `, }, + { + filename: 'test.ts', + code: ` +() => (): void => {}; + `, + options: [{ allowHigherOrderFunctions: true }], + }, + { + filename: 'test.ts', + code: ` +() => function (): void {}; + `, + options: [{ allowHigherOrderFunctions: true }], + }, + { + filename: 'test.ts', + code: ` +() => { return (): void => {} }; + `, + options: [{ allowHigherOrderFunctions: true }], + }, + { + filename: 'test.ts', + code: ` +() => { return function (): void {} }; + `, + options: [{ allowHigherOrderFunctions: true }], + }, + { + filename: 'test.ts', + code: ` +function fn() { return (): void => {} }; + `, + options: [{ allowHigherOrderFunctions: true }], + }, + { + filename: 'test.ts', + code: ` +function fn() { return function (): void {} }; + `, + options: [{ allowHigherOrderFunctions: true }], + }, + { + filename: 'test.ts', + code: ` +function FunctionDeclaration() { + return function FunctionExpression_Within_FunctionDeclaration() { + return function FunctionExpression_Within_FunctionExpression() { + return () => { // ArrowFunctionExpression_Within_FunctionExpression + return () => // ArrowFunctionExpression_Within_ArrowFunctionExpression + (): number => 1 // ArrowFunctionExpression_Within_ArrowFunctionExpression_WithNoBody + } + } + } +} + `, + options: [{ allowHigherOrderFunctions: true }], + }, + { + filename: 'test.ts', + code: ` +() => () => { return (): void => { return; } }; + `, + options: [{ allowHigherOrderFunctions: true }], + }, ], invalid: [ { @@ -364,5 +429,126 @@ const x: Foo = { }, ], }, + { + filename: 'test.ts', + code: ` +() => () => {}; + `, + options: [{ allowHigherOrderFunctions: true }], + errors: [ + { + messageId: 'missingReturnType', + line: 2, + column: 7, + }, + ], + }, + { + filename: 'test.ts', + code: ` +() => function () {}; + `, + options: [{ allowHigherOrderFunctions: true }], + errors: [ + { + messageId: 'missingReturnType', + line: 2, + column: 7, + }, + ], + }, + { + filename: 'test.ts', + code: ` +() => { return () => {} }; + `, + options: [{ allowHigherOrderFunctions: true }], + errors: [ + { + messageId: 'missingReturnType', + line: 2, + column: 16, + }, + ], + }, + { + filename: 'test.ts', + code: ` +() => { return function () {} }; + `, + options: [{ allowHigherOrderFunctions: true }], + errors: [ + { + messageId: 'missingReturnType', + line: 2, + column: 16, + }, + ], + }, + { + filename: 'test.ts', + code: ` +function fn() { return () => {} }; + `, + options: [{ allowHigherOrderFunctions: true }], + errors: [ + { + messageId: 'missingReturnType', + line: 2, + column: 24, + }, + ], + }, + { + filename: 'test.ts', + code: ` +function fn() { return function () {} }; + `, + options: [{ allowHigherOrderFunctions: true }], + errors: [ + { + messageId: 'missingReturnType', + line: 2, + column: 24, + }, + ], + }, + { + filename: 'test.ts', + code: ` +function FunctionDeclaration() { + return function FunctionExpression_Within_FunctionDeclaration() { + return function FunctionExpression_Within_FunctionExpression() { + return () => { // ArrowFunctionExpression_Within_FunctionExpression + return () => // ArrowFunctionExpression_Within_ArrowFunctionExpression + () => 1 // ArrowFunctionExpression_Within_ArrowFunctionExpression_WithNoBody + } + } + } +} + `, + options: [{ allowHigherOrderFunctions: true }], + errors: [ + { + messageId: 'missingReturnType', + line: 7, + column: 11, + }, + ], + }, + { + filename: 'test.ts', + code: ` +() => () => { return () => { return; } }; + `, + options: [{ allowHigherOrderFunctions: true }], + errors: [ + { + messageId: 'missingReturnType', + line: 2, + column: 22, + }, + ], + }, ], });