From c4e3a231798bfa36d6f5e839e877ff9aa75655bc Mon Sep 17 00:00:00 2001 From: yeonjuan Date: Tue, 29 Sep 2020 14:17:42 +0900 Subject: [PATCH 1/5] feat(eslint-plugin): add extension rule `no-duplicate-imports` --- packages/eslint-plugin/README.md | 1 + .../docs/rules/no-duplicate-imports.md | 22 ++++ packages/eslint-plugin/src/configs/all.ts | 6 +- packages/eslint-plugin/src/rules/index.ts | 2 + .../src/rules/no-duplicate-imports.ts | 118 ++++++++++++++++++ .../tests/rules/no-duplicate-imports.test.ts | 109 ++++++++++++++++ .../eslint-plugin/typings/eslint-rules.d.ts | 26 ++++ 7 files changed, 282 insertions(+), 2 deletions(-) create mode 100644 packages/eslint-plugin/docs/rules/no-duplicate-imports.md create mode 100644 packages/eslint-plugin/src/rules/no-duplicate-imports.ts create mode 100644 packages/eslint-plugin/tests/rules/no-duplicate-imports.test.ts diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index 37a27ca9511f..aa76a8d592d1 100644 --- a/packages/eslint-plugin/README.md +++ b/packages/eslint-plugin/README.md @@ -199,6 +199,7 @@ In these cases, we create what we call an extension rule; a rule within our plug | [`@typescript-eslint/lines-between-class-members`](./docs/rules/lines-between-class-members.md) | Require or disallow an empty line between class members | | :wrench: | | | [`@typescript-eslint/no-array-constructor`](./docs/rules/no-array-constructor.md) | Disallow generic `Array` constructors | :heavy_check_mark: | :wrench: | | | [`@typescript-eslint/no-dupe-class-members`](./docs/rules/no-dupe-class-members.md) | Disallow duplicate class members | | | | +| [`@typescript-eslint/no-duplicate-imports`](./docs/rules/no-duplicate-imports.md) | Disallow duplicate imports | | | | | [`@typescript-eslint/no-empty-function`](./docs/rules/no-empty-function.md) | Disallow empty functions | :heavy_check_mark: | | | | [`@typescript-eslint/no-extra-parens`](./docs/rules/no-extra-parens.md) | Disallow unnecessary parentheses | | :wrench: | | | [`@typescript-eslint/no-extra-semi`](./docs/rules/no-extra-semi.md) | Disallow unnecessary semicolons | :heavy_check_mark: | :wrench: | | diff --git a/packages/eslint-plugin/docs/rules/no-duplicate-imports.md b/packages/eslint-plugin/docs/rules/no-duplicate-imports.md new file mode 100644 index 000000000000..140a8a1ece14 --- /dev/null +++ b/packages/eslint-plugin/docs/rules/no-duplicate-imports.md @@ -0,0 +1,22 @@ +# Disallow duplicate imports (`no-duplicate-imports`) + +## Rule Details + +This rule extends the base [`eslint/no-duplicate-imports`](https://eslint.org/docs/rules/no-duplicate-imports) rule. +This version adds support for type-only import and export. + +## How to use + +```jsonc +{ + // note you must disable the base rule as it can report incorrect errors + "no-duplicate-imports": "off", + "@typescript-eslint/no-duplicate-imports": ["error"] +} +``` + +## Options + +See [`eslint/no-duplicate-imports` options](https://eslint.org/docs/rules/no-duplicate-imports#options). + +Taken with ❤️ [from ESLint core](https://github.com/eslint/eslint/blob/master/docs/rules/no-duplicate-imports.md) diff --git a/packages/eslint-plugin/src/configs/all.ts b/packages/eslint-plugin/src/configs/all.ts index 84b4e93fc90c..e32731950ffa 100644 --- a/packages/eslint-plugin/src/configs/all.ts +++ b/packages/eslint-plugin/src/configs/all.ts @@ -14,6 +14,8 @@ export = { 'brace-style': 'off', '@typescript-eslint/brace-style': 'error', '@typescript-eslint/class-literal-property-style': 'error', + 'comma-dangle': 'off', + '@typescript-eslint/comma-dangle': 'error', 'comma-spacing': 'off', '@typescript-eslint/comma-spacing': 'error', '@typescript-eslint/consistent-type-assertions': 'error', @@ -46,6 +48,8 @@ export = { '@typescript-eslint/no-confusing-non-null-assertion': 'error', 'no-dupe-class-members': 'off', '@typescript-eslint/no-dupe-class-members': 'error', + 'no-duplicate-imports': 'off', + '@typescript-eslint/no-duplicate-imports': 'error', '@typescript-eslint/no-dynamic-delete': 'error', 'no-empty-function': 'off', '@typescript-eslint/no-empty-function': 'error', @@ -139,7 +143,5 @@ export = { '@typescript-eslint/typedef': 'error', '@typescript-eslint/unbound-method': 'error', '@typescript-eslint/unified-signatures': 'error', - 'comma-dangle': 'off', - '@typescript-eslint/comma-dangle': 'error', }, }; diff --git a/packages/eslint-plugin/src/rules/index.ts b/packages/eslint-plugin/src/rules/index.ts index 8b5049e25022..35cf3f044445 100644 --- a/packages/eslint-plugin/src/rules/index.ts +++ b/packages/eslint-plugin/src/rules/index.ts @@ -105,6 +105,7 @@ import typeAnnotationSpacing from './type-annotation-spacing'; import typedef from './typedef'; import unboundMethod from './unbound-method'; import unifiedSignatures from './unified-signatures'; +import noDuplicateimports from './no-duplicate-imports'; export default { 'adjacent-overload-signatures': adjacentOverloadSignatures, @@ -210,6 +211,7 @@ export default { 'type-annotation-spacing': typeAnnotationSpacing, 'unbound-method': unboundMethod, 'unified-signatures': unifiedSignatures, + 'no-duplicate-imports': noDuplicateimports, indent: indent, quotes: quotes, semi: semi, diff --git a/packages/eslint-plugin/src/rules/no-duplicate-imports.ts b/packages/eslint-plugin/src/rules/no-duplicate-imports.ts new file mode 100644 index 000000000000..d539845746f7 --- /dev/null +++ b/packages/eslint-plugin/src/rules/no-duplicate-imports.ts @@ -0,0 +1,118 @@ +import { + AST_NODE_TYPES, + TSESTree, +} from '@typescript-eslint/experimental-utils'; +import baseRule from 'eslint/lib/rules/no-duplicate-imports'; +import * as util from '../util'; + +type Options = util.InferOptionsTypeFromRule; +type MessageIds = util.InferMessageIdsTypeFromRule; + +export default util.createRule({ + name: 'no-duplicate-imports', + meta: { + type: 'problem', + docs: { + description: 'Disallow duplicate imports', + category: 'Best Practices', + recommended: false, + extendsBaseRule: true, + }, + schema: baseRule.meta.schema, + messages: { + ...baseRule.meta.messages, + importType: '{{module}} type import is duplicated', + importTypeAs: '{{module}} type import is duplicated as type export', + exportType: '{{module}} type export is duplicated', + exportTypeAs: '{{module}} type export is duplicated as type import', + }, + }, + defaultOptions: [ + { + includeExports: false, + }, + ], + create(context, [option]) { + const rules = baseRule.create(context); + const includeExports = option.includeExports; + const typeImports = new Set(); + const typeExports = new Set(); + + function report( + messageId: MessageIds, + node: TSESTree.Node, + module: string, + ): void { + context.report({ + messageId, + node, + data: { + module, + }, + }); + } + + function isStringLiteral( + node: TSESTree.Node | null, + ): node is TSESTree.StringLiteral { + return ( + !!node && + node.type === AST_NODE_TYPES.Literal && + typeof node.value === 'string' + ); + } + + function checkTypeImport(node: TSESTree.ImportDeclaration): void { + if (isStringLiteral(node.source)) { + const value = node.source.value; + if (typeImports.has(value)) { + report('importType', node, value); + } + if (includeExports && typeExports.has(value)) { + report('importTypeAs', node, value); + } + typeImports.add(value); + } + } + + function checkTypeExport( + node: TSESTree.ExportNamedDeclaration | TSESTree.ExportAllDeclaration, + ): void { + if (isStringLiteral(node.source)) { + const value = node.source.value; + if (typeExports.has(value)) { + report('exportType', node, value); + } + if (typeImports.has(value)) { + report('exportTypeAs', node, value); + } + typeExports.add(value); + } + } + + return { + ...rules, + ImportDeclaration(node): void { + if (node.importKind === 'type') { + checkTypeImport(node); + return; + } + rules.ImportDeclaration(node); + }, + ExportNamedDeclaration(node): void { + if (includeExports && node.exportKind === 'type') { + checkTypeExport(node); + return; + } + rules.ExportNamedDeclaration?.(node); + }, + ExportAllDeclaration(node): void { + if (includeExports && node.exportKind === 'type') { + checkTypeExport(node); + return; + } + rules.ExportAllDeclaration?.(node); + }, + }; + }, +}); diff --git a/packages/eslint-plugin/tests/rules/no-duplicate-imports.test.ts b/packages/eslint-plugin/tests/rules/no-duplicate-imports.test.ts new file mode 100644 index 000000000000..439a18086b1f --- /dev/null +++ b/packages/eslint-plugin/tests/rules/no-duplicate-imports.test.ts @@ -0,0 +1,109 @@ +import rule from '../../src/rules/no-duplicate-imports'; +import { RuleTester } from '../RuleTester'; + +const ruleTester = new RuleTester({ + parser: '@typescript-eslint/parser', +}); + +ruleTester.run('no-dupe-class-members', rule, { + valid: [ + { + code: "import type foo from 'foo';", + }, + { + code: "import type { foo } from 'foo';", + }, + { + code: ` + import foo from 'foo'; + import type bar from 'foo'; + `, + }, + { + code: ` + import { foo } from 'foo'; + import type { bar } from 'foo'; + `, + }, + { + code: ` + import type { foo } from 'foo'; + export type foo = foo; + `, + }, + { + code: ` + import type { foo } from 'foo'; + export type { foo }; + `, + }, + { + code: ` + export { foo } from 'foo'; + export type { foo } from 'foo'; + `, + }, + { + code: ` + export type * as foo from 'foo'; + export type * as bar from 'foo'; + `, + }, + ], + invalid: [ + { + code: ` + import type foo from 'foo'; + import type bar from 'foo'; + `, + errors: [ + { + messageId: 'importType', + data: { + module: 'foo', + }, + }, + ], + }, + { + code: ` + import type { foo } from 'foo'; + import type { bar } from 'foo'; + `, + errors: [{ messageId: 'importType' }], + }, + { + code: ` + import type foo from 'foo'; + export type * from 'foo'; + `, + options: [{ includeExports: true }], + errors: [{ messageId: 'exportTypeAs' }], + }, + { + code: ` + import type { foo } from 'foo'; + export type { foo } from 'foo'; + `, + options: [{ includeExports: true }], + errors: [{ messageId: 'exportTypeAs' }], + }, + { + code: ` + export type * as foo from 'foo'; + export type * as bar from 'foo'; + `, + options: [{ includeExports: true }], + errors: [{ messageId: 'exportType' }], + }, + + // check base rule + { + code: ` + import foo from 'foo'; + import bar from 'foo'; + `, + errors: [{ messageId: 'import' }], + }, + ], +}); diff --git a/packages/eslint-plugin/typings/eslint-rules.d.ts b/packages/eslint-plugin/typings/eslint-rules.d.ts index 39d49db7328e..d11c5589b384 100644 --- a/packages/eslint-plugin/typings/eslint-rules.d.ts +++ b/packages/eslint-plugin/typings/eslint-rules.d.ts @@ -762,3 +762,29 @@ declare module 'eslint/lib/rules/comma-dangle' { >; export = rule; } + +declare module 'eslint/lib/rules/no-duplicate-imports' { + import { TSESLint, TSESTree } from '@typescript-eslint/experimental-utils'; + + const rule: TSESLint.RuleModule< + | 'import' + | 'importAs' + | 'export' + | 'exportAs' + | 'importType' + | 'importTypeAs' + | 'exportType' + | 'exportTypeAs', + [ + { + includeExports?: boolean; + }, + ], + { + ImportDeclaration(node: TSESTree.ImportDeclaration): void; + ExportNamedDeclaration?(node: TSESTree.ExportNamedDeclaration): void; + ExportAllDeclaration?(node: TSESTree.ExportAllDeclaration): void; + } + >; + export = rule; +} From 9b65f592261e261f2c6b1d801c3c46199e51c10b Mon Sep 17 00:00:00 2001 From: yeonjuan Date: Tue, 29 Sep 2020 14:20:10 +0900 Subject: [PATCH 2/5] to camelcase --- packages/eslint-plugin/src/rules/index.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/eslint-plugin/src/rules/index.ts b/packages/eslint-plugin/src/rules/index.ts index 35cf3f044445..6fa670df871d 100644 --- a/packages/eslint-plugin/src/rules/index.ts +++ b/packages/eslint-plugin/src/rules/index.ts @@ -105,7 +105,7 @@ import typeAnnotationSpacing from './type-annotation-spacing'; import typedef from './typedef'; import unboundMethod from './unbound-method'; import unifiedSignatures from './unified-signatures'; -import noDuplicateimports from './no-duplicate-imports'; +import noDuplicateImports from './no-duplicate-imports'; export default { 'adjacent-overload-signatures': adjacentOverloadSignatures, @@ -211,7 +211,7 @@ export default { 'type-annotation-spacing': typeAnnotationSpacing, 'unbound-method': unboundMethod, 'unified-signatures': unifiedSignatures, - 'no-duplicate-imports': noDuplicateimports, + 'no-duplicate-imports': noDuplicateImports, indent: indent, quotes: quotes, semi: semi, From 5b3592f5ae86af30f4d55a76334d3233673e4b56 Mon Sep 17 00:00:00 2001 From: yeonjuan Date: Tue, 29 Sep 2020 15:36:41 +0900 Subject: [PATCH 3/5] add test case --- .../tests/rules/no-duplicate-imports.test.ts | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/packages/eslint-plugin/tests/rules/no-duplicate-imports.test.ts b/packages/eslint-plugin/tests/rules/no-duplicate-imports.test.ts index 439a18086b1f..de956c0fa5d2 100644 --- a/packages/eslint-plugin/tests/rules/no-duplicate-imports.test.ts +++ b/packages/eslint-plugin/tests/rules/no-duplicate-imports.test.ts @@ -49,6 +49,19 @@ ruleTester.run('no-dupe-class-members', rule, { export type * as bar from 'foo'; `, }, + { + code: ` + import type { bar } from 'foo'; + export type { foo } from 'foo'; + `, + }, + { + code: ` + import type { foo } from 'foo'; + export type { bar } from 'bar'; + `, + options: [{includeExports: true}], + }, ], invalid: [ { @@ -72,6 +85,14 @@ ruleTester.run('no-dupe-class-members', rule, { `, errors: [{ messageId: 'importType' }], }, + { + code: ` + export type { foo } from 'foo'; + import type { bar } from 'foo'; + `, + options: [{includeExports: true}], + errors: [{ messageId: 'importTypeAs' }], + }, { code: ` import type foo from 'foo'; From 85dc819c5ef9ad7bb224f8d986339b158b1339c4 Mon Sep 17 00:00:00 2001 From: yeonjuan Date: Tue, 29 Sep 2020 15:44:52 +0900 Subject: [PATCH 4/5] fix lint --- .../eslint-plugin/tests/rules/no-duplicate-imports.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/eslint-plugin/tests/rules/no-duplicate-imports.test.ts b/packages/eslint-plugin/tests/rules/no-duplicate-imports.test.ts index de956c0fa5d2..089c3817a466 100644 --- a/packages/eslint-plugin/tests/rules/no-duplicate-imports.test.ts +++ b/packages/eslint-plugin/tests/rules/no-duplicate-imports.test.ts @@ -60,7 +60,7 @@ ruleTester.run('no-dupe-class-members', rule, { import type { foo } from 'foo'; export type { bar } from 'bar'; `, - options: [{includeExports: true}], + options: [{ includeExports: true }], }, ], invalid: [ @@ -90,7 +90,7 @@ ruleTester.run('no-dupe-class-members', rule, { export type { foo } from 'foo'; import type { bar } from 'foo'; `, - options: [{includeExports: true}], + options: [{ includeExports: true }], errors: [{ messageId: 'importTypeAs' }], }, { From d8aa340d102b1270913dd7f65c083edf1a192ebd Mon Sep 17 00:00:00 2001 From: yeonjuan Date: Tue, 29 Sep 2020 16:35:06 +0900 Subject: [PATCH 5/5] add test cases --- .../tests/rules/no-duplicate-imports.test.ts | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/packages/eslint-plugin/tests/rules/no-duplicate-imports.test.ts b/packages/eslint-plugin/tests/rules/no-duplicate-imports.test.ts index 089c3817a466..d133a7aadd9e 100644 --- a/packages/eslint-plugin/tests/rules/no-duplicate-imports.test.ts +++ b/packages/eslint-plugin/tests/rules/no-duplicate-imports.test.ts @@ -62,6 +62,13 @@ ruleTester.run('no-dupe-class-members', rule, { `, options: [{ includeExports: true }], }, + { + code: ` + import type { foo } from 'foo'; + export type { bar }; + `, + options: [{ includeExports: true }], + }, ], invalid: [ { @@ -126,5 +133,21 @@ ruleTester.run('no-dupe-class-members', rule, { `, errors: [{ messageId: 'import' }], }, + { + code: ` + import foo from 'foo'; + export * from 'foo'; + `, + options: [{ includeExports: true }], + errors: [{ messageId: 'exportAs' }], + }, + { + code: ` + import foo from 'foo'; + export { foo } from 'foo'; + `, + options: [{ includeExports: true }], + errors: [{ messageId: 'exportAs' }], + }, ], });