Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(eslint-plugin): [explicit-module-boundary-types] don't check returned functions if parent function has return type #2084

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 15 additions & 3 deletions packages/eslint-plugin/src/rules/explicit-module-boundary-types.ts
Expand Up @@ -8,6 +8,7 @@ import {
checkFunctionExpressionReturnType,
checkFunctionReturnType,
isTypedFunctionExpression,
ancestorHasReturnType,
} from '../util/explicitReturnTypeUtils';

type Options = [
Expand Down Expand Up @@ -74,6 +75,10 @@ export default util.createRule<Options, MessageIds>({
create(context, [options]) {
const sourceCode = context.getSourceCode();

/**
* Steps up the nodes parents to check if any ancestor nodes are export
* declarations.
*/
function isUnexported(node: TSESTree.Node | undefined): boolean {
let isReturnedValue = false;
while (node) {
Expand Down Expand Up @@ -111,6 +116,9 @@ export default util.createRule<Options, MessageIds>({
return true;
}

/**
* Returns true when the argument node is not typed.
*/
function isArgumentUntyped(node: TSESTree.Identifier): boolean {
return (
!node.typeAnnotation ||
Expand Down Expand Up @@ -265,7 +273,7 @@ export default util.createRule<Options, MessageIds>({
}

/**
* Checks if a function expression follow the rule
* Checks if a function expression follow the rule.
*/
function checkFunctionExpression(
node: TSESTree.ArrowFunctionExpression | TSESTree.FunctionExpression,
Expand All @@ -280,7 +288,8 @@ export default util.createRule<Options, MessageIds>({

if (
isAllowedName(node.parent) ||
isTypedFunctionExpression(node, options)
isTypedFunctionExpression(node, options) ||
ancestorHasReturnType(node.parent, options)
) {
return;
}
Expand All @@ -300,7 +309,10 @@ export default util.createRule<Options, MessageIds>({
* Checks if a function follow the rule
*/
function checkFunction(node: TSESTree.FunctionDeclaration): void {
if (isAllowedName(node.parent)) {
if (
isAllowedName(node.parent) ||
ancestorHasReturnType(node.parent, options)
) {
return;
}

Expand Down
136 changes: 105 additions & 31 deletions packages/eslint-plugin/src/util/explicitReturnTypeUtils.ts
Expand Up @@ -240,31 +240,8 @@ interface Options {
}

/**
* Checks if a function declaration/expression has a return type.
* True when the provided function expression is typed.
*/
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));
}

function isTypedFunctionExpression(
node: TSESTree.ArrowFunctionExpression | TSESTree.FunctionExpression,
options: Options,
Expand All @@ -286,16 +263,15 @@ function isTypedFunctionExpression(
}

/**
* Checks if a function declaration/expression has a return type.
* Check whether the function expression return type is either typed or valid
* with the provided options.
*/
function checkFunctionExpressionReturnType(
function isValidFunctionExpressionReturnType(
node: TSESTree.ArrowFunctionExpression | TSESTree.FunctionExpression,
options: Options,
sourceCode: TSESLint.SourceCode,
report: (loc: TSESTree.SourceLocation) => void,
): void {
): boolean {
if (isTypedFunctionExpression(node, options)) {
return;
return true;
}

const parent = nullThrows(node.parent, NullThrowsReasons.MissingParent);
Expand All @@ -306,7 +282,7 @@ function checkFunctionExpressionReturnType(
parent.type !== AST_NODE_TYPES.ExportDefaultDeclaration &&
parent.type !== AST_NODE_TYPES.ClassProperty
) {
return;
return true;
}

// https://github.com/typescript-eslint/typescript-eslint/issues/653
Expand All @@ -315,14 +291,112 @@ function checkFunctionExpressionReturnType(
node.type === AST_NODE_TYPES.ArrowFunctionExpression &&
returnsConstAssertionDirectly(node)
) {
return true;
}

return false;
}

/**
* Check that the function expression or declaration is valid.
*/
function isValidFunctionReturnType(
node:
| TSESTree.ArrowFunctionExpression
| TSESTree.FunctionDeclaration
| TSESTree.FunctionExpression,
options: Options,
isParentCheck = false,
): boolean {
if (
!isParentCheck &&
options.allowHigherOrderFunctions &&
doesImmediatelyReturnFunctionExpression(node)
) {
return true;
}

if (node.returnType || isConstructor(node.parent) || isSetter(node.parent)) {
return true;
}

return false;
}

/**
* 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 (isValidFunctionReturnType(node, options)) {
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 {
if (isValidFunctionExpressionReturnType(node, options)) {
return;
}

checkFunctionReturnType(node, options, sourceCode, report);
}

/**
* Check whether any ancestor of the provided node has a valid return type, with
* the given options.
*/
function ancestorHasReturnType(
ancestor: TSESTree.Node | undefined,
options: Options,
): boolean {
// Exit early if this ancestor is not a ReturnStatement.
if (ancestor?.type !== AST_NODE_TYPES.ReturnStatement) {
return false;
}

// This boolean tells the `isValidFunctionReturnType` that it is being called
// by an ancestor check.
const isParentCheck = true;

while (ancestor) {
switch (ancestor.type) {
case AST_NODE_TYPES.ArrowFunctionExpression:
case AST_NODE_TYPES.FunctionExpression:
return (
isValidFunctionExpressionReturnType(ancestor, options) ||
isValidFunctionReturnType(ancestor, options, isParentCheck)
);
case AST_NODE_TYPES.FunctionDeclaration:
return isValidFunctionReturnType(ancestor, options, isParentCheck);
}

ancestor = ancestor.parent;
}

/* istanbul ignore next */
return false;
}

export {
checkFunctionReturnType,
checkFunctionExpressionReturnType,
isTypedFunctionExpression,
ancestorHasReturnType,
};
Expand Up @@ -437,6 +437,89 @@ export default { Foo };
`,
options: [{ shouldTrackReferences: true }],
},
{
code: `
export function foo(): (n: number) => string {
return n => String(n);
}
`,
},
{
code: `
export const foo = (a: string): ((n: number) => string) => {
return function (n) {
return String(n);
};
};
`,
},
{
code: `
export function a(): void {
function b() {}
const x = () => {};
(function () {});

function c() {
return () => {};
}

return;
}
`,
},
{
code: `
export function a(): void {
function b() {
function c() {}
}
const x = () => {
return () => 100;
};
(function () {
(function () {});
});

function c() {
return () => {
(function () {});
};
}

return;
}
`,
},
{
code: `
export function a() {
return function b(): () => void {
return function c() {};
};
}
`,
options: [{ allowHigherOrderFunctions: true }],
},
{
code: `
export var arrowFn = () => (): void => {};
`,
},
{
code: `
export function fn() {
return function (): void {};
}
`,
},
{
code: `
export function foo(outer: string) {
return function (inner: string): void {};
}
`,
},
],
invalid: [
{
Expand Down Expand Up @@ -1280,5 +1363,70 @@ export default test;
},
],
},
{
code: `
export const foo = () => (a: string): ((n: number) => string) => {
return function (n) {
return String(n);
};
};
`,
options: [{ allowHigherOrderFunctions: false }],
errors: [
{
messageId: 'missingReturnType',
line: 2,
column: 20,
},
],
},
{
code: `
export var arrowFn = () => () => {};
`,
options: [{ allowHigherOrderFunctions: true }],
errors: [
{
messageId: 'missingReturnType',
line: 2,
column: 28,
},
],
},
{
code: `
export function fn() {
return function () {};
}
`,
options: [{ allowHigherOrderFunctions: true }],
errors: [
{
messageId: 'missingReturnType',
line: 3,
column: 10,
},
],
},
{
code: `
export function foo(outer) {
return function (inner): void {};
}
`,
options: [{ allowHigherOrderFunctions: true }],
errors: [
{
messageId: 'missingArgType',
line: 2,
column: 8,
},
{
messageId: 'missingArgType',
line: 3,
column: 10,
},
],
},
],
});