From fd93d53a12067189f0c396b100ea6248b5040cbc Mon Sep 17 00:00:00 2001 From: Nikita Stefaniak Date: Thu, 5 Mar 2020 23:57:25 +0100 Subject: [PATCH 1/7] feat(eslint-plugin): method-signature-style --- packages/eslint-plugin/src/rules/index.ts | 6 +- .../src/rules/method-signature-style.ts | 59 +++++++++++++ .../rules/method-signature-style.test.ts | 83 +++++++++++++++++++ 3 files changed, 146 insertions(+), 2 deletions(-) create mode 100644 packages/eslint-plugin/src/rules/method-signature-style.ts create mode 100644 packages/eslint-plugin/tests/rules/method-signature-style.test.ts diff --git a/packages/eslint-plugin/src/rules/index.ts b/packages/eslint-plugin/src/rules/index.ts index 486b8a97945..24fa9942a96 100644 --- a/packages/eslint-plugin/src/rules/index.ts +++ b/packages/eslint-plugin/src/rules/index.ts @@ -21,6 +21,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'; @@ -29,10 +30,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'; @@ -98,7 +99,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, @@ -116,8 +116,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..9938eeb16fd --- /dev/null +++ b/packages/eslint-plugin/src/rules/method-signature-style.ts @@ -0,0 +1,59 @@ +import { AST_NODE_TYPES } 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(); + + return { + TSMethodSignature(node): void { + if (mode === 'method') { + return; + } + + context.report({ node, messageId: 'errorMethod' }); + }, + TSPropertySignature(node): void { + if ( + node.typeAnnotation?.typeAnnotation.type !== + AST_NODE_TYPES.TSFunctionType + ) { + return; + } + + if (mode === 'property') { + return; + } + + context.report({ node, messageId: 'errorProperty' }); + }, + }; + }, +}); 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..890b08b6bf0 --- /dev/null +++ b/packages/eslint-plugin/tests/rules/method-signature-style.test.ts @@ -0,0 +1,83 @@ +import rule from '../../src/rules/method-signature-style'; +import { batchedSingleLineTests, RuleTester } from '../RuleTester'; + +const ruleTester = new RuleTester({ + parser: '@typescript-eslint/parser', +}); + +ruleTester.run('method-signature-style', rule, { + valid: [ + ...batchedSingleLineTests({ + code: ` + 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 } + type Test = { f: (a: string) => number } + type Test = { ["f"]: (a: boolean) => void } + type Test = { f: (a: T) => T } + type Test = { ['f']: (a: T, b: T) => T } + `, + }), + ...batchedSingleLineTests({ + options: ['method'], + code: ` + 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 } + type Test = { f(a: string): number } + type Test = { ["f"](a: boolean): void } + type Test = { f(a: T): T } + type Test = { ['f'](a: T, b: T): T } + `, + }), + ], + invalid: [ + ...batchedSingleLineTests({ + code: ` + 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 } + type Test = { f(a: string): number } + type Test = { ["f"](a: boolean): void } + type Test = { f(a: T): T } + type Test = { ['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 }, + ], + }), + ...batchedSingleLineTests({ + options: ['method'], + code: ` + 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 } + type Test = { f: (a: string) => number } + type Test = { ["f"]: (a: boolean) => void } + type Test = { f: (a: T) => T } + type Test = { ['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 }, + ], + }), + ], +}); From 995a4232ff28cda97333c9f92d51b721047581fd Mon Sep 17 00:00:00 2001 From: Nikita Stefaniak Date: Thu, 5 Mar 2020 23:58:17 +0100 Subject: [PATCH 2/7] feat(eslint-plugin): [method-sig-style] add fixers --- .../src/rules/method-signature-style.ts | 77 ++++++++++++++++--- .../rules/method-signature-style.test.ts | 60 ++++++++++----- 2 files changed, 108 insertions(+), 29 deletions(-) diff --git a/packages/eslint-plugin/src/rules/method-signature-style.ts b/packages/eslint-plugin/src/rules/method-signature-style.ts index 9938eeb16fd..bf22e6f2d67 100644 --- a/packages/eslint-plugin/src/rules/method-signature-style.ts +++ b/packages/eslint-plugin/src/rules/method-signature-style.ts @@ -1,4 +1,7 @@ -import { AST_NODE_TYPES } from '@typescript-eslint/experimental-utils'; +import { + AST_NODE_TYPES, + TSESTree, +} from '@typescript-eslint/experimental-utils'; import * as util from '../util'; export type Options = ['property' | 'method']; @@ -32,19 +35,63 @@ export default util.createRule({ 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 = node.params.map(node => sourceCode.getText(node)).join(', '); + params = `(${params})`; + 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(node): void { + TSMethodSignature(methodNode): void { if (mode === 'method') { return; } - context.report({ node, messageId: 'errorMethod' }); + 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(node): void { - if ( - node.typeAnnotation?.typeAnnotation.type !== - AST_NODE_TYPES.TSFunctionType - ) { + TSPropertySignature(propertyNode): void { + const typeNode = propertyNode.typeAnnotation?.typeAnnotation; + if (typeNode?.type !== AST_NODE_TYPES.TSFunctionType) { return; } @@ -52,7 +99,19 @@ export default util.createRule({ return; } - context.report({ node, messageId: 'errorProperty' }); + 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 index 890b08b6bf0..d2cf437e614 100644 --- a/packages/eslint-plugin/tests/rules/method-signature-style.test.ts +++ b/packages/eslint-plugin/tests/rules/method-signature-style.test.ts @@ -10,12 +10,12 @@ ruleTester.run('method-signature-style', rule, { ...batchedSingleLineTests({ code: ` 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"]?: (a: boolean) => void } + interface Test { readonly f: (a?: T) => T } + interface Test { ['f']: (a: T, b: T) => T } type Test = { f: (a: string) => number } - type Test = { ["f"]: (a: boolean) => void } - type Test = { f: (a: T) => T } + type Test = { ["f"]?: (a: boolean) => void } + type Test = { readonly f: (a?: T) => T } type Test = { ['f']: (a: T, b: T) => T } `, }), @@ -23,12 +23,12 @@ ruleTester.run('method-signature-style', rule, { options: ['method'], code: ` 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"]?(a: boolean): void } + interface Test { readonly f(a?: T): T } + interface Test { ['f'](a: T, b: T): T } type Test = { f(a: string): number } - type Test = { ["f"](a: boolean): void } - type Test = { f(a: T): T } + type Test = { ["f"]?(a: boolean): void } + type Test = { readonly f(a?: T): T } type Test = { ['f'](a: T, b: T): T } `, }), @@ -37,12 +37,12 @@ ruleTester.run('method-signature-style', rule, { ...batchedSingleLineTests({ code: ` 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"]?(a: boolean): void } + interface Test { readonly f(a?: T): T } + interface Test { ['f'](a: T, b: T): T } type Test = { f(a: string): number } - type Test = { ["f"](a: boolean): void } - type Test = { f(a: T): T } + type Test = { ["f"]?(a: boolean): void } + type Test = { readonly f(a?: T): T } type Test = { ['f'](a: T, b: T): T } `, errors: [ @@ -55,17 +55,27 @@ ruleTester.run('method-signature-style', rule, { { messageId: 'errorMethod', line: 8 }, { messageId: 'errorMethod', line: 9 }, ], + output: ` + interface Test { f: (a: string) => number } + interface Test { ["f"]?: (a: boolean) => void } + interface Test { readonly f: (a?: T) => T } + interface Test { ['f']: (a: T, b: T) => T } + type Test = { f: (a: string) => number } + type Test = { ["f"]?: (a: boolean) => void } + type Test = { readonly f: (a?: T) => T } + type Test = { ['f']: (a: T, b: T) => T } + `, }), ...batchedSingleLineTests({ options: ['method'], code: ` 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"]?: (a: boolean) => void } + interface Test { readonly f: (a?: T) => T } + interface Test { ['f']: (a: T, b: T) => T } type Test = { f: (a: string) => number } - type Test = { ["f"]: (a: boolean) => void } - type Test = { f: (a: T) => T } + type Test = { ["f"]?: (a: boolean) => void } + type Test = { readonly f: (a?: T) => T } type Test = { ['f']: (a: T, b: T) => T } `, errors: [ @@ -78,6 +88,16 @@ ruleTester.run('method-signature-style', rule, { { messageId: 'errorProperty', line: 8 }, { messageId: 'errorProperty', line: 9 }, ], + output: ` + interface Test { f(a: string): number } + interface Test { ["f"]?(a: boolean): void } + interface Test { readonly f(a?: T): T } + interface Test { ['f'](a: T, b: T): T } + type Test = { f(a: string): number } + type Test = { ["f"]?(a: boolean): void } + type Test = { readonly f(a?: T): T } + type Test = { ['f'](a: T, b: T): T } + `, }), ], }); From be1067ca71ab678124909f81b9b1564bb06adc18 Mon Sep 17 00:00:00 2001 From: Nikita Stefaniak Date: Fri, 6 Mar 2020 02:39:06 +0100 Subject: [PATCH 3/7] docs(eslint-plugin): [method-sig-style] add docs --- .cspell.json | 24 +++--- packages/eslint-plugin/README.md | 1 + .../docs/rules/method-signature-style.md | 77 +++++++++++++++++++ packages/eslint-plugin/src/configs/all.json | 1 + 4 files changed, 92 insertions(+), 11 deletions(-) create mode 100644 packages/eslint-plugin/docs/rules/method-signature-style.md diff --git a/.cspell.json b/.cspell.json index 33f7b70c5f2..a40535cad8c 100644 --- a/.cspell.json +++ b/.cspell.json @@ -33,45 +33,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 b7927757924..abed901aa0c 100644 --- a/packages/eslint-plugin/README.md +++ b/packages/eslint-plugin/README.md @@ -106,6 +106,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..08bc07d46d9 --- /dev/null +++ b/packages/eslint-plugin/docs/rules/method-signature-style.md @@ -0,0 +1,77 @@ +# 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 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 [here](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. +- `"method"`: Enforce using method signature for functions. + +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 variance on all interface/object methods, 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 2626767b84c..fad136708c8 100644 --- a/packages/eslint-plugin/src/configs/all.json +++ b/packages/eslint-plugin/src/configs/all.json @@ -23,6 +23,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", From cca3d68303c0f1539569139aea3a047065f0d0c7 Mon Sep 17 00:00:00 2001 From: Nikita Stefaniak Date: Fri, 6 Mar 2020 02:47:57 +0100 Subject: [PATCH 4/7] test(eslint-plugin): [method-sig-style] more tests --- .../rules/method-signature-style.test.ts | 72 +++++++++---------- 1 file changed, 36 insertions(+), 36 deletions(-) diff --git a/packages/eslint-plugin/tests/rules/method-signature-style.test.ts b/packages/eslint-plugin/tests/rules/method-signature-style.test.ts index d2cf437e614..81378605b82 100644 --- a/packages/eslint-plugin/tests/rules/method-signature-style.test.ts +++ b/packages/eslint-plugin/tests/rules/method-signature-style.test.ts @@ -10,26 +10,26 @@ ruleTester.run('method-signature-style', rule, { ...batchedSingleLineTests({ code: ` interface Test { f: (a: string) => number } - interface Test { ["f"]?: (a: boolean) => void } - interface Test { readonly f: (a?: T) => T } + interface Test { ['f']: (a: boolean) => void } + interface Test { f: (a: T) => T } interface Test { ['f']: (a: T, b: T) => T } - type Test = { f: (a: string) => number } - type Test = { ["f"]?: (a: boolean) => void } - type Test = { readonly f: (a?: T) => T } - type Test = { ['f']: (a: T, b: T) => T } + 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: ` interface Test { f(a: string): number } - interface Test { ["f"]?(a: boolean): void } - interface Test { readonly f(a?: T): T } + interface Test { ['f'](a: boolean): void } + interface Test { f(a: T): T } interface Test { ['f'](a: T, b: T): T } - type Test = { f(a: string): number } - type Test = { ["f"]?(a: boolean): void } - type Test = { readonly f(a?: T): T } - type Test = { ['f'](a: T, b: T): T } + 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 } `, }), ], @@ -37,13 +37,13 @@ ruleTester.run('method-signature-style', rule, { ...batchedSingleLineTests({ code: ` interface Test { f(a: string): number } - interface Test { ["f"]?(a: boolean): void } - interface Test { readonly f(a?: T): T } + interface Test { ['f'](a: boolean): void } + interface Test { f(a: T): T } interface Test { ['f'](a: T, b: T): T } - type Test = { f(a: string): number } - type Test = { ["f"]?(a: boolean): void } - type Test = { readonly f(a?: T): T } - type Test = { ['f'](a: T, b: T): T } + 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 }, @@ -57,26 +57,26 @@ ruleTester.run('method-signature-style', rule, { ], output: ` interface Test { f: (a: string) => number } - interface Test { ["f"]?: (a: boolean) => void } - interface Test { readonly f: (a?: T) => T } + interface Test { ['f']: (a: boolean) => void } + interface Test { f: (a: T) => T } interface Test { ['f']: (a: T, b: T) => T } - type Test = { f: (a: string) => number } - type Test = { ["f"]?: (a: boolean) => void } - type Test = { readonly f: (a?: T) => T } - type Test = { ['f']: (a: T, b: T) => T } + 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: ` interface Test { f: (a: string) => number } - interface Test { ["f"]?: (a: boolean) => void } - interface Test { readonly f: (a?: T) => T } + interface Test { ['f']: (a: boolean) => void } + interface Test { f: (a: T) => T } interface Test { ['f']: (a: T, b: T) => T } - type Test = { f: (a: string) => number } - type Test = { ["f"]?: (a: boolean) => void } - type Test = { readonly f: (a?: T) => T } - type Test = { ['f']: (a: T, b: T) => T } + 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 }, @@ -90,13 +90,13 @@ ruleTester.run('method-signature-style', rule, { ], output: ` interface Test { f(a: string): number } - interface Test { ["f"]?(a: boolean): void } - interface Test { readonly f(a?: T): T } + interface Test { ['f'](a: boolean): void } + interface Test { f(a: T): T } interface Test { ['f'](a: T, b: T): T } - type Test = { f(a: string): number } - type Test = { ["f"]?(a: boolean): void } - type Test = { readonly f(a?: T): T } - type Test = { ['f'](a: T, b: T): T } + 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 } `, }), ], From f42efe0b341a53f4a5598866ce221f15dd710608 Mon Sep 17 00:00:00 2001 From: Nikita Stefaniak Date: Sun, 8 Mar 2020 02:14:31 +0100 Subject: [PATCH 5/7] docs(eslint-plugin): docs --- .../docs/rules/method-signature-style.md | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/method-signature-style.md b/packages/eslint-plugin/docs/rules/method-signature-style.md index 08bc07d46d9..d760c8d0193 100644 --- a/packages/eslint-plugin/docs/rules/method-signature-style.md +++ b/packages/eslint-plugin/docs/rules/method-signature-style.md @@ -14,15 +14,21 @@ interface T2 { } ``` -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 [here](https://github.com/microsoft/TypeScript/pull/18654). +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. -- `"method"`: Enforce using method signature for functions. +- `"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"`. @@ -74,4 +80,4 @@ type T2 = { ## When Not To Use It -If you don't want to enforce a particular variance on all interface/object methods, you don't need this rule. +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. From 291f2b069a3a66c87b4f439f04b6efeedfc18579 Mon Sep 17 00:00:00 2001 From: Nikita Stefaniak Date: Mon, 23 Mar 2020 21:38:41 +0100 Subject: [PATCH 6/7] feat(eslint-plugin): preserve comments in method fixer --- .../eslint-plugin/src/rules/method-signature-style.ts | 10 ++++++++-- .../tests/rules/method-signature-style.test.ts | 8 ++++++++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/packages/eslint-plugin/src/rules/method-signature-style.ts b/packages/eslint-plugin/src/rules/method-signature-style.ts index bf22e6f2d67..6ef8db5a0c9 100644 --- a/packages/eslint-plugin/src/rules/method-signature-style.ts +++ b/packages/eslint-plugin/src/rules/method-signature-style.ts @@ -54,8 +54,14 @@ export default util.createRule({ function getMethodParams( node: TSESTree.TSMethodSignature | TSESTree.TSFunctionType, ): string { - let params = node.params.map(node => sourceCode.getText(node)).join(', '); - params = `(${params})`; + 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}`; diff --git a/packages/eslint-plugin/tests/rules/method-signature-style.test.ts b/packages/eslint-plugin/tests/rules/method-signature-style.test.ts index 81378605b82..543d0a5d864 100644 --- a/packages/eslint-plugin/tests/rules/method-signature-style.test.ts +++ b/packages/eslint-plugin/tests/rules/method-signature-style.test.ts @@ -13,6 +13,7 @@ ruleTester.run('method-signature-style', rule, { 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 } @@ -26,6 +27,7 @@ ruleTester.run('method-signature-style', rule, { 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 } @@ -40,6 +42,7 @@ ruleTester.run('method-signature-style', rule, { 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 } @@ -54,12 +57,14 @@ ruleTester.run('method-signature-style', rule, { { messageId: 'errorMethod', line: 7 }, { messageId: 'errorMethod', line: 8 }, { messageId: 'errorMethod', line: 9 }, + { messageId: 'errorMethod', line: 10 }, ], output: ` 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 } @@ -73,6 +78,7 @@ ruleTester.run('method-signature-style', rule, { 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 } @@ -87,12 +93,14 @@ ruleTester.run('method-signature-style', rule, { { messageId: 'errorProperty', line: 7 }, { messageId: 'errorProperty', line: 8 }, { messageId: 'errorProperty', line: 9 }, + { messageId: 'errorProperty', line: 10 }, ], output: ` 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 } From f355e4ea9a1163a3db417bf03c2e82f1621515bb Mon Sep 17 00:00:00 2001 From: Nikita Stefaniak Date: Fri, 3 Apr 2020 23:36:24 +0200 Subject: [PATCH 7/7] style(eslint-plugin): add workaround for failing lint --- .../tests/rules/method-signature-style.test.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/eslint-plugin/tests/rules/method-signature-style.test.ts b/packages/eslint-plugin/tests/rules/method-signature-style.test.ts index 543d0a5d864..76003e67d84 100644 --- a/packages/eslint-plugin/tests/rules/method-signature-style.test.ts +++ b/packages/eslint-plugin/tests/rules/method-signature-style.test.ts @@ -1,5 +1,5 @@ import rule from '../../src/rules/method-signature-style'; -import { batchedSingleLineTests, RuleTester } from '../RuleTester'; +import { batchedSingleLineTests, noFormat, RuleTester } from '../RuleTester'; const ruleTester = new RuleTester({ parser: '@typescript-eslint/parser', @@ -8,7 +8,7 @@ const ruleTester = new RuleTester({ ruleTester.run('method-signature-style', rule, { valid: [ ...batchedSingleLineTests({ - code: ` + code: noFormat` interface Test { f: (a: string) => number } interface Test { ['f']: (a: boolean) => void } interface Test { f: (a: T) => T } @@ -22,7 +22,7 @@ ruleTester.run('method-signature-style', rule, { }), ...batchedSingleLineTests({ options: ['method'], - code: ` + code: noFormat` interface Test { f(a: string): number } interface Test { ['f'](a: boolean): void } interface Test { f(a: T): T } @@ -37,7 +37,7 @@ ruleTester.run('method-signature-style', rule, { ], invalid: [ ...batchedSingleLineTests({ - code: ` + code: noFormat` interface Test { f(a: string): number } interface Test { ['f'](a: boolean): void } interface Test { f(a: T): T } @@ -59,7 +59,7 @@ ruleTester.run('method-signature-style', rule, { { messageId: 'errorMethod', line: 9 }, { messageId: 'errorMethod', line: 10 }, ], - output: ` + output: noFormat` interface Test { f: (a: string) => number } interface Test { ['f']: (a: boolean) => void } interface Test { f: (a: T) => T } @@ -73,7 +73,7 @@ ruleTester.run('method-signature-style', rule, { }), ...batchedSingleLineTests({ options: ['method'], - code: ` + code: noFormat` interface Test { f: (a: string) => number } interface Test { ['f']: (a: boolean) => void } interface Test { f: (a: T) => T } @@ -95,7 +95,7 @@ ruleTester.run('method-signature-style', rule, { { messageId: 'errorProperty', line: 9 }, { messageId: 'errorProperty', line: 10 }, ], - output: ` + output: noFormat` interface Test { f(a: string): number } interface Test { ['f'](a: boolean): void } interface Test { f(a: T): T }