diff --git a/packages/eslint-plugin/docs/rules/prefer-function-type.md b/packages/eslint-plugin/docs/rules/prefer-function-type.md index a3210a133a6..5c3bc55c9f2 100644 --- a/packages/eslint-plugin/docs/rules/prefer-function-type.md +++ b/packages/eslint-plugin/docs/rules/prefer-function-type.md @@ -24,6 +24,13 @@ interface Foo extends Function { } ``` +```ts +interface MixinMethod { + // returns the function itself, not the `this` argument. + (arg: string): this; +} +``` + Examples of **correct** code for this rule: ```ts @@ -48,6 +55,23 @@ interface Bar extends Foo { } ``` +```ts +// returns the `this` argument of function, retaining it's type. +type MixinMethod = (this: TSelf, arg: string) => TSelf; +// a function that returns itself is much clearer in this form. +type ReturnsSelf = (arg: string) => ReturnsSelf; +``` + +```ts +// multiple call signatures (overloads) is allowed: +interface Overloaded { + (data: string): number; + (id: number): string; +} +// this is equivelent to Overloaded interface. +type Intersection = ((data: string) => number) & ((id: number) => string); +``` + ## When Not To Use It If you specifically want to use an interface or type literal with a single call signature for stylistic reasons, you can disable this rule. diff --git a/packages/eslint-plugin/src/rules/prefer-function-type.ts b/packages/eslint-plugin/src/rules/prefer-function-type.ts index dae4fae3295..1d8c2701210 100644 --- a/packages/eslint-plugin/src/rules/prefer-function-type.ts +++ b/packages/eslint-plugin/src/rules/prefer-function-type.ts @@ -5,6 +5,11 @@ import { } from '@typescript-eslint/experimental-utils'; import * as util from '../util'; +export const phrases = { + [AST_NODE_TYPES.TSTypeLiteral]: 'Type literal', + [AST_NODE_TYPES.TSInterfaceDeclaration]: 'Interface', +} as const; + export default util.createRule({ name: 'prefer-function-type', meta: { @@ -17,7 +22,9 @@ export default util.createRule({ fixable: 'code', messages: { functionTypeOverCallableType: - "{{ type }} has only a call signature - use '{{ sigSuggestion }}' instead.", + '{{ literalOrInterface }} only has a call signature, you should use a function type instead.', + unexpectedThisOnFunctionOnlyInterface: + "`this` refers to the function type '{{ interfaceName }}', did you intend to use a generic `this` parameter like `(this: Self, ...) => Self` instead?", }, schema: [], type: 'suggestion', @@ -104,13 +111,30 @@ export default util.createRule({ */ function checkMember( member: TSESTree.TypeElement, - node: TSESTree.Node, + node: TSESTree.TSInterfaceDeclaration | TSESTree.TSTypeLiteral, + tsThisTypes: TSESTree.TSThisType[] | null = null, ): void { if ( (member.type === AST_NODE_TYPES.TSCallSignatureDeclaration || member.type === AST_NODE_TYPES.TSConstructSignatureDeclaration) && typeof member.returnType !== 'undefined' ) { + if ( + tsThisTypes !== null && + tsThisTypes.length > 0 && + node.type === AST_NODE_TYPES.TSInterfaceDeclaration + ) { + // the message can be confusing if we don't point directly to the `this` node instead of the whole member + // and in favour of generating at most one error we'll only report the first occurrence of `this` if there are multiple + context.report({ + node: tsThisTypes[0], + messageId: 'unexpectedThisOnFunctionOnlyInterface', + data: { + interfaceName: node.id.name, + }, + }); + return; + } const suggestion = renderSuggestion(member, node); const fixStart = node.type === AST_NODE_TYPES.TSTypeLiteral @@ -127,11 +151,7 @@ export default util.createRule({ node: member, messageId: 'functionTypeOverCallableType', data: { - type: - node.type === AST_NODE_TYPES.TSTypeLiteral - ? 'Type literal' - : 'Interface', - sigSuggestion: suggestion, + literalOrInterface: phrases[node.type], }, fix(fixer) { return fixer.replaceTextRange( @@ -142,12 +162,36 @@ export default util.createRule({ }); } } - + let tsThisTypes: TSESTree.TSThisType[] | null = null; + let literalNesting = 0; return { - TSInterfaceDeclaration(node): void { + TSInterfaceDeclaration(): void { + // when entering an interface reset the count of `this`s to empty. + tsThisTypes = []; + }, + 'TSInterfaceDeclaration TSThisType'(node: TSESTree.TSThisType): void { + // inside an interface keep track of all ThisType references. + // unless it's inside a nested type literal in which case it's invalid code anyway + // we don't want to incorrectly say "it refers to name" while typescript says it's completely invalid. + if (literalNesting === 0 && tsThisTypes !== null) { + tsThisTypes.push(node); + } + }, + 'TSInterfaceDeclaration:exit'( + node: TSESTree.TSInterfaceDeclaration, + ): void { if (!hasOneSupertype(node) && node.body.body.length === 1) { - checkMember(node.body.body[0], node); + checkMember(node.body.body[0], node, tsThisTypes); } + // on exit check member and reset the array to nothing. + tsThisTypes = null; + }, + // keep track of nested literals to avoid complaining about invalid `this` uses + 'TSInterfaceDeclaration TSTypeLiteral'(): void { + literalNesting += 1; + }, + 'TSInterfaceDeclaration TSTypeLiteral:exit'(): void { + literalNesting -= 1; }, 'TSTypeLiteral[members.length = 1]'(node: TSESTree.TSTypeLiteral): void { checkMember(node.members[0], node); diff --git a/packages/eslint-plugin/tests/rules/prefer-function-type.test.ts b/packages/eslint-plugin/tests/rules/prefer-function-type.test.ts index 257bd9e20d8..d294041fbaf 100644 --- a/packages/eslint-plugin/tests/rules/prefer-function-type.test.ts +++ b/packages/eslint-plugin/tests/rules/prefer-function-type.test.ts @@ -1,6 +1,6 @@ import { AST_NODE_TYPES } from '@typescript-eslint/experimental-utils'; -import rule from '../../src/rules/prefer-function-type'; -import { RuleTester } from '../RuleTester'; +import rule, { phrases } from '../../src/rules/prefer-function-type'; +import { noFormat, RuleTester } from '../RuleTester'; const ruleTester = new RuleTester({ parserOptions: { @@ -56,6 +56,9 @@ interface Foo { { messageId: 'functionTypeOverCallableType', type: AST_NODE_TYPES.TSCallSignatureDeclaration, + data: { + literalOrInterface: phrases[AST_NODE_TYPES.TSInterfaceDeclaration], + }, }, ], output: ` @@ -72,6 +75,9 @@ type Foo = { { messageId: 'functionTypeOverCallableType', type: AST_NODE_TYPES.TSCallSignatureDeclaration, + data: { + literalOrInterface: phrases[AST_NODE_TYPES.TSTypeLiteral], + }, }, ], output: ` @@ -88,6 +94,9 @@ function foo(bar: { (s: string): number }): number { { messageId: 'functionTypeOverCallableType', type: AST_NODE_TYPES.TSCallSignatureDeclaration, + data: { + literalOrInterface: phrases[AST_NODE_TYPES.TSTypeLiteral], + }, }, ], output: ` @@ -106,6 +115,9 @@ function foo(bar: { (s: string): number } | undefined): number { { messageId: 'functionTypeOverCallableType', type: AST_NODE_TYPES.TSCallSignatureDeclaration, + data: { + literalOrInterface: phrases[AST_NODE_TYPES.TSTypeLiteral], + }, }, ], output: ` @@ -124,6 +136,9 @@ interface Foo extends Function { { messageId: 'functionTypeOverCallableType', type: AST_NODE_TYPES.TSCallSignatureDeclaration, + data: { + literalOrInterface: phrases[AST_NODE_TYPES.TSInterfaceDeclaration], + }, }, ], output: ` @@ -140,11 +155,118 @@ interface Foo { { messageId: 'functionTypeOverCallableType', type: AST_NODE_TYPES.TSCallSignatureDeclaration, + data: { + literalOrInterface: phrases[AST_NODE_TYPES.TSInterfaceDeclaration], + }, }, ], output: ` type Foo = (bar: T) => string; `, }, + { + code: ` +interface Foo { + (this: T): void; +} + `, + errors: [ + { + messageId: 'functionTypeOverCallableType', + type: AST_NODE_TYPES.TSCallSignatureDeclaration, + data: { + literalOrInterface: phrases[AST_NODE_TYPES.TSInterfaceDeclaration], + }, + }, + ], + output: ` +type Foo = (this: T) => void; + `, + }, + { + code: ` +type Foo = { (this: string): T }; + `, + errors: [ + { + messageId: 'functionTypeOverCallableType', + type: AST_NODE_TYPES.TSCallSignatureDeclaration, + data: { + literalOrInterface: phrases[AST_NODE_TYPES.TSTypeLiteral], + }, + }, + ], + output: ` +type Foo = (this: string) => T; + `, + }, + { + code: ` +interface Foo { + (arg: this): void; +} + `, + errors: [ + { + messageId: 'unexpectedThisOnFunctionOnlyInterface', + type: AST_NODE_TYPES.TSThisType, + data: { + interfaceName: 'Foo', + }, + }, + ], + }, + { + code: ` +interface Foo { + (arg: number): this | undefined; +} + `, + errors: [ + { + messageId: 'unexpectedThisOnFunctionOnlyInterface', + type: AST_NODE_TYPES.TSThisType, + data: { + interfaceName: 'Foo', + }, + }, + ], + }, + { + code: ` +interface Foo { + // isn't actually valid ts but want to not give message saying it refers to Foo. + (): { + a: { + nested: this; + }; + between: this; + b: { + nested: string; + }; + }; +} + `, + errors: [ + { + messageId: 'functionTypeOverCallableType', + type: AST_NODE_TYPES.TSCallSignatureDeclaration, + data: { + literalOrInterface: phrases[AST_NODE_TYPES.TSInterfaceDeclaration], + }, + }, + ], + output: noFormat` +type Foo = () => { + a: { + nested: this; + }; + between: this; + b: { + nested: string; + }; + }; + `, + }, ], });