[Fix] no-duplicates: ensure autofix avoids excessive newlines

Fixes #2027

Author: ertrzyiks

CHANGELOG.md

@@ -6,6 +6,9 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel
 
 ## [Unreleased]
 
+### Fixed
+- [`no-duplicates`]: ensure autofix avoids excessive newlines ([#2028], thanks [@ertrzyiks])
+
 ## [2.23.4] - 2021-05-29
 
 ### Fixed
@@ -808,6 +811,7 @@ for info on changes for earlier releases.
 [#2075]: https://github.com/benmosher/eslint-plugin-import/pull/2075
 [#2071]: https://github.com/benmosher/eslint-plugin-import/pull/2071
 [#2034]: https://github.com/benmosher/eslint-plugin-import/pull/2034
+[#2028]: https://github.com/benmosher/eslint-plugin-import/pull/2028
 [#2026]: https://github.com/benmosher/eslint-plugin-import/pull/2026
 [#2022]: https://github.com/benmosher/eslint-plugin-import/pull/2022
 [#2021]: https://github.com/benmosher/eslint-plugin-import/pull/2021
@@ -1273,6 +1277,7 @@ for info on changes for earlier releases.
 [@ephys]: https://github.com/ephys
 [@eps1lon]: https://github.com/eps1lon
 [@ernestostifano]: https://github.com/ernestostifano
+[@ertrzyiks]: https://github.com/ertrzyiks
 [@fa93hws]: https://github.com/fa93hws
 [@fengkfengk]: https://github.com/fengkfengk
 [@fernandopasik]: https://github.com/fernandopasik
@@ -1412,4 +1417,4 @@ for info on changes for earlier releases.
 [@wtgtybhertgeghgtwtg]: https://github.com/wtgtybhertgeghgtwtg
 [@xpl]: https://github.com/xpl
 [@yordis]: https://github.com/yordis
-[@zloirock]: https://github.com/zloirock
+[@zloirock]: https://github.com/zloirock

src/rules/no-duplicates.js

@@ -150,14 +150,27 @@ function getFix(first, rest, sourceCode) {
 
     // Remove imports whose specifiers have been moved into the first import.
     for (const specifier of specifiers) {
-      fixes.push(fixer.remove(specifier.importNode));
+      const importNode = specifier.importNode;
+      fixes.push(fixer.remove(importNode));
+
+      const charAfterImportRange = [importNode.range[1], importNode.range[1] + 1];
+      const charAfterImport = sourceCode.text.substring(charAfterImportRange[0], charAfterImportRange[1]);
+      if (charAfterImport === '\n') {
+        fixes.push(fixer.removeRange(charAfterImportRange));
+      }
     }
 
     // Remove imports whose default import has been moved to the first import,
     // and side-effect-only imports that are unnecessary due to the first
     // import.
     for (const node of unnecessaryImports) {
       fixes.push(fixer.remove(node));
+
+      const charAfterImportRange = [node.range[1], node.range[1] + 1];
+      const charAfterImport = sourceCode.text.substring(charAfterImportRange[0], charAfterImportRange[1]);
+      if (charAfterImport === '\n') {
+        fixes.push(fixer.removeRange(charAfterImportRange));
+      }
     }
 
     return fixes;

tests/src/rules/no-duplicates.js

@@ -302,32 +302,30 @@ ruleTester.run('no-duplicates', rule, {
 
     test({
       code: `
-        import {x} from './foo'
-        import {y} from './foo'
-        // some-tool-disable-next-line
+import {x} from './foo'
+import {y} from './foo'
+// some-tool-disable-next-line
       `,
       // Not autofix bail.
       output: `
-        import {x,y} from './foo'
-        
-        // some-tool-disable-next-line
+import {x,y} from './foo'
+// some-tool-disable-next-line
       `,
       errors: ['\'./foo\' imported multiple times.', '\'./foo\' imported multiple times.'],
     }),
 
     test({
       code: `
-        import {x} from './foo'
-        // comment
+import {x} from './foo'
+// comment
 
-        import {y} from './foo'
+import {y} from './foo'
       `,
       // Not autofix bail.
       output: `
-        import {x,y} from './foo'
-        // comment
+import {x,y} from './foo'
+// comment
 
-        
       `,
       errors: ['\'./foo\' imported multiple times.', '\'./foo\' imported multiple times.'],
     }),
@@ -400,6 +398,20 @@ ruleTester.run('no-duplicates', rule, {
       `,
       errors: ['\'./foo\' imported multiple times.', '\'./foo\' imported multiple times.'],
     }),
+
+    // #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 = {}",
+      errors: ['\'./foo\' imported multiple times.', '\'./foo\' imported multiple times.'],
+    }),
+
+    // #2027 long import list generate empty lines
+    test({
+      code: "import { Foo } from './foo';\nimport Bar from './foo';\nexport const value = {}",
+      output: "import Bar, { Foo } from './foo';\nexport const value = {}",
+      errors: ['\'./foo\' imported multiple times.', '\'./foo\' imported multiple times.'],
+    }),
   ],
 });
 
@@ -430,4 +442,3 @@ context('TypeScript', function() {
       });
     });
 });
-