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

import-js · Oct 21, 2022 · 2b41e42 · 2b41e42
1 parent c2f003a
commit 2b41e42
Show file tree

Hide file tree

Showing 2 changed files with 49 additions and 7 deletions.
diff --git a/src/rules/no-duplicates.js b/src/rules/no-duplicates.js
@@ -75,7 +75,7 @@ function getFix(first, rest, sourceCode, context) {
 
       return {
         importNode: node,
-        text: sourceCode.text.slice(openBrace.range[1], closeBrace.range[0]),
+        identifiers: sourceCode.text.slice(openBrace.range[1], closeBrace.range[0]).split(','), // Split the text into separate identifiers (retaining any whitespace before or after)
         hasTrailingComma: isPunctuator(sourceCode.getTokenBefore(closeBrace), ','),
         isEmpty: !hasSpecifiers(node),
       };
@@ -107,9 +107,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'];
@@ -118,15 +124,24 @@ 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
+          if (existingIdentifiers.has(trimmed)) {
+            return [text, set];
+          }
+          return [text.length > 0 ? `${preferInline && isTypeSpecifier ? 'type ' : ''}${text},${cur}` : cur, 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
@@ -130,6 +130,33 @@ 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.
+    ...[parsers.BABEL_OLD, parsers.TS_NEW].flatMap((parser => [
+      // #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';   ",