Skip to content

fix(eslint-plugin): [explicit-function-return-type] find ReturnStatement inside of BlockStatement #894

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.

Sign up for GitHub

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

Closed
wants to merge 7 commits into from
Closed

fix(eslint-plugin): [explicit-function-return-type] find ReturnStatement inside of BlockStatement #894

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
f6b96ce
fix: when there are multiple arguments in BlockStatement
lonyele Aug 22, 2019
82ae4aa
Merge branch 'master' into fix/finding-return-statement
lonyele Aug 22, 2019
a0fa1a6
Merge branch 'master' into fix/finding-return-statement
lonyele Aug 23, 2019
c56be2d
Merge branch 'master' into fix/finding-return-statement
lonyele Aug 31, 2019
d2ae8e0
Merge branch 'master' into fix/finding-return-statement
lonyele Sep 6, 2019
376a494
Merge branch 'master' into fix/finding-return-statement
lonyele Sep 10, 2019
708a4e6
Merge branch 'master' into fix/finding-return-statement
lonyele Sep 12, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 14 additions & 15 deletions packages/eslint-plugin/src/rules/explicit-function-return-type.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,21 +217,20 @@ export default util.createRule<Options, MessageIds>({
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 body is a block with a return statement
if (body.type === AST_NODE_TYPES.BlockStatement) {
// Check if ReturnStatement exist among body and use that argument
// for checking whether it is a function expression
body.body.some(statement => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's one problem with this code - it'll allow you to have multiple return statements nested in the code, which could have different return types.

ie - the following code will pass:

function foo() {
  if (true) {
    return 1;
  }

  return (): void => {};
}

This is where things get difficult. The best way to approach this is via maintaining state during traversal and then doing checks on the :exit.
Eg https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/src/rules/require-await.ts

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah you are definitely right, I'll make a fix(though I don't have much time right now so it'll be done probably after few weeks...)

if (
statement.type === AST_NODE_TYPES.ReturnStatement &&
!!statement.argument
) {
// If so, check that returned argument as body
body = statement.argument;
return;
}
});
}

// Check if the body being returned is a function expression
Expand Down
11 changes: 11 additions & 0 deletions packages/eslint-plugin/tests/rules/explicit-function-return-type.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,17 @@ function FunctionDeclaration() {
`,
options: [{ allowHigherOrderFunctions: true }],
},
{
filename: 'test.ts',
code: `
const x = (arg1: string) => {
const tempVar1 = 'temporary value1';
const tempVar2 = 'temporary value2';
return (arg2: number): string => 'foo';
}
`,
options: [{ allowHigherOrderFunctions: true }],
},
// https://github.com/typescript-eslint/typescript-eslint/issues/679
{
filename: 'test.ts',
Expand Down