diff --git a/CHANGELOG.md b/CHANGELOG.md index 894aa97b3..8186d8785 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,9 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange ## [Unreleased] +### Fixed +- [`no-duplicates`]: remove duplicate identifiers in duplicate imports ([#2577], thanks [@joe-matsec]) + ### Changed - [Docs] [`no-duplicates`]: fix example schema ([#2684], thanks [@simmo]) @@ -1389,6 +1392,7 @@ for info on changes for earlier releases. [#2668]: https://github.com/import-js/eslint-plugin-import/issues/2668 [#2666]: https://github.com/import-js/eslint-plugin-import/issues/2666 [#2665]: https://github.com/import-js/eslint-plugin-import/issues/2665 +[#2577]: https://github.com/import-js/eslint-plugin-import/issues/2577 [#2444]: https://github.com/import-js/eslint-plugin-import/issues/2444 [#2412]: https://github.com/import-js/eslint-plugin-import/issues/2412 [#2392]: https://github.com/import-js/eslint-plugin-import/issues/2392 @@ -1703,6 +1707,7 @@ for info on changes for earlier releases. [@jimbolla]: https://github.com/jimbolla [@jkimbo]: https://github.com/jkimbo [@joaovieira]: https://github.com/joaovieira +[@joe-matsec]: https://github.com/joe-matsec [@johndevedu]: https://github.com/johndevedu [@johnthagen]: https://github.com/johnthagen [@jonboiser]: https://github.com/jonboiser diff --git a/src/rules/no-duplicates.js b/src/rules/no-duplicates.js index e2df4afdb..15515e675 100644 --- a/src/rules/no-duplicates.js +++ b/src/rules/no-duplicates.js @@ -79,8 +79,7 @@ function getFix(first, rest, sourceCode, context) { return { importNode: node, - text: sourceCode.text.slice(openBrace.range[1], closeBrace.range[0]), - hasTrailingComma: isPunctuator(sourceCode.getTokenBefore(closeBrace), ','), + identifiers: sourceCode.text.slice(openBrace.range[1], closeBrace.range[0]).split(','), // Split the text into separate identifiers (retaining any whitespace before or after) isEmpty: !hasSpecifiers(node), }; }) @@ -111,9 +110,15 @@ function getFix(first, rest, sourceCode, context) { closeBrace != null && isPunctuator(sourceCode.getTokenBefore(closeBrace), ','); const firstIsEmpty = !hasSpecifiers(first); + const firstExistingIdentifiers = firstIsEmpty + ? new Set() + : new Set(sourceCode.text.slice(openBrace.range[1], closeBrace.range[0]) + .split(',') + .map((x) => x.trim()), + ); const [specifiersText] = specifiers.reduce( - ([result, needsComma], specifier) => { + ([result, needsComma, existingIdentifiers], specifier) => { const isTypeSpecifier = specifier.importNode.importKind === 'type'; const preferInline = context.options[0] && context.options[0]['prefer-inline']; @@ -122,15 +127,25 @@ function getFix(first, rest, sourceCode, context) { throw new Error('Your version of TypeScript does not support inline type imports.'); } - const insertText = `${preferInline && isTypeSpecifier ? 'type ' : ''}${specifier.text}`; + // 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)]; + }, ['', existingIdentifiers]); + return [ - needsComma && !specifier.isEmpty - ? `${result},${insertText}` - : `${result}${insertText}`, + needsComma && !specifier.isEmpty && specifierText.length > 0 + ? `${result},${specifierText}` + : `${result}${specifierText}`, specifier.isEmpty ? needsComma : true, + updatedExistingIdentifiers, ]; }, - ['', !firstHasTrailingComma && !firstIsEmpty], + ['', !firstHasTrailingComma && !firstIsEmpty, firstExistingIdentifiers], ); const fixes = []; diff --git a/tests/src/rules/no-duplicates.js b/tests/src/rules/no-duplicates.js index f8a27a743..ac76c3070 100644 --- a/tests/src/rules/no-duplicates.js +++ b/tests/src/rules/no-duplicates.js @@ -5,6 +5,7 @@ import jsxConfig from '../../../config/react'; import { RuleTester } from 'eslint'; import eslintPkg from 'eslint/package.json'; import semver from 'semver'; +import flatMap from 'array.prototype.flatmap'; const ruleTester = new RuleTester(); const rule = require('rules/no-duplicates'); @@ -130,6 +131,36 @@ ruleTester.run('no-duplicates', rule, { errors: ['\'./foo\' imported multiple times.', '\'./foo\' imported multiple times.'], }), + // These test cases use duplicate import identifiers, which causes a fatal parsing error using ESPREE (default) and TS_OLD. + ...flatMap([parsers.BABEL_OLD, parsers.TS_NEW], parser => { + if (!parser) return []; // TS_NEW is not always available + return [ + // #2347: duplicate identifiers should be removed + test({ + code: "import {a} from './foo'; import { a } from './foo'", + output: "import {a} from './foo'; ", + errors: ['\'./foo\' imported multiple times.', '\'./foo\' imported multiple times.'], + parser, + }), + + // #2347: duplicate identifiers should be removed + test({ + code: "import {a,b} from './foo'; import { b, c } from './foo'; import {b,c,d} from './foo'", + output: "import {a,b, c ,d} from './foo'; ", + errors: ['\'./foo\' imported multiple times.', '\'./foo\' imported multiple times.', '\'./foo\' imported multiple times.'], + parser, + }), + + // #2347: duplicate identifiers should be removed, but not if they are adjacent to comments + test({ + code: "import {a} from './foo'; import { a/*,b*/ } from './foo'", + output: "import {a, a/*,b*/ } from './foo'; ", + errors: ['\'./foo\' imported multiple times.', '\'./foo\' imported multiple times.'], + parser, + }), + ]; + }), + test({ code: "import {x} from './foo'; import {} from './foo'; import {/*c*/} from './foo'; import {y} from './foo'", output: "import {x/*c*/,y} from './foo'; ",