From f84d4577dc94600e28b2071ae365d671bfc6420e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Veyret?= Date: Wed, 15 Jan 2020 17:24:02 +0100 Subject: [PATCH] `no-duplicates`: allow duplicate if one is namespace and other not It is a syntax error to put both namespace and non namespace import on the same line, so allow it. Fixes #1538 --- CHANGELOG.md | 3 +++ src/rules/no-duplicates.js | 5 ++++- tests/src/rules/no-duplicates.js | 27 +++++++++++++++++++++------ 3 files changed, 28 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cfd4ad5eb..db4aec82a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel - [`import/external-module-folders` setting] now correctly works with directories containing modules symlinked from `node_modules` ([#1605], thanks [@skozin]) - [`extensions`]: for invalid code where `name` does not exist, do not crash ([#1613], thanks [@ljharb]) - [`extentions`]: Fix scope regex ([#1611], thanks [@yordis]) +- [`no-duplicates`]: allow duplicate imports if one is a namespace and the other not ([#1612], thanks [@sveyret]) ### Changed - [`import/external-module-folders` setting] behavior is more strict now: it will only match complete path segments ([#1605], thanks [@skozin]) @@ -644,6 +645,7 @@ for info on changes for earlier releases. [`memo-parser`]: ./memo-parser/README.md [#1613]: https://github.com/benmosher/eslint-plugin-import/issues/1613 +[#1612]: https://github.com/benmosher/eslint-plugin-import/pull/1612 [#1611]: https://github.com/benmosher/eslint-plugin-import/pull/1611 [#1605]: https://github.com/benmosher/eslint-plugin-import/pull/1605 [#1589]: https://github.com/benmosher/eslint-plugin-import/issues/1589 @@ -1081,3 +1083,4 @@ for info on changes for earlier releases. [@ivo-stefchev]: https://github.com/ivo-stefchev [@skozin]: https://github.com/skozin [@yordis]: https://github.com/yordis +[@sveyret]: https://github.com/sveyret diff --git a/src/rules/no-duplicates.js b/src/rules/no-duplicates.js index 1334a1258..69e5a23a0 100644 --- a/src/rules/no-duplicates.js +++ b/src/rules/no-duplicates.js @@ -257,12 +257,14 @@ module.exports = { }) : defaultResolver const imported = new Map() + const nsImported = new Map() const typesImported = new Map() return { 'ImportDeclaration': function (n) { // resolved path will cover aliased duplicates const resolvedPath = resolver(n.source.value) - const importMap = n.importKind === 'type' ? typesImported : imported + const importMap = n.importKind === 'type' ? typesImported : + (hasNamespace(n) ? nsImported : imported) if (importMap.has(resolvedPath)) { importMap.get(resolvedPath).push(n) @@ -273,6 +275,7 @@ module.exports = { 'Program:exit': function () { checkImports(imported, context) + checkImports(nsImported, context) checkImports(typesImported, context) }, } diff --git a/tests/src/rules/no-duplicates.js b/tests/src/rules/no-duplicates.js index a4c41f677..468c7ab98 100644 --- a/tests/src/rules/no-duplicates.js +++ b/tests/src/rules/no-duplicates.js @@ -31,12 +31,20 @@ ruleTester.run('no-duplicates', rule, { code: "import x from './bar?optionX'; import y from './bar?optionY';", options: [{'considerQueryString': true}], settings: { 'import/resolver': 'webpack' }, - }), + }), test({ code: "import x from './foo'; import y from './bar';", options: [{'considerQueryString': true}], settings: { 'import/resolver': 'webpack' }, - }), + }), + + // #1538: It is impossible to import namespace and other in one line, so allow this. + test({ + code: "import * as ns from './foo'; import {y} from './foo'", + }), + test({ + code: "import {y} from './foo'; import * as ns from './foo'", + }), ], invalid: [ test({ @@ -179,9 +187,16 @@ ruleTester.run('no-duplicates', rule, { }), test({ - code: "import * as ns from './foo'; import {y} from './foo'", - // Autofix bail because first import is a namespace import. - output: "import * as ns from './foo'; import {y} from './foo'", + code: "import * as ns1 from './foo'; import * as ns2 from './foo'", + // Autofix bail because cannot merge namespace imports. + output: "import * as ns1 from './foo'; import * as ns2 from './foo'", + errors: ['\'./foo\' imported multiple times.', '\'./foo\' imported multiple times.'], + }), + + test({ + code: "import * as ns from './foo'; import {x} from './foo'; import {y} from './foo'", + // Autofix could merge some imports, but not the namespace import. + output: "import * as ns from './foo'; import {x,y} from './foo'; ", errors: ['\'./foo\' imported multiple times.', '\'./foo\' imported multiple times.'], }), @@ -189,7 +204,7 @@ ruleTester.run('no-duplicates', rule, { code: "import {x} from './foo'; import * as ns from './foo'; import {y} from './foo'; import './foo'", // Autofix could merge some imports, but not the namespace import. output: "import {x,y} from './foo'; import * as ns from './foo'; ", - errors: ['\'./foo\' imported multiple times.', '\'./foo\' imported multiple times.', '\'./foo\' imported multiple times.', '\'./foo\' imported multiple times.'], + errors: ['\'./foo\' imported multiple times.', '\'./foo\' imported multiple times.', '\'./foo\' imported multiple times.'], }), test({