Skip to content

Commit

Permalink
Fix: no-duplicate-imports allow unmergeable (fixes eslint#12758, fixes …
Browse files Browse the repository at this point in the history 
…eslint#12760)
  • Loading branch information
@boutahlilsoufiane
boutahlilsoufiane committed May 9, 2021
1 parent 5618a4a commit 32d6d6f
Show file tree
Hide file tree
Showing 2 changed files with 306 additions and 298 deletions.
186 changes: 101 additions & 85 deletions lib/rules/no-duplicate-imports.js
View file
Expand Up @@ -8,85 +8,78 @@
// Rule Definition
//------------------------------------------------------------------------------

const IMPORT_NAME_SPACE_SPECIFIER = "ImportNamespaceSpecifier";
const EXPORT_SPECIFIER = "ExportSpecifier";
const IMPORT_SPECIFIER = "ImportSpecifier";
const CONTRADICTORY_IMPORT_EXPORT_TYPES = [
[IMPORT_NAME_SPACE_SPECIFIER],
[IMPORT_SPECIFIER, EXPORT_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|export).
* @param {ASTNode} node A node to get.
* @returns {string} the type of the (import|export).
* @returns {string} The type of the (import|export).
*/
function getImportExportType(node) {
if (
node &&
node.specifiers &&
node.specifiers[0] &&
node.specifiers[0].type
) {
const contradictoryImportTypes = CONTRADICTORY_IMPORT_EXPORT_TYPES.reduce(
(contradictoryImportTypesReduced, contradictoryImportType) => [
...contradictoryImportTypesReduced,
...contradictoryImportType,
],
[]
);
const index = node.specifiers.findIndex((specifier) =>
contradictoryImportTypes.includes(specifier.type)
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 node.specifiers[0].type;
return nodeSpecifiers[i].type;
}
if (node && node.type) {
return node.type;
if (node.type === "ExportAllDeclaration") {
if (node.exported) {
return "ExportNamespaceSpecifier";
}
return "ExportAll";
}
}

/**
* 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();
if (node.type === "ImportDeclaration") {
return "SideEffectImport";
}
return "";
return node.type;
}

/**
* Returns a boolean indicates if the two (import|export) can be merged
* 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 {string} true if both (import|export) can be merged, else we return false.
* @returns {boolean} True if two (import|export) can be merged, false if they can't.
*/
function isImportExportCanBeMerged(node1, node2) {
const importExportType1 = getImportExportType(node1);
const importExportType2 = getImportExportType(node2);
const EXPORT_ALL_DECLARATION = "ExportAllDeclaration";
const IMPORT_DECLARATION = "ImportDeclaration";

if (
(importExportType1 === EXPORT_ALL_DECLARATION &&
importExportType2 !== EXPORT_ALL_DECLARATION &&
importExportType2 !== IMPORT_DECLARATION) ||
(importExportType1 !== EXPORT_ALL_DECLARATION &&
importExportType1 !== IMPORT_DECLARATION &&
importExportType2 === EXPORT_ALL_DECLARATION)
(importExportType1 === "ExportAll" &&
importExportType2 !== "ExportAll" &&
importExportType2 !== "SideEffectImport") ||
(importExportType1 !== "ExportAll" &&
importExportType1 !== "SideEffectImport" &&
importExportType2 === "ExportAll")
) {
return false;
} else if (
importExportType1 !== importExportType2 &&
((CONTRADICTORY_IMPORT_EXPORT_TYPES[0].includes(importExportType1) &&
CONTRADICTORY_IMPORT_EXPORT_TYPES[1].includes(importExportType2)) ||
(CONTRADICTORY_IMPORT_EXPORT_TYPES[1].includes(importExportType1) &&
CONTRADICTORY_IMPORT_EXPORT_TYPES[0].includes(importExportType2)))
}
if (
(isImportExportSpecifier(importExportType1, "namespace") &&
isImportExportSpecifier(importExportType2, "specifier")) ||
(isImportExportSpecifier(importExportType2, "namespace") &&
isImportExportSpecifier(importExportType1, "specifier"))
) {
return false;
}
Expand All @@ -95,13 +88,14 @@ function isImportExportCanBeMerged(node1, node2) {

/**
* 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 same module.
* @returns {boolean} true if the (import/export) should be reported.
* @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;
Expand All @@ -115,25 +109,38 @@ function shouldReportImportExport(node, previousNodes) {
/**
* 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.
* @returns {[{node: ASTNode, declarationType: string}]} An array contains only nodes with declarations types equal to type, if the nodes are null we return [].
* @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 getNodesByDeclarationTypeAndFormat(nodes, type) {
if (nodes) {
return nodes
.filter(({ declarationType }) => declarationType === type)
.map(({ node }) => node);
} else {
return [];
}
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 (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: [{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} messageId A messageId to be reported after the name of the module
* @returns {void} No return value
* @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,
Expand All @@ -145,10 +152,17 @@ function checkAndReport(
const module = getModule(node);
const previousNodes = modules[module];
const messagesIds = [];
const importNodes = getNodesByDeclarationTypeAndFormat(previousNodes, "import");
const importNodes = getNodesByDeclarationTypeAndFormat(
previousNodes,
"import"
);
let exportNodes;

if (includeExports) {
exportNodes = getNodesByDeclarationTypeAndFormat(previousNodes, "export");
exportNodes = getNodesByDeclarationTypeAndFormat(
previousNodes,
"export"
);
}
if (declarationType === "import") {
if (shouldReportImportExport(node, importNodes)) {
Expand All @@ -164,19 +178,18 @@ function checkAndReport(
messagesIds.push("export");
}
if (shouldReportImportExport(node, importNodes)) {
messagesIds.push("exportAs");
messagesIds.push("importAs");
}
}

messagesIds.forEach((messageId) =>
messagesIds.forEach(messageId =>
context.report({
node,
messageId: messageId,
messageId,
data: {
module,
},
})
);
module
}
}));
}

/**
Expand All @@ -188,7 +201,8 @@ function checkAndReport(
* 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} messageId A messageId to be reported after the name of the module.
* @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 {nodeCallback} A function passed to ESLint to handle the statement.
*/
function handleImportsExports(
Expand All @@ -197,8 +211,9 @@ function handleImportsExports(
declarationType,
includeExports
) {
return function (node) {
return function(node) {
const module = getModule(node);

if (module) {
checkAndReport(
context,
Expand All @@ -209,6 +224,7 @@ function handleImportsExports(
);
const previousNodes = modules[module];
const currentNode = { node, declarationType };

if (previousNodes) {
modules[module] = [...previousNodes, currentNode];
} else {
Expand All @@ -226,7 +242,7 @@ module.exports = {
description: "disallow duplicate module imports",
category: "ECMAScript 6",
recommended: false,
url: "https://eslint.org/docs/rules/no-duplicate-imports",
url: "https://eslint.org/docs/rules/no-duplicate-imports"
},

schema: [
Expand All @@ -235,18 +251,18 @@ module.exports = {
properties: {
includeExports: {
type: "boolean",
default: false,
},
default: false
}
},
additionalProperties: false,
},
additionalProperties: false
}
],
messages: {
import: "'{{module}}' import is duplicated.",
importAs: "'{{module}}' import is duplicated as export.",
export: "'{{module}}' export is duplicated.",
export: "'{{module}}' export is duplicated.",
exportAs: "'{{module}}' export is duplicated as import.",
},
exportAs: "'{{module}}' export is duplicated as import."
}
},

create(context) {
Expand All @@ -258,7 +274,7 @@ module.exports = {
modules,
"import",
includeExports
),
)
};

if (includeExports) {
Expand All @@ -276,5 +292,5 @@ module.exports = {
);
}
return handlers;
},
}
};

0 comments on commit 32d6d6f

Please sign in to comment.