diff --git a/lib/rules/no-duplicate-imports.js b/lib/rules/no-duplicate-imports.js index 0520a6e24bea..5c609420730f 100644 --- a/lib/rules/no-duplicate-imports.js +++ b/lib/rules/no-duplicate-imports.js @@ -8,119 +8,188 @@ // Rule Definition //------------------------------------------------------------------------------ -const EXPORT_ALL_DECLARATION = "ExportAllDeclaration"; -const IMPORT_NAME_SPACE_SPECIFIER = "ImportNamespaceSpecifier"; -const IMPORT_SPECIFIER = "ImportSpecifier"; -const CONTRADICTORY_IMPORT_TYPES = [ - IMPORT_NAME_SPACE_SPECIFIER, - IMPORT_SPECIFIER -]; +/** + * Check if (import|export) type is belong to (ImportSpecifier|ExportSpecifier) or (ImportNamespaceSpecifier|ExportNamespaceSpecifier). + * @param {string} importExportType An import type to get. + * @param {string} type Specifier type can be namespace or specifier + * @returns {boolean} True if (import|export) type is belong to (ImportSpecifier|ExportSpecifier) or (ImportNamespaceSpecifier|ExportNamespaceSpecifier) and false if it doesn't. + */ +function isImportExportSpecifier(importExportType, type) { + const specifiersTypes = ["ImportSpecifier", "ExportSpecifier"]; + const namespacesTypes = [ + "ImportNamespaceSpecifier", + "ExportNamespaceSpecifier" + ]; + const arrayToCheck = + type === "specifier" ? specifiersTypes : namespacesTypes; + + return arrayToCheck.includes(importExportType); +} /** - * Return the type of import. + * Return the type of (import|export). * @param {ASTNode} node A node to get. - * @returns {string} the type of the import. + * @returns {string} The type of the (import|export). */ -function getImportType(node) { - if ( - node && - node.specifiers && - node.specifiers[0] && - node.specifiers[0].type - ) { - const index = node.specifiers.findIndex(specifier => - CONTRADICTORY_IMPORT_TYPES.includes( - specifier.type - )); +function getImportExportType(node) { + if (node && node.specifiers && node.specifiers.length > 0) { + const nodeSpecifiers = node.specifiers; + const index = nodeSpecifiers.findIndex( + ({ type }) => + isImportExportSpecifier(type, "specifier") || + isImportExportSpecifier(type, "namespace") + ); + const i = index > -1 ? index : 0; - if (index > -1) { - return node.specifiers[index].type; + return nodeSpecifiers[i].type; + } + if (node.type === "ExportAllDeclaration") { + if (node.exported) { + return "ExportNamespaceSpecifier"; } - return node.specifiers[0].type; + return "ExportAll"; } - if (node && node.type) { - return node.type; + if (node.type === "ImportDeclaration") { + return "SideEffectImport"; } - return ""; + return node.type; } /** - * Returns the name of the module imported or re-exported. - * @param {ASTNode} node A node to get. - * @returns {string} the name of the module, or empty string if no name. + * Returns a boolean indicates if two (import|export) can be merged + * @param {ASTNode} node1 A node to get. + * @param {ASTNode} node2 A node to get. + * @returns {boolean} True if two (import|export) can be merged, false if they can't. */ -function getValue(node) { - if (node && node.source && node.source.value) { - return node.source.value.trim(); - } +function isImportExportCanBeMerged(node1, node2) { + const importExportType1 = getImportExportType(node1); + const importExportType2 = getImportExportType(node2); - return ""; + if ( + (importExportType1 === "ExportAll" && + importExportType2 !== "ExportAll" && + importExportType2 !== "SideEffectImport") || + (importExportType1 !== "ExportAll" && + importExportType1 !== "SideEffectImport" && + importExportType2 === "ExportAll") + ) { + return false; + } + if ( + (isImportExportSpecifier(importExportType1, "namespace") && + isImportExportSpecifier(importExportType2, "specifier")) || + (isImportExportSpecifier(importExportType2, "namespace") && + isImportExportSpecifier(importExportType1, "specifier")) + ) { + return false; + } + return true; } +/** + * Returns a boolean if we should report (import/export). + * @param {ASTNode} node A node to be reported or not. + * @param {[ASTNode]} previousNodes An array contains previous nodes of the module imported or exported. + * @returns {boolean} True if the (import/export) should be reported. + */ +function shouldReportImportExport(node, previousNodes) { + if (previousNodes) { + let i = 0; + + while (i < previousNodes.length) { + if (isImportExportCanBeMerged(node, previousNodes[i])) { + return true; + } + i++; + } + } + return false; +} /** - * Returns a boolean if we should report contradictory import type. - * @param {string[]} importTypes An array contain import types of a module. - * @param {string} importType An contradictory import type to check if we should report it or not. - * @returns {boolean} true if the contradictory import type should be reported. + * Returns array contains only nodes with declarations types equal to type. + * @param {[{node: ASTNode, declarationType: string}]} nodes An array contains objects, each object contains a node and a declaration type. + * @param {string} type Declaration type. + * @returns {[ASTNode]} An array contains only nodes with declarations types equal to type, if the nodes are null we return []. */ -function shouldReportContradictoryImportType(importTypes, importType) { - if (importTypes.indexOf(importType) > -1) { - return true; +function getNodesByDeclarationTypeAndFormat(nodes, type) { + if (nodes) { + return nodes + .filter(({ declarationType }) => declarationType === type) + .map(({ node }) => node); } - return ( - importTypes.findIndex(importTypeItem => CONTRADICTORY_IMPORT_TYPES.includes(importTypeItem)) === -1 - ); + return []; +} +/** + * Returns the name of the module imported or re-exported. + * @param {ASTNode} node A node to get. + * @returns {string} The name of the module, or empty string if no name. + */ +function getModule(node) { + if (node && node.source && node.source.value) { + return node.source.value.trim(); + } + return ""; } /** - * Checks if the name of the import or export exists in the given array, and reports if so. + * Checks if the (import|export) can be merged with at least one import and one export, and reports if so. * @param {RuleContext} context The ESLint rule context object. * @param {ASTNode} node A node to get. - * @param {string} value The name of the imported or exported module. - * @param {string[]} array The array containing other imports or exports in the file. - * @param {string} messageId A messageId to be reported after the name of the module - * @param {{}} modulesWithImportTypes The object containing the name of unique modules with their first import type [specificImport, nameSpaceImport]. - * @param {string} type the name of import type. - * @param {string[]} ExportAllDeclarationsInFile The array containing ExportAllDeclarations in the file. - * - * @returns {void} No return value + * @param {{string: [{node: ASTNode, declarationType: string}]}} modules An object contains as a key a module name and as value an array contains objects, each object contains a node and a declaration type. + * @param {string} declarationType A declaration type can be an import or export. + * @param {boolean} includeExports Whether or not to check for exports in addition to imports. + * @returns {void} No return value. */ function checkAndReport( context, node, - value, - array, - messageId, - modulesWithImportTypes, - type, - ExportAllDeclarationsInFile + modules, + declarationType, + includeExports ) { - let isDuplicate = false; + const module = getModule(node); + const previousNodes = modules[module]; + const messagesIds = []; + const importNodes = getNodesByDeclarationTypeAndFormat( + previousNodes, + "import" + ); + let exportNodes; - if (type === EXPORT_ALL_DECLARATION) { - if (ExportAllDeclarationsInFile.indexOf(value) !== -1) { - isDuplicate = true; + if (includeExports) { + exportNodes = getNodesByDeclarationTypeAndFormat( + previousNodes, + "export" + ); + } + if (declarationType === "import") { + if (shouldReportImportExport(node, importNodes)) { + messagesIds.push("import"); } - } else if (array.indexOf(value) !== -1) { - isDuplicate = true; - if (CONTRADICTORY_IMPORT_TYPES.includes(type)) { - isDuplicate = shouldReportContradictoryImportType( - modulesWithImportTypes[value], - type - ); + if (includeExports) { + if (shouldReportImportExport(node, exportNodes)) { + messagesIds.push("exportAs"); + } + } + } else if (declarationType === "export") { + if (shouldReportImportExport(node, exportNodes)) { + messagesIds.push("export"); + } + if (shouldReportImportExport(node, importNodes)) { + messagesIds.push("importAs"); } } - if (isDuplicate) { + + messagesIds.forEach(messageId => context.report({ node, messageId, data: { - module: value + module } - }); - } + })); } /** @@ -129,110 +198,37 @@ function checkAndReport( */ /** - * Returns a function handling the imports of a given file + * Returns a function handling the (imports|exports) of a given file * @param {RuleContext} context The ESLint rule context object. + * @param {{string: [{node: ASTNode, declarationType: string}]}} modules An object contains as a key a module name and as value an array contains objects, each object contains a node and a declaration type. + * @param {string} declarationType A declaration type can be an import or export. * @param {boolean} includeExports Whether or not to check for exports in addition to imports. - * @param {string[]} importsInFile The array containing other imports in the file. - * @param {string[]} exportsInFile The array containing other exports in the file. - * @param {{}} modulesWithImportTypes The object containing the name of unique modules with their first import type [specificImport, nameSpaceImport]. - * @param {string[]} ExportAllDeclarationsInFile The array containing ExportAllDeclarations in the file. - * * @returns {nodeCallback} A function passed to ESLint to handle the statement. */ -function handleImports( +function handleImportsExports( context, - includeExports, - importsInFile, - exportsInFile, - modulesWithImportTypes, - ExportAllDeclarationsInFile + modules, + declarationType, + includeExports ) { return function(node) { - const value = getValue(node); - const type = getImportType(node); + const module = getModule(node); - if (value) { + if (module) { checkAndReport( context, node, - value, - importsInFile, - "import", - modulesWithImportTypes, - type, - ExportAllDeclarationsInFile + modules, + declarationType, + includeExports ); + const previousNodes = modules[module]; + const currentNode = { node, declarationType }; - if (includeExports) { - checkAndReport( - context, - node, - value, - exportsInFile, - "importAs", - modulesWithImportTypes, - type, - ExportAllDeclarationsInFile - ); - } - importsInFile.push(value); - if (modulesWithImportTypes[value]) { - modulesWithImportTypes[value] = modulesWithImportTypes[ - value - ].concat(type); + if (previousNodes) { + modules[module] = [...previousNodes, currentNode]; } else { - modulesWithImportTypes[value] = [type]; - } - } - }; -} - -/** - * Returns a function handling the exports of a given file - * @param {RuleContext} context The ESLint rule context object. - * @param {string[]} importsInFile The array containing other imports in the file. - * @param {string[]} exportsInFile The array containing other exports in the file. - * @param {{}} modulesWithImportTypes The object containing the name of unique modules with their first import type [specificImport, nameSpaceImport]. - * @param {string[]} ExportAllDeclarationsInFile The array containing ExportAllDeclarations in the file. - * - * @returns {nodeCallback} A function passed to ESLint to handle the statement. - */ -function handleExports( - context, - importsInFile, - exportsInFile, - modulesWithImportTypes, - ExportAllDeclarationsInFile -) { - return function(node) { - const value = getValue(node); - const type = getImportType(node); - - if (value) { - checkAndReport( - context, - node, - value, - exportsInFile, - "export", - modulesWithImportTypes, - type, - ExportAllDeclarationsInFile - ); - if (type === EXPORT_ALL_DECLARATION) { - ExportAllDeclarationsInFile.push(value); - } else { - checkAndReport( - context, - node, - value, - importsInFile, - "exportAs", - modulesWithImportTypes, - type, - ExportAllDeclarationsInFile - ); - exportsInFile.push(value); + modules[module] = [currentNode]; } } }; @@ -271,35 +267,28 @@ module.exports = { create(context) { const includeExports = (context.options[0] || {}).includeExports, - importsInFile = [], - modulesWithImportTypes = {}, - exportsInFile = [], - ExportAllDeclarationsInFile = []; + modules = {}; const handlers = { - ImportDeclaration: handleImports( + ImportDeclaration: handleImportsExports( context, - includeExports, - importsInFile, - exportsInFile, - modulesWithImportTypes, - ExportAllDeclarationsInFile + modules, + "import", + includeExports ) }; if (includeExports) { - handlers.ExportNamedDeclaration = handleExports( + handlers.ExportNamedDeclaration = handleImportsExports( context, - importsInFile, - exportsInFile, - modulesWithImportTypes, - ExportAllDeclarationsInFile + modules, + "export", + includeExports ); - handlers.ExportAllDeclaration = handleExports( + handlers.ExportAllDeclaration = handleImportsExports( context, - importsInFile, - exportsInFile, - modulesWithImportTypes, - ExportAllDeclarationsInFile + modules, + "export", + includeExports ); } return handlers; diff --git a/tests/lib/rules/no-duplicate-imports.js b/tests/lib/rules/no-duplicate-imports.js index 80a99a3037cf..b4098145f424 100644 --- a/tests/lib/rules/no-duplicate-imports.js +++ b/tests/lib/rules/no-duplicate-imports.js @@ -17,7 +17,7 @@ const rule = require("../../../lib/rules/no-duplicate-imports"), //------------------------------------------------------------------------------ const ruleTester = new RuleTester({ - parserOptions: { ecmaVersion: 6, sourceType: "module" } + parserOptions: { ecmaVersion: 12, sourceType: "module" } }); ruleTester.run("no-duplicate-imports", rule, { @@ -34,10 +34,6 @@ ruleTester.run("no-duplicate-imports", rule, { code: 'import os from "os";\nexport { hello } from "hello";', options: [{ includeExports: true }] }, - { - code: 'import os from "os";\nexport * from "hello";', - options: [{ includeExports: true }] - }, { code: 'import os from "os";\nexport { hello as hi } from "hello";', options: [{ includeExports: true }] @@ -48,29 +44,27 @@ ruleTester.run("no-duplicate-imports", rule, { }, { code: - 'import { merge } from "lodash-es";\nexport { merge as lodashMerge }', + 'import { merge } from "lodash-es";\nexport { merge as lodashMerge }', options: [{ includeExports: true }] }, - - // ignore `export * from` declarations, they cannot be merged with any other import/export declarations { - code: "import os from 'os'; export * from 'os';", + code: "import * as foo from 'os';\nexport {too} from 'os';", options: [{ includeExports: true }] }, { - code: "export * from 'os'; import { a } from 'os';", + code: "import os from 'os';\n export * from 'os';", options: [{ includeExports: true }] }, { - code: "import * as ns from 'os'; export * from 'os';", + code: "import * as ns from 'os';\nexport * from 'os';", options: [{ includeExports: true }] }, { - code: "export * from 'os'; export { a } from 'os';", + code: "export * from 'os';\nexport { a } from 'os';", options: [{ includeExports: true }] }, { - code: "export { a as b } from 'os'; export * from 'os';", + code: "export { a as b } from 'os';\nexport * from 'os';", options: [{ includeExports: true }] } ], @@ -87,7 +81,7 @@ ruleTester.run("no-duplicate-imports", rule, { }, { code: - 'import { merge } from "lodash-es";import { find } from "lodash-es";', + 'import { merge } from "lodash-es";\nimport { find } from "lodash-es";', errors: [ { messageId: "import", @@ -98,19 +92,13 @@ ruleTester.run("no-duplicate-imports", rule, { }, { code: - 'import os from "os";\nimport { something } from "os";\nimport * as foobar from "os";', + 'import os from "os";\nimport { something } from "os";\nimport * as foobar from "os";', errors: [ { messageId: "import", data: { module: "os" }, type: "ImportDeclaration" - } - ] - }, - { - code: - 'import os from "os";\nimport * as foobar from "os";\nimport { something } from "os";', - errors: [ + }, { messageId: "import", data: { module: "os" }, @@ -120,7 +108,7 @@ ruleTester.run("no-duplicate-imports", rule, { }, { code: - 'import os from "os";\nimport * as foobar1 from "os";\nimport * as foobar2 from "os";', + 'import os from "os";\nimport * as foobar1 from "os";\nimport * as foobar2 from "os";', errors: [ { messageId: "import", @@ -136,7 +124,7 @@ ruleTester.run("no-duplicate-imports", rule, { }, { code: - 'import { merge } from "lodash-es";import _ from "lodash-es";', + 'import { merge } from "lodash-es";\nimport _ from "lodash-es";', errors: [ { messageId: "import", @@ -147,7 +135,7 @@ ruleTester.run("no-duplicate-imports", rule, { }, { code: - 'import foo, { merge } from "module";\nimport { baz } from "module";', + 'import foo, { merge } from "module";\nimport { baz } from "module";', errors: [ { messageId: "import", @@ -158,7 +146,7 @@ ruleTester.run("no-duplicate-imports", rule, { }, { code: - 'import * as namespace from "lodash-es";\nimport { merge } from "lodash-es";\nimport { baz } from "lodash-es";', + 'import * as modns from "lodash-es";\nimport { merge } from "lodash-es";\nimport { baz } from "lodash-es";', errors: [ { messageId: "import", @@ -180,11 +168,11 @@ ruleTester.run("no-duplicate-imports", rule, { }, { code: - 'import os from "os"; export { os as foobar } from "os";\nexport { something } from "os";', + 'import os from "os"; export { os as foobar } from "os";\nexport { something } from "os";', options: [{ includeExports: true }], errors: [ { - messageId: "exportAs", + messageId: "importAs", data: { module: "os" }, type: "ExportNamedDeclaration" }, @@ -194,7 +182,7 @@ ruleTester.run("no-duplicate-imports", rule, { type: "ExportNamedDeclaration" }, { - messageId: "exportAs", + messageId: "importAs", data: { module: "os" }, type: "ExportNamedDeclaration" } @@ -205,14 +193,26 @@ ruleTester.run("no-duplicate-imports", rule, { options: [{ includeExports: true }], errors: [ { - messageId: "exportAs", + messageId: "importAs", data: { module: "os" }, type: "ExportNamedDeclaration" } ] }, { - code: "export * from 'os'; export * from 'os';", + code: + "import * as modns from 'mod';\nexport * as modns from 'mod';", + options: [{ includeExports: true }], + errors: [ + { + messageId: "importAs", + data: { module: "mod" }, + type: "ExportAllDeclaration" + } + ] + }, + { + code: "export * from 'os';\nexport * from 'os';", options: [{ includeExports: true }], errors: [ { @@ -221,6 +221,17 @@ ruleTester.run("no-duplicate-imports", rule, { type: "ExportAllDeclaration" } ] + }, + { + code: "import 'os';\nexport * from 'os';", + options: [{ includeExports: true }], + errors: [ + { + messageId: "importAs", + data: { module: "os" }, + type: "ExportAllDeclaration" + } + ] } ] });