Skip to content

Commit

Permalink
Fix: ignore unmergable imports when checking no-duplicate-imports (fixes
Browse files Browse the repository at this point in the history
 eslint#13180) & Fix: Ignore re-export all in no-duplicate-imports (fixes eslint#12760)
  • Loading branch information
boutahlilsoufiane committed May 30, 2021
1 parent d515f8d commit af199ef
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 16 deletions.
15 changes: 9 additions & 6 deletions docs/rules/no-duplicate-imports.md
Expand Up @@ -12,9 +12,7 @@ import { find } from 'module';

## Rule Details

An import that can be merged with another is a duplicate of that other.

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

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

Expand All @@ -38,7 +36,9 @@ import something from 'another-module';
Example of **correct** code for this rule:

```js
// not mergable, as they would require new nodes to be created.
/*eslint no-duplicate-imports: "error"*/

// not mergeable
import { merge } from 'module';
import * as something from 'module';
```
Expand Down Expand Up @@ -69,13 +69,16 @@ import { merge, find } from 'module';
export { find };
```

There is a special case even the export is duplicate we ignore it, because it can't be merged with another import/export from the same source, it's when we have export with type export *.

Example of **correct** code for this rule with the `{ "includeExports": true }` option:

```js
/*eslint no-duplicate-imports: ["error", { "includeExports": true }]*/

import { merge, find } from 'module';

// cannot be merged with the above import
export * as something from 'module';

// cannot be written differently
export * from 'module';
```
20 changes: 10 additions & 10 deletions lib/rules/no-duplicate-imports.js
Expand Up @@ -19,10 +19,10 @@ const NAMESPACE_TYPES = [
//------------------------------------------------------------------------------

/**
* 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.
* Check if an import/export type belongs to (ImportSpecifier|ExportSpecifier) or (ImportNamespaceSpecifier|ExportNamespaceSpecifier).
* @param {string} importExportType An import/export type to check.
* @param {string} type Can be "named" or "namespace"
* @returns {boolean} True if import/export type belongs to (ImportSpecifier|ExportSpecifier) or (ImportNamespaceSpecifier|ExportNamespaceSpecifier) and false if it doesn't.
*/
function isImportExportSpecifier(importExportType, type) {
const arrayToCheck = type === "named" ? NAMED_TYPES : NAMESPACE_TYPES;
Expand Down Expand Up @@ -58,8 +58,8 @@ function getImportExportType(node) {

/**
* 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.
* @param {ASTNode} node1 A node to check.
* @param {ASTNode} node2 A node to check.
* @returns {boolean} True if two (import|export) can be merged, false if they can't.
*/
function isImportExportCanBeMerged(node1, node2) {
Expand Down Expand Up @@ -88,10 +88,10 @@ function isImportExportCanBeMerged(node1, node2) {
}

/**
* Returns a boolean if we should report (import/export).
* 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.
* @returns {boolean} True if the (import|export) should be reported.
*/
function shouldReportImportExport(node, previousNodes) {
let i = 0;
Expand All @@ -109,7 +109,7 @@ 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.
* @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 [].
* @returns {[ASTNode]} An array contains only nodes with declarations types equal to type.
*/
function getNodesByDeclarationType(nodes, type) {
return nodes
Expand All @@ -130,7 +130,7 @@ function getModule(node) {
}

/**
* Checks if the (import|export) can be merged with at least one import and one export, and reports if so.
* Checks if the (import|export) can be merged with at least one import or one export, and reports if so.
* @param {RuleContext} context The ESLint rule context object.
* @param {ASTNode} node A node to get.
* @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.
Expand Down
23 changes: 23 additions & 0 deletions tests/lib/rules/no-duplicate-imports.js
Expand Up @@ -28,6 +28,7 @@ ruleTester.run("no-duplicate-imports", rule, {
"import os from \"os\";\nexport { something } from \"os\";",
"import * as bar from \"os\";\nimport { baz } from \"os\";",
"import foo, * as bar from \"os\";\nimport { baz } from \"os\";",
"import foo, { bar } from \"os\";\nimport * as baz from \"os\";",
{
code: "import os from \"os\";\nexport { hello } from \"hello\";",
options: [{ includeExports: true }]
Expand All @@ -48,6 +49,18 @@ ruleTester.run("no-duplicate-imports", rule, {
code: "import { merge } from \"lodash-es\";\nexport { merge as lodashMerge }",
options: [{ includeExports: true }]
},
{
code: "export { something } from \"os\";\nexport * as os from \"os\";",
options: [{ includeExports: true }]
},
{
code: "import { something } from \"os\";\nexport * as os from \"os\";",
options: [{ includeExports: true }]
},
{
code: "import * as os from \"os\";\nexport { something } from \"os\";",
options: [{ includeExports: true }]
},
{
code: "import os from \"os\";\nexport * from \"os\";",
options: [{ includeExports: true }]
Expand Down Expand Up @@ -100,6 +113,16 @@ ruleTester.run("no-duplicate-imports", rule, {
options: [{ includeExports: true }],
errors: [{ messageId: "exportAs", data: { module: "os" }, type: "ExportNamedDeclaration" }]
},
{
code: "import os from \"os\";\nexport * as os from \"os\";",
options: [{ includeExports: true }],
errors: [{ messageId: "exportAs", data: { module: "os" }, type: "ExportAllDeclaration" }]
},
{
code: "export * as os from \"os\";\nimport os from \"os\";",
options: [{ includeExports: true }],
errors: [{ messageId: "importAs", data: { module: "os" }, type: "ImportDeclaration" }]
},
{
code: "import * as modns from \"mod\";\nexport * as modns from \"mod\";",
options: [{ includeExports: true }],
Expand Down

0 comments on commit af199ef

Please sign in to comment.