From 3e7321608c41490eba27b3b56f96a2ec8899c5ea Mon Sep 17 00:00:00 2001 From: Scott Newcomer Date: Sun, 29 Jan 2023 08:20:15 -0600 Subject: [PATCH 01/10] [Fix]: no-duplicates with type imports --- src/rules/no-duplicates.js | 233 ++++++++++++++++++-- tests/src/rules/no-duplicates.js | 365 ++++++++++++++++++++++++++++--- 2 files changed, 553 insertions(+), 45 deletions(-) diff --git a/src/rules/no-duplicates.js b/src/rules/no-duplicates.js index 6b4f4d559..3eb14635f 100644 --- a/src/rules/no-duplicates.js +++ b/src/rules/no-duplicates.js @@ -23,6 +23,59 @@ function checkImports(imported, context) { fix, // Attach the autofix (if any) to the first import. }); + for (const node of rest) { + context.report({ + node: node.source, + 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 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, @@ -33,7 +86,141 @@ function checkImports(imported, context) { } } -function getFix(first, rest, sourceCode, context) { +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 @@ -115,22 +302,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 [ @@ -169,7 +347,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)); } @@ -314,15 +492,18 @@ module.exports = { nsImported: new Map(), defaultTypesImported: new Map(), namedTypesImported: new Map(), + inlineTypesImported: new Map(), }); } const map = moduleMaps.get(n.parent); - const preferInline = context.options[0] && context.options[0]['prefer-inline']; - if (!preferInline && n.importKind === 'type') { + 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 (!preferInline && n.specifiers.some((spec) => spec.importKind === 'type')) { - return map.namedTypesImported; + + if (n.specifiers.some((spec) => spec.importKind === 'type')) { + // import { type foo } + return map.inlineTypesImported; } return hasNamespace(n) ? map.nsImported : map.imported; @@ -347,6 +528,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 d61fda86e..5e921a6a2 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, From 5592e87d6672d66494aa6989bc456f968f8cfbbe Mon Sep 17 00:00:00 2001 From: Scott Newcomer Date: Sun, 19 Feb 2023 13:32:13 -0600 Subject: [PATCH 02/10] Update src/rules/no-duplicates.js Co-authored-by: Jordan Harband --- src/rules/no-duplicates.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/rules/no-duplicates.js b/src/rules/no-duplicates.js index 3eb14635f..020c81fc6 100644 --- a/src/rules/no-duplicates.js +++ b/src/rules/no-duplicates.js @@ -76,12 +76,12 @@ function checkInlineTypeImports(imported, context) { fix, // Attach the autofix (if any) to the first import. }); - for (const node of rest) { + rest.forEach((node) => { context.report({ node: node.source, message, }); - } + }); } } } From eadc5523457c3fd924d384805354bcabcab09a90 Mon Sep 17 00:00:00 2001 From: Scott Newcomer Date: Sun, 19 Feb 2023 13:32:21 -0600 Subject: [PATCH 03/10] Update src/rules/no-duplicates.js Co-authored-by: Jordan Harband --- src/rules/no-duplicates.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/rules/no-duplicates.js b/src/rules/no-duplicates.js index 020c81fc6..b802bca64 100644 --- a/src/rules/no-duplicates.js +++ b/src/rules/no-duplicates.js @@ -111,19 +111,19 @@ function getInlineTypeFix(nodes, sourceCode) { const nodeClosingBrace = nodeTokens.find(token => isPunctuator(token, '}')); // const preferInline = context.options[0] && context.options[0]['prefer-inline']; if (nodeClosingBrace) { - for (const node of rest) { + rest.forEach((node) => { // these will be all Type imports, no Value specifiers // then add inline type specifiers to importKind === 'type' import - for (const specifier of node.specifiers) { + node.specifiers.forEach((specifier) => { 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'); From 8a8655bdce5ae813c24caf65585953f015df2648 Mon Sep 17 00:00:00 2001 From: Scott Newcomer Date: Sun, 19 Feb 2023 13:32:28 -0600 Subject: [PATCH 04/10] Update src/rules/no-duplicates.js Co-authored-by: Jordan Harband --- src/rules/no-duplicates.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rules/no-duplicates.js b/src/rules/no-duplicates.js index b802bca64..a06be47c7 100644 --- a/src/rules/no-duplicates.js +++ b/src/rules/no-duplicates.js @@ -539,7 +539,7 @@ module.exports = { }); checkInlineTypeImports(duplicatedImports, context); - const duplicatedTypeImports = new Map([...map.inlineTypesImported]); + const duplicatedTypeImports = new Map(map.inlineTypesImported); map.namedTypesImported.forEach((value, key) => { if (duplicatedTypeImports.has(key)) { duplicatedTypeImports.get(key).push(...value); From 965a6ba7ea22ffc11fad41aaa5b05d7f44f5b9ca Mon Sep 17 00:00:00 2001 From: Scott Newcomer Date: Sun, 19 Feb 2023 13:32:33 -0600 Subject: [PATCH 05/10] Update src/rules/no-duplicates.js Co-authored-by: Jordan Harband --- src/rules/no-duplicates.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rules/no-duplicates.js b/src/rules/no-duplicates.js index a06be47c7..da7147a84 100644 --- a/src/rules/no-duplicates.js +++ b/src/rules/no-duplicates.js @@ -529,7 +529,7 @@ module.exports = { checkImports(map.defaultTypesImported, context); checkImports(map.namedTypesImported, context); - const duplicatedImports = new Map([...map.inlineTypesImported]); + const duplicatedImports = new Map(map.inlineTypesImported); map.imported.forEach((value, key) => { if (duplicatedImports.has(key)) { duplicatedImports.get(key).push(...value); From b7b9a560a9a3cf2872dbb782dd19afdd6ad5f7b3 Mon Sep 17 00:00:00 2001 From: Scott Newcomer Date: Sun, 19 Feb 2023 13:32:40 -0600 Subject: [PATCH 06/10] Update src/rules/no-duplicates.js Co-authored-by: Jordan Harband --- src/rules/no-duplicates.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/rules/no-duplicates.js b/src/rules/no-duplicates.js index da7147a84..d2a296bbc 100644 --- a/src/rules/no-duplicates.js +++ b/src/rules/no-duplicates.js @@ -51,12 +51,12 @@ function checkTypeImports(imported, context) { fix, // Attach the autofix (if any) to the first import. }); - for (const node of rest) { + rest.forEach((node) => { context.report({ node: node.source, message, }); - } + }); } } } From 085adc814964e2823c59a07afb4ea4edace07be5 Mon Sep 17 00:00:00 2001 From: Scott Newcomer Date: Sun, 19 Feb 2023 15:51:34 -0600 Subject: [PATCH 07/10] one bugfix with trailing commas --- src/rules/no-duplicates.js | 15 ++++++------ tests/src/rules/no-duplicates.js | 39 ++++++++++++++++++++++++++++---- 2 files changed, 43 insertions(+), 11 deletions(-) diff --git a/src/rules/no-duplicates.js b/src/rules/no-duplicates.js index d2a296bbc..f2b621d11 100644 --- a/src/rules/no-duplicates.js +++ b/src/rules/no-duplicates.js @@ -94,13 +94,10 @@ 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')); + // const valueImport = nodes.find((n) => n.specifiers.every((spec) => spec.importKind === 'value')) || nodes.find((n) => n.specifiers.some((spec) => spec.type === 'ImportDefaultSpecifier')); + const valueImport = nodes.find((n) => n.specifiers.some((spec) => spec.type === 'ImportDefaultSpecifier')); if (valueImport) { firstImport = valueImport; rest = nodes.filter((n) => n !== firstImport); @@ -108,9 +105,13 @@ function getInlineTypeFix(nodes, sourceCode) { 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']; + const nodeClosingBraceIndex = nodeTokens.findIndex(token => isPunctuator(token, '}')); + const nodeClosingBrace = nodeTokens[nodeClosingBraceIndex]; + const tokenBeforeClosingBrace = nodeTokens[nodeClosingBraceIndex - 1]; if (nodeClosingBrace) { + if (rest.length && isComma(tokenBeforeClosingBrace)) { + fixes.push(fixer.remove(tokenBeforeClosingBrace)); + } rest.forEach((node) => { // these will be all Type imports, no Value specifiers // then add inline type specifiers to importKind === 'type' import diff --git a/tests/src/rules/no-duplicates.js b/tests/src/rules/no-duplicates.js index 5e921a6a2..bf017a09b 100644 --- a/tests/src/rules/no-duplicates.js +++ b/tests/src/rules/no-duplicates.js @@ -614,6 +614,23 @@ context('TypeScript', function () { }, ], }), + test({ + code: "import {type x} from './foo'; import {y} from './foo'", + ...parserConfig, + output: `import {type x, y} from './foo'; `, + errors: [ + { + line: 1, + column: 22, + message: "'./foo' imported multiple times.", + }, + { + line: 1, + column: 47, + message: "'./foo' imported multiple times.", + }, + ], + }), ].concat(!tsVersionSatisfies('>= 4.5') || !typescriptEslintParserSatisfies('>= 5.7.0') ? [] : [ // without prefer-inline, will dedupe with type import kind test({ @@ -1018,20 +1035,34 @@ context('TypeScript', function () { }, ], }), - // #2834 Detect duplicates across type and regular imports test({ - code: "import {AValue} from './foo'; import type {AType} from './foo'", + code: "import { type C, } from './foo';import {AValue, BValue, } from './foo';", ...parserConfig, options: [{ 'prefer-inline': true }], - output: `import {AValue,type AType} from './foo'; `, + output: "import { type C , AValue, BValue} from './foo';", errors: [ { line: 1, - column: 22, + column: 25, message: "'./foo' imported multiple times.", }, { line: 1, + column: 64, + message: "'./foo' imported multiple times.", + } + ], + }), + // #2834 Detect duplicates across type and regular imports + test({ + code: "import {AValue} from './foo'; import type {AType} from './foo'", + ...parserConfig, + options: [{ 'prefer-inline': true }], + output: `import {AValue,type AType} from './foo'; `, + errors: [ + { + line: 1, + column: 22, column: 56, message: "'./foo' imported multiple times.", }, From 59e433235c72c5ac1729fdabebdca4b6e7ca7194 Mon Sep 17 00:00:00 2001 From: Scott Newcomer Date: Sun, 19 Feb 2023 15:55:57 -0600 Subject: [PATCH 08/10] try this way instead --- src/rules/no-duplicates.js | 14 +++++++++----- tests/src/rules/no-duplicates.js | 2 +- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/src/rules/no-duplicates.js b/src/rules/no-duplicates.js index f2b621d11..6bf5a93fc 100644 --- a/src/rules/no-duplicates.js +++ b/src/rules/no-duplicates.js @@ -109,22 +109,26 @@ function getInlineTypeFix(nodes, sourceCode) { const nodeClosingBrace = nodeTokens[nodeClosingBraceIndex]; const tokenBeforeClosingBrace = nodeTokens[nodeClosingBraceIndex - 1]; if (nodeClosingBrace) { - if (rest.length && isComma(tokenBeforeClosingBrace)) { - fixes.push(fixer.remove(tokenBeforeClosingBrace)); - } + const specifiers = []; rest.forEach((node) => { // these will be all Type imports, no Value specifiers // then add inline type specifiers to importKind === 'type' import node.specifiers.forEach((specifier) => { if (specifier.importKind === 'type') { - fixes.push(fixer.insertTextBefore(nodeClosingBrace, `, type ${specifier.local.name}`)); + specifiers.push(`type ${specifier.local.name}`); } else { - fixes.push(fixer.insertTextBefore(nodeClosingBrace, `, ${specifier.local.name}`)); + specifiers.push(specifier.local.name); } }); fixes.push(fixer.remove(node)); }); + + if (isComma(tokenBeforeClosingBrace)) { + fixes.push(fixer.insertTextBefore(nodeClosingBrace, ` ${specifiers.join(', ')}`)); + } else { + fixes.push(fixer.insertTextBefore(nodeClosingBrace, `, ${specifiers.join(', ')}`)); + } } else { // we have a default import only const defaultSpecifier = firstImport.specifiers.find((spec) => spec.type === 'ImportDefaultSpecifier'); diff --git a/tests/src/rules/no-duplicates.js b/tests/src/rules/no-duplicates.js index bf017a09b..86acfac2a 100644 --- a/tests/src/rules/no-duplicates.js +++ b/tests/src/rules/no-duplicates.js @@ -1039,7 +1039,7 @@ context('TypeScript', function () { code: "import { type C, } from './foo';import {AValue, BValue, } from './foo';", ...parserConfig, options: [{ 'prefer-inline': true }], - output: "import { type C , AValue, BValue} from './foo';", + output: "import { type C, AValue, BValue} from './foo';", errors: [ { line: 1, From 0d2b73739637fdfad37672a7b2e0a1e1993049bc Mon Sep 17 00:00:00 2001 From: Scott Newcomer Date: Mon, 20 Feb 2023 11:47:32 -0600 Subject: [PATCH 09/10] moar feedback --- src/rules/no-duplicates.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/rules/no-duplicates.js b/src/rules/no-duplicates.js index 6bf5a93fc..f9c320c5e 100644 --- a/src/rules/no-duplicates.js +++ b/src/rules/no-duplicates.js @@ -35,11 +35,11 @@ function checkImports(imported, context) { } function checkTypeImports(imported, context) { - for (const [module, nodes] of imported.entries()) { + Array.from(imported).forEach(([module, nodes]) => { 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 someInlineTypeImports = nodes.some((node) => node.specifiers.some((spec) => spec.importKind === 'type')); + if (typeImports.length > 0 && someInlineTypeImports) { const message = `'${module}' imported multiple times.`; const sourceCode = context.getSourceCode(); const fix = getTypeFix(nodes, sourceCode, context); @@ -59,11 +59,11 @@ function checkTypeImports(imported, context) { }); } } - } + }); } function checkInlineTypeImports(imported, context) { - for (const [module, nodes] of imported.entries()) { + Array.from(imported).forEach(([module, nodes]) => { if (nodes.length > 1) { const message = `'${module}' imported multiple times.`; const sourceCode = context.getSourceCode(); @@ -83,7 +83,7 @@ function checkInlineTypeImports(imported, context) { }); }); } - } + }); } function isComma(token) { From 864effb638a796135ef463568335bed92cf19a48 Mon Sep 17 00:00:00 2001 From: Scott Newcomer Date: Fri, 28 Jul 2023 10:00:39 -0500 Subject: [PATCH 10/10] comment out for now --- tests/src/rules/no-duplicates.js | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/tests/src/rules/no-duplicates.js b/tests/src/rules/no-duplicates.js index 86acfac2a..29b3829db 100644 --- a/tests/src/rules/no-duplicates.js +++ b/tests/src/rules/no-duplicates.js @@ -1053,21 +1053,21 @@ context('TypeScript', function () { } ], }), - // #2834 Detect duplicates across type and regular imports - test({ - code: "import {AValue} from './foo'; import type {AType} from './foo'", - ...parserConfig, - options: [{ 'prefer-inline': true }], - output: `import {AValue,type AType} from './foo'; `, - errors: [ - { - line: 1, - column: 22, - column: 56, - message: "'./foo' imported multiple times.", - }, - ], - }), + // // #2834 Detect duplicates across type and regular imports + // test({ + // code: "import {AValue} from './foo'; import type {AType} from './foo'", + // ...parserConfig, + // options: [{ 'prefer-inline': true }], + // output: `import {AValue,type AType} from './foo'; `, + // errors: [ + // { + // line: 1, + // column: 22, + // column: 56, + // message: "'./foo' imported multiple times.", + // }, + // ], + // }), ]); ruleTester.run('no-duplicates', rule, {