From 9e7d1616d78a0f94521f4e6d4b48344e5df2d9f7 Mon Sep 17 00:00:00 2001 From: Brad Zacher Date: Sun, 2 Feb 2020 09:18:46 -0800 Subject: [PATCH] fix(eslint-plugin): [embt] fix allowTypedFunctionExpressions (#1553) --- .../rules/explicit-module-boundary-types.md | 65 ++++++++-- .../rules/explicit-module-boundary-types.ts | 7 +- .../src/util/explicitReturnTypeUtils.ts | 64 ++++++---- .../explicit-module-boundary-types.test.ts | 116 +++++++++--------- 4 files changed, 156 insertions(+), 96 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/explicit-module-boundary-types.md b/packages/eslint-plugin/docs/rules/explicit-module-boundary-types.md index cc7d5f1cc03..a5327a1e547 100644 --- a/packages/eslint-plugin/docs/rules/explicit-module-boundary-types.md +++ b/packages/eslint-plugin/docs/rules/explicit-module-boundary-types.md @@ -65,13 +65,26 @@ The rule accepts an options object with the following properties: ```ts type Options = { - // 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 arguments/return value directly. + */ allowTypedFunctionExpressions?: boolean; - // if true, functions immediately returning another function expression will not be checked + /** + * If true, functions immediately returning another function expression will not + * require an explicit return value annotation. + * You must still type the parameters of the function. + */ allowHigherOrderFunctions?: boolean; - // if true, body-less arrow functions are allowed to return an object as const + /** + * If true, body-less arrow functions that return an `as const` type assertion will not + * require an explicit return value annotation. + * You must still type the parameters of the function. + */ allowDirectConstAssertionInArrowFunctions?: boolean; - // an array of function/method names that will not be checked + /** + * An array of function/method names that will not have their arguments or their return values checked. + */ allowedNames?: string[]; }; @@ -118,6 +131,8 @@ export let funcExpr = function() { export let objectProp = { foo: () => 1, }; + +export const foo = bar => {}; ``` Examples of additional **correct** code for this rule with `{ allowTypedFunctionExpressions: true }`: @@ -146,6 +161,9 @@ export let objectPropAs = { export let objectPropCast = { foo: () => 1, }; + +type FooType = (bar: string) => void; +export const foo: FooType = bar => {}; ``` ### `allowHigherOrderFunctions` @@ -158,6 +176,10 @@ export var arrowFn = () => () => {}; export function fn() { return function() {}; } + +export function foo(outer) { + return function(inner): void {}; +} ``` Examples of **correct** code for this rule with `{ allowHigherOrderFunctions: true }`: @@ -168,17 +190,15 @@ export var arrowFn = () => (): void => {}; export function fn() { return function(): void {}; } + +export function foo(outer: string) { + return function(inner: string): void {}; +} ``` ### `allowDirectConstAssertionInArrowFunctions` -Examples of additional **correct** code for this rule with `{ allowDirectConstAssertionInArrowFunctions: true }`: - -```ts -export const func = (value: number) => ({ type: 'X', value } as const); -``` - -Examples of additional **incorrect** code for this rule with `{ allowDirectConstAssertionInArrowFunctions: true }`: +Examples of **incorrect** code for this rule with `{ allowDirectConstAssertionInArrowFunctions: true }`: ```ts export const func = (value: number) => ({ type: 'X', value }); @@ -187,15 +207,34 @@ export const foo = () => { bar: true, } as const; }; +export const bar = () => 1; +export const baz = arg => arg as const; +``` + +Examples of **correct** code for this rule with `{ allowDirectConstAssertionInArrowFunctions: true }`: + +```ts +export const func = (value: number) => ({ type: 'X', value } as const); +export const foo = () => + ({ + bar: true, + } as const); +export const bar = () => 1 as const; +export const baz = (arg: string) => arg as const; ``` ### `allowedNames` You may pass function/method names you would like this rule to ignore, like so: -```cjson +```json { - "@typescript-eslint/explicit-module-boundary-types": ["error", { "allowedName": ["ignoredFunctionName", "ignoredMethodName"] }] + "@typescript-eslint/explicit-module-boundary-types": [ + "error", + { + "allowedName": ["ignoredFunctionName", "ignoredMethodName"] + } + ] } ``` 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 0bd872622f1..f003977d8fb 100644 --- a/packages/eslint-plugin/src/rules/explicit-module-boundary-types.ts +++ b/packages/eslint-plugin/src/rules/explicit-module-boundary-types.ts @@ -6,6 +6,7 @@ import * as util from '../util'; import { checkFunctionExpressionReturnType, checkFunctionReturnType, + isTypedFunctionExpression, } from '../util/explicitReturnTypeUtils'; type Options = [ @@ -178,7 +179,11 @@ export default util.createRule({ return; } - if (isAllowedName(node.parent) || isUnexported(node)) { + if ( + isAllowedName(node.parent) || + isUnexported(node) || + isTypedFunctionExpression(node, options) + ) { return; } diff --git a/packages/eslint-plugin/src/util/explicitReturnTypeUtils.ts b/packages/eslint-plugin/src/util/explicitReturnTypeUtils.ts index 81d43590eab..03a2260289e 100644 --- a/packages/eslint-plugin/src/util/explicitReturnTypeUtils.ts +++ b/packages/eslint-plugin/src/util/explicitReturnTypeUtils.ts @@ -5,6 +5,7 @@ import { AST_TOKEN_TYPES, } from '@typescript-eslint/experimental-utils'; import { isTypeAssertion, isConstructor, isSetter } from './astUtils'; +import { nullThrows, NullThrowsReasons } from './nullThrows'; type FunctionNode = | TSESTree.ArrowFunctionExpression @@ -264,6 +265,26 @@ function checkFunctionReturnType( report(getReporLoc(node, sourceCode)); } +function isTypedFunctionExpression( + node: TSESTree.ArrowFunctionExpression | TSESTree.FunctionExpression, + options: Options, +): boolean { + const parent = nullThrows(node.parent, NullThrowsReasons.MissingParent); + + if (!options.allowTypedFunctionExpressions) { + return false; + } + + return ( + isTypeAssertion(parent) || + isVariableDeclaratorWithTypeAnnotation(parent) || + isClassPropertyWithTypeAnnotation(parent) || + isPropertyOfObjectWithType(parent) || + isFunctionArgument(parent, node) || + isConstructorArgument(parent) + ); +} + /** * Checks if a function declaration/expression has a return type. */ @@ -273,36 +294,25 @@ function checkFunctionExpressionReturnType( 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 (isTypedFunctionExpression(node, options)) { + 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; - } + const parent = nullThrows(node.parent, NullThrowsReasons.MissingParent); + if ( + options.allowExpressions && + parent.type !== AST_NODE_TYPES.VariableDeclarator && + parent.type !== AST_NODE_TYPES.MethodDefinition && + parent.type !== AST_NODE_TYPES.ExportDefaultDeclaration && + 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 && + node.type === AST_NODE_TYPES.ArrowFunctionExpression && returnsConstAssertionDirectly(node) ) { return; @@ -311,4 +321,8 @@ function checkFunctionExpressionReturnType( checkFunctionReturnType(node, options, sourceCode, report); } -export { checkFunctionReturnType, checkFunctionExpressionReturnType }; +export { + checkFunctionReturnType, + checkFunctionExpressionReturnType, + isTypedFunctionExpression, +}; 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 fc2413ba011..20a9f14a57e 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 @@ -8,7 +8,6 @@ const ruleTester = new RuleTester({ ruleTester.run('explicit-module-boundary-types', rule, { valid: [ { - filename: 'test.ts', code: ` function test(): void { return; @@ -16,7 +15,6 @@ function test(): void { `, }, { - filename: 'test.ts', code: ` export function test(): void { return; @@ -24,7 +22,6 @@ export function test(): void { `, }, { - filename: 'test.ts', code: ` export var fn = function(): number { return 1; @@ -32,13 +29,11 @@ export var fn = function(): number { `, }, { - filename: 'test.ts', code: ` export var arrowFn = (): string => 'test'; `, }, { - filename: 'test.ts', code: ` class Test { constructor() {} @@ -54,7 +49,6 @@ class Test { `, }, { - filename: 'test.ts', code: ` export class Test { constructor() {} @@ -70,7 +64,6 @@ export class Test { `, }, { - filename: 'test.ts', code: ` export function test(): void { nested(); @@ -81,7 +74,6 @@ export function test(): void { `, }, { - filename: 'test.ts', code: ` export function test(): string { const nested = () => 'value'; @@ -90,7 +82,6 @@ export function test(): string { `, }, { - filename: 'test.ts', code: ` export function test(): string { class Nested { @@ -103,7 +94,6 @@ export function test(): string { `, }, { - filename: 'test.ts', code: ` export var arrowFn: Foo = () => 'test'; `, @@ -114,7 +104,6 @@ export var arrowFn: Foo = () => 'test'; ], }, { - filename: 'test.ts', code: ` export var funcExpr: Foo = function() { return 'test'; }; `, @@ -125,17 +114,14 @@ export var funcExpr: Foo = function() { return 'test'; }; ], }, { - filename: 'test.ts', code: `const x = (() => {}) as Foo`, options: [{ allowTypedFunctionExpressions: true }], }, { - filename: 'test.ts', code: `const x = (() => {})`, options: [{ allowTypedFunctionExpressions: true }], }, { - filename: 'test.ts', code: ` export const x = { foo: () => {}, @@ -144,7 +130,6 @@ export const x = { options: [{ allowTypedFunctionExpressions: true }], }, { - filename: 'test.ts', code: ` export const x = { foo: () => {}, @@ -153,7 +138,6 @@ export const x = { options: [{ allowTypedFunctionExpressions: true }], }, { - filename: 'test.ts', code: ` export const x: Foo = { foo: () => {}, @@ -163,7 +147,6 @@ export const x: Foo = { }, // https://github.com/typescript-eslint/typescript-eslint/issues/484 { - filename: 'test.ts', code: ` type MethodType = () => void; @@ -175,7 +158,6 @@ export class App { }, // https://github.com/typescript-eslint/typescript-eslint/issues/525 { - filename: 'test.ts', code: ` export const myObj = { set myProp(val: number) { @@ -185,49 +167,42 @@ export const myObj = { `, }, { - filename: 'test.ts', code: ` export default () => (): void => {}; `, options: [{ allowHigherOrderFunctions: true }], }, { - filename: 'test.ts', code: ` export default () => function (): void {}; `, options: [{ allowHigherOrderFunctions: true }], }, { - filename: 'test.ts', code: ` export default () => { return (): void => {} }; `, options: [{ allowHigherOrderFunctions: true }], }, { - filename: 'test.ts', code: ` export default () => { return function (): void {} }; `, options: [{ allowHigherOrderFunctions: true }], }, { - filename: 'test.ts', code: ` export function fn() { return (): void => {} }; `, options: [{ allowHigherOrderFunctions: true }], }, { - filename: 'test.ts', code: ` export function fn() { return function (): void {} }; `, options: [{ allowHigherOrderFunctions: true }], }, { - filename: 'test.ts', code: ` export function FunctionDeclaration() { return function FunctionExpression_Within_FunctionDeclaration() { @@ -243,14 +218,12 @@ export function FunctionDeclaration() { options: [{ allowHigherOrderFunctions: true }], }, { - filename: 'test.ts', code: ` export default () => () => { return (): void => { return; } }; `, options: [{ allowHigherOrderFunctions: true }], }, { - filename: 'test.ts', code: ` export class Accumulator { private count: number = 0; @@ -269,7 +242,6 @@ new Accumulator().accumulate(() => 1); ], }, { - filename: 'test.ts', code: ` export const func1 = (value: number) => (({ type: "X", value }) as const); export const func2 = (value: number) => ({ type: "X", value } as const); @@ -283,7 +255,6 @@ export const func4 = (value: number) => x as const; ], }, { - filename: 'test.ts', code: ` export const func1 = (value: string) => value; export const func2 = (value: number) => ({ type: "X", value }); @@ -295,7 +266,6 @@ export const func2 = (value: number) => ({ type: "X", value }); ], }, { - filename: 'test.ts', code: ` export class Test { constructor() {} @@ -315,10 +285,36 @@ export class Test { }, ], }, + { + code: ` + export function foo(outer: string) { + return function(inner: string): void {}; + } + `, + options: [ + { + allowHigherOrderFunctions: true, + }, + ], + }, + // https://github.com/typescript-eslint/typescript-eslint/issues/1552 + { + code: ` + export type Ensurer = (blocks: TFBlock[]) => TFBlock[]; + + export const myEnsurer: Ensurer = blocks => { + return blocks; + }; + `, + options: [ + { + allowTypedFunctionExpressions: true, + }, + ], + }, ], invalid: [ { - filename: 'test.ts', code: ` export function test( a: number, @@ -338,7 +334,6 @@ export function test( ], }, { - filename: 'test.ts', code: ` export function test() { return; @@ -355,7 +350,6 @@ export function test() { ], }, { - filename: 'test.ts', code: ` export var fn = function() { return 1; @@ -372,7 +366,6 @@ export var fn = function() { ], }, { - filename: 'test.ts', code: ` export var arrowFn = () => 'test'; `, @@ -387,7 +380,6 @@ export var arrowFn = () => 'test'; ], }, { - filename: 'test.ts', code: ` export class Test { constructor() {} @@ -443,7 +435,6 @@ export class Test { ], }, { - filename: 'test.ts', code: ` export class Foo { public a = () => {}; @@ -493,7 +484,6 @@ export class Foo { ], }, { - filename: 'test.ts', code: 'export default () => true ? (() => {}) : ((): void => {});', errors: [ { @@ -513,7 +503,6 @@ export class Foo { ], }, { - filename: 'test.ts', code: "export var arrowFn = () => 'test';", options: [{ allowTypedFunctionExpressions: true }], errors: [ @@ -527,7 +516,6 @@ export class Foo { ], }, { - filename: 'test.ts', code: "export var funcExpr = function() { return 'test'; };", options: [{ allowTypedFunctionExpressions: true }], errors: [ @@ -541,7 +529,6 @@ export class Foo { ], }, { - filename: 'test.ts', code: 'export const x = (() => {}) as Foo', options: [{ allowTypedFunctionExpressions: false }], errors: [ @@ -555,7 +542,6 @@ export class Foo { ], }, { - filename: 'test.ts', code: ` interface Foo {} export const x = { @@ -574,7 +560,6 @@ export const x = { ], }, { - filename: 'test.ts', code: ` interface Foo {} export const x: Foo = { @@ -593,7 +578,6 @@ export const x: Foo = { ], }, { - filename: 'test.ts', code: 'export default () => () => {};', options: [{ allowHigherOrderFunctions: true }], errors: [ @@ -607,7 +591,6 @@ export const x: Foo = { ], }, { - filename: 'test.ts', code: 'export default () => function () {};', options: [{ allowHigherOrderFunctions: true }], errors: [ @@ -621,7 +604,6 @@ export const x: Foo = { ], }, { - filename: 'test.ts', code: 'export default () => { return () => {} };', options: [{ allowHigherOrderFunctions: true }], errors: [ @@ -635,7 +617,6 @@ export const x: Foo = { ], }, { - filename: 'test.ts', code: 'export default () => { return function () {} };', options: [{ allowHigherOrderFunctions: true }], errors: [ @@ -649,7 +630,6 @@ export const x: Foo = { ], }, { - filename: 'test.ts', code: 'export function fn() { return () => {} };', options: [{ allowHigherOrderFunctions: true }], errors: [ @@ -663,7 +643,6 @@ export const x: Foo = { ], }, { - filename: 'test.ts', code: 'export function fn() { return function () {} };', options: [{ allowHigherOrderFunctions: true }], errors: [ @@ -677,7 +656,6 @@ export const x: Foo = { ], }, { - filename: 'test.ts', code: ` export function FunctionDeclaration() { return function FunctionExpression_Within_FunctionDeclaration() { @@ -702,7 +680,6 @@ export function FunctionDeclaration() { ], }, { - filename: 'test.ts', code: 'export default () => () => { return () => { return; } };', options: [{ allowHigherOrderFunctions: true }], errors: [ @@ -716,7 +693,6 @@ export function FunctionDeclaration() { ], }, { - filename: 'test.ts', code: 'export default (() => true)()', options: [ { @@ -734,7 +710,6 @@ export function FunctionDeclaration() { ], }, { - filename: 'test.ts', code: ` export const func1 = (value: number) => ({ type: "X", value } as any); export const func2 = (value: number) => ({ type: "X", value } as Action); @@ -762,7 +737,6 @@ export const func2 = (value: number) => ({ type: "X", value } as Action); ], }, { - filename: 'test.ts', code: ` export const func = (value: number) => ({ type: "X", value } as const); `, @@ -782,7 +756,6 @@ export const func = (value: number) => ({ type: "X", value } as const); ], }, { - filename: 'test.ts', code: ` export class Test { constructor() {} @@ -812,7 +785,6 @@ export class Test { ], }, { - filename: 'test.ts', code: ` export const func1 = (value: number) => value; export const func2 = (value: number) => value; @@ -833,7 +805,6 @@ export const func2 = (value: number) => value; ], }, { - filename: 'test.ts', code: 'export function fn(test): string { return "123" };', errors: [ { @@ -846,7 +817,6 @@ export const func2 = (value: number) => value; ], }, { - filename: 'test.ts', code: 'export const fn = (one: number, two): string => "123";', errors: [ { @@ -858,5 +828,37 @@ export const func2 = (value: number) => value; }, ], }, + { + code: ` + export function foo(outer) { + return function(inner) {}; + } + `, + options: [{ allowHigherOrderFunctions: true }], + errors: [ + { + messageId: 'missingArgType', + line: 2, + }, + { + messageId: 'missingReturnType', + line: 3, + }, + { + messageId: 'missingArgType', + line: 3, + }, + ], + }, + { + code: 'export const baz = arg => arg as const;', + options: [{ allowDirectConstAssertionInArrowFunctions: true }], + errors: [ + { + messageId: 'missingArgType', + line: 1, + }, + ], + }, ], });