From d2292bb7a6eddf37c159d43ec6e7a91c461407e4 Mon Sep 17 00:00:00 2001 From: Scott Newcomer Date: Sun, 29 Jan 2023 08:20:15 -0600 Subject: [PATCH] [Fix]: no-duplicates with type imports --- src/rules/no-duplicates.js | 228 +++++++++++++++++-- tests/src/rules/no-duplicates.js | 365 ++++++++++++++++++++++++++++--- 2 files changed, 551 insertions(+), 42 deletions(-) diff --git a/src/rules/no-duplicates.js b/src/rules/no-duplicates.js index 15515e675..81431d5bf 100644 --- a/src/rules/no-duplicates.js +++ b/src/rules/no-duplicates.js @@ -27,11 +27,198 @@ function checkImports(imported, context) { message, }); } + } } } -function getFix(first, rest, sourceCode, context) { +function checkTypeImports(imported, context) { + for (const [module, nodes] of imported.entries()) { + const typeImports = nodes.filter((node) => node.importKind === 'type'); + if (nodes.length > 1) { + const someInlineTypeImports = nodes.filter((node) => node.specifiers.some((spec) => spec.importKind === 'type')); + if (typeImports.length > 0 && someInlineTypeImports.length > 0) { + const message = `'${module}' imported multiple times.`; + const sourceCode = context.getSourceCode(); + const fix = getTypeFix(nodes, sourceCode, context); + + const [first, ...rest] = nodes; + context.report({ + node: first.source, + message, + fix, // Attach the autofix (if any) to the first import. + }); + + for (const node of rest) { + context.report({ + node: node.source, + message, + }); + } + } + } + } +} + +function checkInlineTypeImports(imported, context) { + for (const [module, nodes] of imported.entries()) { + if (nodes.length > 1) { + const message = `'${module}' imported multiple times.`; + const sourceCode = context.getSourceCode(); + const fix = getInlineTypeFix(nodes, sourceCode); + + const [first, ...rest] = nodes; + context.report({ + node: first.source, + message, + fix, // Attach the autofix (if any) to the first import. + }); + + for (const node of rest) { + context.report({ + node: node.source, + message, + }); + } + } + } +} + +function isComma(token) { + return token.type === 'Punctuator' && token.value === ','; +} + +function getInlineTypeFix(nodes, sourceCode) { + return fixer => { + const fixes = []; + + // if (!semver.satisfies(typescriptPkg.version, '>= 4.5')) { + // throw new Error('Your version of TypeScript does not support inline type imports.'); + // } + + // push to first import + let [firstImport, ...rest] = nodes; + const valueImport = nodes.find((n) => n.specifiers.every((spec) => spec.importKind === 'value')) || nodes.find((n) => n.specifiers.some((spec) => spec.type === 'ImportDefaultSpecifier')); + if (valueImport) { + firstImport = valueImport; + rest = nodes.filter((n) => n !== firstImport); + } + + const nodeTokens = sourceCode.getTokens(firstImport); + // we are moving the rest of the Type or Inline Type imports here. + const nodeClosingBrace = nodeTokens.find(token => isPunctuator(token, '}')); + // const preferInline = context.options[0] && context.options[0]['prefer-inline']; + if (nodeClosingBrace) { + for (const node of rest) { + // these will be all Type imports, no Value specifiers + // then add inline type specifiers to importKind === 'type' import + for (const specifier of node.specifiers) { + if (specifier.importKind === 'type') { + fixes.push(fixer.insertTextBefore(nodeClosingBrace, `, type ${specifier.local.name}`)); + } else { + fixes.push(fixer.insertTextBefore(nodeClosingBrace, `, ${specifier.local.name}`)); + } + } + + fixes.push(fixer.remove(node)); + } + } else { + // we have a default import only + const defaultSpecifier = firstImport.specifiers.find((spec) => spec.type === 'ImportDefaultSpecifier'); + const inlineTypeImports = []; + for (const node of rest) { + // these will be all Type imports, no Value specifiers + // then add inline type specifiers to importKind === 'type' import + for (const specifier of node.specifiers) { + if (specifier.importKind === 'type') { + inlineTypeImports.push(`type ${specifier.local.name}`); + } else { + inlineTypeImports.push(specifier.local.name); + } + } + + fixes.push(fixer.remove(node)); + } + + fixes.push(fixer.insertTextAfter(defaultSpecifier, `, {${inlineTypeImports.join(', ')}}`)); + } + + return fixes; + }; +} + +function getTypeFix(nodes, sourceCode, context) { + return fixer => { + const fixes = []; + + const preferInline = context.options[0] && context.options[0]['prefer-inline']; + + if (preferInline) { + if (!semver.satisfies(typescriptPkg.version, '>= 4.5')) { + throw new Error('Your version of TypeScript does not support inline type imports.'); + } + + // collapse all type imports to the inline type import + const typeImports = nodes.filter((node) => node.importKind === 'type'); + const someInlineTypeImports = nodes.filter((node) => node.specifiers.some((spec) => spec.importKind === 'type')); + // push to first import + const firstImport = someInlineTypeImports[0]; + + if (firstImport) { + const nodeTokens = sourceCode.getTokens(firstImport); + // we are moving the rest of the Type imports here + const nodeClosingBrace = nodeTokens.find(token => isPunctuator(token, '}')); + + for (const node of typeImports) { + for (const specifier of node.specifiers) { + fixes.push(fixer.insertTextBefore(nodeClosingBrace, `, type ${specifier.local.name}`)); + } + + fixes.push(fixer.remove(node)); + } + } + } else { + // move inline types to type imports + const typeImports = nodes.filter((node) => node.importKind === 'type'); + const someInlineTypeImports = nodes.filter((node) => node.specifiers.some((spec) => spec.importKind === 'type')); + + const firstImport = typeImports[0]; + + if (firstImport) { + const nodeTokens = sourceCode.getTokens(firstImport); + // we are moving the rest of the Type imports here + const nodeClosingBrace = nodeTokens.find(token => isPunctuator(token, '}')); + + for (const node of someInlineTypeImports) { + for (const specifier of node.specifiers) { + if (specifier.importKind === 'type') { + fixes.push(fixer.insertTextBefore(nodeClosingBrace, `, ${specifier.local.name}`)); + } + } + + if (node.specifiers.every((spec) => spec.importKind === 'type')) { + fixes.push(fixer.remove(node)); + } else { + for (const specifier of node.specifiers) { + if (specifier.importKind === 'type') { + const maybeComma = sourceCode.getTokenAfter(specifier); + if (isComma(maybeComma)) { + fixes.push(fixer.remove(maybeComma)); + } + // TODO: remove `type`? + fixes.push(fixer.remove(specifier)); + } + } + } + } + } + } + + return fixes; + }; +} + +function getFix(first, rest, sourceCode) { // 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 @@ -119,22 +306,13 @@ function getFix(first, rest, sourceCode, context) { const [specifiersText] = specifiers.reduce( ([result, needsComma, existingIdentifiers], 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 && (!typescriptPkg || !semver.satisfies(typescriptPkg.version, '>= 4.5'))) { - throw new Error('Your version of TypeScript does not support inline type imports.'); - } - // Add *only* the new identifiers that don't already exist, and track any new identifiers so we don't add them again in the next loop const [specifierText, updatedExistingIdentifiers] = specifier.identifiers.reduce(([text, set], cur) => { const trimmed = cur.trim(); // Trim whitespace before/after to compare to our set of existing identifiers - const curWithType = trimmed.length > 0 && preferInline && isTypeSpecifier ? `type ${cur}` : cur; if (existingIdentifiers.has(trimmed)) { return [text, set]; } - return [text.length > 0 ? `${text},${curWithType}` : curWithType, set.add(trimmed)]; + return [text.length > 0 ? `${text},${cur}` : cur, set.add(trimmed)]; }, ['', existingIdentifiers]); return [ @@ -173,7 +351,7 @@ function getFix(first, rest, sourceCode, context) { // `import def from './foo'` → `import def, {...} from './foo'` fixes.push(fixer.insertTextAfter(first.specifiers[0], `, {${specifiersText}}`)); } - } else if (!shouldAddDefault && openBrace != null && closeBrace != null) { + } else if (!shouldAddDefault && openBrace != null && closeBrace != null && specifiersText) { // `import {...} './foo'` → `import {..., ...} from './foo'` fixes.push(fixer.insertTextBefore(closeBrace, specifiersText)); } @@ -318,14 +496,18 @@ module.exports = { nsImported: new Map(), defaultTypesImported: new Map(), namedTypesImported: new Map(), + inlineTypesImported: new Map(), }); } const map = moduleMaps.get(n.parent); if (n.importKind === 'type') { + // import type Foo | import type { foo } 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; + // import { type foo } + return map.inlineTypesImported; } return hasNamespace(n) ? map.nsImported : map.imported; @@ -350,6 +532,26 @@ module.exports = { checkImports(map.nsImported, context); checkImports(map.defaultTypesImported, context); checkImports(map.namedTypesImported, context); + + const duplicatedImports = new Map([...map.inlineTypesImported]); + map.imported.forEach((value, key) => { + if (duplicatedImports.has(key)) { + duplicatedImports.get(key).push(...value); + } else { + duplicatedImports.set(key, [value]); + } + }); + checkInlineTypeImports(duplicatedImports, context); + + const duplicatedTypeImports = new Map([...map.inlineTypesImported]); + map.namedTypesImported.forEach((value, key) => { + if (duplicatedTypeImports.has(key)) { + duplicatedTypeImports.get(key).push(...value); + } else { + duplicatedTypeImports.set(key, value); + } + }); + checkTypeImports(duplicatedTypeImports, context); } }, }; diff --git a/tests/src/rules/no-duplicates.js b/tests/src/rules/no-duplicates.js index ac76c3070..5d9cff418 100644 --- a/tests/src/rules/no-duplicates.js +++ b/tests/src/rules/no-duplicates.js @@ -107,7 +107,7 @@ ruleTester.run('no-duplicates', rule, { }), test({ - code: "import type { x } from './foo'; import type { y } from './foo'", + code: "import type { x } from './foo'; import type { y } from './foo';", output: "import type { x , y } from './foo'; ", parser: parsers.BABEL_OLD, errors: ['\'./foo\' imported multiple times.', '\'./foo\' imported multiple times.'], @@ -455,12 +455,12 @@ import {x,y} from './foo' import { BULK_ACTIONS_ENABLED } from '../constants'; - + const TestComponent = () => { return
; } - + export default TestComponent; `, output: ` @@ -471,12 +471,12 @@ import {x,y} from './foo' BULK_ACTIONS_ENABLED } from '../constants'; import React from 'react'; - + const TestComponent = () => { return
; } - + export default TestComponent; `, errors: ["'../constants' imported multiple times.", "'../constants' imported multiple times."], @@ -538,15 +538,6 @@ context('TypeScript', function () { ...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, @@ -554,9 +545,27 @@ context('TypeScript', function () { ]); const invalid = [ + // if this is what we find, then inline regardless of TS version. We don't convert back to `type { x }` + test({ + code: "import { type x } from './foo'; import y from './foo';", + output: " import y, {type x} from './foo';", + ...parserConfig, + errors: [ + { + line: 1, + column: 24, + message: "'./foo' imported multiple times.", + }, + { + line: 1, + column: 47, + message: "'./foo' imported multiple times.", + }, + ], + }), test({ code: "import type x from './foo'; import type y from './foo'", - output: "import type x from './foo'; import type y from './foo'", + output: "import type x from './foo'; import type y from './foo'", // warn but no fixes ...parserConfig, errors: [ { @@ -606,11 +615,12 @@ context('TypeScript', function () { ], }), ].concat(!tsVersionSatisfies('>= 4.5') || !typescriptEslintParserSatisfies('>= 5.7.0') ? [] : [ + // without prefer-inline, will dedupe with type import kind test({ - code: "import {type x} from './foo'; import type {y} from './foo'", + code: "import type {x} from './foo'; import {type y} from './foo'", ...parserConfig, options: [{ 'prefer-inline': false }], - output: `import {type x,y} from './foo'; `, + output: `import type {x, y} from './foo'; `, errors: [ { line: 1, @@ -624,11 +634,50 @@ context('TypeScript', function () { }, ], }), + // with prefer-inline, will dedupe with inline type specifier + test({ + code: "import type {x} from './foo';import {type y} from './foo';", + ...parserConfig, + options: [{ 'prefer-inline': true }], + output: `import {type y, type x} from './foo';`, + errors: [ + { + line: 1, + column: 22, + message: "'./foo' imported multiple times.", + }, + { + line: 1, + column: 51, + message: "'./foo' imported multiple times.", + }, + ], + }), + // (same as above with imports switched up) without prefer-inline, will dedupe with type import kind + test({ + code: "import {type x} from './foo'; import type {y} from './foo';", + ...parserConfig, + options: [{ 'prefer-inline': false }], + output: ` import type {y, x} from './foo';`, + errors: [ + { + line: 1, + column: 22, + message: "'./foo' imported multiple times.", + }, + { + line: 1, + column: 52, + message: "'./foo' imported multiple times.", + }, + ], + }), + // with prefer-inline, will dedupe with inline type specifier 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'; `, + output: `import {type x, type y} from 'foo'; `, errors: [ { line: 1, @@ -642,28 +691,69 @@ context('TypeScript', function () { }, ], }), + // throw in a Value import test({ - code: "import {type x} from 'foo'; import type {y} from 'foo'", + code: "import {type x, C} from './foo'; import type {y} from './foo';", ...parserConfig, - output: `import {type x,y} from 'foo'; `, + options: [{ 'prefer-inline': false }], + output: `import { C} from './foo'; import type {y, x} from './foo';`, + errors: [ + { + line: 1, + column: 25, + message: "'./foo' imported multiple times.", + }, + { + line: 1, + column: 55, + message: "'./foo' imported multiple times.", + }, + ], + }), + // (same as above but import statements switched) + test({ + code: "import type {y} from './foo'; import {type x, C} from './foo';", + ...parserConfig, + options: [{ 'prefer-inline': false }], + output: `import type {y, x} from './foo'; import { C} from './foo';`, errors: [ { line: 1, column: 22, + message: "'./foo' imported multiple times.", + }, + { + line: 1, + column: 55, + message: "'./foo' imported multiple times.", + }, + ], + }), + // with prefer-inline, will dedupe with inline type specifier + test({ + code: "import {type x, C} from 'foo';import type {y} from 'foo';", + ...parserConfig, + options: [{ 'prefer-inline': true }], + output: `import {type x, C, type y} from 'foo';`, + errors: [ + { + line: 1, + column: 25, message: "'foo' imported multiple times.", }, { line: 1, - column: 50, + column: 52, message: "'foo' imported multiple times.", }, ], }), + // (same as above but import statements switched) test({ - code: "import {type x} from './foo'; import {type y} from './foo'", + code: "import type {y} from './foo'; import {type x, C} from './foo';", ...parserConfig, options: [{ 'prefer-inline': true }], - output: `import {type x,type y} from './foo'; `, + output: ` import {type x, C, type y} from './foo';`, errors: [ { line: 1, @@ -672,15 +762,17 @@ context('TypeScript', function () { }, { line: 1, - column: 52, + column: 55, message: "'./foo' imported multiple times.", }, ], }), + // inlines types will still dedupe without prefer-inline test({ - code: "import {type x} from './foo'; import {type y} from './foo'", + code: `import {type x} from './foo';import {type y} from './foo';`, ...parserConfig, - output: `import {type x,type y} from './foo'; `, + options: [{ 'prefer-inline': false }], + output: `import {type x, type y} from './foo';`, errors: [ { line: 1, @@ -689,15 +781,230 @@ context('TypeScript', function () { }, { line: 1, - column: 52, + column: 51, + message: "'./foo' imported multiple times.", + }, + ], + }), + // inlines types will dedupe with prefer-inline + 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: 51, message: "'./foo' imported multiple times.", }, ], }), + // 3 imports test({ - code: "import {AValue, type x, BValue} from './foo'; import {type y} from './foo'", + code: `import {type x} from './foo';import {type y} from './foo';import {type z} from './foo';`, ...parserConfig, - output: `import {AValue, type x, BValue,type y} from './foo'; `, + output: "import {type x, type y, type z} from './foo';", + errors: [ + { + line: 1, + column: 22, + message: "'./foo' imported multiple times.", + }, + { + line: 1, + column: 51, + message: "'./foo' imported multiple times.", + }, + { + line: 1, + column: 80, + message: "'./foo' imported multiple times.", + }, + ], + }), + // 3 imports with default import + test({ + code: "import {type x} from './foo';import {type y} from './foo';import A from './foo';", + ...parserConfig, + output: "import A, {type x, type y} from './foo';", + errors: [ + { + line: 1, + column: 22, + message: "'./foo' imported multiple times.", + }, + { + line: 1, + column: 51, + message: "'./foo' imported multiple times.", + }, + { + line: 1, + column: 73, + message: "'./foo' imported multiple times.", + }, + ], + }), + // 3 imports with default import + value import + test({ + code: "import {type x} from './foo';import {type y} from './foo';import A, { C } from './foo';", + ...parserConfig, + output: "import A, { C , type x, type y} from './foo';", + errors: [ + { + line: 1, + column: 22, + message: "'./foo' imported multiple times.", + }, + { + line: 1, + column: 51, + message: "'./foo' imported multiple times.", + }, + { + line: 1, + column: 80, + message: "'./foo' imported multiple times.", + }, + ], + }), + // mixed imports - dedupe existing inline types without prefer-inline + test({ + code: "import {AValue, type x, BValue} from './foo';import {type y, CValue} from './foo';", + ...parserConfig, + output: "import {AValue, type x, BValue, type y, CValue} from './foo';", + errors: [ + { + line: 1, + column: 38, + message: "'./foo' imported multiple times.", + }, + { + line: 1, + column: 75, + message: "'./foo' imported multiple times.", + }, + ], + }), + test({ + code: "import AValue from './foo'; import {type y} from './foo';", + ...parserConfig, + output: "import AValue, {type y} from './foo'; ", + errors: [ + { + line: 1, + column: 20, + message: "'./foo' imported multiple times.", + }, + { + line: 1, + column: 50, + message: "'./foo' imported multiple times.", + }, + ], + }), + // switch it up + test({ + code: "import {type y} from './foo';import AValue from './foo';", + ...parserConfig, + output: `import AValue, {type y} from './foo';`, + errors: [ + { + line: 1, + column: 22, + message: "'./foo' imported multiple times.", + }, + { + line: 1, + column: 49, + message: "'./foo' imported multiple times.", + }, + ], + }), + test({ + code: "import AValue, {BValue} from './foo'; import {type y, CValue} from './foo';", + ...parserConfig, + output: "import AValue, {BValue, type y, CValue} from './foo'; ", + errors: [ + { + line: 1, + column: 30, + message: "'./foo' imported multiple times.", + }, + { + line: 1, + column: 68, + message: "'./foo' imported multiple times.", + }, + ], + }), + // will unfurl inline types to type import if not prefer-inline + test({ + code: "import {AValue, type x, BValue} from './foo'; import type {y} from './foo';", + ...parserConfig, + output: "import {AValue, BValue} from './foo'; import type {y, x} from './foo';", + errors: [ + { + line: 1, + column: 38, + message: "'./foo' imported multiple times.", + }, + { + line: 1, + column: 68, + message: "'./foo' imported multiple times.", + }, + ], + }), + // will dedupe inline type imports with prefer-inline + test({ + code: "import {AValue, type x, BValue} from './foo'; import type {y} from './foo'", + ...parserConfig, + options: [{ 'prefer-inline': true }], + 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.", + }, + ], + }), + test({ + code: "import AValue, {type x, BValue} from './foo';import type {y} from './foo';", + ...parserConfig, + options: [{ 'prefer-inline': false }], + // don't want to lose type information + output: "import AValue, { BValue} from './foo';import type {y, x} from './foo';", + errors: [ + { + line: 1, + column: 38, + message: "'./foo' imported multiple times.", + }, + { + line: 1, + column: 67, + message: "'./foo' imported multiple times.", + }, + ], + }), + test({ + code: "import AValue, {type x, BValue} from './foo'; import type {y} from './foo'", + ...parserConfig, + options: [{ 'prefer-inline': true }], + output: `import AValue, {type x, BValue, type y} from './foo'; `, errors: [ { line: 1,