diff --git a/.cspell.json b/.cspell.json index 7ee50214e7f..9df7d65a1b5 100644 --- a/.cspell.json +++ b/.cspell.json @@ -35,45 +35,47 @@ "\\(#.+?\\)" ], "words": [ - "ASTs", "Airbnb", "Airbnb's", - "Codecov", - "Crockford", - "errored", - "IDE's", - "IIFE", - "IIFEs", - "OOM", - "OOMs", - "Premade", - "ROADMAP", + "ASTs", "autofix", "autofixers", "backticks", "bigint", + "bivariant", "blockless", "codebases", + "Codecov", + "contravariant", + "Crockford", "declarators", "destructure", "destructured", + "errored", "erroring", "ESLint", "ESLint's", "espree", "estree", + "IDE's", + "IIFE", + "IIFEs", "linebreaks", "necroing", "nocheck", "nullish", + "OOM", + "OOMs", "parameterised", "performant", "pluggable", "postprocess", + "Premade", "prettier's", "recurse", "reimplement", "resync", + "ROADMAP", "ruleset", "rulesets", "superset", diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index fa049b2cfb2..92131acf458 100644 --- a/packages/eslint-plugin/README.md +++ b/packages/eslint-plugin/README.md @@ -107,6 +107,7 @@ Pro Tip: For larger codebases you may want to consider splitting our linting int | [`@typescript-eslint/explicit-module-boundary-types`](./docs/rules/explicit-module-boundary-types.md) | Require explicit return and argument types on exported functions' and classes' public class methods | | | | | [`@typescript-eslint/member-delimiter-style`](./docs/rules/member-delimiter-style.md) | Require a specific member delimiter style for interfaces and type literals | :heavy_check_mark: | :wrench: | | | [`@typescript-eslint/member-ordering`](./docs/rules/member-ordering.md) | Require a consistent member declaration order | | | | +| [`@typescript-eslint/method-signature-style`](./docs/rules/method-signature-style.md) | Enforces using a particular method signature syntax. | | :wrench: | | | [`@typescript-eslint/naming-convention`](./docs/rules/naming-convention.md) | Enforces naming conventions for everything across a codebase | | | :thought_balloon: | | [`@typescript-eslint/no-base-to-string`](./docs/rules/no-base-to-string.md) | Requires that `.toString()` is only called on objects which provide useful information when stringified | | | :thought_balloon: | | [`@typescript-eslint/no-dynamic-delete`](./docs/rules/no-dynamic-delete.md) | Disallow the delete operator with computed key expressions | | :wrench: | | diff --git a/packages/eslint-plugin/docs/rules/method-signature-style.md b/packages/eslint-plugin/docs/rules/method-signature-style.md new file mode 100644 index 00000000000..d760c8d0193 --- /dev/null +++ b/packages/eslint-plugin/docs/rules/method-signature-style.md @@ -0,0 +1,83 @@ +# Enforces using a particular method signature syntax. (`method-signature-style`) + +There are two ways to define an object/interface function property. + +```ts +// method shorthand syntax +interface T1 { + func(arg: string): number; +} + +// regular property with function type +interface T2 { + func: (arg: string) => number; +} +``` + +A good practice is to use the TypeScript's `strict` option (which implies `strictFunctionTypes`) which enables correct typechecking for function properties only (method signatures get old behavior). + +TypeScript FAQ: + +> A method and a function property of the same type behave differently. +> Methods are always bivariant in their argument, while function properties are contravariant in their argument under `strictFunctionTypes`. + +See the reasoning behind that in the [TypeScript PR for the compiler option](https://github.com/microsoft/TypeScript/pull/18654). + +## Options + +This rule accepts one string option: + +- `"property"`: Enforce using property signature for functions. Use this to enforce maximum correctness together with TypeScript's strict mode. +- `"method"`: Enforce using method signature for functions. Use this if you aren't using TypeScript's strict mode and prefer this style. + +The default is `"property"`. + +## Rule Details + +Examples of **incorrect** code with `property` option. + +```ts +interface T1 { + func(arg: string): number; +} +type T2 = { + func(arg: boolean): void; +}; +``` + +Examples of **correct** code with `property` option. + +```ts +interface T1 { + func: (arg: string) => number; +} +type T2 = { + func: (arg: boolean) => void; +}; +``` + +Examples of **incorrect** code with `method` option. + +```ts +interface T1 { + func: (arg: string) => number; +} +type T2 = { + func: (arg: boolean) => void; +}; +``` + +Examples of **correct** code with `method` option. + +```ts +interface T1 { + func(arg: string): number; +} +type T2 = { + func(arg: boolean): void; +}; +``` + +## When Not To Use It + +If you don't want to enforce a particular style for object/interface function types, and/or if you don't use `strictFunctionTypes`, then you don't need this rule. diff --git a/packages/eslint-plugin/src/configs/all.json b/packages/eslint-plugin/src/configs/all.json index c5575d4970d..cfcb38ced24 100644 --- a/packages/eslint-plugin/src/configs/all.json +++ b/packages/eslint-plugin/src/configs/all.json @@ -24,6 +24,7 @@ "@typescript-eslint/indent": "error", "@typescript-eslint/member-delimiter-style": "error", "@typescript-eslint/member-ordering": "error", + "@typescript-eslint/method-signature-style": "error", "@typescript-eslint/naming-convention": "error", "no-array-constructor": "off", "@typescript-eslint/no-array-constructor": "error", diff --git a/packages/eslint-plugin/src/rules/index.ts b/packages/eslint-plugin/src/rules/index.ts index 29db58b681a..332c706c832 100644 --- a/packages/eslint-plugin/src/rules/index.ts +++ b/packages/eslint-plugin/src/rules/index.ts @@ -22,6 +22,7 @@ import interfaceNamePrefix from './interface-name-prefix'; import memberDelimiterStyle from './member-delimiter-style'; import memberNaming from './member-naming'; import memberOrdering from './member-ordering'; +import methodSignatureStyle from './method-signature-style'; import namingConvention from './naming-convention'; import noArrayConstructor from './no-array-constructor'; import noBaseToString from './no-base-to-string'; @@ -30,10 +31,10 @@ import noDynamicDelete from './no-dynamic-delete'; import noEmptyFunction from './no-empty-function'; import noEmptyInterface from './no-empty-interface'; import noExplicitAny from './no-explicit-any'; -import noExtraneousClass from './no-extraneous-class'; import noExtraNonNullAssertion from './no-extra-non-null-assertion'; import noExtraParens from './no-extra-parens'; import noExtraSemi from './no-extra-semi'; +import noExtraneousClass from './no-extraneous-class'; import noFloatingPromises from './no-floating-promises'; import noForInArray from './no-for-in-array'; import noImpliedEval from './no-implied-eval'; @@ -99,7 +100,6 @@ export default { 'ban-ts-comment': banTsComment, 'ban-ts-ignore': banTsIgnore, 'ban-types': banTypes, - 'no-base-to-string': noBaseToString, 'brace-style': braceStyle, camelcase: camelcase, 'class-name-casing': classNameCasing, @@ -118,8 +118,10 @@ export default { 'member-delimiter-style': memberDelimiterStyle, 'member-naming': memberNaming, 'member-ordering': memberOrdering, + 'method-signature-style': methodSignatureStyle, 'naming-convention': namingConvention, 'no-array-constructor': noArrayConstructor, + 'no-base-to-string': noBaseToString, 'no-dupe-class-members': noDupeClassMembers, 'no-dynamic-delete': noDynamicDelete, 'no-empty-function': noEmptyFunction, diff --git a/packages/eslint-plugin/src/rules/method-signature-style.ts b/packages/eslint-plugin/src/rules/method-signature-style.ts new file mode 100644 index 00000000000..6ef8db5a0c9 --- /dev/null +++ b/packages/eslint-plugin/src/rules/method-signature-style.ts @@ -0,0 +1,124 @@ +import { + AST_NODE_TYPES, + TSESTree, +} from '@typescript-eslint/experimental-utils'; +import * as util from '../util'; + +export type Options = ['property' | 'method']; + +export type MessageId = 'errorMethod' | 'errorProperty'; + +export default util.createRule({ + name: 'method-signature-style', + meta: { + type: 'suggestion', + docs: { + description: 'Enforces using a particular method signature syntax.', + category: 'Best Practices', + recommended: false, + }, + fixable: 'code', + messages: { + errorMethod: + 'Shorthand method signature is forbidden. Use a function property instead.', + errorProperty: + 'Function property signature is forbidden. Use a method shorthand instead.', + }, + schema: [ + { + enum: ['property', 'method'], + }, + ], + }, + defaultOptions: ['property'], + + create(context, [mode]) { + const sourceCode = context.getSourceCode(); + + function getMethodKey( + node: TSESTree.TSMethodSignature | TSESTree.TSPropertySignature, + ): string { + let key = sourceCode.getText(node.key); + if (node.computed) { + key = `[${key}]`; + } + if (node.optional) { + key = `${key}?`; + } + if (node.readonly) { + key = `readonly ${key}`; + } + return key; + } + + function getMethodParams( + node: TSESTree.TSMethodSignature | TSESTree.TSFunctionType, + ): string { + let params = '()'; + if (node.params.length > 0) { + params = sourceCode.text.substring( + sourceCode.getTokenBefore(node.params[0])!.range[0], + sourceCode.getTokenAfter(node.params[node.params.length - 1])! + .range[1], + ); + } + if (node.typeParameters != null) { + const typeParams = sourceCode.getText(node.typeParameters); + params = `${typeParams}${params}`; + } + return params; + } + + function getMethodReturnType( + node: TSESTree.TSMethodSignature | TSESTree.TSFunctionType, + ): string { + return sourceCode.getText(node.returnType!.typeAnnotation); + } + + return { + TSMethodSignature(methodNode): void { + if (mode === 'method') { + return; + } + + context.report({ + node: methodNode, + messageId: 'errorMethod', + fix: fixer => { + const key = getMethodKey(methodNode); + const params = getMethodParams(methodNode); + const returnType = getMethodReturnType(methodNode); + return fixer.replaceText( + methodNode, + `${key}: ${params} => ${returnType}`, + ); + }, + }); + }, + TSPropertySignature(propertyNode): void { + const typeNode = propertyNode.typeAnnotation?.typeAnnotation; + if (typeNode?.type !== AST_NODE_TYPES.TSFunctionType) { + return; + } + + if (mode === 'property') { + return; + } + + context.report({ + node: propertyNode, + messageId: 'errorProperty', + fix: fixer => { + const key = getMethodKey(propertyNode); + const params = getMethodParams(typeNode); + const returnType = getMethodReturnType(typeNode); + return fixer.replaceText( + propertyNode, + `${key}${params}: ${returnType}`, + ); + }, + }); + }, + }; + }, +}); diff --git a/packages/eslint-plugin/tests/rules/method-signature-style.test.ts b/packages/eslint-plugin/tests/rules/method-signature-style.test.ts new file mode 100644 index 00000000000..76003e67d84 --- /dev/null +++ b/packages/eslint-plugin/tests/rules/method-signature-style.test.ts @@ -0,0 +1,111 @@ +import rule from '../../src/rules/method-signature-style'; +import { batchedSingleLineTests, noFormat, RuleTester } from '../RuleTester'; + +const ruleTester = new RuleTester({ + parser: '@typescript-eslint/parser', +}); + +ruleTester.run('method-signature-style', rule, { + valid: [ + ...batchedSingleLineTests({ + code: noFormat` + interface Test { f: (a: string) => number } + interface Test { ['f']: (a: boolean) => void } + interface Test { f: (a: T) => T } + interface Test { ['f']: (a: T, b: T) => T } + interface Test { 'f!': (/* b */ x: any /* c */) => void } + type Test = { readonly f: (a: string) => number } + type Test = { ['f']?: (a: boolean) => void } + type Test = { readonly f?: (a?: T) => T } + type Test = { readonly ['f']?: (a: T, b: T) => T } + `, + }), + ...batchedSingleLineTests({ + options: ['method'], + code: noFormat` + interface Test { f(a: string): number } + interface Test { ['f'](a: boolean): void } + interface Test { f(a: T): T } + interface Test { ['f'](a: T, b: T): T } + interface Test { 'f!'(/* b */ x: any /* c */): void } + type Test = { readonly f(a: string): number } + type Test = { ['f']?(a: boolean): void } + type Test = { readonly f?(a?: T): T } + type Test = { readonly ['f']?(a: T, b: T): T } + `, + }), + ], + invalid: [ + ...batchedSingleLineTests({ + code: noFormat` + interface Test { f(a: string): number } + interface Test { ['f'](a: boolean): void } + interface Test { f(a: T): T } + interface Test { ['f'](a: T, b: T): T } + interface Test { 'f!'(/* b */ x: any /* c */): void } + type Test = { readonly f(a: string): number } + type Test = { ['f']?(a: boolean): void } + type Test = { readonly f?(a?: T): T } + type Test = { readonly ['f']?(a: T, b: T): T } + `, + errors: [ + { messageId: 'errorMethod', line: 2 }, + { messageId: 'errorMethod', line: 3 }, + { messageId: 'errorMethod', line: 4 }, + { messageId: 'errorMethod', line: 5 }, + { messageId: 'errorMethod', line: 6 }, + { messageId: 'errorMethod', line: 7 }, + { messageId: 'errorMethod', line: 8 }, + { messageId: 'errorMethod', line: 9 }, + { messageId: 'errorMethod', line: 10 }, + ], + output: noFormat` + interface Test { f: (a: string) => number } + interface Test { ['f']: (a: boolean) => void } + interface Test { f: (a: T) => T } + interface Test { ['f']: (a: T, b: T) => T } + interface Test { 'f!': (/* b */ x: any /* c */) => void } + type Test = { readonly f: (a: string) => number } + type Test = { ['f']?: (a: boolean) => void } + type Test = { readonly f?: (a?: T) => T } + type Test = { readonly ['f']?: (a: T, b: T) => T } + `, + }), + ...batchedSingleLineTests({ + options: ['method'], + code: noFormat` + interface Test { f: (a: string) => number } + interface Test { ['f']: (a: boolean) => void } + interface Test { f: (a: T) => T } + interface Test { ['f']: (a: T, b: T) => T } + interface Test { 'f!': (/* b */ x: any /* c */) => void } + type Test = { readonly f: (a: string) => number } + type Test = { ['f']?: (a: boolean) => void } + type Test = { readonly f?: (a?: T) => T } + type Test = { readonly ['f']?: (a: T, b: T) => T } + `, + errors: [ + { messageId: 'errorProperty', line: 2 }, + { messageId: 'errorProperty', line: 3 }, + { messageId: 'errorProperty', line: 4 }, + { messageId: 'errorProperty', line: 5 }, + { messageId: 'errorProperty', line: 6 }, + { messageId: 'errorProperty', line: 7 }, + { messageId: 'errorProperty', line: 8 }, + { messageId: 'errorProperty', line: 9 }, + { messageId: 'errorProperty', line: 10 }, + ], + output: noFormat` + interface Test { f(a: string): number } + interface Test { ['f'](a: boolean): void } + interface Test { f(a: T): T } + interface Test { ['f'](a: T, b: T): T } + interface Test { 'f!'(/* b */ x: any /* c */): void } + type Test = { readonly f(a: string): number } + type Test = { ['f']?(a: boolean): void } + type Test = { readonly f?(a?: T): T } + type Test = { readonly ['f']?(a: T, b: T): T } + `, + }), + ], +});