From 6304ddc70fc187e248aa65c69bc8983c5051ecd3 Mon Sep 17 00:00:00 2001 From: Scott Newcomer Date: Fri, 10 Jun 2022 07:08:47 -0500 Subject: [PATCH] [New] `no-duplicates`: support inline type import with `inlineTypeImport` option --- CHANGELOG.md | 3 + docs/rules/no-duplicates.md | 27 +++ src/rules/no-duplicates.js | 25 ++- tests/src/rules/no-duplicates.js | 307 ++++++++++++++++++++++--------- 4 files changed, 267 insertions(+), 95 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b247632c2..3e8e88924 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange - Add [`no-empty-named-blocks`] rule ([#2568], thanks [@guilhermelimak]) - [`prefer-default-export`]: add "target" option ([#2602], thanks [@azyzz228]) - [`no-absolute-path`]: add fixer ([#2613], thanks [@adipascu]) +- [`no-duplicates`]: support inline type import with `inlineTypeImport` option ([#2475], thanks [@snewcomer]) ### Fixed - [`order`]: move nested imports closer to main import entry ([#2396], thanks [@pri1311]) @@ -1045,6 +1046,7 @@ for info on changes for earlier releases. [#2506]: https://github.com/import-js/eslint-plugin-import/pull/2506 [#2503]: https://github.com/import-js/eslint-plugin-import/pull/2503 [#2490]: https://github.com/import-js/eslint-plugin-import/pull/2490 +[#2475]: https://github.com/import-js/eslint-plugin-import/pull/2475 [#2473]: https://github.com/import-js/eslint-plugin-import/pull/2473 [#2466]: https://github.com/import-js/eslint-plugin-import/pull/2466 [#2459]: https://github.com/import-js/eslint-plugin-import/pull/2459 @@ -1760,6 +1762,7 @@ for info on changes for earlier releases. [@singles]: https://github.com/singles [@skozin]: https://github.com/skozin [@skyrpex]: https://github.com/skyrpex +[@snewcomer]: https://github.com/snewcomer [@sompylasar]: https://github.com/sompylasar [@soryy708]: https://github.com/soryy708 [@sosukesuzuki]: https://github.com/sosukesuzuki diff --git a/docs/rules/no-duplicates.md b/docs/rules/no-duplicates.md index 3ca8d1af2..553fbbcc3 100644 --- a/docs/rules/no-duplicates.md +++ b/docs/rules/no-duplicates.md @@ -67,6 +67,33 @@ import SomeDefaultClass from './mod?minify' import * from './mod.js?minify' ``` +### Inline Type imports + +TypeScript 4.5 introduced a new [feature](https://devblogs.microsoft.com/typescript/announcing-typescript-4-5/#type-on-import-names) that allows mixing of named value and type imports. In order to support fixing to an inline type import when duplicate imports are detected, `prefer-inline` can be set to true. + +Config: + +```json +"import/no-duplicates": ["error", {"prefer-inline": true}] +``` + + + +❌ Invalid `["error", "prefer-inline"]` + +```js +import { AValue, type AType } from './mama-mia' +import type { BType } from './mama-mia' +``` + +✅ Valid with `["error", "prefer-inline"]` + +```js +import { AValue, type AType, type BType } from './mama-mia' +``` + + + ## When Not To Use It If the core ESLint version is good enough (i.e. you're _not_ using Flow and you _are_ using [`import/extensions`](./extensions.md)), keep it and don't use this. diff --git a/src/rules/no-duplicates.js b/src/rules/no-duplicates.js index 4aec2d1e7..b896f442a 100644 --- a/src/rules/no-duplicates.js +++ b/src/rules/no-duplicates.js @@ -1,5 +1,7 @@ import resolve from 'eslint-module-utils/resolve'; import docsUrl from '../docsUrl'; +import semver from 'semver'; +import typescriptPkg from 'typescript/package.json'; function checkImports(imported, context) { for (const [module, nodes] of imported.entries()) { @@ -7,7 +9,7 @@ function checkImports(imported, context) { const message = `'${module}' imported multiple times.`; const [first, ...rest] = nodes; const sourceCode = context.getSourceCode(); - const fix = getFix(first, rest, sourceCode); + const fix = getFix(first, rest, sourceCode, context); context.report({ node: first.source, @@ -25,7 +27,7 @@ function checkImports(imported, context) { } } -function getFix(first, rest, sourceCode) { +function getFix(first, rest, sourceCode, context) { // Sorry ESLint <= 3 users, no autofix for you. Autofixing duplicate imports // requires multiple `fixer.whatever()` calls in the `fix`: We both need to // update the first one, and remove the rest. Support for multiple @@ -108,10 +110,19 @@ function getFix(first, rest, sourceCode) { const [specifiersText] = specifiers.reduce( ([result, needsComma], specifier) => { + const isTypeSpecifier = specifier.importNode.importKind === 'type'; + + const preferInline = context.options[0] && context.options[0]['prefer-inline']; + // a user might set prefer-inline but not have a supporting TypeScript version. Flow does not support inline types so this should fail in that case as well. + if (preferInline && !semver.satisfies(typescriptPkg.version, '>= 4.5')) { + throw new Error('Your version of TypeScript does not support inline type imports.'); + } + + const insertText = `${preferInline && isTypeSpecifier ? 'type ' : ''}${specifier.text}`; return [ needsComma && !specifier.isEmpty - ? `${result},${specifier.text}` - : `${result}${specifier.text}`, + ? `${result},${insertText}` + : `${result}${insertText}`, specifier.isEmpty ? needsComma : true, ]; }, @@ -257,6 +268,9 @@ module.exports = { considerQueryString: { type: 'boolean', }, + 'prefer-inline': { + type: 'boolean', + }, }, additionalProperties: false, }, @@ -291,6 +305,9 @@ module.exports = { if (n.importKind === 'type') { return n.specifiers.length > 0 && n.specifiers[0].type === 'ImportDefaultSpecifier' ? map.defaultTypesImported : map.namedTypesImported; } + if (n.specifiers.some((spec) => spec.importKind === 'type')) { + return map.namedTypesImported; + } return hasNamespace(n) ? map.nsImported : map.imported; } diff --git a/tests/src/rules/no-duplicates.js b/tests/src/rules/no-duplicates.js index 14a9f2009..f8a27a743 100644 --- a/tests/src/rules/no-duplicates.js +++ b/tests/src/rules/no-duplicates.js @@ -1,5 +1,5 @@ import * as path from 'path'; -import { test as testUtil, getNonDefaultParsers, parsers } from '../utils'; +import { test as testUtil, getNonDefaultParsers, parsers, tsVersionSatisfies, typescriptEslintParserSatisfies } from '../utils'; import jsxConfig from '../../../config/react'; import { RuleTester } from 'eslint'; @@ -467,99 +467,224 @@ context('TypeScript', function () { }, }; - ruleTester.run('no-duplicates', rule, { - valid: [ + const valid = [ // #1667: ignore duplicate if is a typescript type import - test({ - code: "import type { x } from './foo'; import y from './foo'", - ...parserConfig, - }), - test({ - code: "import type x from './foo'; import type y from './bar'", - ...parserConfig, - }), - test({ - code: "import type {x} from './foo'; import type {y} from './bar'", - ...parserConfig, - }), - test({ - code: "import type x from './foo'; import type {y} from './foo'", - ...parserConfig, - }), - test({ - code: ` - import type {} from './module'; - import {} from './module2'; - `, - ...parserConfig, - }), - test({ - code: ` + test({ + code: "import type { x } from './foo'; import y from './foo'", + ...parserConfig, + }), + test({ + code: "import type x from './foo'; import type y from './bar'", + ...parserConfig, + }), + test({ + code: "import type {x} from './foo'; import type {y} from './bar'", + ...parserConfig, + }), + test({ + code: "import type x from './foo'; import type {y} from './foo'", + ...parserConfig, + }), + test({ + code: ` + import type {} from './module'; + import {} from './module2'; + `, + ...parserConfig, + }), + test({ + code: ` + import type { Identifier } from 'module'; + + declare module 'module2' { + import type { Identifier } from 'module'; + } + + declare module 'module3' { import type { Identifier } from 'module'; + } + `, + ...parserConfig, + }), + ].concat(!tsVersionSatisfies('>= 4.5') || !typescriptEslintParserSatisfies('>= 5.7.0') ? [] : [ + // #2470: ignore duplicate if is a typescript inline type import + test({ + code: "import { type x } from './foo'; import y from './foo'", + ...parserConfig, + }), + test({ + code: "import { type x } from './foo'; import { y } from './foo'", + ...parserConfig, + }), + test({ + code: "import { type x } from './foo'; import type y from 'foo'", + ...parserConfig, + }), + ]); + + const invalid = [ + test({ + code: "import type x from './foo'; import type y from './foo'", + output: "import type x from './foo'; import type y from './foo'", + ...parserConfig, + errors: [ + { + line: 1, + column: 20, + message: "'./foo' imported multiple times.", + }, + { + line: 1, + column: 48, + message: "'./foo' imported multiple times.", + }, + ], + }), + test({ + code: "import type x from './foo'; import type x from './foo'", + output: "import type x from './foo'; ", + ...parserConfig, + errors: [ + { + line: 1, + column: 20, + message: "'./foo' imported multiple times.", + }, + { + line: 1, + column: 48, + message: "'./foo' imported multiple times.", + }, + ], + }), + test({ + code: "import type {x} from './foo'; import type {y} from './foo'", + ...parserConfig, + output: `import type {x,y} from './foo'; `, + errors: [ + { + line: 1, + column: 22, + message: "'./foo' imported multiple times.", + }, + { + line: 1, + column: 52, + message: "'./foo' imported multiple times.", + }, + ], + }), + ].concat(!tsVersionSatisfies('>= 4.5') || !typescriptEslintParserSatisfies('>= 5.7.0') ? [] : [ + test({ + code: "import {type x} from './foo'; import type {y} from './foo'", + ...parserConfig, + options: [{ 'prefer-inline': false }], + output: `import {type x,y} from './foo'; `, + errors: [ + { + line: 1, + column: 22, + message: "'./foo' imported multiple times.", + }, + { + line: 1, + column: 52, + message: "'./foo' imported multiple times.", + }, + ], + }), + test({ + code: "import {type x} from 'foo'; import type {y} from 'foo'", + ...parserConfig, + options: [{ 'prefer-inline': true }], + output: `import {type x,type y} from 'foo'; `, + errors: [ + { + line: 1, + column: 22, + message: "'foo' imported multiple times.", + }, + { + line: 1, + column: 50, + message: "'foo' imported multiple times.", + }, + ], + }), + test({ + code: "import {type x} from 'foo'; import type {y} from 'foo'", + ...parserConfig, + output: `import {type x,y} from 'foo'; `, + errors: [ + { + line: 1, + column: 22, + message: "'foo' imported multiple times.", + }, + { + line: 1, + column: 50, + message: "'foo' imported multiple times.", + }, + ], + }), + test({ + code: "import {type x} from './foo'; import {type y} from './foo'", + ...parserConfig, + options: [{ 'prefer-inline': true }], + output: `import {type x,type y} from './foo'; `, + errors: [ + { + line: 1, + column: 22, + message: "'./foo' imported multiple times.", + }, + { + line: 1, + column: 52, + message: "'./foo' imported multiple times.", + }, + ], + }), + test({ + code: "import {type x} from './foo'; import {type y} from './foo'", + ...parserConfig, + output: `import {type x,type y} from './foo'; `, + errors: [ + { + line: 1, + column: 22, + message: "'./foo' imported multiple times.", + }, + { + line: 1, + column: 52, + message: "'./foo' imported multiple times.", + }, + ], + }), + test({ + code: "import {AValue, type x, BValue} from './foo'; import {type y} from './foo'", + ...parserConfig, + output: `import {AValue, type x, BValue,type y} from './foo'; `, + errors: [ + { + line: 1, + column: 38, + message: "'./foo' imported multiple times.", + }, + { + line: 1, + column: 68, + message: "'./foo' imported multiple times.", + }, + ], + }), + ]); - declare module 'module2' { - import type { Identifier } from 'module'; - } - - declare module 'module3' { - import type { Identifier } from 'module'; - } - `, - ...parserConfig, - }), - ], - invalid: [ - test({ - code: "import type x from './foo'; import type y from './foo'", - ...parserConfig, - errors: [ - { - line: 1, - column: 20, - message: "'./foo' imported multiple times.", - }, - { - line: 1, - column: 48, - message: "'./foo' imported multiple times.", - }, - ], - }), - test({ - code: "import type x from './foo'; import type x from './foo'", - output: "import type x from './foo'; ", - ...parserConfig, - errors: [ - { - line: 1, - column: 20, - message: "'./foo' imported multiple times.", - }, - { - line: 1, - column: 48, - message: "'./foo' imported multiple times.", - }, - ], - }), - test({ - code: "import type {x} from './foo'; import type {y} from './foo'", - ...parserConfig, - output: `import type {x,y} from './foo'; `, - errors: [ - { - line: 1, - column: 22, - message: "'./foo' imported multiple times.", - }, - { - line: 1, - column: 52, - message: "'./foo' imported multiple times.", - }, - ], - }), - ], + ruleTester.run('no-duplicates', rule, { + valid, + invalid, }); }); });