Skip to content

Commit

Permalink
fix(eslint-plugin): [explicit-function-return-type] support AllowType…
Browse files Browse the repository at this point in the history
…dFunctionExpression within AllowHigherOrderFunction (#4250)

* refactor: move function to be used more widely

* feat: find if the ancestor(top level) is typed

* test: add more test cases

* fix: lint fail on test case

* fix: make working with option combos

* chore: remove unnecessary comment
  • Loading branch information
lonyele committed Jan 17, 2022
1 parent 8a30108 commit d053cde
Show file tree
Hide file tree
Showing 4 changed files with 182 additions and 51 deletions.
17 changes: 15 additions & 2 deletions packages/eslint-plugin/src/rules/explicit-function-return-type.ts
Expand Up @@ -2,7 +2,8 @@ import { AST_NODE_TYPES, TSESTree } from '@typescript-eslint/utils';
import * as util from '../util';
import {
checkFunctionReturnType,
checkFunctionExpressionReturnType,
isValidFunctionExpressionReturnType,
ancestorHasReturnType,
} from '../util/explicitReturnTypeUtils';

type Options = [
Expand Down Expand Up @@ -78,7 +79,15 @@ export default util.createRule<Options, MessageIds>({
return;
}

checkFunctionExpressionReturnType(node, options, sourceCode, loc =>
if (
options.allowTypedFunctionExpressions &&
(isValidFunctionExpressionReturnType(node, options) ||
ancestorHasReturnType(node))
) {
return;
}

checkFunctionReturnType(node, options, sourceCode, loc =>
context.report({
node,
loc,
Expand All @@ -87,6 +96,10 @@ export default util.createRule<Options, MessageIds>({
);
},
FunctionDeclaration(node): void {
if (options.allowTypedFunctionExpressions && node.returnType) {
return;
}

checkFunctionReturnType(node, options, sourceCode, loc =>
context.report({
node,
Expand Down
Expand Up @@ -8,6 +8,7 @@ import {
FunctionExpression,
FunctionNode,
isTypedFunctionExpression,
ancestorHasReturnType,
} from '../util/explicitReturnTypeUtils';

type Options = [
Expand Down Expand Up @@ -387,55 +388,6 @@ export default util.createRule<Options, MessageIds>({
}
}

/**
* Check whether any ancestor of the provided function has a valid return type.
* This function assumes that the function either:
* - belongs to an exported function chain validated by isExportedHigherOrderFunction
* - is directly exported itself
*/
function ancestorHasReturnType(node: FunctionNode): boolean {
let ancestor = node.parent;

if (ancestor?.type === AST_NODE_TYPES.Property) {
ancestor = ancestor.value;
}

// if the ancestor is not a return, then this function was not returned at all, so we can exit early
const isReturnStatement =
ancestor?.type === AST_NODE_TYPES.ReturnStatement;
const isBodylessArrow =
ancestor?.type === AST_NODE_TYPES.ArrowFunctionExpression &&
ancestor.body.type !== AST_NODE_TYPES.BlockStatement;
if (!isReturnStatement && !isBodylessArrow) {
return false;
}

while (ancestor) {
switch (ancestor.type) {
case AST_NODE_TYPES.ArrowFunctionExpression:
case AST_NODE_TYPES.FunctionExpression:
case AST_NODE_TYPES.FunctionDeclaration:
if (ancestor.returnType) {
return true;
}
// assume
break;

// const x: Foo = () => {};
// Assume that a typed variable types the function expression
case AST_NODE_TYPES.VariableDeclarator:
if (ancestor.id.typeAnnotation) {
return true;
}
break;
}

ancestor = ancestor.parent;
}

return false;
}

function checkEmptyBodyFunctionExpression(
node: TSESTree.TSEmptyBodyFunctionExpression,
): void {
Expand Down
46 changes: 46 additions & 0 deletions packages/eslint-plugin/src/util/explicitReturnTypeUtils.ts
Expand Up @@ -293,11 +293,57 @@ function checkFunctionExpressionReturnType(
checkFunctionReturnType(node, options, sourceCode, report);
}

/**
* Check whether any ancestor of the provided function has a valid return type.
*/
function ancestorHasReturnType(node: FunctionNode): boolean {
let ancestor = node.parent;

if (ancestor?.type === AST_NODE_TYPES.Property) {
ancestor = ancestor.value;
}

// if the ancestor is not a return, then this function was not returned at all, so we can exit early
const isReturnStatement = ancestor?.type === AST_NODE_TYPES.ReturnStatement;
const isBodylessArrow =
ancestor?.type === AST_NODE_TYPES.ArrowFunctionExpression &&
ancestor.body.type !== AST_NODE_TYPES.BlockStatement;
if (!isReturnStatement && !isBodylessArrow) {
return false;
}

while (ancestor) {
switch (ancestor.type) {
case AST_NODE_TYPES.ArrowFunctionExpression:
case AST_NODE_TYPES.FunctionExpression:
case AST_NODE_TYPES.FunctionDeclaration:
if (ancestor.returnType) {
return true;
}
break;

// const x: Foo = () => {};
// Assume that a typed variable types the function expression
case AST_NODE_TYPES.VariableDeclarator:
if (ancestor.id.typeAnnotation) {
return true;
}
break;
}

ancestor = ancestor.parent;
}

return false;
}

export {
checkFunctionExpressionReturnType,
checkFunctionReturnType,
doesImmediatelyReturnFunctionExpression,
FunctionExpression,
FunctionNode,
isTypedFunctionExpression,
isValidFunctionExpressionReturnType,
ancestorHasReturnType,
};
Expand Up @@ -400,6 +400,68 @@ new Foo(1, () => {});
code: 'const log = (message: string) => void console.log(message);',
options: [{ allowConciseArrowFunctionExpressionsStartingWithVoid: true }],
},
{
filename: 'test.ts',
code: `
type HigherOrderType = () => (arg1: string) => (arg2: number) => string;
const x: HigherOrderType = () => arg1 => arg2 => 'foo';
`,
options: [
{
allowTypedFunctionExpressions: true,
allowHigherOrderFunctions: true,
},
],
},
{
filename: 'test.ts',
code: `
type HigherOrderType = () => (arg1: string) => (arg2: number) => string;
const x: HigherOrderType = () => arg1 => arg2 => 'foo';
`,
options: [
{
allowTypedFunctionExpressions: true,
allowHigherOrderFunctions: false,
},
],
},
{
filename: 'test.ts',
code: `
interface Foo {
foo: string;
arrowFn: () => string;
}
function foo(): Foo {
return {
foo: 'foo',
arrowFn: () => 'test',
};
}
`,
options: [
{
allowTypedFunctionExpressions: true,
allowHigherOrderFunctions: true,
},
],
},
{
filename: 'test.ts',
code: `
type Foo = (arg1: string) => string;
type Bar<T> = (arg2: string) => T;
const x: Bar<Foo> = arg1 => arg2 => arg1 + arg2;
`,
options: [
{
allowTypedFunctionExpressions: true,
allowHigherOrderFunctions: true,
},
],
},
],
invalid: [
{
Expand Down Expand Up @@ -1027,6 +1089,64 @@ foo({
{
filename: 'test.ts',
code: `
type HigherOrderType = () => (arg1: string) => (arg2: number) => string;
const x: HigherOrderType = () => arg1 => arg2 => 'foo';
`,
options: [
{
allowTypedFunctionExpressions: false,
allowHigherOrderFunctions: true,
},
],
errors: [
{
messageId: 'missingReturnType',
line: 3,
endLine: 3,
column: 42,
endColumn: 49,
},
],
},
{
filename: 'test.ts',
code: `
type HigherOrderType = () => (arg1: string) => (arg2: number) => string;
const x: HigherOrderType = () => arg1 => arg2 => 'foo';
`,
options: [
{
allowTypedFunctionExpressions: false,
allowHigherOrderFunctions: false,
},
],
errors: [
{
messageId: 'missingReturnType',
line: 3,
endLine: 3,
column: 28,
endColumn: 33,
},
{
messageId: 'missingReturnType',
line: 3,
endLine: 3,
column: 34,
endColumn: 41,
},
{
messageId: 'missingReturnType',
line: 3,
endLine: 3,
column: 42,
endColumn: 49,
},
],
},
{
filename: 'test.ts',
code: `
const func = (value: number) => ({ type: 'X', value } as any);
const func = (value: number) => ({ type: 'X', value } as Action);
`,
Expand Down

0 comments on commit d053cde

Please sign in to comment.