From ec87d067a83c3b36790fb0f6c07145a80e738ec5 Mon Sep 17 00:00:00 2001 From: Hidemi Yukita Date: Fri, 21 Jun 2019 03:41:24 +0900 Subject: [PATCH] feat(eslint-plugin): add `consistent-type-definitions` rule (#463) Deprecates `prefer-interface` --- packages/eslint-plugin/README.md | 2 +- .../docs/rules/consistent-type-definitions.md | 74 +++++++ .../docs/rules/prefer-interface.md | 4 +- packages/eslint-plugin/src/configs/all.json | 4 +- .../src/rules/consistent-type-definitions.ts | 103 +++++++++ packages/eslint-plugin/src/rules/index.ts | 2 + .../src/rules/prefer-interface.ts | 2 + .../rules/consistent-type-definitions.test.ts | 209 ++++++++++++++++++ .../validate-docs/check-for-rule-docs.ts | 4 +- .../validate-docs/validate-table-rules.ts | 4 +- .../validate-docs/validate-table-structure.ts | 8 +- .../experimental-utils/src/ts-eslint/Rule.ts | 2 +- 12 files changed, 407 insertions(+), 11 deletions(-) create mode 100644 packages/eslint-plugin/docs/rules/consistent-type-definitions.md create mode 100644 packages/eslint-plugin/src/rules/consistent-type-definitions.ts create mode 100644 packages/eslint-plugin/tests/rules/consistent-type-definitions.test.ts diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index 1d0e220adbd..68ff8434d8a 100644 --- a/packages/eslint-plugin/README.md +++ b/packages/eslint-plugin/README.md @@ -131,6 +131,7 @@ Then you should add `airbnb` (or `airbnb-base`) to your `extends` section of `.e | [`@typescript-eslint/ban-types`](./docs/rules/ban-types.md) | Enforces that types will not to be used | :heavy_check_mark: | :wrench: | | | [`@typescript-eslint/camelcase`](./docs/rules/camelcase.md) | Enforce camelCase naming convention | :heavy_check_mark: | | | | [`@typescript-eslint/class-name-casing`](./docs/rules/class-name-casing.md) | Require PascalCased class and interface names | :heavy_check_mark: | | | +| [`@typescript-eslint/consistent-type-definitions`](./docs/rules/consistent-type-definitions.md) | Consistent with type definition either `interface` or `type` | :heavy_check_mark: | :wrench: | | | [`@typescript-eslint/explicit-function-return-type`](./docs/rules/explicit-function-return-type.md) | Require explicit return types on functions and class methods | :heavy_check_mark: | | | | [`@typescript-eslint/explicit-member-accessibility`](./docs/rules/explicit-member-accessibility.md) | Require explicit accessibility modifiers on class properties and methods | :heavy_check_mark: | | | | [`@typescript-eslint/func-call-spacing`](./docs/rules/func-call-spacing.md) | Require or disallow spacing between function identifiers and their invocations | | :wrench: | | @@ -169,7 +170,6 @@ Then you should add `airbnb` (or `airbnb-base`) to your `extends` section of `.e | [`@typescript-eslint/prefer-for-of`](./docs/rules/prefer-for-of.md) | Prefer a ‘for-of’ loop over a standard ‘for’ loop if the index is only used to access the array being iterated | | | | | [`@typescript-eslint/prefer-function-type`](./docs/rules/prefer-function-type.md) | Use function types instead of interfaces with call signatures | | :wrench: | | | [`@typescript-eslint/prefer-includes`](./docs/rules/prefer-includes.md) | Enforce `includes` method over `indexOf` method | | :wrench: | :thought_balloon: | -| [`@typescript-eslint/prefer-interface`](./docs/rules/prefer-interface.md) | Prefer an interface declaration over a type literal (type T = { ... }) | :heavy_check_mark: | :wrench: | | | [`@typescript-eslint/prefer-namespace-keyword`](./docs/rules/prefer-namespace-keyword.md) | Require the use of the `namespace` keyword instead of the `module` keyword to declare custom TypeScript modules | :heavy_check_mark: | :wrench: | | | [`@typescript-eslint/prefer-regexp-exec`](./docs/rules/prefer-regexp-exec.md) | Prefer RegExp#exec() over String#match() if no global flag is provided | | | :thought_balloon: | | [`@typescript-eslint/prefer-string-starts-ends-with`](./docs/rules/prefer-string-starts-ends-with.md) | Enforce the use of `String#startsWith` and `String#endsWith` instead of other equivalent methods of checking substrings | | :wrench: | :thought_balloon: | diff --git a/packages/eslint-plugin/docs/rules/consistent-type-definitions.md b/packages/eslint-plugin/docs/rules/consistent-type-definitions.md new file mode 100644 index 00000000000..3e60f2ced7c --- /dev/null +++ b/packages/eslint-plugin/docs/rules/consistent-type-definitions.md @@ -0,0 +1,74 @@ +# Consistent with type definition either `interface` or `type` (consistent-type-definitions) + +There are two ways to define a type. + +```ts +// type alias +type T1 = { + a: string; + b: number; +}; + +// interface keyword +interface T2 { + a: string; + b: number; +} +``` + +## Options + +This rule accepts one string option: + +- `"interface"`: enforce using `interface`s for object type definitions. +- `"type"`: enforce using `type`s for object type definitions. + +For example: + +```CJSON +{ + // Use type for object definitions + "@typescript-eslint/consistent-type-definitions": ["error", "type"] +} +``` + +## Rule Details + +Examples of **incorrect** code with `interface` option. + +```ts +type T = { x: number }; +``` + +Examples of **correct** code with `interface` option. + +```ts +type T = string; +type Foo = string | {}; + +interface T { + x: number; +} +``` + +Examples of **incorrect** code with `type` option. + +```ts +interface T { + x: number; +} +``` + +Examples of **correct** code with `type` option. + +```ts +type T = { x: number }; +``` + +## When Not To Use It + +If you specifically want to use an interface or type literal for stylistic reasons, you can disable this rule. + +## Compatibility + +- TSLint: [interface-over-type-literal](https://palantir.github.io/tslint/rules/interface-over-type-literal/) diff --git a/packages/eslint-plugin/docs/rules/prefer-interface.md b/packages/eslint-plugin/docs/rules/prefer-interface.md index 39c8ca24321..8b995e6b06c 100644 --- a/packages/eslint-plugin/docs/rules/prefer-interface.md +++ b/packages/eslint-plugin/docs/rules/prefer-interface.md @@ -1,7 +1,9 @@ -# Prefer an interface declaration over a type literal (type T = { ... }) (prefer-interface) +# Prefer an interface declaration over a type literal (type T = { ... }) (prefer-interface)\ Interfaces are generally preferred over type literals because interfaces can be implemented, extended and merged. +## DEPRECATED - this rule has been deprecated in favour of [`consistent-type-definitions`](./consistent-type-definitions.md) + ## Rule Details Examples of **incorrect** code for this rule. diff --git a/packages/eslint-plugin/src/configs/all.json b/packages/eslint-plugin/src/configs/all.json index cc2f9778671..2efc8d24323 100644 --- a/packages/eslint-plugin/src/configs/all.json +++ b/packages/eslint-plugin/src/configs/all.json @@ -9,6 +9,7 @@ "camelcase": "off", "@typescript-eslint/camelcase": "error", "@typescript-eslint/class-name-casing": "error", + "@typescript-eslint/consistent-type-definitions": "error", "@typescript-eslint/explicit-function-return-type": "error", "@typescript-eslint/explicit-member-accessibility": "error", "func-call-spacing": "off", @@ -23,11 +24,13 @@ "@typescript-eslint/no-angle-bracket-type-assertion": "error", "no-array-constructor": "off", "@typescript-eslint/no-array-constructor": "error", + "@typescript-eslint/no-empty-function": "error", "@typescript-eslint/no-empty-interface": "error", "@typescript-eslint/no-explicit-any": "error", "no-extra-parens": "off", "@typescript-eslint/no-extra-parens": "error", "@typescript-eslint/no-extraneous-class": "error", + "@typescript-eslint/no-floating-promises": "error", "@typescript-eslint/no-for-in-array": "error", "@typescript-eslint/no-inferrable-types": "error", "no-magic-numbers": "off", @@ -53,7 +56,6 @@ "@typescript-eslint/prefer-for-of": "error", "@typescript-eslint/prefer-function-type": "error", "@typescript-eslint/prefer-includes": "error", - "@typescript-eslint/prefer-interface": "error", "@typescript-eslint/prefer-namespace-keyword": "error", "@typescript-eslint/prefer-regexp-exec": "error", "@typescript-eslint/prefer-string-starts-ends-with": "error", diff --git a/packages/eslint-plugin/src/rules/consistent-type-definitions.ts b/packages/eslint-plugin/src/rules/consistent-type-definitions.ts new file mode 100644 index 00000000000..d8d9bb675a2 --- /dev/null +++ b/packages/eslint-plugin/src/rules/consistent-type-definitions.ts @@ -0,0 +1,103 @@ +import { TSESLint, TSESTree } from '@typescript-eslint/experimental-utils'; +import * as util from '../util'; + +export default util.createRule({ + name: 'consistent-type-definitions', + meta: { + type: 'suggestion', + docs: { + description: + 'Consistent with type definition either `interface` or `type`', + category: 'Stylistic Issues', + recommended: 'error', + }, + messages: { + interfaceOverType: 'Use an `interface` instead of a `type`', + typeOverInterface: 'Use a `type` instead of an `interface`', + }, + schema: [ + { + enum: ['interface', 'type'], + }, + ], + fixable: 'code', + }, + defaultOptions: ['interface'], + create(context, [option]) { + const sourceCode = context.getSourceCode(); + + return { + // VariableDeclaration with kind type has only one VariableDeclarator + "TSTypeAliasDeclaration[typeAnnotation.type='TSTypeLiteral']"( + node: TSESTree.TSTypeAliasDeclaration, + ) { + if (option === 'interface') { + context.report({ + node: node.id, + messageId: 'interfaceOverType', + fix(fixer) { + const typeNode = node.typeParameters || node.id; + const fixes: TSESLint.RuleFix[] = []; + + const firstToken = sourceCode.getFirstToken(node); + if (firstToken) { + fixes.push(fixer.replaceText(firstToken, 'interface')); + fixes.push( + fixer.replaceTextRange( + [typeNode.range[1], node.typeAnnotation.range[0]], + ' ', + ), + ); + } + + const afterToken = sourceCode.getTokenAfter(node.typeAnnotation); + if ( + afterToken && + afterToken.type === 'Punctuator' && + afterToken.value === ';' + ) { + fixes.push(fixer.remove(afterToken)); + } + + return fixes; + }, + }); + } + }, + TSInterfaceDeclaration(node) { + if (option === 'type') { + context.report({ + node: node.id, + messageId: 'typeOverInterface', + fix(fixer) { + const typeNode = node.typeParameters || node.id; + const fixes: TSESLint.RuleFix[] = []; + + const firstToken = sourceCode.getFirstToken(node); + if (firstToken) { + fixes.push(fixer.replaceText(firstToken, 'type')); + fixes.push( + fixer.replaceTextRange( + [typeNode.range[1], node.body.range[0]], + ' = ', + ), + ); + } + + if (node.extends) { + node.extends.forEach(heritage => { + const typeIdentifier = sourceCode.getText(heritage); + fixes.push( + fixer.insertTextAfter(node.body, ` & ${typeIdentifier}`), + ); + }); + } + + return fixes; + }, + }); + } + }, + }; + }, +}); diff --git a/packages/eslint-plugin/src/rules/index.ts b/packages/eslint-plugin/src/rules/index.ts index ec00de43335..3b799b1cc80 100644 --- a/packages/eslint-plugin/src/rules/index.ts +++ b/packages/eslint-plugin/src/rules/index.ts @@ -5,6 +5,7 @@ import banTsIgnore from './ban-ts-ignore'; import banTypes from './ban-types'; import camelcase from './camelcase'; import classNameCasing from './class-name-casing'; +import consistentTypeDefinitions from './consistent-type-definitions'; import explicitFunctionReturnType from './explicit-function-return-type'; import explicitMemberAccessibility from './explicit-member-accessibility'; import funcCallSpacing from './func-call-spacing'; @@ -63,6 +64,7 @@ export default { 'ban-types': banTypes, camelcase: camelcase, 'class-name-casing': classNameCasing, + 'consistent-type-definitions': consistentTypeDefinitions, 'explicit-function-return-type': explicitFunctionReturnType, 'explicit-member-accessibility': explicitMemberAccessibility, 'func-call-spacing': funcCallSpacing, diff --git a/packages/eslint-plugin/src/rules/prefer-interface.ts b/packages/eslint-plugin/src/rules/prefer-interface.ts index 3197d579540..555e9646b2b 100644 --- a/packages/eslint-plugin/src/rules/prefer-interface.ts +++ b/packages/eslint-plugin/src/rules/prefer-interface.ts @@ -16,6 +16,8 @@ export default util.createRule({ interfaceOverType: 'Use an interface instead of a type literal.', }, schema: [], + deprecated: true, + replacedBy: ['consistent-type-definitions'], }, defaultOptions: [], create(context) { diff --git a/packages/eslint-plugin/tests/rules/consistent-type-definitions.test.ts b/packages/eslint-plugin/tests/rules/consistent-type-definitions.test.ts new file mode 100644 index 00000000000..b5daf40a90e --- /dev/null +++ b/packages/eslint-plugin/tests/rules/consistent-type-definitions.test.ts @@ -0,0 +1,209 @@ +import rule from '../../src/rules/consistent-type-definitions'; +import { RuleTester } from '../RuleTester'; + +const ruleTester = new RuleTester({ + parser: '@typescript-eslint/parser', +}); + +ruleTester.run('consistent-type-definitions', rule, { + valid: [ + { + code: `var foo = { };`, + options: ['interface'], + }, + { + code: `interface A {}`, + options: ['interface'], + }, + { + code: `interface A extends B { x: number; }`, + options: ['interface'], + }, + { + code: `type U = string;`, + options: ['interface'], + }, + { + code: `type V = { x: number; } | { y: string; };`, + options: ['interface'], + }, + { + code: ` +type Record = { + [K in T]: U; +} +`, + options: ['interface'], + }, + { + code: `type V = { x: number; } | { y: string; };`, + options: ['interface'], + }, + { + code: `type T = { x: number; }`, + options: ['type'], + }, + { + code: `type T = { x: number; }`, + options: ['type'], + }, + { + code: `type T = { x: number; }`, + options: ['type'], + }, + { + code: `type A = { x: number; } & B & C;`, + options: ['type'], + }, + { + code: `type A = { x: number; } & B & C;`, + options: ['type'], + }, + { + code: ` +export type W = { + x: T, +}; +`, + options: ['type'], + }, + ], + invalid: [ + { + code: `type T = { x: number; };`, + output: `interface T { x: number; }`, + options: ['interface'], + errors: [ + { + messageId: 'interfaceOverType', + line: 1, + column: 6, + }, + ], + }, + { + code: `type T={ x: number; };`, + output: `interface T { x: number; }`, + options: ['interface'], + errors: [ + { + messageId: 'interfaceOverType', + line: 1, + column: 6, + }, + ], + }, + { + code: `type T= { x: number; };`, + output: `interface T { x: number; }`, + options: ['interface'], + errors: [ + { + messageId: 'interfaceOverType', + line: 1, + column: 6, + }, + ], + }, + { + code: ` +export type W = { + x: T, +}; +`, + output: ` +export interface W { + x: T, +} +`, + options: ['interface'], + errors: [ + { + messageId: 'interfaceOverType', + line: 2, + column: 13, + }, + ], + }, + { + code: `interface T { x: number; }`, + output: `type T = { x: number; }`, + options: ['type'], + errors: [ + { + messageId: 'typeOverInterface', + line: 1, + column: 11, + }, + ], + }, + { + code: `interface T{ x: number; }`, + output: `type T = { x: number; }`, + options: ['type'], + errors: [ + { + messageId: 'typeOverInterface', + line: 1, + column: 11, + }, + ], + }, + { + code: `interface T { x: number; }`, + output: `type T = { x: number; }`, + options: ['type'], + errors: [ + { + messageId: 'typeOverInterface', + line: 1, + column: 11, + }, + ], + }, + { + code: `interface A extends B, C { x: number; };`, + output: `type A = { x: number; } & B & C;`, + options: ['type'], + errors: [ + { + messageId: 'typeOverInterface', + line: 1, + column: 11, + }, + ], + }, + { + code: `interface A extends B, C { x: number; };`, + output: `type A = { x: number; } & B & C;`, + options: ['type'], + errors: [ + { + messageId: 'typeOverInterface', + line: 1, + column: 11, + }, + ], + }, + { + code: ` +export interface W { + x: T, +}; +`, + output: ` +export type W = { + x: T, +}; +`, + options: ['type'], + errors: [ + { + messageId: 'typeOverInterface', + line: 2, + column: 18, + }, + ], + }, + ], +}); diff --git a/packages/eslint-plugin/tools/validate-docs/check-for-rule-docs.ts b/packages/eslint-plugin/tools/validate-docs/check-for-rule-docs.ts index 6012d46a3f1..eeb67c9d528 100644 --- a/packages/eslint-plugin/tools/validate-docs/check-for-rule-docs.ts +++ b/packages/eslint-plugin/tools/validate-docs/check-for-rule-docs.ts @@ -3,8 +3,8 @@ import fs from 'fs'; import path from 'path'; import { logRule } from './log'; -function checkForRuleDocs( - rules: Record>, +function checkForRuleDocs( + rules: Record>>, ): boolean { const ruleDocs = new Set( fs.readdirSync(path.resolve(__dirname, '../../docs/rules')), diff --git a/packages/eslint-plugin/tools/validate-docs/validate-table-rules.ts b/packages/eslint-plugin/tools/validate-docs/validate-table-rules.ts index 5268bc67556..1ab532e3160 100644 --- a/packages/eslint-plugin/tools/validate-docs/validate-table-rules.ts +++ b/packages/eslint-plugin/tools/validate-docs/validate-table-rules.ts @@ -5,8 +5,8 @@ import marked from 'marked'; import path from 'path'; import { logRule } from './log'; -function validateTableRules( - rules: Record>, +function validateTableRules( + rules: Record>>, rulesTable: marked.Tokens.Table, ): boolean { let hasErrors = false; diff --git a/packages/eslint-plugin/tools/validate-docs/validate-table-structure.ts b/packages/eslint-plugin/tools/validate-docs/validate-table-structure.ts index 4b5bb4151f4..02da5a66151 100644 --- a/packages/eslint-plugin/tools/validate-docs/validate-table-structure.ts +++ b/packages/eslint-plugin/tools/validate-docs/validate-table-structure.ts @@ -3,11 +3,13 @@ import chalk from 'chalk'; import marked from 'marked'; import { logError } from './log'; -function validateTableStructure( - rules: Record>, +function validateTableStructure( + rules: Record>>, rulesTable: marked.Tokens.Table, ): boolean { - const ruleNames = Object.keys(rules).sort(); + const ruleNames = Object.keys(rules) + .filter(ruleName => rules[ruleName].meta.deprecated !== true) + .sort(); let hasErrors = false; rulesTable.cells.forEach((row, rowIndex) => { diff --git a/packages/experimental-utils/src/ts-eslint/Rule.ts b/packages/experimental-utils/src/ts-eslint/Rule.ts index 388f64e99fc..2759a3d33d2 100644 --- a/packages/experimental-utils/src/ts-eslint/Rule.ts +++ b/packages/experimental-utils/src/ts-eslint/Rule.ts @@ -62,7 +62,7 @@ interface RuleMetaData { /** * The name of the rule this rule was replaced by, if it was deprecated. */ - replacedBy?: string; + replacedBy?: string[]; /** * The options schema. Supply an empty array if there are no options. */