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

feat(eslint-plugin): [explicit-function-return-type] allowHigherOrderFunctions (#193) #538

Merged
merged 17 commits into from Jun 3, 2019
Merged
Show file tree
Hide file tree
Changes from 14 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
33 changes: 31 additions & 2 deletions packages/eslint-plugin/docs/rules/explicit-function-return-type.md
Expand Up @@ -65,13 +65,16 @@ The rule accepts an options object with the following properties:
type Options = {
// if true, only functions which are part of a declaration will be checked
allowExpressions?: boolean;
// if true, type annotations are also allowed on the variable of a function expression rather than on the function directly.
// if true, type annotations are also allowed on the variable of a function expression rather than on the function directly
allowTypedFunctionExpressions?: boolean;
// if true, functions immediately returning another function expression will not be checked
allowHigherOrderFunctions?: boolean;
};

const defaults = {
allowExpressions: false,
allowTypedFunctionExpressions: false,
allowHigherOrderFunctions: false,
};
```

Expand Down Expand Up @@ -121,7 +124,7 @@ let funcExpr: FuncType = function() {
};

let asTyped = (() => '') as () => string;
let caasTyped = <() => string>(() => '');
let castTyped = <() => string>(() => '');

interface ObjectType {
foo(): number;
Expand All @@ -137,6 +140,32 @@ let objectPropCast = <ObjectType>{
};
```

### allowHigherOrderFunctions

Examples of **incorrect** code for this rule with `{ allowHigherOrderFunctions: true }`:

```ts
var arrowFn = (x: number) => (y: number) => x + y;

function fn(x: number) {
return function(y: number) {
return x + y;
};
}
```

Examples of **correct** code for this rule with `{ allowHigherOrderFunctions: true }`:

```ts
var arrowFn = (x: number) => (y: number): number => x + y;

function fn(x: number) {
return function(y: number): number {
return x + y;
};
}
```

## When Not To Use It

If you don't wish to prevent calling code from using function return values in unexpected ways, then
Expand Down
Expand Up @@ -8,6 +8,7 @@ type Options = [
{
allowExpressions?: boolean;
allowTypedFunctionExpressions?: boolean;
allowHigherOrderFunctions?: boolean;
}
];
type MessageIds = 'missingReturnType';
Expand Down Expand Up @@ -35,6 +36,9 @@ export default util.createRule<Options, MessageIds>({
allowTypedFunctionExpressions: {
type: 'boolean',
},
allowHigherOrderFunctions: {
type: 'boolean',
},
},
additionalProperties: false,
},
Expand All @@ -44,6 +48,7 @@ export default util.createRule<Options, MessageIds>({
{
allowExpressions: false,
allowTypedFunctionExpressions: false,
allowHigherOrderFunctions: false,
},
],
create(context, [options]) {
Expand Down Expand Up @@ -138,6 +143,50 @@ export default util.createRule<Options, MessageIds>({
);
}

/**
* 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 function declaration/expression has a return type.
*/
Expand All @@ -147,6 +196,13 @@ export default util.createRule<Options, MessageIds>({
| TSESTree.FunctionDeclaration
| TSESTree.FunctionExpression,
): void {
if (
options.allowHigherOrderFunctions &&
doesImmediatelyReturnFunctionExpression(node)
) {
return;
}

if (
node.returnType ||
isConstructor(node.parent) ||
Expand All @@ -169,7 +225,8 @@ export default util.createRule<Options, MessageIds>({
function checkFunctionExpressionReturnType(
node: TSESTree.ArrowFunctionExpression | TSESTree.FunctionExpression,
): void {
if (node.parent) {
// Should always have a parent; checking just in case
/* istanbul ignore else */ if (node.parent) {
if (options.allowTypedFunctionExpressions) {
if (
isTypeCast(node.parent) ||
Expand Down
Expand Up @@ -180,6 +180,71 @@ const myObj = {
};
`,
},
{
filename: 'test.ts',
code: `
() => (): void => {};
`,
options: [{ allowHigherOrderFunctions: true }],
},
{
filename: 'test.ts',
code: `
() => function (): void {};
`,
options: [{ allowHigherOrderFunctions: true }],
},
{
filename: 'test.ts',
code: `
() => { return (): void => {} };
`,
options: [{ allowHigherOrderFunctions: true }],
},
{
filename: 'test.ts',
code: `
() => { return function (): void {} };
`,
options: [{ allowHigherOrderFunctions: true }],
},
{
filename: 'test.ts',
code: `
function fn() { return (): void => {} };
`,
options: [{ allowHigherOrderFunctions: true }],
},
{
filename: 'test.ts',
code: `
function fn() { return function (): void {} };
`,
options: [{ allowHigherOrderFunctions: true }],
},
{
filename: 'test.ts',
code: `
function FunctionDeclaration() {
return function FunctionExpression_Within_FunctionDeclaration() {
return function FunctionExpression_Within_FunctionExpression() {
return () => { // ArrowFunctionExpression_Within_FunctionExpression
return () => // ArrowFunctionExpression_Within_ArrowFunctionExpression
(): number => 1 // ArrowFunctionExpression_Within_ArrowFunctionExpression_WithNoBody
}
}
}
}
`,
options: [{ allowHigherOrderFunctions: true }],
},
{
filename: 'test.ts',
code: `
() => () => { return (): void => { return; } };
`,
options: [{ allowHigherOrderFunctions: true }],
},
],
invalid: [
{
Expand Down Expand Up @@ -364,5 +429,126 @@ const x: Foo = {
},
],
},
{
filename: 'test.ts',
code: `
() => () => {};
`,
options: [{ allowHigherOrderFunctions: true }],
errors: [
{
messageId: 'missingReturnType',
line: 2,
column: 7,
},
],
},
{
filename: 'test.ts',
code: `
() => function () {};
`,
options: [{ allowHigherOrderFunctions: true }],
errors: [
{
messageId: 'missingReturnType',
line: 2,
column: 7,
},
],
},
{
filename: 'test.ts',
code: `
() => { return () => {} };
`,
options: [{ allowHigherOrderFunctions: true }],
errors: [
{
messageId: 'missingReturnType',
line: 2,
column: 16,
},
],
},
{
filename: 'test.ts',
code: `
() => { return function () {} };
`,
options: [{ allowHigherOrderFunctions: true }],
errors: [
{
messageId: 'missingReturnType',
line: 2,
column: 16,
},
],
},
{
filename: 'test.ts',
code: `
function fn() { return () => {} };
`,
options: [{ allowHigherOrderFunctions: true }],
errors: [
{
messageId: 'missingReturnType',
line: 2,
column: 24,
},
],
},
{
filename: 'test.ts',
code: `
function fn() { return function () {} };
`,
options: [{ allowHigherOrderFunctions: true }],
errors: [
{
messageId: 'missingReturnType',
line: 2,
column: 24,
},
],
},
{
filename: 'test.ts',
code: `
function FunctionDeclaration() {
return function FunctionExpression_Within_FunctionDeclaration() {
return function FunctionExpression_Within_FunctionExpression() {
return () => { // ArrowFunctionExpression_Within_FunctionExpression
return () => // ArrowFunctionExpression_Within_ArrowFunctionExpression
() => 1 // ArrowFunctionExpression_Within_ArrowFunctionExpression_WithNoBody
}
}
}
}
`,
options: [{ allowHigherOrderFunctions: true }],
errors: [
{
messageId: 'missingReturnType',
line: 7,
column: 11,
},
],
},
{
filename: 'test.ts',
code: `
() => () => { return () => { return; } };
`,
options: [{ allowHigherOrderFunctions: true }],
errors: [
{
messageId: 'missingReturnType',
line: 2,
column: 22,
},
],
},
],
});