From f991764fad7d56e85a789ffa191919b0895d781a Mon Sep 17 00:00:00 2001 From: Brad Zacher Date: Sun, 26 Jan 2020 14:04:07 -0800 Subject: [PATCH] chore(eslint-plugin): refactor explicit return type rules to share code (#1493) Co-authored-by: Armano --- .vscode/settings.json | 10 +- .../rules/explicit-function-return-type.ts | 326 ++------------- .../rules/explicit-module-boundary-types.ts | 385 +++--------------- packages/eslint-plugin/src/util/astUtils.ts | 45 +- .../src/util/explicitReturnTypeUtils.ts | 314 ++++++++++++++ .../explicit-module-boundary-types.test.ts | 17 +- 6 files changed, 452 insertions(+), 645 deletions(-) create mode 100644 packages/eslint-plugin/src/util/explicitReturnTypeUtils.ts diff --git a/.vscode/settings.json b/.vscode/settings.json index 0174acc7be3..c03c3aac4dd 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -4,14 +4,8 @@ "eslint.validate": [ "javascript", "javascriptreact", - { - "language": "typescript", - "autoFix": true - }, - { - "language": "typescriptreact", - "autoFix": true - } + "typescript", + "typescriptreact" ], // When enabled, will trim trailing whitespace when saving a file. 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 b8542098011..b8ee7ff584b 100644 --- a/packages/eslint-plugin/src/rules/explicit-function-return-type.ts +++ b/packages/eslint-plugin/src/rules/explicit-function-return-type.ts @@ -1,9 +1,9 @@ -import { - TSESTree, - AST_NODE_TYPES, - AST_TOKEN_TYPES, -} from '@typescript-eslint/experimental-utils'; +import { TSESTree } from '@typescript-eslint/experimental-utils'; import * as util from '../util'; +import { + checkFunctionReturnType, + checkFunctionExpressionReturnType, +} from '../util/explicitReturnTypeUtils'; type Options = [ { @@ -60,303 +60,27 @@ export default util.createRule({ create(context, [options]) { const sourceCode = context.getSourceCode(); - /** - * Returns start column position - * @param node - */ - function getLocStart( - node: - | TSESTree.ArrowFunctionExpression - | TSESTree.FunctionDeclaration - | TSESTree.FunctionExpression, - ): TSESTree.LineAndColumnData { - /* highlight method name */ - const parent = node.parent; - if ( - parent && - (parent.type === AST_NODE_TYPES.MethodDefinition || - (parent.type === AST_NODE_TYPES.Property && parent.method)) - ) { - return parent.loc.start; - } - - return node.loc.start; - } - - /** - * Returns end column position - * @param node - */ - function getLocEnd( - node: - | TSESTree.ArrowFunctionExpression - | TSESTree.FunctionDeclaration - | TSESTree.FunctionExpression, - ): TSESTree.LineAndColumnData { - /* highlight `=>` */ - if (node.type === AST_NODE_TYPES.ArrowFunctionExpression) { - return sourceCode.getTokenBefore( - node.body, - token => - token.type === AST_TOKEN_TYPES.Punctuator && token.value === '=>', - )!.loc.end; - } - - return sourceCode.getTokenBefore(node.body!)!.loc.end; - } - - /** - * 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 { - 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 belongs to: - * new Foo(() => {}) - * ^^^^^^^^ - */ - function isConstructorArgument(parent: TSESTree.Node): boolean { - return parent.type === AST_NODE_TYPES.NewExpression; - } - - /** - * 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 ( - util.isTypeAssertion(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 ( - 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 node belongs to: - * `foo(() => 1)` - */ - function isFunctionArgument( - parent: TSESTree.Node, - callee?: TSESTree.ArrowFunctionExpression | TSESTree.FunctionExpression, - ): boolean { - return ( - (parent.type === AST_NODE_TYPES.CallExpression || - parent.type === AST_NODE_TYPES.OptionalCallExpression) && - // 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 (util.isTypeAssertion(body)) { - 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 ( - options.allowHigherOrderFunctions && - doesImmediatelyReturnFunctionExpression(node) - ) { - return; - } - - if ( - node.returnType || - isConstructor(node.parent) || - isSetter(node.parent) - ) { - return; - } - - context.report({ - node, - loc: { start: getLocStart(node), end: getLocEnd(node) }, - messageId: 'missingReturnType', - }); - } - - /** - * 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 ( - util.isTypeAssertion(node.parent) || - isVariableDeclaratorWithTypeAnnotation(node.parent) || - isClassPropertyWithTypeAnnotation(node.parent) || - isPropertyOfObjectWithType(node.parent) || - isFunctionArgument(node.parent, node) || - isConstructorArgument(node.parent) - ) { - return; - } - } - - if ( - options.allowExpressions && - node.parent.type !== AST_NODE_TYPES.VariableDeclarator && - node.parent.type !== AST_NODE_TYPES.MethodDefinition && - node.parent.type !== AST_NODE_TYPES.ExportDefaultDeclaration && - node.parent.type !== AST_NODE_TYPES.ClassProperty - ) { - return; - } - } - - // https://github.com/typescript-eslint/typescript-eslint/issues/653 - if ( - node.type === AST_NODE_TYPES.ArrowFunctionExpression && - options.allowDirectConstAssertionInArrowFunctions && - returnsConstAssertionDirectly(node) - ) { - return; - } - - checkFunctionReturnType(node); - } - return { - ArrowFunctionExpression: checkFunctionExpressionReturnType, - FunctionDeclaration: checkFunctionReturnType, - FunctionExpression: checkFunctionExpressionReturnType, + 'ArrowFunctionExpression, FunctionExpression'( + node: TSESTree.ArrowFunctionExpression | TSESTree.FunctionExpression, + ): void { + checkFunctionExpressionReturnType(node, options, sourceCode, loc => + context.report({ + node, + loc, + messageId: 'missingReturnType', + }), + ); + }, + FunctionDeclaration(node): void { + checkFunctionReturnType(node, options, sourceCode, loc => + context.report({ + node, + loc, + messageId: 'missingReturnType', + }), + ); + }, }; }, }); 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 7871035672f..0bd872622f1 100644 --- a/packages/eslint-plugin/src/rules/explicit-module-boundary-types.ts +++ b/packages/eslint-plugin/src/rules/explicit-module-boundary-types.ts @@ -1,9 +1,12 @@ import { TSESTree, AST_NODE_TYPES, - AST_TOKEN_TYPES, } from '@typescript-eslint/experimental-utils'; import * as util from '../util'; +import { + checkFunctionExpressionReturnType, + checkFunctionReturnType, +} from '../util/explicitReturnTypeUtils'; type Options = [ { @@ -64,152 +67,6 @@ export default util.createRule({ create(context, [options]) { const sourceCode = context.getSourceCode(); - /** - * Returns start column position - * @param node - */ - function getLocStart( - node: - | TSESTree.ArrowFunctionExpression - | TSESTree.FunctionDeclaration - | TSESTree.FunctionExpression, - ): TSESTree.LineAndColumnData { - /* highlight method name */ - const parent = node.parent; - if ( - parent && - (parent.type === AST_NODE_TYPES.MethodDefinition || - (parent.type === AST_NODE_TYPES.Property && parent.method)) - ) { - return parent.loc.start; - } - - return node.loc.start; - } - - /** - * Returns end column position - * @param node - */ - function getLocEnd( - node: - | TSESTree.ArrowFunctionExpression - | TSESTree.FunctionDeclaration - | TSESTree.FunctionExpression, - ): TSESTree.LineAndColumnData { - /* highlight `=>` */ - if (node.type === AST_NODE_TYPES.ArrowFunctionExpression) { - return sourceCode.getTokenBefore( - node.body, - token => - token.type === AST_TOKEN_TYPES.Punctuator && token.value === '=>', - )!.loc.end; - } - - return sourceCode.getTokenBefore(node.body!)!.loc.end; - } - - /** - * 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 { - 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 belongs to: - * new Foo(() => {}) - * ^^^^^^^^ - */ - function isConstructorArgument(parent: TSESTree.Node): boolean { - return parent.type === AST_NODE_TYPES.NewExpression; - } - - /** - * 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) - ); - } - function isUnexported(node: TSESTree.Node | undefined): boolean { let isReturnedValue = false; while (node) { @@ -243,189 +100,33 @@ export default util.createRule({ return true; } - function isPrivateMethod( - node: TSESTree.MethodDefinition | TSESTree.TSAbstractMethodDefinition, - ): boolean { - return node.accessibility === 'private'; - } - - /** - * 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 node belongs to: - * `foo(() => 1)` - */ - function isFunctionArgument( - parent: TSESTree.Node, - callee?: TSESTree.ArrowFunctionExpression | TSESTree.FunctionExpression, - ): boolean { + function isArgumentUntyped(node: TSESTree.Identifier): boolean { return ( - (parent.type === AST_NODE_TYPES.CallExpression || - parent.type === AST_NODE_TYPES.OptionalCallExpression) && - // make sure this isn't an IIFE - parent.callee !== callee + !node.typeAnnotation || + node.typeAnnotation.typeAnnotation.type === AST_NODE_TYPES.TSAnyKeyword ); } - /** - * 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( + function checkArguments( node: | TSESTree.ArrowFunctionExpression | TSESTree.FunctionDeclaration | TSESTree.FunctionExpression, ): void { - const paramIdentifiers = node.params.filter( - param => param.type === AST_NODE_TYPES.Identifier, - ) as TSESTree.Identifier[]; + const paramIdentifiers = node.params.filter(util.isIdentifier); const untypedArgs = paramIdentifiers.filter(isArgumentUntyped); - if (untypedArgs.length) { - untypedArgs.forEach(untypedArg => - context.report({ - node, - messageId: 'missingArgType', - data: { - name: untypedArg.name, - }, - }), - ); - } - - if (isAllowedName(node.parent)) { - return; - } - - if (isUnexported(node.parent)) { - return; - } - - if ( - options.allowHigherOrderFunctions && - doesImmediatelyReturnFunctionExpression(node) - ) { - return; - } - - if ( - node.returnType || - isConstructor(node.parent) || - isSetter(node.parent) - ) { - return; - } - - context.report({ - node, - loc: { start: getLocStart(node), end: getLocEnd(node) }, - messageId: 'missingReturnType', - }); - } - - /** - * 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 ( - node.parent.type === AST_NODE_TYPES.MethodDefinition && - isPrivateMethod(node.parent) - ) { - return; - } - - if (options.allowTypedFunctionExpressions) { - if ( - isTypeCast(node.parent) || - isVariableDeclaratorWithTypeAnnotation(node.parent) || - isClassPropertyWithTypeAnnotation(node.parent) || - isPropertyOfObjectWithType(node.parent) || - isFunctionArgument(node.parent, node) || - isConstructorArgument(node.parent) - ) { - return; - } - } - } - - if ( - node.type === AST_NODE_TYPES.ArrowFunctionExpression && - options.allowDirectConstAssertionInArrowFunctions && - returnsConstAssertionDirectly(node) - ) { - return; - } - - checkFunctionReturnType(node); + untypedArgs.forEach(untypedArg => + context.report({ + node, + messageId: 'missingArgType', + data: { + name: untypedArg.name, + }, + }), + ); } /** @@ -465,17 +166,47 @@ export default util.createRule({ return false; } - function isArgumentUntyped(node: TSESTree.Identifier): boolean { - return ( - !node.typeAnnotation || - node.typeAnnotation.typeAnnotation.type === AST_NODE_TYPES.TSAnyKeyword - ); - } - return { - ArrowFunctionExpression: checkFunctionExpressionReturnType, - FunctionDeclaration: checkFunctionReturnType, - FunctionExpression: checkFunctionExpressionReturnType, + 'ArrowFunctionExpression, FunctionExpression'( + node: TSESTree.ArrowFunctionExpression | TSESTree.FunctionExpression, + ): void { + if ( + node.parent?.type === AST_NODE_TYPES.MethodDefinition && + node.parent.accessibility === 'private' + ) { + // don't check private methods as they aren't part of the public signature + return; + } + + if (isAllowedName(node.parent) || isUnexported(node)) { + return; + } + + checkFunctionExpressionReturnType(node, options, sourceCode, loc => + context.report({ + node, + loc, + messageId: 'missingReturnType', + }), + ); + + checkArguments(node); + }, + FunctionDeclaration(node): void { + if (isAllowedName(node.parent) || isUnexported(node)) { + return; + } + + checkFunctionReturnType(node, options, sourceCode, loc => + context.report({ + node, + loc, + messageId: 'missingReturnType', + }), + ); + + checkArguments(node); + }, }; }, }); diff --git a/packages/eslint-plugin/src/util/astUtils.ts b/packages/eslint-plugin/src/util/astUtils.ts index 6c92d90b637..367bd93350b 100644 --- a/packages/eslint-plugin/src/util/astUtils.ts +++ b/packages/eslint-plugin/src/util/astUtils.ts @@ -65,8 +65,10 @@ function isTokenOnSameLine( /** * Checks if a node is a type assertion: - * - x as foo - * - x + * ``` + * x as foo + * x + * ``` */ function isTypeAssertion( node: TSESTree.Node | undefined | null, @@ -80,14 +82,49 @@ function isTypeAssertion( ); } +/** + * Checks if a node is a constructor method. + */ +function isConstructor( + node: TSESTree.Node | undefined, +): node is TSESTree.MethodDefinition { + return ( + node?.type === AST_NODE_TYPES.MethodDefinition && + node.kind === 'constructor' + ); +} + +/** + * Checks if a node is a setter method. + */ +function isSetter( + node: TSESTree.Node | undefined, +): node is TSESTree.MethodDefinition | TSESTree.Property { + return ( + !!node && + (node.type === AST_NODE_TYPES.MethodDefinition || + node.type === AST_NODE_TYPES.Property) && + node.kind === 'set' + ); +} + +function isIdentifier( + node: TSESTree.Node | undefined, +): node is TSESTree.Identifier { + return node?.type === AST_NODE_TYPES.Identifier; +} + export { - isTypeAssertion, + isConstructor, + isIdentifier, + isLogicalOrOperator, isNonNullAssertionPunctuator, isNotNonNullAssertionPunctuator, isNotOptionalChainPunctuator, isOptionalChainPunctuator, isOptionalOptionalChain, + isSetter, isTokenOnSameLine, - isLogicalOrOperator, + isTypeAssertion, LINEBREAK_MATCHER, }; diff --git a/packages/eslint-plugin/src/util/explicitReturnTypeUtils.ts b/packages/eslint-plugin/src/util/explicitReturnTypeUtils.ts new file mode 100644 index 00000000000..81d43590eab --- /dev/null +++ b/packages/eslint-plugin/src/util/explicitReturnTypeUtils.ts @@ -0,0 +1,314 @@ +import { + TSESTree, + AST_NODE_TYPES, + TSESLint, + AST_TOKEN_TYPES, +} from '@typescript-eslint/experimental-utils'; +import { isTypeAssertion, isConstructor, isSetter } from './astUtils'; + +type FunctionNode = + | TSESTree.ArrowFunctionExpression + | TSESTree.FunctionDeclaration + | TSESTree.FunctionExpression; + +/** + * Creates a report location for the given function. + * The location only encompasses the "start" of the function, and not the body + * + * eg. + * function foo(args) {} + * ^^^^^^^^^^^^^^^^^^ + * + * get y(args) {} + * ^^^^^^^^^^^ + * + * const x = (args) => {} + * ^^^^^^^^^ + */ +function getReporLoc( + node: FunctionNode, + sourceCode: TSESLint.SourceCode, +): TSESTree.SourceLocation { + /** + * Returns start column position + * @param node + */ + function getLocStart(): TSESTree.LineAndColumnData { + /* highlight method name */ + const parent = node.parent; + if ( + parent && + (parent.type === AST_NODE_TYPES.MethodDefinition || + (parent.type === AST_NODE_TYPES.Property && parent.method)) + ) { + return parent.loc.start; + } + + return node.loc.start; + } + + /** + * Returns end column position + * @param node + */ + function getLocEnd(): TSESTree.LineAndColumnData { + /* highlight `=>` */ + if (node.type === AST_NODE_TYPES.ArrowFunctionExpression) { + return sourceCode.getTokenBefore( + node.body, + token => + token.type === AST_TOKEN_TYPES.Punctuator && token.value === '=>', + )!.loc.end; + } + + return sourceCode.getTokenBefore(node.body!)!.loc.end; + } + + return { + start: getLocStart(), + end: getLocEnd(), + }; +} + +/** + * Checks if a node is a variable declarator with a type annotation. + * ``` + * const x: Foo = ... + * ``` + */ +function isVariableDeclaratorWithTypeAnnotation( + node: TSESTree.Node, +): node is TSESTree.VariableDeclarator { + 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, +): node is TSESTree.ClassProperty { + return node.type === AST_NODE_TYPES.ClassProperty && !!node.typeAnnotation; +} + +/** + * Checks if a node belongs to: + * ``` + * new Foo(() => {}) + * ^^^^^^^^ + * ``` + */ +function isConstructorArgument( + node: TSESTree.Node, +): node is TSESTree.NewExpression { + return node.type === AST_NODE_TYPES.NewExpression; +} + +/** + * 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 ( + isTypeAssertion(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 (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 node belongs to: + * ``` + * foo(() => 1) + * ``` + */ +function isFunctionArgument( + parent: TSESTree.Node, + callee?: TSESTree.ArrowFunctionExpression | TSESTree.FunctionExpression, +): parent is TSESTree.CallExpression | TSESTree.OptionalCallExpression { + return ( + (parent.type === AST_NODE_TYPES.CallExpression || + parent.type === AST_NODE_TYPES.OptionalCallExpression) && + // 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 (isTypeAssertion(body)) { + 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; +} + +interface Options { + allowExpressions?: boolean; + allowTypedFunctionExpressions?: boolean; + allowHigherOrderFunctions?: boolean; + allowDirectConstAssertionInArrowFunctions?: boolean; +} + +/** + * 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 ( + options.allowHigherOrderFunctions && + doesImmediatelyReturnFunctionExpression(node) + ) { + return; + } + + if (node.returnType || isConstructor(node.parent) || isSetter(node.parent)) { + 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 { + // Should always have a parent; checking just in case + /* istanbul ignore else */ if (node.parent) { + if (options.allowTypedFunctionExpressions) { + if ( + isTypeAssertion(node.parent) || + isVariableDeclaratorWithTypeAnnotation(node.parent) || + isClassPropertyWithTypeAnnotation(node.parent) || + isPropertyOfObjectWithType(node.parent) || + isFunctionArgument(node.parent, node) || + isConstructorArgument(node.parent) + ) { + return; + } + } + + if ( + options.allowExpressions && + node.parent.type !== AST_NODE_TYPES.VariableDeclarator && + node.parent.type !== AST_NODE_TYPES.MethodDefinition && + node.parent.type !== AST_NODE_TYPES.ExportDefaultDeclaration && + node.parent.type !== AST_NODE_TYPES.ClassProperty + ) { + 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, report); +} + +export { checkFunctionReturnType, checkFunctionExpressionReturnType }; 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 c257d60f715..fc2413ba011 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 @@ -394,7 +394,7 @@ export class Test { get prop() { return 1; } - set prop() {} + set prop(value) {} method() { return; } @@ -412,6 +412,13 @@ export class Test { column: 3, endColumn: 13, }, + { + messageId: 'missingArgType', + line: 7, + endLine: 7, + column: 11, + endColumn: 21, + }, { messageId: 'missingReturnType', line: 8, @@ -420,18 +427,18 @@ export class Test { endColumn: 11, }, { - messageId: 'missingArgType', + messageId: 'missingReturnType', line: 11, endLine: 11, column: 11, - endColumn: 27, + endColumn: 19, }, { - messageId: 'missingReturnType', + messageId: 'missingArgType', line: 11, endLine: 11, column: 11, - endColumn: 19, + endColumn: 27, }, ], },