From 182bf9aa042f76101252a0f8f0efc522014036dd Mon Sep 17 00:00:00 2001 From: yosuke ota Date: Fri, 7 Aug 2020 00:09:22 +0900 Subject: [PATCH] feat(eslint-plugin): add `consistent-type-imports` rule --- packages/eslint-plugin/README.md | 1 + .../docs/rules/consistent-type-imports.md | 52 ++++ packages/eslint-plugin/src/configs/all.ts | 1 + .../src/rules/consistent-type-imports.ts | 153 +++++++++++ packages/eslint-plugin/src/rules/index.ts | 2 + .../rules/consistent-type-imports.test.ts | 250 ++++++++++++++++++ 6 files changed, 459 insertions(+) create mode 100644 packages/eslint-plugin/docs/rules/consistent-type-imports.md create mode 100644 packages/eslint-plugin/src/rules/consistent-type-imports.ts create mode 100644 packages/eslint-plugin/tests/rules/consistent-type-imports.test.ts diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index 6786bedba5e5..8d6c3229e50d 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/class-literal-property-style`](./docs/rules/class-literal-property-style.md) | Ensures that literals on classes are exposed in a consistent style | | :wrench: | | | [`@typescript-eslint/consistent-type-assertions`](./docs/rules/consistent-type-assertions.md) | Enforces consistent usage of type assertions | | | | | [`@typescript-eslint/consistent-type-definitions`](./docs/rules/consistent-type-definitions.md) | Consistent with type definition either `interface` or `type` | | :wrench: | | +| [`@typescript-eslint/consistent-type-imports`](./docs/rules/consistent-type-imports.md) | Enforces consistent usage of type imports | | :wrench: | | | [`@typescript-eslint/explicit-function-return-type`](./docs/rules/explicit-function-return-type.md) | Require explicit return types on functions and class methods | | | | | [`@typescript-eslint/explicit-member-accessibility`](./docs/rules/explicit-member-accessibility.md) | Require explicit accessibility modifiers on class properties and methods | | :wrench: | | | [`@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 | :heavy_check_mark: | | | diff --git a/packages/eslint-plugin/docs/rules/consistent-type-imports.md b/packages/eslint-plugin/docs/rules/consistent-type-imports.md new file mode 100644 index 000000000000..c60f3b9cd1cd --- /dev/null +++ b/packages/eslint-plugin/docs/rules/consistent-type-imports.md @@ -0,0 +1,52 @@ +# Enforces consistent usage of type imports (`consistent-type-imports`) + +## Rule Details + +This rule aims to standardize the use of type imports style across the codebase. + +```ts +import type { Foo } from './foo'; +let foo: Foo; +``` + +```ts +import { Foo } from './foo'; +let foo: Foo; +``` + +```ts +let foo: import('foo').Foo; +``` + +## Options + +```ts +type Options = + | 'type-imports' + | 'no-type-imports' + | { + prefer: 'type-imports' | 'no-type-imports'; + disallowTypeAnnotations: boolean; + }; + +const defaultOptions: Options = { + prefer: 'type-imports', + disallowTypeAnnotations: true, +}; +``` + +### `prefer` + +This option defines the expected import kind for type-only imports. Valid values for `prefer` are: + +- `type-imports` will enforce that you always use `import type Foo from '...'`. It is default. +- `no-type-imports` will enforce that you always use `import Foo from '...'`. + +### `disallowTypeAnnotations` + +If `true`, type imports in type annotations (`import()`) is not allowed. +Default is `true`. + +## When Not To Use It + +If you specifically want to use both import kinds for stylistic reasons, you can disable this rule. diff --git a/packages/eslint-plugin/src/configs/all.ts b/packages/eslint-plugin/src/configs/all.ts index 98455e551562..86e40bab6f2f 100644 --- a/packages/eslint-plugin/src/configs/all.ts +++ b/packages/eslint-plugin/src/configs/all.ts @@ -18,6 +18,7 @@ export = { '@typescript-eslint/comma-spacing': 'error', '@typescript-eslint/consistent-type-assertions': 'error', '@typescript-eslint/consistent-type-definitions': 'error', + '@typescript-eslint/consistent-type-imports': 'error', 'default-param-last': 'off', '@typescript-eslint/default-param-last': 'error', 'dot-notation': 'off', diff --git a/packages/eslint-plugin/src/rules/consistent-type-imports.ts b/packages/eslint-plugin/src/rules/consistent-type-imports.ts new file mode 100644 index 000000000000..cbe71d916698 --- /dev/null +++ b/packages/eslint-plugin/src/rules/consistent-type-imports.ts @@ -0,0 +1,153 @@ +import { TSESTree } from '@typescript-eslint/experimental-utils'; +import * as util from '../util'; + +type Prefer = 'type-imports' | 'no-type-imports'; + +type Options = [ + | Prefer + | { + prefer?: Prefer; + disallowTypeAnnotations?: boolean; + }, +]; +type MessageIds = 'typeOverValue' | 'valueOverType' | 'noImportTypeAnnotations'; +export default util.createRule({ + name: 'consistent-type-imports', + meta: { + type: 'suggestion', + docs: { + description: 'Enforces consistent usage of type imports', + category: 'Stylistic Issues', + recommended: false, + }, + messages: { + typeOverValue: 'Use an `import type` instead of an `import`.', + valueOverType: 'Use an `import` instead of an `import type`.', + noImportTypeAnnotations: '`import()` type annotations are forbidden.', + }, + schema: [ + { + oneOf: [ + { + enum: ['type-imports', 'no-type-imports'], + }, + { + type: 'object', + properties: { + prefer: { + enum: ['type-imports', 'no-type-imports'], + }, + disallowTypeAnnotations: { + type: 'boolean', + }, + }, + additionalProperties: false, + }, + ], + }, + ], + fixable: 'code', + }, + + defaultOptions: [ + { + prefer: 'type-imports', + disallowTypeAnnotations: true, + }, + ], + + create(context, [option]) { + const prefer = + (typeof option === 'string' ? option : option.prefer) ?? 'type-imports'; + const disallowTypeAnnotations = + typeof option === 'string' + ? true + : option.disallowTypeAnnotations !== false; + const sourceCode = context.getSourceCode(); + + const allValueImports: TSESTree.ImportDeclaration[] = []; + const referenceIdToDeclMap = new Map< + TSESTree.Identifier, + TSESTree.ImportDeclaration + >(); + return { + ...(prefer === 'type-imports' + ? { + // prefer type imports + 'ImportDeclaration[importKind=value]'( + node: TSESTree.ImportDeclaration, + ): void { + let used = false; + for (const specifier of node.specifiers) { + const id = specifier.local; + const variable = context + .getScope() + .variables.find(v => v.name === id.name)!; + for (const ref of variable.references) { + if (ref.identifier !== id) { + referenceIdToDeclMap.set(ref.identifier, node); + used = true; + } + } + } + if (used) { + allValueImports.push(node); + } + }, + 'TSTypeReference Identifier'(node: TSESTree.Identifier): void { + // Remove type reference ids + referenceIdToDeclMap.delete(node); + }, + 'Program:exit'(): void { + const usedAsValueImports = new Set(referenceIdToDeclMap.values()); + for (const valueImport of allValueImports) { + if (usedAsValueImports.has(valueImport)) { + continue; + } + context.report({ + node: valueImport, + messageId: 'typeOverValue', + fix(fixer) { + // import type Foo from 'foo' + // ^^^^^ insert + const importToken = sourceCode.getFirstToken(valueImport)!; + return fixer.insertTextAfter(importToken, ' type'); + }, + }); + } + }, + } + : { + // prefer no type imports + 'ImportDeclaration[importKind=type]'( + node: TSESTree.ImportDeclaration, + ): void { + context.report({ + node: node, + messageId: 'valueOverType', + fix(fixer) { + // import type Foo from 'foo' + // ^^^^^ remove + const importToken = sourceCode.getFirstToken(node)!; + return fixer.removeRange([ + importToken.range[1], + sourceCode.getTokenAfter(importToken)!.range[1], + ]); + }, + }); + }, + }), + ...(disallowTypeAnnotations + ? { + // disallow `import()` type + TSImportType(node: TSESTree.TSImportType): void { + context.report({ + node: node, + messageId: 'noImportTypeAnnotations', + }); + }, + } + : {}), + }; + }, +}); diff --git a/packages/eslint-plugin/src/rules/index.ts b/packages/eslint-plugin/src/rules/index.ts index 524bc3d59da0..4df9fb63a1d0 100644 --- a/packages/eslint-plugin/src/rules/index.ts +++ b/packages/eslint-plugin/src/rules/index.ts @@ -10,6 +10,7 @@ import commaSpacing from './comma-spacing'; import confusingNonNullAssertionLikeNotEqual from './no-confusing-non-null-assertion'; import consistentTypeAssertions from './consistent-type-assertions'; import consistentTypeDefinitions from './consistent-type-definitions'; +import consistentTypeImports from './consistent-type-imports'; import defaultParamLast from './default-param-last'; import dotNotation from './dot-notation'; import explicitFunctionReturnType from './explicit-function-return-type'; @@ -115,6 +116,7 @@ export default { 'no-confusing-non-null-assertion': confusingNonNullAssertionLikeNotEqual, 'consistent-type-assertions': consistentTypeAssertions, 'consistent-type-definitions': consistentTypeDefinitions, + 'consistent-type-imports': consistentTypeImports, 'default-param-last': defaultParamLast, 'dot-notation': dotNotation, 'explicit-function-return-type': explicitFunctionReturnType, diff --git a/packages/eslint-plugin/tests/rules/consistent-type-imports.test.ts b/packages/eslint-plugin/tests/rules/consistent-type-imports.test.ts new file mode 100644 index 000000000000..33047ea0321e --- /dev/null +++ b/packages/eslint-plugin/tests/rules/consistent-type-imports.test.ts @@ -0,0 +1,250 @@ +import rule from '../../src/rules/consistent-type-imports'; +import { RuleTester } from '../RuleTester'; + +const ruleTester = new RuleTester({ + parser: '@typescript-eslint/parser', + parserOptions: { + ecmaVersion: 2020, + sourceType: 'module', + }, +}); + +ruleTester.run('consistent-type-imports', rule, { + valid: [ + ` + import Foo from 'foo'; + const foo: Foo = new Foo(); + `, + ` + import foo from 'foo'; + const foo: foo.Foo = foo.fn(); + `, + ` + import { A, B } from 'foo'; + const foo: A = B(); + `, + ` + import Foo from 'foo'; + `, + ` + import Foo from 'foo'; + function fn() { + type Foo = {}; + let foo: Foo; + } + `, + ` + import A, { B } from 'foo'; + const b = B; + `, + ` + import A, { B, C as c } from 'foo'; + const d = c; + `, + { + code: ` + let foo: import('foo'); + let bar: import('foo').Bar; + `, + options: [{ disallowTypeAnnotations: false }], + }, + { + code: ` + import Foo from 'foo'; + let foo: Foo; + `, + options: [{ prefer: 'no-type-imports' }], + }, + ], + invalid: [ + { + code: ` + import Foo from 'foo'; + let foo: Foo; + type Bar = Foo; + interface Baz { + foo: Foo; + } + function fn(a: Foo): Foo {} + `, + output: ` + import type Foo from 'foo'; + let foo: Foo; + type Bar = Foo; + interface Baz { + foo: Foo; + } + function fn(a: Foo): Foo {} + `, + errors: [ + { + messageId: 'typeOverValue', + line: 2, + column: 9, + }, + ], + }, + { + code: ` + import Foo from 'foo'; + let foo: Foo; + `, + output: ` + import type Foo from 'foo'; + let foo: Foo; + `, + options: ['type-imports'], + errors: [ + { + messageId: 'typeOverValue', + line: 2, + column: 9, + }, + ], + }, + { + code: ` + import { A, B } from 'foo'; + let foo: A; + let bar: B; + `, + output: ` + import type { A, B } from 'foo'; + let foo: A; + let bar: B; + `, + errors: [ + { + messageId: 'typeOverValue', + line: 2, + column: 9, + }, + ], + }, + { + code: ` + import { A as a, B as b } from 'foo'; + let foo: a; + let bar: b; + `, + output: ` + import type { A as a, B as b } from 'foo'; + let foo: a; + let bar: b; + `, + errors: [ + { + messageId: 'typeOverValue', + line: 2, + column: 9, + }, + ], + }, + { + code: ` + import A, { B } from 'foo'; + let foo: A; + let bar: B; + `, + output: ` + import type A, { B } from 'foo'; + let foo: A; + let bar: B; + `, + errors: [ + { + messageId: 'typeOverValue', + line: 2, + column: 9, + }, + ], + }, + { + code: ` + let foo: import('foo'); + let bar: import('foo').Bar; + `, + output: null, + errors: [ + { + messageId: 'noImportTypeAnnotations', + line: 2, + column: 18, + }, + { + messageId: 'noImportTypeAnnotations', + line: 3, + column: 18, + }, + ], + }, + { + code: ` + let foo: import('foo'); + `, + output: null, + options: ['type-imports'], + errors: [ + { + messageId: 'noImportTypeAnnotations', + line: 2, + column: 18, + }, + ], + }, + { + code: ` + import type Foo from 'foo'; + let foo: Foo; + `, + options: [{ prefer: 'no-type-imports' }], + output: ` + import Foo from 'foo'; + let foo: Foo; + `, + errors: [ + { + messageId: 'valueOverType', + line: 2, + column: 9, + }, + ], + }, + { + code: ` + import type Foo from 'foo'; + let foo: Foo; + `, + options: ['no-type-imports'], + output: ` + import Foo from 'foo'; + let foo: Foo; + `, + errors: [ + { + messageId: 'valueOverType', + line: 2, + column: 9, + }, + ], + }, + { + code: ` + import type { Foo } from 'foo'; + let foo: Foo; + `, + options: ['no-type-imports'], + output: ` + import { Foo } from 'foo'; + let foo: Foo; + `, + errors: [ + { + messageId: 'valueOverType', + line: 2, + column: 9, + }, + ], + }, + ], +});