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 1f69834d4784..da3e7334b6ab 100644 --- a/packages/eslint-plugin/src/rules/explicit-function-return-type.ts +++ b/packages/eslint-plugin/src/rules/explicit-function-return-type.ts @@ -1,10 +1,8 @@ -import { AST_NODE_TYPES, TSESTree } from '@typescript-eslint/utils'; -import * as util from '../util'; import { - checkFunctionReturnType, - isValidFunctionExpressionReturnType, - ancestorHasReturnType, -} from '../util/explicitReturnTypeUtils'; + TSESTree, + AST_NODE_TYPES, +} from '@typescript-eslint/experimental-utils'; +import * as util from '../util'; type Options = [ { @@ -12,8 +10,6 @@ type Options = [ allowTypedFunctionExpressions?: boolean; allowHigherOrderFunctions?: boolean; allowDirectConstAssertionInArrowFunctions?: boolean; - allowConciseArrowFunctionExpressionsStartingWithVoid?: boolean; - allowedNames?: string[]; }, ]; type MessageIds = 'missingReturnType'; @@ -25,7 +21,8 @@ export default util.createRule({ docs: { description: 'Require explicit return types on functions and class methods', - recommended: false, + category: 'Stylistic Issues', + recommended: 'warn', }, messages: { missingReturnType: 'Missing return type on function.', @@ -46,15 +43,6 @@ export default util.createRule({ allowDirectConstAssertionInArrowFunctions: { type: 'boolean', }, - allowConciseArrowFunctionExpressionsStartingWithVoid: { - type: 'boolean', - }, - allowedNames: { - type: 'array', - items: { - type: 'string', - }, - }, }, additionalProperties: false, }, @@ -66,115 +54,260 @@ export default util.createRule({ allowTypedFunctionExpressions: true, allowHigherOrderFunctions: true, allowDirectConstAssertionInArrowFunctions: true, - allowConciseArrowFunctionExpressionsStartingWithVoid: false, - allowedNames: [], }, ], create(context, [options]) { - const sourceCode = context.getSourceCode(); - function isAllowedName( - node: - | TSESTree.ArrowFunctionExpression - | TSESTree.FunctionExpression - | TSESTree.FunctionDeclaration, + /** + * Checks if a node is a constructor. + * @param node The node to check + */ + function isConstructor(node: TSESTree.Node | undefined): boolean { + return ( + !!node && + node.type === AST_NODE_TYPES.MethodDefinition && + node.kind === 'constructor' + ); + } + + /** + * Checks if a node is a setter. + */ + function isSetter(node: TSESTree.Node | undefined): boolean { + return ( + !!node && + (node.type === AST_NODE_TYPES.MethodDefinition || + node.type === AST_NODE_TYPES.Property) && + node.kind === 'set' + ); + } + + /** + * Checks if a node is a variable declarator with a type annotation. + * `const x: Foo = ...` + */ + function isVariableDeclaratorWithTypeAnnotation( + node: TSESTree.Node, ): boolean { - if (!options.allowedNames || !options.allowedNames.length) { + return ( + node.type === AST_NODE_TYPES.VariableDeclarator && + !!node.id.typeAnnotation + ); + } + + /** + * Checks if a node is a class property with a type annotation. + * `public x: Foo = ...` + */ + function isClassPropertyWithTypeAnnotation(node: TSESTree.Node): boolean { + return ( + node.type === AST_NODE_TYPES.ClassProperty && !!node.typeAnnotation + ); + } + + /** + * Checks if a node is a type cast + * `(() => {}) as Foo` + * `(() => {})` + */ + function isTypeCast(node: TSESTree.Node): boolean { + return ( + node.type === AST_NODE_TYPES.TSAsExpression || + node.type === AST_NODE_TYPES.TSTypeAssertion + ); + } + + /** + * Checks if a node belongs to: + * `const x: Foo = { prop: () => {} }` + * `const x = { prop: () => {} } as Foo` + * `const x = { prop: () => {} }` + */ + function isPropertyOfObjectWithType( + property: TSESTree.Node | undefined, + ): boolean { + if (!property || property.type !== AST_NODE_TYPES.Property) { + return false; + } + const objectExpr = property.parent; // this shouldn't happen, checking just in case + /* istanbul ignore if */ if ( + !objectExpr || + objectExpr.type !== AST_NODE_TYPES.ObjectExpression + ) { + return false; + } + + const parent = objectExpr.parent; // this shouldn't happen, checking just in case + /* istanbul ignore if */ if (!parent) { + return false; + } + + return ( + isTypeCast(parent) || + isClassPropertyWithTypeAnnotation(parent) || + isVariableDeclaratorWithTypeAnnotation(parent) || + isFunctionArgument(parent) + ); + } + + /** + * 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 ( - node.type === AST_NODE_TYPES.ArrowFunctionExpression || - node.type === AST_NODE_TYPES.FunctionExpression + body.type === AST_NODE_TYPES.BlockStatement && + body.body.length === 1 ) { - const parent = node.parent; - let funcName; - if (node.id?.name) { - funcName = node.id.name; - } else if (parent) { - switch (parent.type) { - case AST_NODE_TYPES.VariableDeclarator: { - if (parent.id.type === AST_NODE_TYPES.Identifier) { - funcName = parent.id.name; - } - break; - } - case AST_NODE_TYPES.MethodDefinition: - case AST_NODE_TYPES.PropertyDefinition: - case AST_NODE_TYPES.Property: { - if ( - parent.key.type === AST_NODE_TYPES.Identifier && - parent.computed === false - ) { - funcName = parent.key.name; - } - break; - } - } + 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; } - if (!!funcName && !!options.allowedNames.includes(funcName)) { - return true; + } + + // 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 node belongs to: + * `foo(() => 1)` + */ + function isFunctionArgument( + parent: TSESTree.Node, + callee?: TSESTree.ArrowFunctionExpression | TSESTree.FunctionExpression, + ): boolean { + return ( + parent.type === AST_NODE_TYPES.CallExpression && + // make sure this isn't an IIFE + parent.callee !== callee + ); + } + + /** + * Checks if a function belongs to: + * `() => ({ action: 'xxx' }) as const` + */ + function returnsConstAssertionDirectly( + node: TSESTree.ArrowFunctionExpression, + ): boolean { + const { body } = node; + if (body.type === AST_NODE_TYPES.TSAsExpression) { + const { typeAnnotation } = body; + if (typeAnnotation.type === AST_NODE_TYPES.TSTypeReference) { + const { typeName } = typeAnnotation; + if ( + typeName.type === AST_NODE_TYPES.Identifier && + typeName.name === 'const' + ) { + return true; + } } } + + return false; + } + + /** + * Checks if a function declaration/expression has a return type. + */ + function checkFunctionReturnType( + node: + | TSESTree.ArrowFunctionExpression + | TSESTree.FunctionDeclaration + | TSESTree.FunctionExpression, + ): void { if ( - node.type === AST_NODE_TYPES.FunctionDeclaration && - node.id && - node.id.type === AST_NODE_TYPES.Identifier && - !!options.allowedNames.includes(node.id.name) + options.allowHigherOrderFunctions && + doesImmediatelyReturnFunctionExpression(node) ) { - return true; + return; } - return false; + + if ( + node.returnType || + isConstructor(node.parent) || + isSetter(node.parent) + ) { + return; + } + + context.report({ + node, + messageId: 'missingReturnType', + }); } - return { - 'ArrowFunctionExpression, FunctionExpression'( - node: TSESTree.ArrowFunctionExpression | TSESTree.FunctionExpression, - ): void { - if ( - options.allowConciseArrowFunctionExpressionsStartingWithVoid && - node.type === AST_NODE_TYPES.ArrowFunctionExpression && - node.expression && - node.body.type === AST_NODE_TYPES.UnaryExpression && - node.body.operator === 'void' - ) { - return; - } - if (isAllowedName(node)) { - return; + /** + * Checks if a function declaration/expression has a return type. + */ + function checkFunctionExpressionReturnType( + node: TSESTree.ArrowFunctionExpression | TSESTree.FunctionExpression, + ): void { + // Should always have a parent; checking just in case + /* istanbul ignore else */ if (node.parent) { + if (options.allowTypedFunctionExpressions) { + if ( + isTypeCast(node.parent) || + isVariableDeclaratorWithTypeAnnotation(node.parent) || + isClassPropertyWithTypeAnnotation(node.parent) || + isPropertyOfObjectWithType(node.parent) || + isFunctionArgument(node.parent, node) + ) { + return; + } } if ( - options.allowTypedFunctionExpressions && - (isValidFunctionExpressionReturnType(node, options) || - ancestorHasReturnType(node)) + options.allowExpressions && + node.parent.type !== AST_NODE_TYPES.VariableDeclarator && + node.parent.type !== AST_NODE_TYPES.MethodDefinition && + node.parent.type !== AST_NODE_TYPES.ExportDefaultDeclaration ) { return; } + } - checkFunctionReturnType(node, options, sourceCode, loc => - context.report({ - node, - loc, - messageId: 'missingReturnType', - }), - ); - }, - FunctionDeclaration(node): void { - if (isAllowedName(node)) { - return; - } - if (options.allowTypedFunctionExpressions && node.returnType) { - return; - } + // https://github.com/typescript-eslint/typescript-eslint/issues/653 + if ( + node.type === AST_NODE_TYPES.ArrowFunctionExpression && + options.allowDirectConstAssertionInArrowFunctions && + returnsConstAssertionDirectly(node) + ) { + return; + } - checkFunctionReturnType(node, options, sourceCode, loc => - context.report({ - node, - loc, - messageId: 'missingReturnType', - }), - ); - }, + checkFunctionReturnType(node); + } + + return { + ArrowFunctionExpression: checkFunctionExpressionReturnType, + FunctionDeclaration: checkFunctionReturnType, + FunctionExpression: checkFunctionExpressionReturnType, }; }, }); 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 40c15a89a2f1..eef15b63ccf6 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 @@ -555,6 +555,20 @@ const x: Bar = arg1 => arg2 => arg1 + arg2; }, ], }, + { + filename: 'test.ts', + code: ` +const func = (value: number) => (({ type: "X", value }) as const); +const func = (value: number) => ({ type: "X", value } as const); +const func = (value: number) => (x as const); +const func = (value: number) => x as const; + `, + options: [ + { + allowDirectConstAssertionInArrowFunctions: true, + }, + ], + }, ], invalid: [ { @@ -1414,5 +1428,47 @@ class Foo { }, ], }, + { + filename: 'test.ts', + code: ` +const func = (value: number) => ({ type: "X", value } as any); +const func = (value: number) => ({ type: "X", value } as Action); + `, + options: [ + { + allowDirectConstAssertionInArrowFunctions: true, + }, + ], + errors: [ + { + messageId: 'missingReturnType', + line: 2, + column: 14, + }, + { + messageId: 'missingReturnType', + line: 3, + column: 14, + }, + ], + }, + { + filename: 'test.ts', + code: ` +const func = (value: number) => ({ type: "X", value } as const); + `, + options: [ + { + allowDirectConstAssertionInArrowFunctions: false, + }, + ], + errors: [ + { + messageId: 'missingReturnType', + line: 2, + column: 14, + }, + ], + }, ], });