From ab7a2d8d0545d765dcd6318416977c617fbe4ff2 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 --- package.json | 4 +- src/rules/no-duplicates.js | 249 +++++++++++++++++++-- tests/src/rules/no-duplicates.js | 365 ++++++++++++++++++++++++++++--- 3 files changed, 564 insertions(+), 54 deletions(-) diff --git a/package.json b/package.json index 299cf52b0c..5548158a45 100644 --- a/package.json +++ b/package.json @@ -25,8 +25,8 @@ "watch": "npm run tests-only -- -- --watch", "pretest": "linklocal", "posttest": "eslint . && npm run update:eslint-docs -- --check", - "mocha": "cross-env BABEL_ENV=test nyc mocha", - "tests-only": "npm run mocha tests/src", + "mocha": "cross-env BABEL_ENV=test nyc mocha --watch", + "tests-only": "npm run mocha tests/src/rules/no-duplicates", "test": "npm run tests-only", "test-compiled": "npm run prepublish && BABEL_ENV=testCompiled mocha --compilers js:babel-register tests/src", "test-all": "node --require babel-register ./scripts/testAll", diff --git a/src/rules/no-duplicates.js b/src/rules/no-duplicates.js index e2df4afdb4..03100f96a2 100644 --- a/src/rules/no-duplicates.js +++ b/src/rules/no-duplicates.js @@ -27,11 +27,198 @@ function checkImports(imported, context) { message, }); } + + } + } +} + +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 getFix(first, rest, sourceCode, context) { +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 @@ -112,26 +299,18 @@ function getFix(first, rest, sourceCode, context) { isPunctuator(sourceCode.getTokenBefore(closeBrace), ','); const firstIsEmpty = !hasSpecifiers(first); - 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 && (!typescriptPkg || !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},${insertText}` - : `${result}${insertText}`, - specifier.isEmpty ? needsComma : true, - ]; - }, - ['', !firstHasTrailingComma && !firstIsEmpty], - ); + const [specifiersText] = specifiers + .reduce( + ([result, needsComma], specifier) => { + return [ + needsComma && !specifier.isEmpty + ? `${result},${specifier.text}` + : `${result}${specifier.text}`, + specifier.isEmpty ? needsComma : true, + ]; + }, + ['', !firstHasTrailingComma && !firstIsEmpty], + ); const fixes = []; @@ -158,7 +337,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)); } @@ -303,14 +482,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; @@ -335,6 +518,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 f8a27a743b..8a37478a55 100644 --- a/tests/src/rules/no-duplicates.js +++ b/tests/src/rules/no-duplicates.js @@ -106,7 +106,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.'], @@ -424,12 +424,12 @@ import {x,y} from './foo' import { BULK_ACTIONS_ENABLED } from '../constants'; - + const TestComponent = () => { return
; } - + export default TestComponent; `, output: ` @@ -440,12 +440,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."], @@ -507,15 +507,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, @@ -523,9 +514,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: [ { @@ -575,11 +584,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, @@ -593,11 +603,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, @@ -611,28 +660,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, @@ -641,15 +731,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, @@ -658,15 +750,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,