From f29d1c9d37b0a9865ec2eeda38ffac6e93fb2d3a Mon Sep 17 00:00:00 2001 From: Gilbert Gilb's Date: Thu, 28 Mar 2019 18:43:00 +0100 Subject: [PATCH] fix(eslint-plugin): Fix `allowExpressions` false positives in explicit-function-return-type and incorrect documentation (#388) Fixes #387 --- .../rules/explicit-function-return-type.md | 6 +++-- .../rules/explicit-function-return-type.ts | 22 +++++++++---------- .../explicit-function-return-type.test.ts | 19 ++++++++++++++++ 3 files changed, 34 insertions(+), 13 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 e63d3d351a2..b6f37038aba 100644 --- a/packages/eslint-plugin/docs/rules/explicit-function-return-type.md +++ b/packages/eslint-plugin/docs/rules/explicit-function-return-type.md @@ -62,9 +62,11 @@ class Test { The rule accepts an options object with the following properties: - `allowExpressions` if true, only functions which are part of a declaration will be checked +- `allowTypedFunctionExpressions` if true, type annotations are also allowed on the variable + of a function expression rather than on the function directly. -By default, `allowExpressions: false` is used, meaning all declarations and -expressions _must_ have a return type. +By default, `allowExpressions: false` and `allowTypedFunctionExpressions: false` are used, +meaning all declarations and expressions _must_ have a return type. ### allowExpressions 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 9cb9c983104..0d9f9304d2a 100644 --- a/packages/eslint-plugin/src/rules/explicit-function-return-type.ts +++ b/packages/eslint-plugin/src/rules/explicit-function-return-type.ts @@ -39,7 +39,7 @@ export default util.createRule({ }, defaultOptions: [ { - allowExpressions: true, + allowExpressions: false, allowTypedFunctionExpressions: false, }, ], @@ -88,6 +88,16 @@ export default util.createRule({ | TSESTree.FunctionDeclaration | TSESTree.FunctionExpression, ): void { + if ( + options.allowExpressions && + node.type !== AST_NODE_TYPES.FunctionDeclaration && + node.parent && + node.parent.type !== AST_NODE_TYPES.VariableDeclarator && + node.parent.type !== AST_NODE_TYPES.MethodDefinition + ) { + return; + } + if ( !node.returnType && node.parent && @@ -112,16 +122,6 @@ export default util.createRule({ | TSESTree.FunctionDeclaration | TSESTree.FunctionExpression, ): void { - if ( - options.allowExpressions && - node.type !== AST_NODE_TYPES.ArrowFunctionExpression && - node.parent && - node.parent.type !== AST_NODE_TYPES.VariableDeclarator && - node.parent.type !== AST_NODE_TYPES.MethodDefinition - ) { - return; - } - if ( options.allowTypedFunctionExpressions && 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 6786b92f4b1..45ff620d616 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 @@ -54,6 +54,7 @@ function test() { `, }, { + filename: 'test.ts', code: `fn(() => {});`, options: [ { @@ -62,6 +63,7 @@ function test() { ], }, { + filename: 'test.ts', code: `fn(function() {});`, options: [ { @@ -70,6 +72,7 @@ function test() { ], }, { + filename: 'test.ts', code: `[function() {}, () => {}]`, options: [ { @@ -78,6 +81,7 @@ function test() { ], }, { + filename: 'test.ts', code: `(function() {});`, options: [ { @@ -86,6 +90,7 @@ function test() { ], }, { + filename: 'test.ts', code: `(() => {})();`, options: [ { @@ -193,6 +198,20 @@ class Test { }, ], }, + { + filename: 'test.ts', + code: `function test() { + return; + }`, + options: [{ allowExpressions: true }], + errors: [ + { + messageId: 'missingReturnType', + line: 1, + column: 1, + }, + ], + }, { filename: 'test.ts', code: `const foo = () => {};`,