[Fix] no-duplicates: remove duplicate identifiers in duplicate imports

Author: jportner

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

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 = [];

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';   ",