From dfe6ab8f301e60b19adbf908cb59267ce1a7e05b Mon Sep 17 00:00:00 2001 From: Jordan Harband Date: Fri, 22 May 2026 12:18:35 +0200 Subject: [PATCH] [Fix] `no-duplicates`: remove extra space when merging import specifiers When auto-fixing duplicate imports, the no-duplicates rule was using untrimmed specifier strings (with leading spaces from splitting by comma), causing extra spaces. Now uses the trimmed identifier string when building the merged specifier text. Fixes #3222 --- CHANGELOG.md | 3 ++ src/rules/no-duplicates.js | 53 ++++++++++++++++++++++++++++++-- tests/src/rules/no-duplicates.js | 26 ++++++++-------- 3 files changed, 66 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f7829286f0..16da8e207b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange - [`no-duplicates`]: avoid false positives for TypeScript namespace and default type imports that cannot be merged ([#3195]) - ExportMap: resolve export * as ns re-exports under modern parsers ([#3250]) - [`order`]: make the alphabetize comparator transitive for sibling/parent imports in the same group, so the autofix converges under Node 25's updated V8 sort ([#3235]) +- [`no-duplicates`]: remove extra space when merging import specifiers ([#3246], thanks [@mixelburg]) ### Changed - [Docs] `no-unused-modules`: Fix docs of `ignoreUnusedTypeExports` option ([#3233], thanks [@ej612]) @@ -1194,6 +1195,7 @@ for info on changes for earlier releases. [`memo-parser`]: ./memo-parser/README.md [#3250]: https://github.com/import-js/eslint-plugin-import/pull/3250 +[#3246]: https://github.com/import-js/eslint-plugin-import/pull/3246 [#3237]: https://github.com/import-js/eslint-plugin-import/pull/3237 [#3236]: https://github.com/import-js/eslint-plugin-import/pull/3236 [#3235]: https://github.com/import-js/eslint-plugin-import/issues/3235 @@ -2000,6 +2002,7 @@ for info on changes for earlier releases. [@mihkeleidast]: https://github.com/mihkeleidast [@MikeyBeLike]: https://github.com/MikeyBeLike [@minervabot]: https://github.com/minervabot +[@mixelburg]: https://github.com/mixelburg [@mpint]: https://github.com/mpint [@mplewis]: https://github.com/mplewis [@mrmckeb]: https://github.com/mrmckeb diff --git a/src/rules/no-duplicates.js b/src/rules/no-duplicates.js index b5d4bdd6e1..369a249692 100644 --- a/src/rules/no-duplicates.js +++ b/src/rules/no-duplicates.js @@ -2,6 +2,7 @@ import { getSourceCode } from 'eslint-module-utils/contextCompat'; import resolve from 'eslint-module-utils/resolve'; import semver from 'semver'; import flatMap from 'array.prototype.flatmap'; +import trimEnd from 'string.prototype.trimend'; import docsUrl from '../docsUrl'; @@ -171,7 +172,21 @@ function getFix(first, rest, sourceCode, context) { // 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; + const hasLineComment = (/\/\/[^\n]*$/).test(trimmed); + let curWithType; + if (trimmed.length > 0 && preferInline && isTypeSpecifier) { + curWithType = `type ${trimmed}`; + } else if (hasLineComment && trimmed.length > 0) { + // Preserve a trailing newline after line comments so the closing brace + // is not accidentally commented out when the specifier is spliced in. + curWithType = `${trimmed}\n`; + } else if (cur.includes('\n')) { + // Preserve leading newline+indentation for multiline import specifiers + // so merged specifiers stay on their own lines. + curWithType = trimEnd(cur).replace(/^[^\S\n]+/, ''); + } else { + curWithType = trimmed; + } if (existingIdentifiers.has(trimmed)) { return [text, set]; } @@ -217,7 +232,22 @@ function getFix(first, rest, sourceCode, context) { fixes.push(fixer.insertTextAfter(firstToken, ` ${defaultImportName},`)); if (shouldAddSpecifiers) { // `import def, {...} from './foo'` → `import def, {..., ...} from './foo'` - fixes.push(fixer.insertTextBefore(closeBrace, specifiersText)); + const textBeforeClose2 = sourceCode.text.slice(openBrace.range[1], closeBrace.range[0]); + const trailingMatch2 = textBeforeClose2.match(/(\s+)$/); + const trailingWhitespace2 = trailingMatch2 ? trailingMatch2[1] : ''; + if (textBeforeClose2.trim() === '') { + fixes.push(fixer.replaceTextRange( + [openBrace.range[1], closeBrace.range[0]], + specifiersText, + )); + } else if (trailingWhitespace2) { + fixes.push(fixer.replaceTextRange( + [closeBrace.range[0] - trailingWhitespace2.length, closeBrace.range[1]], + `${specifiersText}${trailingWhitespace2}}`, + )); + } else { + fixes.push(fixer.insertTextBefore(closeBrace, specifiersText)); + } } } else if (!shouldAddDefault && openBrace == null && shouldAddSpecifiers) { if (first.specifiers.length === 0) { @@ -229,7 +259,24 @@ function getFix(first, rest, sourceCode, context) { } } else if (!shouldAddDefault && openBrace != null && closeBrace != null) { // `import {...} './foo'` → `import {..., ...} from './foo'` - fixes.push(fixer.insertTextBefore(closeBrace, specifiersText)); + // Preserve trailing whitespace before the closing brace (symmetric spacing, multiline formatting). + // For empty or whitespace-only imports, replace all content between braces instead. + const textBeforeClose = sourceCode.text.slice(openBrace.range[1], closeBrace.range[0]); + const trailingMatch = textBeforeClose.match(/(\s+)$/); + const trailingWhitespace = trailingMatch ? trailingMatch[1] : ''; + if (textBeforeClose.trim() === '') { + fixes.push(fixer.replaceTextRange( + [openBrace.range[1], closeBrace.range[0]], + specifiersText, + )); + } else if (trailingWhitespace) { + fixes.push(fixer.replaceTextRange( + [closeBrace.range[0] - trailingWhitespace.length, closeBrace.range[1]], + `${specifiersText}${trailingWhitespace}}`, + )); + } else { + fixes.push(fixer.insertTextBefore(closeBrace, specifiersText)); + } } // Remove imports whose specifiers have been moved into the first import. diff --git a/tests/src/rules/no-duplicates.js b/tests/src/rules/no-duplicates.js index 0318240660..68f30fc65b 100644 --- a/tests/src/rules/no-duplicates.js +++ b/tests/src/rules/no-duplicates.js @@ -53,20 +53,20 @@ ruleTester.run('no-duplicates', rule, { invalid: [ test({ code: "import { x } from './foo'; import { y } from './foo'", - output: "import { x , y } from './foo'; ", + output: "import { x,y } from './foo'; ", errors: ['\'./foo\' imported multiple times.', '\'./foo\' imported multiple times.'], }), test({ code: "import {x} from './foo'; import {y} from './foo'; import { z } from './foo'", - output: "import {x,y, z } from './foo'; ", + output: "import {x,y,z} from './foo'; ", errors: ['\'./foo\' imported multiple times.', '\'./foo\' imported multiple times.', '\'./foo\' imported multiple times.'], }), // ensure resolved path results in warnings test({ code: "import { x } from './bar'; import { y } from 'bar';", - output: "import { x , y } from './bar'; ", + output: "import { x,y } from './bar'; ", settings: { 'import/resolve': { paths: [path.join(process.cwd(), 'tests', 'files')], @@ -107,7 +107,7 @@ ruleTester.run('no-duplicates', rule, { test({ code: "import type { x } from './foo'; import type { y } from './foo'", - output: "import type { x , y } from './foo'; ", + output: "import type { x,y } from './foo'; ", parser: parsers.BABEL_OLD, errors: ['\'./foo\' imported multiple times.', '\'./foo\' imported multiple times.'], }), @@ -120,7 +120,7 @@ ruleTester.run('no-duplicates', rule, { test({ code: "import { x, /* x */ } from './foo'; import {//y\ny//y2\n} from './foo'", - output: "import { x, /* x */ //y\ny//y2\n} from './foo'; ", + output: "import { x, /* x *///y\ny//y2\n } from './foo'; ", errors: ['\'./foo\' imported multiple times.', '\'./foo\' imported multiple times.'], }), @@ -145,7 +145,7 @@ ruleTester.run('no-duplicates', rule, { // #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'; ", + output: "import {a,b,c,d} from './foo'; ", errors: ['\'./foo\' imported multiple times.', '\'./foo\' imported multiple times.', '\'./foo\' imported multiple times.'], parser, }), @@ -153,7 +153,7 @@ ruleTester.run('no-duplicates', rule, { // #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'; ", + output: "import {a,a/*,b*/} from './foo'; ", errors: ['\'./foo\' imported multiple times.', '\'./foo\' imported multiple times.'], parser, }), @@ -168,7 +168,7 @@ ruleTester.run('no-duplicates', rule, { test({ code: "import { } from './foo'; import {x} from './foo'", - output: "import { x} from './foo'; ", + output: "import {x} from './foo'; ", errors: ['\'./foo\' imported multiple times.', '\'./foo\' imported multiple times.'], }), @@ -186,7 +186,8 @@ ruleTester.run('no-duplicates', rule, { test({ code: "import './foo'; import { /*x*/} from './foo'; import {//y\n} from './foo'; import {z} from './foo'", - output: "import { /*x*///y\nz} from './foo'; ", + output: "import {/*x*///y\nz} from './foo'; ", + errors: ['\'./foo\' imported multiple times.', '\'./foo\' imported multiple times.', '\'./foo\' imported multiple times.', '\'./foo\' imported multiple times.'], }), @@ -373,7 +374,7 @@ import {x,y} from './foo' // #2027 long import list generate empty lines test({ code: "import { Foo } from './foo';\nimport { Bar } from './foo';\nexport const value = {}", - output: "import { Foo , Bar } from './foo';\nexport const value = {}", + output: "import { Foo,Bar } from './foo';\nexport const value = {}", errors: ['\'./foo\' imported multiple times.', '\'./foo\' imported multiple times.'], }), @@ -406,7 +407,6 @@ import {x,y} from './foo' import { DEFAULT_FILTER_KEYS, BULK_DISABLED, - ${''} BULK_ACTIONS_ENABLED } from '../constants'; import React from 'react'; @@ -444,9 +444,9 @@ import {x,y} from './foo' ${''} import { A2, - ${''} B2, - C2} from 'bar'; + C2 + } from 'bar'; ${''} `, errors: [