Skip to content

Commit e44ce0a

Browse files
boutahlilsoufianeboutahlilsoufiane
authored
Fix: no-duplicate-imports allow unmergeable (fixes #12758, fixes #12760) (#14238)
* Fix: ignore unmergable imports when checking no-duplicate-imports (fixes #13180) & Fix: Ignore re-export all in no-duplicate-imports (fixes #12760) * Fix: ignore unmergable imports when checking no-duplicate-imports (fixes #13180) & Fix: Ignore re-export all in no-duplicate-imports (fixes #12760) * Fix: ignore unmergable imports when checking no-duplicate-imports (fixes #13180) & Fix: Ignore re-export all in no-duplicate-imports (fixes #12760) * Fix: ignore unmergable imports when checking no-duplicate-imports (fixes #13180) & Fix: Ignore re-export all in no-duplicate-imports (fixes #12760) * Fix: ignore unmergable imports when checking no-duplicate-imports (fixes #13180) & Fix: Ignore re-export all in no-duplicate-imports (fixes #12760) * Fix: ignore unmergable imports when checking no-duplicate-imports (fixes #13180) & Fix: Ignore re-export all in no-duplicate-imports (fixes #12760) * Fix: ignore unmergable imports when checking no-duplicate-imports (fixes #13180) & Fix: Ignore re-export all in no-duplicate-imports (fixes #12760) * Fix: ignore unmergable imports when checking no-duplicate-imports (fixes #13180) & Fix: Ignore re-export all in no-duplicate-imports (fixes #12760) * Fix: ignore unmergable imports when checking no-duplicate-imports (fixes #13180) & Fix: Ignore re-export all in no-duplicate-imports (fixes #12760)
1 parent bb66a3d commit e44ce0a

File tree

3 files changed

+298
-72
lines changed

3 files changed

+298
-72
lines changed

‎docs/rules/no-duplicate-imports.md

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import { find } from 'module';
1212

1313
## Rule Details
1414

15-
This rule requires that all imports from a single module exists in a single `import` statement.
15+
This rule requires that all imports from a single module that can be merged exist in a single `import` statement.
1616

1717
Example of **incorrect** code for this rule:
1818

@@ -33,6 +33,16 @@ import { merge, find } from 'module';
3333
import something from 'another-module';
3434
```
3535

36+
Example of **correct** code for this rule:
37+
38+
```js
39+
/*eslint no-duplicate-imports: "error"*/
40+
41+
// not mergeable
42+
import { merge } from 'module';
43+
import * as something from 'module';
44+
```
45+
3646
## Options
3747

3848
This rule takes one optional argument, an object with a single key, `includeExports` which is a `boolean`. It defaults to `false`.
@@ -58,3 +68,17 @@ import { merge, find } from 'module';
5868

5969
export { find };
6070
```
71+
72+
Example of **correct** code for this rule with the `{ "includeExports": true }` option:
73+
74+
```js
75+
/*eslint no-duplicate-imports: ["error", { "includeExports": true }]*/
76+
77+
import { merge, find } from 'module';
78+
79+
// cannot be merged with the above import
80+
export * as something from 'module';
81+
82+
// cannot be written differently
83+
export * from 'module';
84+
```

‎lib/rules/no-duplicate-imports.js

Lines changed: 214 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -4,92 +4,225 @@
44
*/
55
"use strict";
66

7+
//------------------------------------------------------------------------------
8+
// Helpers
9+
//------------------------------------------------------------------------------
10+
11+
const NAMED_TYPES = ["ImportSpecifier", "ExportSpecifier"];
12+
const NAMESPACE_TYPES = [
13+
"ImportNamespaceSpecifier",
14+
"ExportNamespaceSpecifier"
15+
];
16+
717
//------------------------------------------------------------------------------
818
// Rule Definition
919
//------------------------------------------------------------------------------
1020

1121
/**
12-
* Returns the name of the module imported or re-exported.
22+
* Check if an import/export type belongs to (ImportSpecifier|ExportSpecifier) or (ImportNamespaceSpecifier|ExportNamespaceSpecifier).
23+
* @param {string} importExportType An import/export type to check.
24+
* @param {string} type Can be "named" or "namespace"
25+
* @returns {boolean} True if import/export type belongs to (ImportSpecifier|ExportSpecifier) or (ImportNamespaceSpecifier|ExportNamespaceSpecifier) and false if it doesn't.
26+
*/
27+
function isImportExportSpecifier(importExportType, type) {
28+
const arrayToCheck = type === "named" ? NAMED_TYPES : NAMESPACE_TYPES;
29+
30+
return arrayToCheck.includes(importExportType);
31+
}
32+
33+
/**
34+
* Return the type of (import|export).
1335
* @param {ASTNode} node A node to get.
14-
* @returns {string} the name of the module, or empty string if no name.
36+
* @returns {string} The type of the (import|export).
1537
*/
16-
function getValue(node) {
17-
if (node && node.source && node.source.value) {
18-
return node.source.value.trim();
38+
function getImportExportType(node) {
39+
if (node.specifiers && node.specifiers.length > 0) {
40+
const nodeSpecifiers = node.specifiers;
41+
const index = nodeSpecifiers.findIndex(
42+
({ type }) =>
43+
isImportExportSpecifier(type, "named") ||
44+
isImportExportSpecifier(type, "namespace")
45+
);
46+
const i = index > -1 ? index : 0;
47+
48+
return nodeSpecifiers[i].type;
1949
}
50+
if (node.type === "ExportAllDeclaration") {
51+
if (node.exported) {
52+
return "ExportNamespaceSpecifier";
53+
}
54+
return "ExportAll";
55+
}
56+
return "SideEffectImport";
57+
}
2058

21-
return "";
59+
/**
60+
* Returns a boolean indicates if two (import|export) can be merged
61+
* @param {ASTNode} node1 A node to check.
62+
* @param {ASTNode} node2 A node to check.
63+
* @returns {boolean} True if two (import|export) can be merged, false if they can't.
64+
*/
65+
function isImportExportCanBeMerged(node1, node2) {
66+
const importExportType1 = getImportExportType(node1);
67+
const importExportType2 = getImportExportType(node2);
68+
69+
if (
70+
(importExportType1 === "ExportAll" &&
71+
importExportType2 !== "ExportAll" &&
72+
importExportType2 !== "SideEffectImport") ||
73+
(importExportType1 !== "ExportAll" &&
74+
importExportType1 !== "SideEffectImport" &&
75+
importExportType2 === "ExportAll")
76+
) {
77+
return false;
78+
}
79+
if (
80+
(isImportExportSpecifier(importExportType1, "namespace") &&
81+
isImportExportSpecifier(importExportType2, "named")) ||
82+
(isImportExportSpecifier(importExportType2, "namespace") &&
83+
isImportExportSpecifier(importExportType1, "named"))
84+
) {
85+
return false;
86+
}
87+
return true;
2288
}
2389

2490
/**
25-
* Checks if the name of the import or export exists in the given array, and reports if so.
26-
* @param {RuleContext} context The ESLint rule context object.
27-
* @param {ASTNode} node A node to get.
28-
* @param {string} value The name of the imported or exported module.
29-
* @param {string[]} array The array containing other imports or exports in the file.
30-
* @param {string} messageId A messageId to be reported after the name of the module
31-
*
32-
* @returns {void} No return value
91+
* Returns a boolean if we should report (import|export).
92+
* @param {ASTNode} node A node to be reported or not.
93+
* @param {[ASTNode]} previousNodes An array contains previous nodes of the module imported or exported.
94+
* @returns {boolean} True if the (import|export) should be reported.
3395
*/
34-
function checkAndReport(context, node, value, array, messageId) {
35-
if (array.indexOf(value) !== -1) {
36-
context.report({
37-
node,
38-
messageId,
39-
data: {
40-
module: value
41-
}
42-
});
96+
function shouldReportImportExport(node, previousNodes) {
97+
let i = 0;
98+
99+
while (i < previousNodes.length) {
100+
if (isImportExportCanBeMerged(node, previousNodes[i])) {
101+
return true;
102+
}
103+
i++;
43104
}
105+
return false;
44106
}
45107

46108
/**
47-
* @callback nodeCallback
48-
* @param {ASTNode} node A node to handle.
109+
* Returns array contains only nodes with declarations types equal to type.
110+
* @param {[{node: ASTNode, declarationType: string}]} nodes An array contains objects, each object contains a node and a declaration type.
111+
* @param {string} type Declaration type.
112+
* @returns {[ASTNode]} An array contains only nodes with declarations types equal to type.
113+
*/
114+
function getNodesByDeclarationType(nodes, type) {
115+
return nodes
116+
.filter(({ declarationType }) => declarationType === type)
117+
.map(({ node }) => node);
118+
}
119+
120+
/**
121+
* Returns the name of the module imported or re-exported.
122+
* @param {ASTNode} node A node to get.
123+
* @returns {string} The name of the module, or empty string if no name.
49124
*/
125+
function getModule(node) {
126+
if (node && node.source && node.source.value) {
127+
return node.source.value.trim();
128+
}
129+
return "";
130+
}
50131

51132
/**
52-
* Returns a function handling the imports of a given file
133+
* Checks if the (import|export) can be merged with at least one import or one export, and reports if so.
53134
* @param {RuleContext} context The ESLint rule context object.
135+
* @param {ASTNode} node A node to get.
136+
* @param {Map} modules A Map object contains as a key a module name and as value an array contains objects, each object contains a node and a declaration type.
137+
* @param {string} declarationType A declaration type can be an import or export.
54138
* @param {boolean} includeExports Whether or not to check for exports in addition to imports.
55-
* @param {string[]} importsInFile The array containing other imports in the file.
56-
* @param {string[]} exportsInFile The array containing other exports in the file.
57-
*
58-
* @returns {nodeCallback} A function passed to ESLint to handle the statement.
139+
* @returns {void} No return value.
59140
*/
60-
function handleImports(context, includeExports, importsInFile, exportsInFile) {
61-
return function(node) {
62-
const value = getValue(node);
141+
function checkAndReport(
142+
context,
143+
node,
144+
modules,
145+
declarationType,
146+
includeExports
147+
) {
148+
const module = getModule(node);
63149

64-
if (value) {
65-
checkAndReport(context, node, value, importsInFile, "import");
150+
if (modules.has(module)) {
151+
const previousNodes = modules.get(module);
152+
const messagesIds = [];
153+
const importNodes = getNodesByDeclarationType(previousNodes, "import");
154+
let exportNodes;
66155

156+
if (includeExports) {
157+
exportNodes = getNodesByDeclarationType(previousNodes, "export");
158+
}
159+
if (declarationType === "import") {
160+
if (shouldReportImportExport(node, importNodes)) {
161+
messagesIds.push("import");
162+
}
67163
if (includeExports) {
68-
checkAndReport(context, node, value, exportsInFile, "importAs");
164+
if (shouldReportImportExport(node, exportNodes)) {
165+
messagesIds.push("importAs");
166+
}
167+
}
168+
} else if (declarationType === "export") {
169+
if (shouldReportImportExport(node, exportNodes)) {
170+
messagesIds.push("export");
171+
}
172+
if (shouldReportImportExport(node, importNodes)) {
173+
messagesIds.push("exportAs");
69174
}
70-
71-
importsInFile.push(value);
72175
}
73-
};
176+
messagesIds.forEach(messageId =>
177+
context.report({
178+
node,
179+
messageId,
180+
data: {
181+
module
182+
}
183+
}));
184+
}
74185
}
75186

76187
/**
77-
* Returns a function handling the exports of a given file
188+
* @callback nodeCallback
189+
* @param {ASTNode} node A node to handle.
190+
*/
191+
192+
/**
193+
* Returns a function handling the (imports|exports) of a given file
78194
* @param {RuleContext} context The ESLint rule context object.
79-
* @param {string[]} importsInFile The array containing other imports in the file.
80-
* @param {string[]} exportsInFile The array containing other exports in the file.
81-
*
195+
* @param {Map} modules A Map object contains as a key a module name and as value an array contains objects, each object contains a node and a declaration type.
196+
* @param {string} declarationType A declaration type can be an import or export.
197+
* @param {boolean} includeExports Whether or not to check for exports in addition to imports.
82198
* @returns {nodeCallback} A function passed to ESLint to handle the statement.
83199
*/
84-
function handleExports(context, importsInFile, exportsInFile) {
200+
function handleImportsExports(
201+
context,
202+
modules,
203+
declarationType,
204+
includeExports
205+
) {
85206
return function(node) {
86-
const value = getValue(node);
207+
const module = getModule(node);
208+
209+
if (module) {
210+
checkAndReport(
211+
context,
212+
node,
213+
modules,
214+
declarationType,
215+
includeExports
216+
);
217+
const currentNode = { node, declarationType };
218+
let nodes = [currentNode];
87219

88-
if (value) {
89-
checkAndReport(context, node, value, exportsInFile, "export");
90-
checkAndReport(context, node, value, importsInFile, "exportAs");
220+
if (modules.has(module)) {
221+
const previousNodes = modules.get(module);
91222

92-
exportsInFile.push(value);
223+
nodes = [...previousNodes, currentNode];
224+
}
225+
modules.set(module, nodes);
93226
}
94227
};
95228
}
@@ -105,16 +238,19 @@ module.exports = {
105238
url: "https://eslint.org/docs/rules/no-duplicate-imports"
106239
},
107240

108-
schema: [{
109-
type: "object",
110-
properties: {
111-
includeExports: {
112-
type: "boolean",
113-
default: false
114-
}
115-
},
116-
additionalProperties: false
117-
}],
241+
schema: [
242+
{
243+
type: "object",
244+
properties: {
245+
includeExports: {
246+
type: "boolean",
247+
default: false
248+
}
249+
},
250+
additionalProperties: false
251+
}
252+
],
253+
118254
messages: {
119255
import: "'{{module}}' import is duplicated.",
120256
importAs: "'{{module}}' import is duplicated as export.",
@@ -125,18 +261,30 @@ module.exports = {
125261

126262
create(context) {
127263
const includeExports = (context.options[0] || {}).includeExports,
128-
importsInFile = [],
129-
exportsInFile = [];
130-
264+
modules = new Map();
131265
const handlers = {
132-
ImportDeclaration: handleImports(context, includeExports, importsInFile, exportsInFile)
266+
ImportDeclaration: handleImportsExports(
267+
context,
268+
modules,
269+
"import",
270+
includeExports
271+
)
133272
};
134273

135274
if (includeExports) {
136-
handlers.ExportNamedDeclaration = handleExports(context, importsInFile, exportsInFile);
137-
handlers.ExportAllDeclaration = handleExports(context, importsInFile, exportsInFile);
275+
handlers.ExportNamedDeclaration = handleImportsExports(
276+
context,
277+
modules,
278+
"export",
279+
includeExports
280+
);
281+
handlers.ExportAllDeclaration = handleImportsExports(
282+
context,
283+
modules,
284+
"export",
285+
includeExports
286+
);
138287
}
139-
140288
return handlers;
141289
}
142290
};

0 commit comments

Comments
 (0)